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

On Windows + AdoptOpenJDK 8 232-b09, ProjectDirectories.fromPath returns "null" directories #26

Closed
eed3si9n opened this issue Jan 2, 2020 · 26 comments · Fixed by #27
Closed

Comments

@eed3si9n
Copy link

eed3si9n commented Jan 2, 2020

This was originally reported as sbt/sbt#5206 On Windows, Coursier creates "./null/Coursier/cache/v1" directories as cache and coursier/coursier#1438

steps

  1. Use Windows.
  2. Download the AdoptOpenJDK 8 232-b09 zip file (https://github.com/AdoptOpenJDK/openjdk8-binaries/releases/download/jdk8u232-b09/OpenJDK8U-jdk_x86-32_windows_hotspot_8u232b09.zip)
  3. Launch sbt as follows:
sbt --java-home /cygdrive/c/Users/foo/Downloads/OpenJDK8U-jdk_x86-32_windows_hotspot_8u232b09/jdk8u232-b09/
  1. add directories-jvm to libraryDependencies and call
scala> io.github.soc.directories.ProjectDirectories.fromPath("Coursier")

problem

ProjectDirectories (Windows 7):
  projectPath  = 'Coursier'
  cacheDir     = 'null\Corusier\cache'
  configDir    = 'null\Corusier\config'
  dataDir      = 'null\Corusier\data'
  dataLocalDir = 'null\Corusier\data'
  runtimeDir   = 'null'

It points a relative path named "null".

expectation

https://github.com/soc/directories-jvm#projectdirectories

notes

https://github.com/soc/directories-jvm/blob/8849872b9b71a378e6356f69bcf58427cb681e2f/src/main/java/io/github/soc/directories/Util.java#L113-L145

@soc
Copy link
Collaborator

soc commented Jan 2, 2020

@eed3si9n Thanks, I will look into this!

@eed3si9n
Copy link
Author

eed3si9n commented Jan 2, 2020

Why not use the environment variable LOCALAPPDATA? It seems less scarier than shelling out to powershell.

@alexarchambault
Copy link
Contributor

I've been wondering why we're not using the LocalAppData and AppData env vars too.

@eed3si9n
Copy link
Author

eed3si9n commented Jan 2, 2020

Especially since sbt + Coursier doesn't need "Music" directory.

@alexarchambault
Copy link
Contributor

The powershell stuff seems to be required for UserDirectories, possibly less so for ProjectDirectories, yes.

@soc
Copy link
Collaborator

soc commented Jan 2, 2020

@eed3si9n @alexarchambault There was a reason why I abandoned that approach, but I don't remember the reason anymore...

Not sure it is this: https://stackoverflow.com/questions/1198911/how-to-get-local-application-data-folder-in-java#comment4791986_1198954.

To make things complete: The corresponding registry keys are deprecated and not necessarily in sync¹, using paths with %LocalAppData% doesn't really work either, because you need something to resolve it.

¹ https://devblogs.microsoft.com/oldnewthing/?p=41973

@eed3si9n
Copy link
Author

eed3si9n commented Jan 3, 2020

If you're ok with JNA here's an implementation from a library that's doing similar things - https://github.com/harawata/appdirs/blob/master/src/main/java/net/harawata/appdirs/impl/ShellFolderResolver.java

@alexarchambault
Copy link
Contributor

alexarchambault commented Jan 3, 2020

@eed3si9n See #16 (comment) for a discussion about JNA. AFAIC, I'm glad directories-jvm doesn't use it, so that it's more lightweight.

@alexarchambault
Copy link
Contributor

I was able to reproduce the steps above on a Windows 10 machine. Something really weird though: I can't reproduce that error from Ammonite.

From sbt (getting nulls):

C:\Users\Alexandre\projects\test-directories>type project\build.properties
sbt.version=1.3.6

C:\Users\Alexandre\projects\test-directories>type build.sbt
libraryDependencies += "io.github.soc" % "directories" % "11"

C:\Users\Alexandre\projects\test-directories>sbt console
...
scala> io.github.soc.directories.ProjectDirectories.fromPath("Coursier")
res0: io.github.soc.directories.ProjectDirectories =
ProjectDirectories (Windows 10):
  projectPath  = 'Coursier'
  cacheDir     = 'null\Coursier\cache'
  configDir    = 'null\Coursier\config'
  dataDir      = 'null\Coursier\data'
  dataLocalDir = 'null\Coursier\data'
  runtimeDir   = 'null'
scala> :q

From Ammonite (almost no nulls):

C:\Users\Alexandre\projects\test-directories>coursier launch ammonite:2.0.1
...
@ import $ivy.`io.github.soc:directories:11`
@ io.github.soc.directories.ProjectDirectories.fromPath("Coursier")
res1: io.github.soc.directories.ProjectDirectories = ProjectDirectories (Windows 10):
  projectPath  = 'Coursier'
  cacheDir     = 'C:\Users\Alexandre\AppData\Local\Coursier\cache'
  configDir    = 'C:\Users\Alexandre\AppData\Roaming\Coursier\config'
  dataDir      = 'C:\Users\Alexandre\AppData\Roaming\Coursier\data'
  dataLocalDir = 'C:\Users\Alexandre\AppData\Local\Coursier\data'
  runtimeDir   = 'null'
@ sys.props("java.version") // check we're using the right JDK
res2: String = "1.8.0_232"
@ sys.props("java.home")
res3: String = "C:\\Users\\Alexandre\\.jabba\\jdk\\adopt@1.8.0-232\\jre
@ sys.props("java.vm.version")
res4: String = "25.232-b09"

@alexarchambault
Copy link
Contributor

alexarchambault commented Jan 3, 2020

Same when loading lm-coursier-shaded from Ammonite (no nulls):

C:\...> coursier launch ammonite:2.0.1
@ import $ivy.`io.get-coursier::lm-coursier-shaded:2.0.0-RC5-2`
@ val cls = Thread.currentThread.getContextClassLoader.loadClass("lmcoursier.internal.shaded.coursier.cache.
CacheDefaults$")
@ val defaults = cls.getFields.head.get(null)
@ val method = defaults.getClass.getMethod("location")
@ println(method.invoke(defaults))
C:\Users\Alexandre\AppData\Local\Coursier\cache\v1

@alexarchambault
Copy link
Contributor

alexarchambault commented Jan 3, 2020

Could sbt somehow change things (in... the environment? something else?) that make powershell or the windows calls behave differently?

I tried printing the error stream of the powershell process too, and saw nothing suspicious there.

Also, for an easy repro, one can just run sbt console from directories-jvm itself.

@alexarchambault
Copy link
Contributor

alexarchambault commented Jan 3, 2020

Ok, that seems to originate from these JDK changes (AdoptOpenJDK/openjdk-jdk8u@048eb42). I think sbt sets up a security manager, and Ammonite doesn't hence the discrepancy above.

Calling System.setProperty("jdk.lang.Process.allowAmbiguousCommands", "true") before the calls to directories-jvm seems to fix the issue \o/

@soc
Copy link
Collaborator

soc commented Jan 3, 2020

@alexarchambault Thanks for digging through this!

Seems to be a response to a CVE: https://bugzilla.redhat.com/show_bug.cgi?id=1777929

I can't find anything helpful on how to escape/rewrite the command though, because that would have been my first approach: The JDK doesn't like ambiguous commands anymore? Fine, I will change them to make them unambiguous.

@soc
Copy link
Collaborator

soc commented Jan 3, 2020

Nevertheless, I think there is also a bug in handling the nulls – the user shouldn't receive a path if the operation to retrieve it has failed.

@szeiger
Copy link

szeiger commented Jan 4, 2020

using paths with %LocalAppData% doesn't really work either, because you need something to resolve it.

What do you mean by something to resolve it?

@soc
Copy link
Collaborator

soc commented Jan 4, 2020

@szeiger What I meant is that you can't use %LocalAppData% in your file/path APIs expecting that it gets turned into a real path when it hits the Win32 API.

@soc soc closed this as completed in #27 Jan 5, 2020
@szeiger
Copy link

szeiger commented Jan 14, 2020

I still don't see the problem. It's an environment variable. Use System.getenv to read it and you have an absolute path. Is there any situation where it would be something else?

@dwijnand
Copy link

@szeiger this comment https://stackoverflow.com/questions/1198911/how-to-get-local-application-data-folder-in-java#comment4791986_1198954:

Note that the APPDATA environment variable is not available in applications that has been started via "Run As ..."

@soc
Copy link
Collaborator

soc commented Jan 14, 2020

I'd also be unsure whether the Win32 API takes a modified environment variable into account (and I think it's reasonable to consider the API the source of truth).

@soc
Copy link
Collaborator

soc commented Jan 23, 2020

Probably not: https://devblogs.microsoft.com/oldnewthing/20200115-00/?p=103329

Changing Local_AppData just got axed, it's likely that setting %LocalAppData% will have no effect, except setting %LocalAppData% to a place without a local app data folder.

@alexarchambault
Copy link
Contributor

I found out that powershell accepts base64-encoded programs via -EncodedCommand, so that we don't have to fight with escaping like in #27.

It seems to work well (using it in another context in coursier).

@soc
Copy link
Collaborator

soc commented Jun 5, 2020

Heads up: I just published v12.

@philwalk
Copy link

philwalk commented Feb 11, 2021

The powershell command with -EncodedCommand still fails on my Windows 10 system, due to a missing .NET Framework:

C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -version 2 -NoProfile -EncodedCommand blah-blah-blah

Version v2.0.50727 of the .NET Framework is not installed and it is required to run version 2 of Windows PowerShell.

I never use powershell.

A possible workaround:

export COURSIER_JVM_CACHE=$COURSIER_CACHE

Not sure what the ramifications of that might be, but it does hide the problem.

@soc
Copy link
Collaborator

soc commented Feb 12, 2021

@philwalk I don't think this related to this issue, I think this is #47.

@philwalk
Copy link

@soc ahh, thanks, sorry for the noise!

@soc
Copy link
Collaborator

soc commented Feb 15, 2021

no problem!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants