You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "uschindler (via GitHub)" <gi...@apache.org> on 2023/02/06 16:02:25 UTC

[GitHub] [lucene] uschindler opened a new pull request, #12131: Port over gradle setting generator from Solr

uschindler opened a new pull request, #12131:
URL: https://github.com/apache/lucene/pull/12131

   In Apache Solr we improved the local settings generation to be done directly in gardlew startup (similar to gradle downloader).
   
   This has several positive effects:
   - We can do our Github CI and Jenkins checks in one go, as the file is now generated before gradle even starts, so the build will succeed on first run.
   - The template file is editable by committers without going into script files. Number of processors for threads is inserted by templating
   
   See https://github.com/apache/solr/pull/1320 and https://issues.apache.org/jira/browse/SOLR-16641 for details.


-- 
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@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] colvinco commented on pull request #12131: Port over gradle setting generator from Solr

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #12131:
URL: https://github.com/apache/lucene/pull/12131#issuecomment-1419339752

   There's another reference in smokeTestRelease.py https://github.com/apache/lucene/blob/8df59fc878795dd94e10d4c15a7bc4f1a919843b/dev-tools/scripts/smokeTestRelease.py#L612-L613


-- 
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@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Port over gradle setting generator from Solr [lucene]

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #12131:
URL: https://github.com/apache/lucene/pull/12131#issuecomment-2084544680

   Hi,
   I agree with Dawid. I also don't like the non-runtime settings enforcing you to put it into the per project properties file.
   
   We should not discuss this here, maybe a better place would we the corresponding Gradle issue that makes some of the dynamic settings configurable at Gradle runtime.
   
   An other option that I loved in our previous ant build was that the actual files can be configured before loading.
   
   If Gradle won't fix the issue with dynamic properties, it would be great to have an include like mechanism.


-- 
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@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Port over gradle setting generator from Solr [lucene]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #12131:
URL: https://github.com/apache/lucene/pull/12131#issuecomment-2080324742

   IMO the complexity inherent (admittedly isn't a lot) in generating gradle.properties doesn't seem worth the pickiness of the preferences that some of us apparently have which brought it about.  Customizing gradlew... having a template somewhere someone must find.  The unusualness of it.  Code to maintain is not free / without consequence; we have a complex build here.  Can someone reiterate why its so important that we do this?
   
   The alternative to this is simple -- just commit gradle.properties to source control, and leave it mentioned in `.gitignore` so that anyone can adjust as they wish.  We have control over tests.jvms having a computable default (since this is a setting of our invention read by our own scripts, not by gradle itself) -- thus if not specified (and it wouldn't unless someone sets it), we can implement the cap at 12.


-- 
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@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Port over gradle setting generator from Solr [lucene]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #12131:
URL: https://github.com/apache/lucene/pull/12131#issuecomment-2085413668

   Of course I agree that Gradle could be improved in the ways mentioned here.
   
   RE why is this bothering me today:  Where I work, I have a Gradle project and within it, a copy of Solr, also using Gradle.  The parent project uses the Gradle "includeBuild" feature to refer to parts of Solr needed by the parent project.  Since the Solr copy has no gradle.properties and we also don't need to call its `gradlew`, its build runs with defaults.  Of course, I have options.  I can commit this file, which is my preference; albeit my colleagues are unsure.  I can also add more code/complexity to the build to invoke the generation of this file if it isn't present.  
   
   [This Gradle issue](https://github.com/gradle/gradle/issues/14431) is pertinent; I thumb'ed up it.  


-- 
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@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Port over gradle setting generator from Solr [lucene]

Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #12131:
URL: https://github.com/apache/lucene/pull/12131#issuecomment-2084491642

   > I'm happy to report that we can put individual settings in ~/.gradle/gradle.properties and these settings will overlay in priority over those in the project's properties. Nice; ehh?
   
   Sorry, I wasn't clear - this option has been possible since the very beginning of gradle. The problem is that it's a global setting and, if I recall right, we wanted to have the possibility to set those options locally, per-project. It may seem like a no-brainer to set those parallel worker (or test jvms) globally but it's tricky. Many people won't know about it or bother and - what's even more problematic - it's a global setting, it'll affect you if you have other projects with very heavy tests. Then forking the same number of gradle or test workers would result in an OOM for one project but not for others.
   
   It would be ideal if you could specify something like what we had for ant previously - a "gradle.properties.local" but this isn't possible.
   
   Can I ask why is that automatically generated gradle.properties file bothering you? You can "override" those per-project settings with your global gradle.properties file or env variables [1], if you don't have a problem with sharing those settings across projects.
   
   [1] https://docs.gradle.org/current/userguide/build_environment.html#sec:gradle_configuration_properties


-- 
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@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Port over gradle setting generator from Solr [lucene]

Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #12131:
URL: https://github.com/apache/lucene/pull/12131#issuecomment-2080376757

   if your file is in git, adding it to .gitignore doesn't make local changes ignorable - it's still versioned and git status would show it as modified. 
   
   I have not found a way to override some of those properties using other means (environment variables or another file). This was a while ago - maybe something has changed, worth checking.
   
   The most problematic machine specific setting is this one:
   
   ```
   # Maximum number of parallel gradle workers.
   org.gradle.workers.max=12
   ```
   
   And you can't set it from within gradle (it has to be in that file). Some people have machines with 64 cores, some have machines with 4 cores - there is no preset that will work for everyone, that's why it's computed.
   


-- 
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@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] uschindler commented on pull request #12131: Port over gradle setting generator from Solr

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #12131:
URL: https://github.com/apache/lucene/pull/12131#issuecomment-1419364815

   > There's another reference in smokeTestRelease.py
   > 
   > https://github.com/apache/lucene/blob/8df59fc878795dd94e10d4c15a7bc4f1a919843b/dev-tools/scripts/smokeTestRelease.py#L612-L613
   
   Fixed.


-- 
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@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Port over gradle setting generator from Solr [lucene]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #12131:
URL: https://github.com/apache/lucene/pull/12131#issuecomment-2084240048

   > if your file is in git, adding it to .gitignore doesn't make local changes ignorable - it's still versioned and git status would show it as modified.
   
   True; I had thought .gitignore kept this but it doesn't.  In my projects where I have changed files that I never want to commit, I use an IntelliJ changelist I name "Ignored" that is never active.  It prevents accidentally committing it.
   
   > org.gradle.workers.max=12
   
   FYI: Runtime.runtime.availableProcessors() is 16 on my machine).  I haven't found that particular setting to be so problematic  for me since a build exclusive of tests (a) doesn't do *that* much (won't cripple my machine for 5+ minutes), and (b) is only partially parallelizable to a high level due to many dependencies.
   
   An advantage to a checked-in gradle.properties is keeping it up to date.  A new setting might be added and it'll be used without concern over whether or not someone generated an old one -- that's a non-issue.
   
   I reviewed Gradle's [options](https://docs.gradle.org/current/userguide/build_environment.html) for configuration and did a little experiment.  I'm happy to report that we can put individual settings in `~/.gradle/gradle.properties` and these settings will overlay in priority over those in the project's properties.  Nice; ehh?


-- 
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@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] uschindler commented on pull request #12131: Port over gradle setting generator from Solr

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #12131:
URL: https://github.com/apache/lucene/pull/12131#issuecomment-1419575660

   Hi @colvinco, I merged your PR into my branch and found only a small difference in the windows script, which I fixed. Not sure why Solr did not apply the JAVA_OPTS for the generator.
   
   


-- 
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@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] uschindler merged pull request #12131: Port over gradle setting generator from Solr

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler merged PR #12131:
URL: https://github.com/apache/lucene/pull/12131


-- 
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@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Port over gradle setting generator from Solr [lucene]

Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #12131:
URL: https://github.com/apache/lucene/pull/12131#issuecomment-2088223120

   Ah, I see your point. The only crude workaround I see for now is to set those props via an environment variable - these should be picked up by all included builds. For example, when I run:
   ```
   $ GRADLE_OPTS="-Dorg.gradle.workers.max=0" ./gradlew help
   ```
   this breaks Lucene's build, even though gradle.properties is there. So if you can set it in your build IDE or shell, you can override the defaults across all composite builds. 
   
   I really think it's kind of dumb that the number of workers can't be configured in settings.gradle, for example.


-- 
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@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org