You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "tylerbertrand (via GitHub)" <gi...@apache.org> on 2023/01/30 22:34:56 UTC
[GitHub] [solr] tylerbertrand opened a new pull request, #1319: Gradle optimizations
tylerbertrand opened a new pull request, #1319:
URL: https://github.com/apache/solr/pull/1319
# Description
<!-- Please provide a short description of the changes you're making with this pull request. -->
Addressed various build script problems uncovered by Gradle Enterprise that should result in improved build times.
The following build scans show "before and after" build times and other stats for these changes:
* Before: https://scans.gradle.com/s/hxh3ugogtrqv4
* After: https://scans.gradle.com/s/fnusdvrk4xbkm
# Solution
<!-- Please provide a short description of the approach taken to implement your solution. -->
* Resolved overlapping outputs issue caused by `copyTestResources` outputs overlapping with `processTestResources`.
Removed `copyTestResources` and moved its functionality into the existing `processTestResources` task. Overlapping outputs between these two tasks was causing them both to not be cacheable.
* Configured inputs and outputs for `validateJarLicenses` task.
Properly configuring task inputs and outputs prevents a task from being needlessly re-executed if its inputs and outputs have not changed.
* Converted absolute paths to relative paths where necessary. Implemented `CommandLineArgumentProvider`s to properly configure the problematic input files/directories and their pathing.
Referencing certain files/directories by absolute path was causing tasks to be re-executed even though their outputs should've been read from the build cache without re-execution.
* Marked some properties as `@Internal`, as they should not be considered for `UP-TO-DATE` checking.
* Updated tasks touched by these changes to take advantage of lazy task configuration
Lazily configuring tasks avoids wasting time configuring tasks that will not be executed
# Tests
<!-- Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem. -->
N/A - only build logic was changed
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `main` branch.
- [x] I have run `./gradlew check`.
- [x] I have added tests for my changes.
- [x] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
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] risdenk commented on pull request #1319: Gradle optimizations
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on PR #1319:
URL: https://github.com/apache/solr/pull/1319#issuecomment-1419442129
Thanks @tylerbertrand. if needed, I'm happy to help port to Lucene.
--
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] tylerbertrand commented on pull request #1319: Gradle optimizations
Posted by "tylerbertrand (via GitHub)" <gi...@apache.org>.
tylerbertrand commented on PR #1319:
URL: https://github.com/apache/solr/pull/1319#issuecomment-1419423953
> @tylerbertrand I see a few potentially outstanding comments on this from @erichaagdev - do you plan to address those? If not happy to merge as is.
If it's alright with you I think it can be merged as-is, and I'll apply those changes in a later PR.
--
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] risdenk commented on pull request #1319: Gradle optimizations
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on PR #1319:
URL: https://github.com/apache/solr/pull/1319#issuecomment-1428410080
I created https://github.com/apache/lucene/issues/12145 to track adding this to Lucene.
--
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] risdenk commented on pull request #1319: Gradle optimizations
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on PR #1319:
URL: https://github.com/apache/solr/pull/1319#issuecomment-1419267953
@tylerbertrand I see a few potentially outstanding comments on this from @erichaagdev - do you plan to address those? If not happy to merge as is.
--
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] erichaagdev commented on a diff in pull request #1319: Gradle optimizations
Posted by "erichaagdev (via GitHub)" <gi...@apache.org>.
erichaagdev commented on code in PR #1319:
URL: https://github.com/apache/solr/pull/1319#discussion_r1092291823
##########
gradle/testing/defaults-tests.gradle:
##########
@@ -179,3 +182,20 @@ allprojects {
}
}
}
+
+class LoggingFileArgumentProvider implements CommandLineArgumentProvider {
+ @InputFile
+ @PathSensitive(PathSensitivity.RELATIVE)
+ File loggingConfigFile
Review Comment:
I suggest using an appropriate lazy file type, versus `File`: https://docs.gradle.org/current/userguide/lazy_configuration.html#working_with_files_in_lazy_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@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] erichaagdev commented on a diff in pull request #1319: Gradle optimizations
Posted by "erichaagdev (via GitHub)" <gi...@apache.org>.
erichaagdev commented on code in PR #1319:
URL: https://github.com/apache/solr/pull/1319#discussion_r1092301663
##########
gradle/testing/randomization.gradle:
##########
@@ -34,6 +34,28 @@ buildscript {
}
}
+class SecurityArgumentProvider implements CommandLineArgumentProvider {
+ @Internal
+ File commonSolrDir
Review Comment:
Same suggestion about using lazy file type.
--
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] risdenk merged pull request #1319: Gradle optimizations
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk merged PR #1319:
URL: https://github.com/apache/solr/pull/1319
--
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] tylerbertrand commented on pull request #1319: Gradle optimizations
Posted by "tylerbertrand (via GitHub)" <gi...@apache.org>.
tylerbertrand commented on PR #1319:
URL: https://github.com/apache/solr/pull/1319#issuecomment-1419430398
> LGTM
>
> Could you please port over the changes to APache Lucene? It should mostly merge across checkout (you can relatively easy cherry-pick the commit from the solr repo into the lucene repo).
@uschindler Sure, I'll take a look at doing that 👍🏼
--
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] erichaagdev commented on pull request #1319: Gradle optimizations
Posted by "erichaagdev (via GitHub)" <gi...@apache.org>.
erichaagdev commented on PR #1319:
URL: https://github.com/apache/solr/pull/1319#issuecomment-1419428027
> If it's alright with you I think it can be merged as-is, and I'll apply those changes in a later PR.
I'm fine with this. It was only a suggestion. 👍
--
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
Re: [PR] Gradle optimizations [solr]
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on PR #1319:
URL: https://github.com/apache/solr/pull/1319#issuecomment-1929782103
I think this change is causing https://issues.apache.org/jira/browse/SOLR-17142 - which is `WARNING: there were unreferenced files under license folder:` when repeated builds are run without clean.
--
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
Re: [PR] Gradle optimizations [solr]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #1319:
URL: https://github.com/apache/solr/pull/1319#issuecomment-1929905954
Yes, I have seen the same problem on Lucene.
--
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