You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "janhoy (via GitHub)" <gi...@apache.org> on 2023/03/28 19:56:09 UTC
[GitHub] [solr] janhoy opened a new pull request, #1502: SOLR-16721 Java version detection fails when `_JAVA_OPTIONS` is set
janhoy opened a new pull request, #1502:
URL: https://github.com/apache/solr/pull/1502
https://issues.apache.org/jira/browse/SOLR-16721
Simple fix. Removing the `head` filter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1502: SOLR-16721 Java version detection fails when `_JAVA_OPTIONS` is set
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1502:
URL: https://github.com/apache/solr/pull/1502#discussion_r1151719775
##########
solr/bin/solr:
##########
@@ -163,7 +163,7 @@ if [[ $? -ne 0 ]] ; then
echo >&2 "${PATH}"
exit 1
else
- JAVA_VER_NUM=$(echo "$JAVA_VER" | head -1 | awk -F '"' '/version/ {print $2}' | sed -e's/^1\.//' | sed -e's/[._-].*$//')
+ JAVA_VER_NUM=$(echo "$JAVA_VER" | awk -F '"' '/version/ {print $2}' | sed -e's/^1\.//' | sed -e's/[._-].*$//')
Review Comment:
Sure, good point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy merged pull request #1502: SOLR-16721 Java version detection fails when `_JAVA_OPTIONS` is set
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy merged PR #1502:
URL: https://github.com/apache/solr/pull/1502
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #1502: SOLR-16721 Java version detection fails when `_JAVA_OPTIONS` is set
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #1502:
URL: https://github.com/apache/solr/pull/1502#issuecomment-1489275643
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #1502: SOLR-16721 Java version detection fails when `_JAVA_OPTIONS` is set
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #1502:
URL: https://github.com/apache/solr/pull/1502#issuecomment-1489280511
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] ZhongRuoyu commented on a diff in pull request #1502: SOLR-16721 Java version detection fails when `_JAVA_OPTIONS` is set
Posted by "ZhongRuoyu (via GitHub)" <gi...@apache.org>.
ZhongRuoyu commented on code in PR #1502:
URL: https://github.com/apache/solr/pull/1502#discussion_r1151667097
##########
solr/bin/solr:
##########
@@ -163,7 +163,7 @@ if [[ $? -ne 0 ]] ; then
echo >&2 "${PATH}"
exit 1
else
- JAVA_VER_NUM=$(echo "$JAVA_VER" | head -1 | awk -F '"' '/version/ {print $2}' | sed -e's/^1\.//' | sed -e's/[._-].*$//')
+ JAVA_VER_NUM=$(echo "$JAVA_VER" | awk -F '"' '/version/ {print $2}' | sed -e's/^1\.//' | sed -e's/[._-].*$//')
Review Comment:
To make the patch more robust, I suggest this:
```suggestion
JAVA_VER_NUM=$(echo "$JAVA_VER" | grep -v '_JAVA_OPTIONS' | awk -F '"' '/version/ {print $2}' | sed -e's/^1\.//' | sed -e's/[._-].*$//')
```
This prevents matching any occurrence of `version` found in `_JAVA_OPTIONS`:
```console
$ export _JAVA_OPTIONS='-Dmy.package.version="123"'
$ JAVA_VER=$(java -version 2>&1)
$ echo "$JAVA_VER" | awk -F '"' '/version/ {print $2}' | sed -e's/^1\.//' | sed -e's/[._-].*$//'
123
19
$ echo "$JAVA_VER" | grep -v '_JAVA_OPTIONS' | awk -F '"' '/version/ {print $2}' | sed -e's/^1\.//' | sed -e's/[._-].*$//'
19
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] ZhongRuoyu commented on pull request #1502: SOLR-16721 Java version detection fails when `_JAVA_OPTIONS` is set
Posted by "ZhongRuoyu (via GitHub)" <gi...@apache.org>.
ZhongRuoyu commented on PR #1502:
URL: https://github.com/apache/solr/pull/1502#issuecomment-1488270639
Thanks @janhoy! I can confirm that this patch works in https://github.com/Homebrew/homebrew-core/pull/126571.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1502: SOLR-16721 Java version detection fails when `_JAVA_OPTIONS` is set
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1502:
URL: https://github.com/apache/solr/pull/1502#discussion_r1151721890
##########
solr/CHANGES.txt:
##########
@@ -83,6 +83,8 @@ Bug Fixes
* SOLR-16638: Fix Http2SolrClient's exception message when serverBaseUrl is null (Alex Deparvu via Kevin Risden)
+* SOLR-16721: Java version detection fails when `_JAVA_OPTIONS` is set (janhoy)
Review Comment:
```suggestion
* SOLR-16721: Java version detection fails when `_JAVA_OPTIONS` is set (janhoy, Ruoyu Zhong)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #1502: SOLR-16721 Java version detection fails when `_JAVA_OPTIONS` is set
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #1502:
URL: https://github.com/apache/solr/pull/1502#issuecomment-1489281287
> 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] ZhongRuoyu commented on a diff in pull request #1502: SOLR-16721 Java version detection fails when `_JAVA_OPTIONS` is set
Posted by "ZhongRuoyu (via GitHub)" <gi...@apache.org>.
ZhongRuoyu commented on code in PR #1502:
URL: https://github.com/apache/solr/pull/1502#discussion_r1151667097
##########
solr/bin/solr:
##########
@@ -163,7 +163,7 @@ if [[ $? -ne 0 ]] ; then
echo >&2 "${PATH}"
exit 1
else
- JAVA_VER_NUM=$(echo "$JAVA_VER" | head -1 | awk -F '"' '/version/ {print $2}' | sed -e's/^1\.//' | sed -e's/[._-].*$//')
+ JAVA_VER_NUM=$(echo "$JAVA_VER" | awk -F '"' '/version/ {print $2}' | sed -e's/^1\.//' | sed -e's/[._-].*$//')
Review Comment:
To make the patch more robust, I suggest this:
```suggestion
JAVA_VER_NUM=$(echo "$JAVA_VER" | grep -v '_JAVA_OPTIONS' | awk -F '"' '/version/ {print $2}' | sed -e's/^1\.//' | sed -e's/[._-].*$//')
```
This prevents matching any occurrence of `version` found in `_JAVA_OPTIONS`:
```console
$ export _JAVA_OPTIONS='-Dmy.package.version="123"'
$ java --version
Picked up _JAVA_OPTIONS: -Dmy.package.version="123"
openjdk version "19.0.2" 2023-01-17
OpenJDK Runtime Environment Homebrew (build 19.0.2)
OpenJDK 64-Bit Server VM Homebrew (build 19.0.2, mixed mode, sharing)
$ JAVA_VER=$(java -version 2>&1)
$ echo "$JAVA_VER" | awk -F '"' '/version/ {print $2}' | sed -e's/^1\.//' | sed -e's/[._-].*$//'
123
19
$ echo "$JAVA_VER" | grep -v '_JAVA_OPTIONS' | awk -F '"' '/version/ {print $2}' | sed -e's/^1\.//' | sed -e's/[._-].*$//'
19
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] andywebb1975 commented on pull request #1502: SOLR-16721 Java version detection fails when `_JAVA_OPTIONS` is set
Posted by "andywebb1975 (via GitHub)" <gi...@apache.org>.
andywebb1975 commented on PR #1502:
URL: https://github.com/apache/solr/pull/1502#issuecomment-1489819681
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #1502: SOLR-16721 Java version detection fails when `_JAVA_OPTIONS` is set
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #1502:
URL: https://github.com/apache/solr/pull/1502#issuecomment-1491630140
Yep, with your example you'd get this output:
```
8
42
```
Agree it won't hurt adding back the head
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org