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

JAVA_HOME is not set correctly on MacOS for local (jdkFile) installed JDKs that are present in the tool cache #396

Closed
3 of 5 tasks
erwin1 opened this issue Oct 28, 2022 · 10 comments · Fixed by #397
Closed
3 of 5 tasks
Assignees
Labels
bug Something isn't working

Comments

@erwin1
Copy link
Contributor

erwin1 commented Oct 28, 2022

Description:
Local installed JDKs using the jdkFile option that are already present in the tool cache are not checked for suffix 'Contents/Home' on macos, so as a result the JAVA_HOME is not set correctly.

Task version:
v3

Platform:

  • Ubuntu
  • macOS
  • Windows

Runner type:

  • Hosted
  • Self-hosted

Repro steps:

  • run a workflow with setup-java with a jdkFile option on a self-hosted runner on MacOS using a JDK that places files in 'Contents/Home'
  • the first time this works fine. JAVA_HOME is set correctly including 'Contents/Home'
  • run the workflow again, this time the JDK will already be present in the tool cache
  • setup-java now sets JAVA_HOME without the 'Contents/Home' suffix

Expected behavior:
When the JDK is available in the tool cache, JAVA_HOME must be set correctly.

Actual behavior:
When the JDK is available in the tool cache, JAVA_HOME is set without checking if the JDK is placed in 'Contents/Home'

@erwin1 erwin1 added bug Something isn't working needs triage labels Oct 28, 2022
@panticmilos
Copy link
Contributor

hi @erwin1, thank you for the report. We will take a look at it

@panticmilos
Copy link
Contributor

By the way, could you provide us public repo with repro steps?

@erwin1
Copy link
Contributor Author

erwin1 commented Oct 28, 2022

Thanks.

Here's a public repo: https://github.com/erwin1/setup-java-mac-sscce
On a hosted runner it's a bit harder to reproduce because every run happens on a different runner (so less chance of having a tool cache).

Although I was able to reproduce with a trick to use setup-java again in the same job.

This fails:
https://github.com/erwin1/setup-java-mac-sscce/actions/runs/3344915283/jobs/5539822614

And using the proposed fix from #397 it works:
https://github.com/erwin1/setup-java-mac-sscce/actions/runs/3344942099/jobs/5539881412

@erwin1
Copy link
Contributor Author

erwin1 commented Oct 30, 2022

added 2 unit tests of which one fails (java is resolved from toolcache including Contents/Home on MacOS) without the proposed fix

@panticmilos panticmilos self-assigned this Nov 1, 2022
@erwin1
Copy link
Contributor Author

erwin1 commented Dec 2, 2022

Any chance we can move this forward?

@panticmilos
Copy link
Contributor

hi @erwin1, sorry for the delayed answer, thank you for the repo I will take a look again :)

@erwin1
Copy link
Contributor Author

erwin1 commented Feb 4, 2023

We are still regularly running into this issue, so it would be great to get a review on the PR. Thanks!

@dmitry-shibanov
Copy link
Contributor

Hello @erwin1. We've merged your pull request. For now you can try to use this changes with actions/setup-java@main. We'll ping you when new release is ready.

For now I'm going to reopen it until we release a new version.

@erwin1
Copy link
Contributor Author

erwin1 commented Apr 4, 2023

Thanks, using @main works as expected now:
https://github.com/erwin1/setup-java-mac-sscce/actions/runs/4606608821

@IvanZosimov
Copy link
Contributor

Hi, @erwin1 👋 The new version of the setup-java is released. Please, check it out.

I'm going to close this issue, If you have any additions or questions feel free to ping us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants