Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make default framework location detection on Linux XDG compliant #2924

Merged
merged 1 commit into from
Nov 8, 2022
Merged

Conversation

Miepee
Copy link
Contributor

@Miepee Miepee commented Nov 7, 2022

As per the XDG basedir spec:

Environment variables: [...] $XDG_DATA_HOME defines the base directory relative to which user-specific data files should be stored. If $XDG_DATA_HOME is either not set or empty, a default equal to $HOME/.local/share should be used.

Implementing this, makes Apktool work in special environments (such as Flatpak) which by default set XDG_DATA_HOME to something else, without the need to override the path with --frame-dir

As far as I know, the BSDs also try to adhere to the XDG spec, so this should be beneficial for them too.

Copy link
Owner

@iBotPeaches iBotPeaches left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do store a few things either temporarily or persisted.

  • framework files (persisted)
  • temp files (building the apk)
  • aapt binaries (temp during building)

So I believe if we are going to respect some environment variables. I might have to do some research how to classify other file storage mechanisms.

On Linux, check first if $XDG_DATA_HOME is set, if not use path as it was before (~/.local/share/apktool)
@Miepee
Copy link
Contributor Author

Miepee commented Nov 7, 2022

We do store a few things either temporarily or persisted.

Are all of these currently stored in whatever path getFrameworkDir() spews out?

@iBotPeaches
Copy link
Owner

Are all of these currently stored in whatever path getFrameworkDir() spews out?

Negative. At the base those are like File.createTempFile stuff. https://github.com/iBotPeaches/Apktool/blob/master/brut.j.util/src/main/java/brut/util/OS.java#L131

@Miepee
Copy link
Contributor Author

Miepee commented Nov 7, 2022

Then AFAIK the current approach seems fine.
Only thing that might be worth keeping in mind, is that if Apktool introduces some persistent configuration files, or log files, that those should go into $XDG_CONFIG_HOME / $XDG_STATE_HOME.
Distro maintainers would probably know more about where which files should go on Linux, I'm just some end-user trying to fix a bug that affected me :)

@iBotPeaches iBotPeaches merged commit 6a70be6 into iBotPeaches:master Nov 8, 2022
iBotPeaches added a commit that referenced this pull request Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants