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

fix(shim): Run JAR file from app's root directory #5872

Merged
merged 2 commits into from
Apr 11, 2024
Merged

Conversation

niheaven
Copy link
Member

@niheaven niheaven commented Apr 9, 2024

Description

For Java apps, the working directory should be apps' root directory since config files are normally located beside .jar.

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

@niheaven
Copy link
Member Author

niheaven commented Apr 9, 2024

I'm not sure if .py or other executives need such tweaks.

@rashil2000
Copy link
Member

Why WSL check?

@niheaven
Copy link
Member Author

WSL couldn't recognize D:/aaa/bbb-type path, since it uses /mnt/D/aaa/bbb, while Cygwin could access D:/aaa/bbb correctly.

@niheaven
Copy link
Member Author

Some use cases: ScoopInstaller/Main#5702, ScoopInstaller/Main#5704

@rashil2000
Copy link
Member

rashil2000 commented Apr 11, 2024

I recognize the usefulness of this feature, but still don't get the WSL part - do we really need to handle the use case where bash scripts (meant to run in Git Bash etc.) would be run inside WSL Bash? Wouldn't that lead to additional problems (w.r.t to finding the correct binaries like java.exe etc.)?

@niheaven
Copy link
Member Author

WSL shares PATH with the host machine, so if you install Java and can run in pwsh, you can run it in WSL too. That's why we should consider such cases.

@niheaven niheaven merged commit 6327146 into develop Apr 11, 2024
2 checks passed
@niheaven niheaven deleted the fix-jar-cmd branch April 11, 2024 07:56
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.

None yet

2 participants