Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
epugh merged PR #3347: URL: https://github.com/apache/solr/pull/3347 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
rahulgoswami commented on PR #3347: URL: https://github.com/apache/solr/pull/3347#issuecomment-2902122027 Flaky tests on Crave ? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
rahulgoswami commented on code in PR #3347:
URL: https://github.com/apache/solr/pull/3347#discussion_r2101285379
##
solr/core/src/java/org/apache/solr/cli/RunExampleTool.java:
##
@@ -230,17 +230,13 @@ public void runImpl(CommandLine cli) throws Exception {
throw new IllegalArgumentException(
"Value of --script option is invalid! " + script + " not found");
} else {
- Path scriptFile = serverDir.getParent().resolve("bin").resolve("solr");
+ Path scriptFile =
+ serverDir.getParent().resolve("bin").resolve(CLIUtils.isWindows() ?
"solr.cmd" : "solr");
if (Files.isRegularFile(scriptFile)) {
script = scriptFile.toAbsolutePath().toString();
} else {
-scriptFile = serverDir.getParent().resolve("bin").resolve("solr.cmd");
-if (Files.isRegularFile(scriptFile)) {
- script = scriptFile.toAbsolutePath().toString();
-} else {
- throw new IllegalArgumentException(
- "Cannot locate the bin/solr script! Please pass --script to this
application.");
-}
+throw new IllegalArgumentException(
+"Cannot locate the bin/solr script! Please pass --script to this
application.");
Review Comment:
Resolved!
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
epugh commented on code in PR #3347:
URL: https://github.com/apache/solr/pull/3347#discussion_r2098882553
##
solr/core/src/java/org/apache/solr/cli/RunExampleTool.java:
##
@@ -230,17 +230,13 @@ public void runImpl(CommandLine cli) throws Exception {
throw new IllegalArgumentException(
"Value of --script option is invalid! " + script + " not found");
} else {
- Path scriptFile = serverDir.getParent().resolve("bin").resolve("solr");
+ Path scriptFile =
+ serverDir.getParent().resolve("bin").resolve(CLIUtils.isWindows() ?
"solr.cmd" : "solr");
if (Files.isRegularFile(scriptFile)) {
script = scriptFile.toAbsolutePath().toString();
} else {
-scriptFile = serverDir.getParent().resolve("bin").resolve("solr.cmd");
-if (Files.isRegularFile(scriptFile)) {
- script = scriptFile.toAbsolutePath().toString();
-} else {
- throw new IllegalArgumentException(
- "Cannot locate the bin/solr script! Please pass --script to this
application.");
-}
+throw new IllegalArgumentException(
+"Cannot locate the bin/solr script! Please pass --script to this
application.");
Review Comment:
maybe do the same `solr.cmd : solr` thing in the error message? nit pick
;_0
##
solr/CHANGES.txt:
##
@@ -291,6 +291,9 @@ Bug Fixes
* SOLR-17758: The NumFieldLimitingUpdateRequestProcessor's "warnOnly" mode has
been fixed, and now processes documents even
when the limit has been exceeded. (Jason Gerlowski, Rahul Goswami)
+* SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts"
(formerly "-a") in conjunction with "-e" (examples like 'techproducts')
Review Comment:
Nice description!
##
solr/packaging/test/test_example.bats:
##
@@ -0,0 +1,41 @@
+#!/usr/bin/env bats
+
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load bats_helper
+
+setup() {
+ common_clean_setup
+}
+
+teardown() {
+ # save a snapshot of SOLR_HOME for failed tests
+ save_home_on_failure
+
+ solr stop --all >/dev/null 2>&1
+}
+
+@test "start -e cloud works with --jvm-opts" {
+ solr start -e cloud --no-prompt --jvm-opts "-Dcustom.prop=helloworld"
Review Comment:
nice that this is also a test that our cloud example starts! we didn't have
that in bats at all..
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
rahulgoswami commented on PR #3347: URL: https://github.com/apache/solr/pull/3347#issuecomment-2893265563 Ok so this is ready. When trying to run an example on MacOS with -e, I am getting into a weird AccessControlException with Jetty. I do not see this behavior on Windows. I have a feeling something broke on the main branch and when I merged main into this PR branch, it is breaking the example run here too. I ran the example on the main branch thereafter, and indeed the same issue there. In case this is something environmental with my Mac, I would really appreciate if you could help run the bats test (test_example.bats) in this PR on your Linux/MacOS machine. And if everything looks good, kindly send these changes to the promised land! :-) Of course if something needs to be addressed, I'll take a stab at it in the morning. Thank you for your collaboration throughout! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
epugh commented on PR #3347: URL: https://github.com/apache/solr/pull/3347#issuecomment-2891762702 > > There is a point of diminishing returns on tests... If you have manually tested things on windows and linux... then that is good > > Makes sense. And although I hate to admit it, I _do_ think I might have hit that point with this PR. I am going to revert some of the test changes and take a quick look at the Linux bats test later tonight. And hope to get this PR polished off thereafter. Sounds like a plan! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
rahulgoswami commented on PR #3347: URL: https://github.com/apache/solr/pull/3347#issuecomment-2891512762 > There is a point of diminishing returns on tests... If you have manually tested things on windows and linux... then that is good Makes sense. And although I hate to admit it, I _do_ think I might have hit that point with this PR. I am going to revert some of the test changes and take a quick look at the Linux bats test later tonight. And hope to get this PR polished off thereafter. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
epugh commented on PR #3347: URL: https://github.com/apache/solr/pull/3347#issuecomment-2891086272 Another thought is that maybe we need to rethink how we test these things, and maybe we do a whole new approach to starting Solr, that *also* covers this need. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
epugh commented on PR #3347: URL: https://github.com/apache/solr/pull/3347#issuecomment-2891083379 So... There is a point of diminishing returns on tests... If you have manually tested things on windows and linux... then that is good... We *do* I think have a bats style test right for LInux, so that is good... Progress over perfection.. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
rahulgoswami commented on PR #3347: URL: https://github.com/apache/solr/pull/3347#issuecomment-2889551976 > The bug is in the test, not the fix, and it is seems to be because of a nuance in the /admin/info/properties that got inadvertently uncovered as part of this testing. I take back the part where I said the issue with tests is because of a nuance in /admin/info/properties. I copy/pasted certain parts of the test from another test in TestSolrCLIRunExample (testExample()), which causes RunExampleTool's startSolr() method to end up calling "solr" script instead of "solr.cmd" (I would expect this to throw an error on Windows, but it doesn't. I am guessing it's got something to do with the custom executor (RunExampleExecutor) that the tests use). Upon further examination seems like the existing testExample() uses a custom RunExampleExecutor to execute the launching process. This launches Solr in embedded mode binding to a pre-declared server Socket. In effect, it seems like a new JVM instance is never launched. Which is why the system property sent with --jvm-opts doesn't reflect. This is not the case when actually running the example from CLI, and hence that works as expected. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
rahulgoswami commented on PR #3347: URL: https://github.com/apache/solr/pull/3347#issuecomment-2889517954 > Great progress, and this stuff is gnarly to test! (and easy to break, so great to get some tests) Sigh! Cannot get tests for this part to run correctly. At this point I have spent wa more time trying to get the tests to run successfully than I spent fixing the issue. The CLI launches the Java process which launches the CLI which launches the actual Solr process. Easy peasy! ...Not! Not when I have to replicate this in tests. :( [Just to recap, with this fix, launching the example with --jvm-opts via CLI works fine on both Windows and Solr. It's just that it's harder than I thought to replicate this behavior in tests, which is what I am working on.] The other tests for RunExampleTool are launching Solr in embedded mode which won't work for this PR. This is since I need to test for the presence of --jvm-args which needs to be passed at the launch of the Solr instance and cannot be tested "within" an already running JVM of the test suite. Rant overback to fixing the tests. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
rahulgoswami commented on PR #3347: URL: https://github.com/apache/solr/pull/3347#issuecomment-2887202955 Thanks for taking a look Eric. I converted the PR to a draft since I am still polishing off the tests. It is currently failing, but I committed it anyway in order to be able to pull it on macOS to see if the test fails for Linux too (it does!). The bug is in the test, not the fix, and it is seems to be because of a nuance in the /admin/info/properties that got inadvertently uncovered as part of this testing. I am fairly certain this would work if I retrieve the system property from the Java Runtime of the launched instance instead of calling /admin/info/properties. Now that I know the reason, I am a little more at peace and will revisit this later today. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
epugh commented on code in PR #3347:
URL: https://github.com/apache/solr/pull/3347#discussion_r2093331439
##
solr/core/src/test/org/apache/solr/cli/TestSolrCLIRunExample.java:
##
@@ -325,6 +330,99 @@ public void testFilmsExample() throws Exception {
testExample("films");
}
+ @Test
+ public void testAdditionalArgs() throws Exception {
+String testStandaloneMode = LuceneTestCase.rarely() ? "--user-managed" :
"";
+Path defaultSolrHomeDir = ExternalPaths.SERVER_HOME;
+if (!Files.isDirectory(defaultSolrHomeDir)) {
Review Comment:
I don't know if this is required...I believe that all our tests that use
`ExternalPaths.SERVER_HOME` assume it's valid.. Altenratively, I think there
is a assert or assume type operation that only runs the test when a criteria is
met for things that ARE variable!
##
solr/core/src/test/org/apache/solr/cli/TestSolrCLIRunExample.java:
##
@@ -325,6 +330,99 @@ public void testFilmsExample() throws Exception {
testExample("films");
}
+ @Test
+ public void testAdditionalArgs() throws Exception {
+String testStandaloneMode = LuceneTestCase.rarely() ? "--user-managed" :
"";
+Path defaultSolrHomeDir = ExternalPaths.SERVER_HOME;
+if (!Files.isDirectory(defaultSolrHomeDir)) {
+ fail(defaultSolrHomeDir + " not found and is required to run this
test!");
+}
+
+Path testSolrHomeDir = createTempDir();
+Path solrServerDir = defaultSolrHomeDir.getParent();
+
+// need a port to start the example server on
+int bindPort = -1;
+try (ServerSocket socket = new ServerSocket(0)) {
+ bindPort = socket.getLocalPort();
+}
+
+log.info("Selected port {} to start techproducts example Solr instance on
...", bindPort);
+
+String sysProp = "solr.customprop.greet";
+String[] toolArgs =
+new String[] {
+ "-e",
+ "techproducts",
+ "--server-dir",
+ solrServerDir.toString(),
+ "--solr-home",
+ testSolrHomeDir.toString(),
+ "-p",
+ String.valueOf(bindPort),
+ testStandaloneMode,
+ "--jvm-opts",
+ "-D" + sysProp + "=hello"
+};
+
+// capture tool output to stdout
+CLITestHelper.TestingRuntime runtime = new
CLITestHelper.TestingRuntime(true);
+
+RunExampleExecutor executor = new RunExampleExecutor();
+closeables.add(executor);
+
+RunExampleTool tool = new RunExampleTool(executor, System.in, runtime);
+try {
+ int status = tool.runTool(SolrCLI.processCommandLineArgs(tool,
toolArgs));
+
+ if (status == -1) {
+// maybe it's the port, try again
+try (ServerSocket socket = new ServerSocket(0)) {
+ bindPort = socket.getLocalPort();
+}
+Thread.sleep(100);
+status = tool.runTool(SolrCLI.processCommandLineArgs(tool, toolArgs));
+ }
+
+ assertEquals("it should be ok " + tool + " " +
Arrays.toString(toolArgs), 0, status);
+
+} catch (Exception e) {
+ log.error("RunExampleTool failed due to: ", e); // nowarn
+ throw e;
+}
+
+String propValue = null;
+try (SolrClient solrClient =
+new Http2SolrClient.Builder("http://localhost:"; + bindPort +
"/solr").build()) {
+ propValue = getSystemProperty(solrClient, sysProp);
+}
+assertEquals("System property provided with --jvm-opts not found",
"hello", propValue);
+// stop the test instance
+executor.execute(org.apache.commons.exec.CommandLine.parse("bin/solr stop
-p " + bindPort));
+ }
+
+ public String getSystemProperty(SolrClient solrClient, String propertyName)
throws Exception {
Review Comment:
I expected this to call the java system propertly method, or be a utility
method that we already have in Solr! On reading it, maybe a name like
"assertSolrRunningWithSystemProperty" would make it clearer?
##
solr/core/src/test/org/apache/solr/cli/TestSolrCLIRunExample.java:
##
@@ -325,6 +330,99 @@ public void testFilmsExample() throws Exception {
testExample("films");
}
+ @Test
+ public void testAdditionalArgs() throws Exception {
+String testStandaloneMode = LuceneTestCase.rarely() ? "--user-managed" :
"";
+Path defaultSolrHomeDir = ExternalPaths.SERVER_HOME;
+if (!Files.isDirectory(defaultSolrHomeDir)) {
+ fail(defaultSolrHomeDir + " not found and is required to run this
test!");
+}
+
+Path testSolrHomeDir = createTempDir();
+Path solrServerDir = defaultSolrHomeDir.getParent();
+
+// need a port to start the example server on
+int bindPort = -1;
+try (ServerSocket socket = new ServerSocket(0)) {
+ bindPort = socket.getLocalPort();
+}
+
+log.info("Selected port {} to start techproducts example Solr instance on
...", bindPort);
+
+String sysProp = "solr.customprop.greet";
+String[] toolArgs =
+new String[] {
+ "-e",
+ "techproducts",
+
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
epugh commented on code in PR #3347:
URL: https://github.com/apache/solr/pull/3347#discussion_r2087142370
##
solr/core/src/java/org/apache/solr/cli/SolrCLI.java:
##
@@ -124,7 +124,7 @@ public static CommandLine parseCmdLine(Tool tool, String[]
args) {
List dashDList = new ArrayList<>();
for (int a = 1; a < args.length; a++) {
String arg = args[a];
- if (arg.startsWith("-D")) {
+ if (!args[a - 1].equals("--jvm-opts") && arg.startsWith("-D")) {
Review Comment:
I think we probably hsould add some tests around this kind of tricky logic?
##
solr/core/src/java/org/apache/solr/cli/RunExampleTool.java:
##
@@ -723,12 +724,13 @@ protected Map startSolr(
forceArg,
verboseArg,
extraArgs,
-jvmOptsArg,
syspropArg);
startCmd = startCmd.replaceAll("\\s+", " ").trim(); // for pretty printing
+String startCmdWithJvmOpts = startCmd + jvmOptsArg;
Review Comment:
I don't love this variable name... Maybe it's just `startCmd`? and a
comment why we moved `jvmOptsArg` out of the previous format command?
##
solr/core/src/java/org/apache/solr/cli/RunExampleTool.java:
##
@@ -723,12 +724,13 @@ protected Map startSolr(
forceArg,
verboseArg,
extraArgs,
-jvmOptsArg,
syspropArg);
startCmd = startCmd.replaceAll("\\s+", " ").trim(); // for pretty printing
+String startCmdWithJvmOpts = startCmd + jvmOptsArg;
Review Comment:
Is this maybe not startCmd, but instead "About logging parameters"?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
rahulgoswami commented on code in PR #3347: URL: https://github.com/apache/solr/pull/3347#discussion_r2087128207 ## solr/bin/solr.cmd: ## @@ -752,7 +752,7 @@ IF NOT "%EXAMPLE%"=="" ( -Dlog4j.configurationFile="file:///%DEFAULT_SERVER_DIR%\resources\log4j2-console.xml" ^ -Dsolr.install.symDir="%SOLR_TIP%" ^ -classpath "%DEFAULT_SERVER_DIR%\solr-webapp\webapp\WEB-INF\lib\*;%DEFAULT_SERVER_DIR%\lib\ext\*" ^ -org.apache.solr.cli.SolrCLI run_example --script "%SDIR%\solr.cmd" -e %EXAMPLE% --server-dir "%SOLR_SERVER_DIR%" ^ +org.apache.solr.cli.SolrCLI run_example --script "%SDIR%\solr.cmd" -e %EXAMPLE% --server-dir "%SOLR_SERVER_DIR%" --jvm-opts "%SOLR_ADDL_ARGS%" ^ Review Comment: This was definitely the main reason. However even after passing it, it was breaking apart in RunExampleTool downstream. Looks like CommandLine.parse() was not preserving the quotes around the additional arguments causing cmd invoked by executor.execute() to break up the argument value in unexpected ways. Hence start.cmd called by RunExampleTool gets broken arguments. Linux shell doesn't seem to be that finicky and handles this fine. Fixed that for Windows. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
rahulgoswami commented on code in PR #3347: URL: https://github.com/apache/solr/pull/3347#discussion_r2087128207 ## solr/bin/solr.cmd: ## @@ -752,7 +752,7 @@ IF NOT "%EXAMPLE%"=="" ( -Dlog4j.configurationFile="file:///%DEFAULT_SERVER_DIR%\resources\log4j2-console.xml" ^ -Dsolr.install.symDir="%SOLR_TIP%" ^ -classpath "%DEFAULT_SERVER_DIR%\solr-webapp\webapp\WEB-INF\lib\*;%DEFAULT_SERVER_DIR%\lib\ext\*" ^ -org.apache.solr.cli.SolrCLI run_example --script "%SDIR%\solr.cmd" -e %EXAMPLE% --server-dir "%SOLR_SERVER_DIR%" ^ +org.apache.solr.cli.SolrCLI run_example --script "%SDIR%\solr.cmd" -e %EXAMPLE% --server-dir "%SOLR_SERVER_DIR%" --jvm-opts "%SOLR_ADDL_ARGS%" ^ Review Comment: This was definitely the main reason. However even after passing it, it was breaking apart in RunExampleTool downstream. Looks like CommandLine.parse() was not preserving the quotes around the additional arguments causing cmd invoked by executor.execute() to break up the argument value in unexpected ways. Hence start.cmd called by RunExampleTool gets broken arguments. Fixed that 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-7962: Passing additional arguments to solr.cmd using "--jvm-opts" (formerly "-a") does not work with "-e" on Windows [solr]
epugh commented on code in PR #3347: URL: https://github.com/apache/solr/pull/3347#discussion_r2087106199 ## solr/bin/solr.cmd: ## @@ -752,7 +752,7 @@ IF NOT "%EXAMPLE%"=="" ( -Dlog4j.configurationFile="file:///%DEFAULT_SERVER_DIR%\resources\log4j2-console.xml" ^ -Dsolr.install.symDir="%SOLR_TIP%" ^ -classpath "%DEFAULT_SERVER_DIR%\solr-webapp\webapp\WEB-INF\lib\*;%DEFAULT_SERVER_DIR%\lib\ext\*" ^ -org.apache.solr.cli.SolrCLI run_example --script "%SDIR%\solr.cmd" -e %EXAMPLE% --server-dir "%SOLR_SERVER_DIR%" ^ +org.apache.solr.cli.SolrCLI run_example --script "%SDIR%\solr.cmd" -e %EXAMPLE% --server-dir "%SOLR_SERVER_DIR%" --jvm-opts "%SOLR_ADDL_ARGS%" ^ Review Comment: I find it interesting that `SOLR_ADDL_ARGS` already existed as a variable but wasn't being passed in, was this the bug? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
