You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/08/30 23:40:31 UTC
[GitHub] [lucene-solr] uschindler opened a new pull request #1806: LUCENE-9475: Switch validateSourcePatters away from Ant legacy
uschindler opened a new pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806
This PR changes the legacy setp of the checkSourcePatterns task to directly execute the checker code in the running Gradle VM.
Because of Ant compatibility we preserved the old code using Ant's Groovy task. So it was executing Groovy shell inside Groovy. The code changes are minimal, further improvements are coming later.
This task has no outputs, so itรs declared to run always. We may improve this, if we make the checked files and patterns a task input. Quick tests on different branches showed that task works.
----------------------------------------------------------------
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.
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-solr] uschindler commented on pull request #1806: LUCENE-9475: Switch validateSourcePatterns away from Ant legacy
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806#issuecomment-683614711
I wanted to change the task in the same way after 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.
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-solr] uschindler commented on pull request #1806: LUCENE-9475: Switch validateSourcePatterns away from Ant legacy
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806#issuecomment-683688206
I think it's now ready. I did not change the inner closures to be real class members. The most important thing to me was to make the inputs declared and the outputs always uptodate, so the task only runs when inputs change. The temporary file by @madrob was not needed (the trick is `outputs.uptodateWhen{true}` (with no outputs, gradle always executes task - well known to the forbiddenapis policeman, trick is userd there, 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.
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-solr] uschindler commented on pull request #1806: LUCENE-9475: Switch validateSourcePatterns away from Ant legacy
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806#issuecomment-683614441
Oh haven't seen that PR. I was just annoyed by the warning.
----------------------------------------------------------------
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.
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-solr] uschindler commented on pull request #1806: LUCENE-9475: Switch validateSourcePatterns away from Ant legacy
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806#issuecomment-683701375
Thanks Dawid,
I will merge this soon. Robert's issue about `var` should be decided later. In #1802 I will add an exclude just for him.
----------------------------------------------------------------
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.
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-solr] dweiss commented on a change in pull request #1806: LUCENE-9475: Switch validateSourcePatterns away from Ant legacy
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806#discussion_r480031302
##########
File path: gradle/validation/validate-source-patterns.gradle
##########
@@ -15,27 +15,233 @@
* limitations under the License.
*/
-// This should be eventually rewritten in plain gradle. For now, delegate to
-// the ant/groovy script we already have.
+import org.apache.rat.Defaults;
+import org.apache.rat.document.impl.FileDocument;
+import org.apache.rat.api.MetaData;
-configure(rootProject) {
- configurations {
- checkSourceDeps
+buildscript {
+ repositories {
+ mavenCentral()
}
dependencies {
- checkSourceDeps "org.codehaus.groovy:groovy-all:2.4.17"
- checkSourceDeps "org.apache.rat:apache-rat:${scriptDepVersions['apache-rat']}"
+ classpath "org.apache.rat:apache-rat:${scriptDepVersions['apache-rat']}"
+ }
+}
+
+configure(rootProject) {
+ task("validateSourcePatterns", type: ValidateSourcePatternsTask) {
+ group = 'Verification'
+ description = 'Validate Source Patterns'
+
+ // this task has no outputs, so it's always uptodate (if inputs don't change).
+ outputs.upToDateWhen { true }
}
+}
+
+class ValidateSourcePatternsTask extends DefaultTask {
+
+ @InputFiles
+ final ConfigurableFileTree sourceFiles = project.fileTree(project.rootDir) {
+ [
Review comment:
And this I'd move to the task declaration block (so that it's not hardcoded but configured :)
----------------------------------------------------------------
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.
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-solr] uschindler commented on pull request #1806: LUCENE-9475: Switch validateSourcePatterns away from Ant legacy
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806#issuecomment-683689216
In case you ask: Making is a class extends DefaultTask allows to better declare inputs/outputs. I am not a fan of manually adding them. This code is much more readable (because of annotations).
----------------------------------------------------------------
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.
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-solr] uschindler commented on pull request #1806: LUCENE-9475: Switch validateSourcePatterns away from Ant legacy
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806#issuecomment-683666774
Hi I converted this PR to a draft, because I am incorporating changes from #1441 (which I closed, as outdated).
----------------------------------------------------------------
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.
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-solr] uschindler commented on pull request #1806: LUCENE-9475: Switch validateSourcePatterns away from Ant legacy
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806#issuecomment-683701983
By the way: Due to my changes, also `gradle/**`` is now checked by validateSourcePatterns. I also added the `.gradle` file extension (you might have noticed).
----------------------------------------------------------------
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.
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-solr] uschindler merged pull request #1806: LUCENE-9475: Switch validateSourcePatterns away from Ant legacy
Posted by GitBox <gi...@apache.org>.
uschindler merged pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806
----------------------------------------------------------------
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.
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-solr] dweiss commented on pull request #1806: LUCENE-9475: Switch validateSourcePatterns away from Ant legacy
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806#issuecomment-683612482
Mike Drob did some work on inputs/ outputs here, it was too early though (ant compatibility).
https://github.com/apache/lucene-solr/pull/1441
The outputs can be empty (but declared). The inputs as per usual - file collection. Should work out of the box.
----------------------------------------------------------------
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.
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-solr] uschindler edited a comment on pull request #1806: LUCENE-9475: Switch validateSourcePatterns away from Ant legacy
Posted by GitBox <gi...@apache.org>.
uschindler edited a comment on pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806#issuecomment-683701983
By the way: Due to my changes, also `gradle/**` is now checked by validateSourcePatterns. I also added the `.gradle` file extension (you might have noticed).
----------------------------------------------------------------
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.
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-solr] dweiss commented on a change in pull request #1806: LUCENE-9475: Switch validateSourcePatterns away from Ant legacy
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1806:
URL: https://github.com/apache/lucene-solr/pull/1806#discussion_r480030991
##########
File path: gradle/validation/validate-source-patterns.gradle
##########
@@ -15,27 +15,233 @@
* limitations under the License.
*/
-// This should be eventually rewritten in plain gradle. For now, delegate to
-// the ant/groovy script we already have.
+import org.apache.rat.Defaults;
+import org.apache.rat.document.impl.FileDocument;
+import org.apache.rat.api.MetaData;
-configure(rootProject) {
- configurations {
- checkSourceDeps
+buildscript {
+ repositories {
+ mavenCentral()
}
dependencies {
- checkSourceDeps "org.codehaus.groovy:groovy-all:2.4.17"
- checkSourceDeps "org.apache.rat:apache-rat:${scriptDepVersions['apache-rat']}"
+ classpath "org.apache.rat:apache-rat:${scriptDepVersions['apache-rat']}"
+ }
+}
+
+configure(rootProject) {
+ task("validateSourcePatterns", type: ValidateSourcePatternsTask) {
+ group = 'Verification'
+ description = 'Validate Source Patterns'
+
+ // this task has no outputs, so it's always uptodate (if inputs don't change).
+ outputs.upToDateWhen { true }
Review comment:
I would move it to the class's constructor? Then inputs and outputs are consistently in the same code.
----------------------------------------------------------------
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.
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