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 2021/04/12 18:52:05 UTC
[GitHub] [lucene] dweiss opened a new pull request #80: LUCENE-9925: add checksums to snowball-generated files
dweiss opened a new pull request #80:
URL: https://github.com/apache/lucene/pull/80
--
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] rmuir commented on a change in pull request #80: LUCENE-9925: add checksums to snowball-generated files
Posted by GitBox <gi...@apache.org>.
rmuir commented on a change in pull request #80:
URL: https://github.com/apache/lucene/pull/80#discussion_r611904118
##########
File path: lucene/analysis/common/src/java/org/apache/lucene/analysis/email/ASCIITLD.jflex
##########
@@ -15,7 +15,7 @@
* limitations under the License.
*/
// Generated from IANA TLD Database <https://data.iana.org/TLD/tlds-alpha-by-domain.txt>
-// file version from 2021 Apr 10, Sat 07:07:01 Coordinated Universal Time
+// file version from 2021 Apr 12, Mon 07:07:01 Coordinated Universal Time
Review comment:
I opened https://issues.apache.org/jira/browse/LUCENE-9926 to fix this, we gotta remove the date as it seems they just bump it at this time every single day.
--
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] dweiss merged pull request #80: LUCENE-9925: add checksums to snowball-generated files
Posted by GitBox <gi...@apache.org>.
dweiss merged pull request #80:
URL: https://github.com/apache/lucene/pull/80
--
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] rmuir commented on a change in pull request #80: LUCENE-9925: add checksums to snowball-generated files
Posted by GitBox <gi...@apache.org>.
rmuir commented on a change in pull request #80:
URL: https://github.com/apache/lucene/pull/80#discussion_r612614117
##########
File path: gradle/generation/snowball.gradle
##########
@@ -114,5 +133,5 @@ configure(project(":lucene:analysis:common")) {
}
}
- regenerate.dependsOn mustRunInOrder([snowball, "spotlessApply"])
+ regenerate.dependsOn wrapWithPersistentChecksums(snowball, [ andThenTasks: "spotlessApply", ignoreWithSource: [downloadSnowballStemmers, downloadSnowballWebsite, downloadSnowballData] ])
Review comment:
@dweiss I like that idea! There's always a good time to call them over and over, if you are actually wrestling with grammar change or something. Calling the whole "regenerate" even without the big DFA takes a couple minutes for me (the ICU compilation is costly as an example). So its hard to remember and properly exclude just the slow parts.
It's helpful to be able to just target regeneration of something specific like `snowball` when doing some real concrete work with it. But it isn't useful to iterate this way unless it also triggers reformatting and checksums, so that tests and checks pass appropriately (if they should).
--
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] rmuir commented on a change in pull request #80: LUCENE-9925: add checksums to snowball-generated files
Posted by GitBox <gi...@apache.org>.
rmuir commented on a change in pull request #80:
URL: https://github.com/apache/lucene/pull/80#discussion_r611893685
##########
File path: gradle/generation/snowball.gradle
##########
@@ -114,5 +133,5 @@ configure(project(":lucene:analysis:common")) {
}
}
- regenerate.dependsOn mustRunInOrder([snowball, "spotlessApply"])
+ regenerate.dependsOn wrapWithPersistentChecksums(snowball, [ andThenTasks: "spotlessApply", ignoreWithSource: [downloadSnowballStemmers, downloadSnowballWebsite, downloadSnowballData] ])
Review comment:
from a user perspective this is a little frustrating. let's leave it for now but please think about it, it is a bit frustrating that `gradlew snowball` is essentially an incomplete, useless task now, becuase it doesnt run tidy nor update checksums. Maybe these individual tasks simply shouldnt be exposed at all, because they only cause pain, only `regenerate` really does everything (such as updating the checksums files).
I tried out your branch and wanted to remove some now-obselete logic from the shell script (hey, the less it does the better, would love to get rid of it). So i ran `./gradlew snowball` with my modified shell script and it did nothing: UP-TO-DATE everywhere.
ok, take two: i ran `./gradlew snowball --rerun-tasks` to test my changes, but it didn't run tidy.
ok, take three: i ran `./gradlew snowball tidy --rerun-tasks`, which accomplished what I wanted, unfortunately, precommit now fails because it neglected to update any checksums :(
take four: i ran `./gradlew -p lucene/analysis/common regenerate --rerun-tasks` to apply a more blunt instrument. But i forgot that damn 12GB DFA is in there :)
--
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] dweiss commented on a change in pull request #80: LUCENE-9925: add checksums to snowball-generated files
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #80:
URL: https://github.com/apache/lucene/pull/80#discussion_r612177428
##########
File path: gradle/generation/snowball.gradle
##########
@@ -114,5 +133,5 @@ configure(project(":lucene:analysis:common")) {
}
}
- regenerate.dependsOn mustRunInOrder([snowball, "spotlessApply"])
+ regenerate.dependsOn wrapWithPersistentChecksums(snowball, [ andThenTasks: "spotlessApply", ignoreWithSource: [downloadSnowballStemmers, downloadSnowballWebsite, downloadSnowballData] ])
Review comment:
> Maybe these individual tasks simply shouldnt be exposed at all, because they only cause pain, only regenerate really does everything (such as updating the checksums files).
Agreed - they shouldn't be exposed. And they sort of are not - if you run the task list they won't show up (their "wrapped" equivalents are shown - snowballIfNeeded). I agree the per-project or global convention "regenerate" should be the "standard" way of regenerating things and I explicitly warned about using low-level regeneration tasks in the docs:
https://github.com/apache/lucene/blob/main/help/regeneration.txt#L135-L143
Don't get me wrong, I'm not too happy about how it currently works (wrapper tasks, complex wire-up logic) but I spent a day or two thinking whether this can be done easier (Occam's razor) and I didn't find a better solution... If I come up with something I'm certainly up for replacing those wrapper tasks...
> But i forgot that damn 12GB DFA is in there :)
LOL. https://github.com/apache/lucene/blob/main/help/regeneration.txt#L145-L148
--
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] dweiss commented on a change in pull request #80: LUCENE-9925: add checksums to snowball-generated files
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #80:
URL: https://github.com/apache/lucene/pull/80#discussion_r612178745
##########
File path: gradle/generation/snowball.gradle
##########
@@ -114,5 +133,5 @@ configure(project(":lucene:analysis:common")) {
}
}
- regenerate.dependsOn mustRunInOrder([snowball, "spotlessApply"])
+ regenerate.dependsOn wrapWithPersistentChecksums(snowball, [ andThenTasks: "spotlessApply", ignoreWithSource: [downloadSnowballStemmers, downloadSnowballWebsite, downloadSnowballData] ])
Review comment:
Maybe we can do a trivial change of suffixing the names of wrapped tasks "*Internal" and then strip this suffix for the wrapper (instead of appending "IfNeeded")... This would make those wrapped tasks more intuitive in isolated calls?...
--
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] dweiss commented on pull request #80: LUCENE-9925: add checksums to snowball-generated files
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #80:
URL: https://github.com/apache/lucene/pull/80#issuecomment-818059624
The last commit is somewhat optional but I think we should clean up these generated sources. Makes the code more consistent and elegant, even if these classes are spaghetti-generated 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