Verify checksums on generated files [LUCENE-9868]

[asfimport]

This would prevent accidental changes to generated resources/ files.

---

Migrated from LUCENE-9868 by Dawid Weiss (@dweiss), resolved Apr 01 2021
Parent: #10910
Attachments: quietExec.patch
Linked issues:

• Make the most painful tasks in regenerate fully incremental [LUCENE-9872] #10911
• Enforce @Override annotation with ecj static analysis configuration [LUCENE-9864] #10903

[asfimport]

Dawid Weiss (@dweiss) (migrated from JIRA)

I think I know how to do it efficiently.

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

I tend to think that all regenerate tasks should maybe add a separate file containing an md5 (that's a valid hash here, because does not need to be bullet-proof; also crc32 would be enough!) next to each regenerated file.

The precommit check task should simply search for such checksum files in source tree and validate the corrsponding file. The check task then won't even need to know the regeneration task responsible or any other semantics are needed, it just searches for checksums and validates them. Of course somebody could break this by removing the checksum file entirely.

The it is up to all regenerate tasks to know which files they modify and checksum them. We may add a function in the build for reusage.

[asfimport]

Dawid Weiss (@dweiss) (migrated from JIRA)

I have the same idea but with a different implementation angle. I'll get to it, can't do it now.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

I think hashing could be extremely difficult to debug, if something goes wrong. It could be frustrating to not know why the hash changed.
I'm thinking typical stuff we've seen before in the past such as non-idempotent regeneration because of hashmap ordering and stuff like that.

On the other hand: a nightly job that would regenerate and fail if the source code tree is dirty is easy to implement and debug: gradlew regenerate precommit || git diff would work perfectly, and if it failed you would see a DIFF!

I do agree we need to guard against this, but I'm concerned we are going about it correctly. I think we might be biased by the large DFA and some desire for it to happen on every single PR, but I think a smaller step (checking nightly) would be a significant improvement. We can always back changes out and then fix them!

And consider theres a lot we currently don't do to test PRs already: in particular we don't even run the unit tests against them.

[asfimport]

Zach Chen (@zacharymorn) (migrated from JIRA)

I wonder if this can be implemented "cheaply" inside / similar to checkWorkingCopyClean task? i.e. specify a common path for generated files ("/generated/") , and then warn / fail gradle build if changes to generated files are detected and uncommitted? I guess this won't be able to stop someone from committing in changed generated files ignoring warnings / failures, but with a determined mind in auto-pilot mode, that someone can probably also commit in changed checksum as well. :D

[asfimport]

Dawid Weiss (@dweiss) (migrated from JIRA)

I think hashing could be extremely difficult to debug, if something goes wrong. It could be frustrating to not know why the hash changed.

I'm thinking typical stuff we've seen before in the past such as non-idempotent regeneration because of hashmap ordering and stuff like that.

I think you'd still know what went wrong because the failure would happen on check (before you commit)? And then you can regenerate and see the diff in inverse...

This said, the benefit of actually having a job doing the regeneration would be twofold - to detect accidental changes AND to make sure these tasks are up-to-date and actually run from time to time (so that URLs don't become stale, etc.). Remember we already have a task to verify the contents of the local working tree is "clean"... so that job could just run this command:

gradlew regenerate -x jflexUAX29URLEmailTokenizerImpl checkWorkingCopyClean

and this would both regenerate AND verify nothing's been changed. I excluded the jflex killer task above. This is an exceptional task that may require other handling.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Yeah, probably a few other things we want to do to make this less annoying to debug (as someone who has regenerated a lot recently, sorry these are random)...

• cache things (especially downloads) better. It is unclear to me if this currently happens. As an example, previously, with the ant-ivy build, when we'd download the mecab dictionary, we would store it in ivy's cache, which works across clean invocations.
• reduce noise from icu regenerate. It is too verbose and dominates all the output of my scrollback buffer. We should change the tar -xvf to tar -xf to not list every file extracted from the icu tgz. As for the compiler output from configure/make, it is only important if the thing fails, but not something you really care to see every day. Maybe we can output this stuff to a file and only print the file if the thing fails.

[asfimport]

Dawid Weiss (@dweiss) (migrated from JIRA)

> cache things (especially downloads) better.

Possible, but not easy. You can make it similar to ivy - emulate a "repository" with a specific layout and make gradle thing the artifact is downloaded from there. The problem is when to update such artifacts since they contain no "version". I personally think a "clean" should really be a last resort thing in gradle. The build benefits from not cleaning - that's the point of those incremental tasks and caches, etc.

> reduce noise from icu regenerate.

Can be done similar to how we invoke exec in render-javadoc.gradle. Perhaps it'd be best to pull it up into a utility... but it does involve some magic. See attached (haven't tried on Linux).