[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties

["colvinco (via GitHub)" <gi...@apache.org>]

colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1092385694


##########
gradlew:
##########
@@ -163,11 +163,8 @@ fi
 
 CLASSPATH=$GRADLE_WRAPPER_JAR
 
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
-if [ ! -e "$APP_HOME/gradle.properties" ]; then
-    GRADLE_DAEMON_CTRL=--no-daemon
-fi
+# Hold on to the original arguments, don't want to pass them in when generating gradle.properties
+TASK_ARGS=$@

Review Comment:
   I'm not going to claim to be an expert at bash, so I could be wrong on the details of this. I have tested it though.
   
   The positional parameters referenced by `$@` are mutated by the `set` cmd at various points in the script 
   https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L226
   https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L236
   https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L268
   which push more arguments onto the parameters. That's described in https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L45-L48
   
   Initially `$@` will refer to the parameters passed in when execution of `gradlew` began, i.e. any parameters supplied by the user `./gradlew updateLicenses check -x test --dry-run` -> `updateLicenses check -x test --dry-run`. The set commands are pushing additional arguments into that, for the JRE, for the main class `org.gradle.wrapper.GradleWrapperMain`, etc.
   
   So when https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L275 is called it contains all the arguments for executing gradle, including the user supplied arguments. However, for calling `localSettings` I don't want the user supplied arguments, so the easiest way I could see to do it without rewriting the whole script was to stash them and not append them into the arguments being built with `set`.
   
   The Windows .bat file doesn't do any complex argument building, so I didn't need to mess around in it to get the same effect.
   
   > Also does $@ need to be quoted?
   Quoting it here appeared to quote the entire string of arguments rather than quoting the individual arguments. Maybe I'm missing a trick as that sounds like it would be the normal expansion behavior https://stackoverflow.com/a/3898681
   



-- 
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