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

Revert "Compare bin subdirs of java home on root mismatch [fix 187]" #241

Merged

Conversation

runningcode
Copy link
Owner

@runningcode runningcode commented Jan 18, 2023

Reverts #240

This doesn't work. I have this issue on my local machine:

  | Gradle is not using JAVA_HOME.                                                                       |
  | JAVA_HOME is /Users/no/.asdf/installs/java/adoptopenjdk-11.0.11+9                                    |
  | Gradle is using /Users/no/.asdf/installs/java/adoptopenjdk-11.0.11+9                                 |
  | This can slow down your build significantly when switching from Android Studio to the terminal.      |
  | To fix: Project Structure -> JDK Location.                                                           |
  | Set this to your JAVA_HOME.

See build scan here: https://scans.gradle.com/s/utvdqanc3oila

@runningcode runningcode merged commit 6c114d8 into master Jan 18, 2023
@runningcode runningcode deleted the revert-240-compare-only-bin-subdirs-of-java-home-187 branch January 18, 2023 21:03
@Rattenkrieg
Copy link
Contributor

Really weird, seems both of us are using sdkman on mac.
If you happen to have time to look at this, I suggest logging both
gradleJavaHomePath.toRealPath().resolve("bin") and environmentJavaHomePath.resolve("bin").toRealPath() perhaps this could provide a clue.

@slowpacer
Copy link
Contributor

Getting back to this thread, I believe the "zulu-sdkman" issue is still present and troubles people from time to time. The fix that was reverted within this MR is actually partially fixing the java home check. The problem is - it's focusing only on the issue but won't work in normal circumstances, so only works for zulu distributions.

The if (gradleJavaHomePath.toRealPath() != environmentJavaHomePath.toRealPath()) logic handles the fail scenario, so the java home is appended with "bin" into its path and gets a real path comparison(links are unwrapped). But in case the initial condition returns true - the outer logic returns false. And that's why on local machine the java home check failed for oracle distribution(even with proper setup), because there were no issues with linking.

In order to handle it properly I believe we can keep the reverted check, but as a fallback, so that we will execute it only in case the primary check fails. So it can end up being something like this snippet(to be polished ofc):

private fun isGradleUsingJavaHome(): Boolean {
        // Follow symlinks when checking that java home matches.
        return environmentJavaHome?.let {
            val gradleJavaHomePath = gradleJavaHome.toPath()
            val environmentJavaHomePath = File(it).toPath()
            return gradleJavaHomePath.toRealPath() != environmentJavaHomePath.toRealPath() || javaHomeFallbackCheck(
                gradleJavaHomePath,
                environmentJavaHomePath
            )
        } ?: false
    }

    private fun javaHomeFallbackCheck(gradleJavaHomePath: Path, environmentJavaHomePath: Path) =
        gradleJavaHomePath.toRealPath().resolve("bin") == environmentJavaHomePath.resolve("bin").toRealPath()

P.S. @runningcode let me know if that makes sense and isn't introducing more complexity. I can throw an MR.
P.P.S. the gradle fix with zulu support(mentioned here) is doing something similar, in a nutshell it just gets the right java exec from the bin folder(I believe so). But the initial java home resolution logic relies on System.getProperty("java.home") and the check gradle doctor relies on system env(System.getenv("JAVA_HOME")) hence the difference. But conceptually, adding bin and unwrapping the real path makes both approaches close(according to me).

@runningcode
Copy link
Owner Author

Hi @slowpacer thanks for trying to find a solution here.
I think I understand what you are trying to get at but the logic you wrote seems to do something else.

return gradleJavaHomePath.toRealPath() != environmentJavaHomePath.toRealPath() ||

Wouldn't this part above return true if the paths are not equal? Did you mean for that part to have == instead of !=?

Other than that, your solutions seems like a good idea.

@slowpacer
Copy link
Contributor

slowpacer commented Mar 10, 2024

Hey @runningcode, yeah, apologies, my bad, blindly copy pasted. Indeed should be == instead of !=.
EDIT: Well, on that note, maybe it might be easier just to have the fallback check(the one with "bin" in the pass). I guess it should handle all scenarios.🤔🤔🤔

@runningcode
Copy link
Owner Author

Isn't only checking "the bin in the path" the issue with what was reverted?

@slowpacer
Copy link
Contributor

From what I see the issue was that your local machine oracle distribution was evaluated against this condition:
gradleJavaHomePath.toRealPath() != environmentJavaHomePath.toRealPath()
and it returned false as both were the same (/Users/no/.asdf/installs/java/adoptopenjdk-11.0.11+9), and then the outer return just went on with the false negative result.

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.

3 participants