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

SOLR-16721 Java version detection fails when _JAVA_OPTIONS is set #1502

Merged
merged 8 commits into from Mar 31, 2023

Conversation

janhoy
Copy link
Contributor

@janhoy janhoy commented Mar 28, 2023

Signed-off-by: Jan Høydahl <janhoy@users.noreply.github.com>
@ZhongRuoyu
Copy link

Thanks @janhoy! I can confirm that this patch works in Homebrew/homebrew-core#126571.

solr/bin/solr Outdated Show resolved Hide resolved
janhoy and others added 2 commits March 29, 2023 12:23
Co-authored-by: Ruoyu Zhong <zhongruoyu@outlook.com>
Signed-off-by: Jan Høydahl <janhoy@users.noreply.github.com>
andywebb1975
andywebb1975 previously approved these changes Mar 29, 2023
Copy link
Contributor

@andywebb1975 andywebb1975 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This made me wonder if there's a way to get just the version number from java more cleanly, but I don't see anything in the man page :-(

# Conflicts:
#	solr/CHANGES.txt
@janhoy
Copy link
Contributor Author

janhoy commented Mar 29, 2023

There's also JAVA_TOOL_OPTIONS which is treated the same way, so there may be one extra line above the version line. See https://stackoverflow.com/questions/28327620/difference-between-java-options-java-tool-options-and-java-opts

Signed-off-by: Jan Høydahl <janhoy@users.noreply.github.com>
@janhoy
Copy link
Contributor Author

janhoy commented Mar 29, 2023

This made me wonder if there's a way to get just the version number from java more cleanly, but I don't see anything in the man page :-(

Yea, that's crazy - imagine all the parsing of versions in all kind of scripts that could be avoided. But I think there's no such option.

@janhoy
Copy link
Contributor Author

janhoy commented Mar 29, 2023

There's also JAVA_TOOL_OPTIONS which is treated the same way, so there may be one extra line above the version line. See https://stackoverflow.com/questions/28327620/difference-between-java-options-java-tool-options-and-java-opts

Changed the grep to grep -v '_OPTIONS' to handle this case too

@andywebb1975
Copy link
Contributor

Removing the head -1 means multiple lines will pass through - so if java -version changed to something along these lines we'd get a multi-line value back:

openjdk version "1.8.0_362"
OpenJDK Runtime Environment (build 1.8.0_362-b09)
OpenJDK 64-Bit Server VM (build 25.362-b09, mixed mode)
SomeOS version "1.42.5"

Keeping the head -1 (after the grep) would protect against this - so I think that's worth putting back in.

@andywebb1975 andywebb1975 dismissed their stale review March 30, 2023 07:22

(See comment above)

@janhoy
Copy link
Contributor Author

janhoy commented Mar 31, 2023

Yep, with your example you'd get this output:

8
42

Agree it won't hurt adding back the head

Signed-off-by: Jan Høydahl <janhoy@users.noreply.github.com>
Signed-off-by: Jan Høydahl <janhoy@users.noreply.github.com>
@janhoy janhoy merged commit f7fe594 into apache:main Mar 31, 2023
3 of 4 checks passed
@janhoy janhoy deleted the SOLR-16721 branch March 31, 2023 20:50
janhoy added a commit that referenced this pull request Apr 10, 2023
…1502)

Signed-off-by: Jan Høydahl <janhoy@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants