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

EXT_DIR value depends on Android version #223

Open
andrew-tpz opened this issue Dec 11, 2020 · 2 comments
Open

EXT_DIR value depends on Android version #223

andrew-tpz opened this issue Dec 11, 2020 · 2 comments
Labels
Milestone

Comments

@andrew-tpz
Copy link

Related to commit #208:

getExternalStorageDirectory() returns root card path, while getExternalFilesDirectoryPath() returns app data specific path. So property EXT_DIR in logback.xml would get different values depend on Android version.

I think it would be something like this:

  public String getExternalStorageDirectoryPath() {
    if (Build.VERSION.SDK_INT >= 29) {
      return getExternalFilesDirectoryPath();
    } else {
      return Environment.getExternalStorageDirectory().getAbsolutePath() + "/Android/data/" + getPackageName() + "/files";
    }

@andrew-tpz andrew-tpz added the bug label Dec 11, 2020
@stale stale bot added the stale label Jun 11, 2021
@tony19 tony19 removed the stale label Oct 30, 2021
Repository owner deleted a comment from stale bot Oct 30, 2021
@stale stale bot added the stale label Jan 9, 2022
Repository owner deleted a comment from stale bot Jan 9, 2022
@stale stale bot removed the stale label Jan 9, 2022
@tony19 tony19 added this to the 2.0.1 milestone Nov 6, 2022
@tony19
Copy link
Owner

tony19 commented Nov 6, 2022

Your suggestion is to make $EXT_DIR/getExternalStorageDirectoryPath() always return the app-specific external files dir, but I think that defeats the purpose of the function, which is to return the root of the external storage path. I missed that in my original review of #208.

The more correct fix to me is to make $EXT_DIR/getExternalStorageDirectoryPath() always return the root of the external storage like it did before #208. In either case, this would be as a breaking change, as users might be relying on the current behavior in their configs.

I think I'll address this by:

  1. documenting the current behavior with a warning
  2. deprecating $EXT_DIR, emitting a runtime deprecation warning in the logs
  3. adding a new special variable to replace $EXT_DIR (e.g., $EXTERNAL_STORAGE_DIR)
  4. adding a new special variable specifically for external files dir (e.g., $EXTERNAL_FILES_DIR) (related Make externalFilesDir available as a property #181).

🤔

@tony19 tony19 modified the milestones: 2.0.1, 2.1.0 Dec 4, 2022
@ajburley
Copy link

Any update on this? We are having to use a custom PropertyDefiner to work around this issue, whereas in earlier versions we were directly using EXT_DIR in the config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 📋 Backlog
Development

No branches or pull requests

3 participants