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

Incorrect path for cache directories on windows #5590

Closed
johanneshardt opened this issue Aug 26, 2023 · 9 comments · Fixed by #5595
Closed

Incorrect path for cache directories on windows #5590

johanneshardt opened this issue Aug 26, 2023 · 9 comments · Fixed by #5595
Milestone

Comments

@johanneshardt
Copy link

Describe the bug

Seems that this issue (#2451) has resurfaced:
image

Steps to reproduce:

  • Minimal install: I've removed, sbt, coursier and scala-cli (only Metals installed).
  • Removed everything relevant in %APPDATA%, %USERPROFILE% (metals, ammonite, coursier, bloop folders etc.)
  • Created a build.sbt with only scalaVersion := "3.3.0" and main.scala
  • Set JAVA_HOME = C:\Program Files\OpenJDK\jdk-19.0.1
  • Opened VS Code in that directory

The "null" folder is created as soon as metals starts, the "powershell" folder is created when clicking "import build' on the popup from metals.

Output when running ProjectDirectories.from() as described in #2451:
image

Setting env vars (COURSIER_BIN_DIR, COURSIER_CACHE, COURSIER_JVM_CACHE) as was recommended at some time in the past makes no difference for me. Building with scala-cli instead of sbt, by running scala-cli setup-ide . before opening VS Code makes no difference either.

This seems to be an issue with directories-jvm again (dirs-dev/directories-jvm#51, dirs-dev/directories-jvm#49), so I'm not sure what can be done on metals' side. Mostly posting this in case someone else runs into this and only finds #2451.

Expected behavior

No response

Operating system

Windows

Editor/Extension

VS Code

Version of Metals

v1.24.0

Extra context or search terms

No response

@tgodzik
Copy link
Contributor

tgodzik commented Aug 28, 2023

I wonder if in such case we should do a workaround and replace null with %APPDATA% value and resolving .cache on it. That's the expected place, right? Alternatively, we could try %APPDATA%/metals-config to not interfere with possible existing cache.

Not the pretties workaround, but should work for us. What do you think?

@tgodzik
Copy link
Contributor

tgodzik commented Aug 28, 2023

Alternatively, we could just return None if the directory starts with null/

tgodzik added a commit to tgodzik/metals that referenced this issue Aug 28, 2023
This is a bug in ProjectDirectories, which seems to be super hard to fix, so instead we can special case it. This is not super important to use that directory anyway, we fall back to local workspace.

Fixes scalameta#5590
tgodzik added a commit to tgodzik/metals that referenced this issue Aug 28, 2023
This is a bug in ProjectDirectories, which seems to be super hard to fix, so instead we can special case it. This is not super important to use that directory anyway, we fall back to local workspace.

Fixes scalameta#5590
@tgodzik
Copy link
Contributor

tgodzik commented Aug 28, 2023

Let's try #5595 -> at least we will not create the null directory

tgodzik added a commit to tgodzik/metals that referenced this issue Aug 29, 2023
This is a bug in ProjectDirectories, which seems to be super hard to fix, so instead we can special case it. This is not super important to use that directory anyway, we fall back to local workspace.

Fixes scalameta#5590
@johanneshardt
Copy link
Author

Seems reasonable 👍

tgodzik added a commit to tgodzik/metals that referenced this issue Aug 29, 2023
This is a bug in ProjectDirectories, which seems to be super hard to fix, so instead we can special case it. This is not super important to use that directory anyway, we fall back to local workspace.

Fixes scalameta#5590
tgodzik added a commit to tgodzik/metals that referenced this issue Aug 29, 2023
This is a bug in ProjectDirectories, which seems to be super hard to fix, so instead we can special case it. This is not super important to use that directory anyway, we fall back to local workspace.

Fixes scalameta#5590
tgodzik added a commit that referenced this issue Aug 29, 2023
This is a bug in ProjectDirectories, which seems to be super hard to fix, so instead we can special case it. This is not super important to use that directory anyway, we fall back to local workspace.

Fixes #5590
@tgodzik
Copy link
Contributor

tgodzik commented Aug 29, 2023

I changed the fix a bit, so do let us know if that is not fixed in the end.

@johanneshardt
Copy link
Author

Yes, that seems to work on my PC. Reverting metals to 1.0.1 reintroduced the bug after a restart as well. Thanks for the fix!

@kasiaMarek kasiaMarek added this to the Metals v1.1.0 milestone Oct 17, 2023
@NoahELE
Copy link

NoahELE commented Oct 25, 2024

I found that the issue persists to exist on Windows. Importing Metals build keeps creating null and PowerShell 7.4.5 folders under the project dir.

@tgodzik
Copy link
Contributor

tgodzik commented Oct 25, 2024

That's unexpected, but best to create new issue.

@NoahELE
Copy link

NoahELE commented Oct 25, 2024

#6885

New issue created.

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 a pull request may close this issue.

4 participants