Many fixes to antlr4-maven-plugin dependency analysis #1945
Conversation
Otherwise, we could pick up stale changes from prior tests.
Adding a newline should actually not change the generated lexer.
Some systems have low-granularity timestamps, so that file modification dates are rounded to seconds. This causes false negatives when detecting if a grammar needs to be recompiled if it changes a second after producing its tokens. This likely only causes an issue for tests that frequently mutate files; real humans are unlikely to compile within 1s of changing a grammar. Still, this seems a cleaner solution that hacking the failing test to use force a different modification time, as there will almost never be false positives. This fixes the failing test after making the test correct.
For some reason, the grammar import dependency analysis only included the first import on a line. This does not work so well...
|
Thanks, I'll get to this as soon as I can. I started teaching a 5-week course this week :( for which I've been prepping like crazy. |
|
Hi @parrt – any chance you'll have time to look at this? I'd not like to lose context when you come back asking for changes :) |
The buildContext.hasDelta function is ignorant of importants. Since we have more advanced dependency analysis, stop relying on hasDelta and instead just refresh grammars where we know the dependencies have changed.
|
hmm...no response. let me take another peek |
|
Ok, seems harmless, and useful if your comments are correct. haha. thanks! |
|
Hiya @dhalperi Can you check the build at travis-ci.com? it appears this PR broke the dotnet mac builds. weird. thanks! |
|
Hi @parrt , Can you send a link to an example failure? I did see this in one of the runs on but that is clearly some sort of network error. Do you have additional Travis runs that I can't see? (I can only access travis-ci.org, not travis-ci.com) |
|
hi. check this one https://travis-ci.org/antlr/antlr4/builds/257836911 I think it is the build for merge of your stuff. Hmm...error doesn't seem related but maybe. for dotnet says: "Error: openssl 1.0.2j is already installed" etc... |
|
hmm...maybe this is spurious travis crap again. @ericvergnaud do you see anything weird in the build log? |
|
You can rerun an individual build on travis, if you're logged in and a repository owner. That often helps to check if errors are transient, assuming it's not a flaky unit test (which this does not seem to be). |
|
it does seem travis is flaky. ok, thanks! BTW, @dhalperi is it reasonable to move the antlr4 mojo to a separate repo under antlr org do you think? |
We were having some issues with getting grammars to recompile with changed imports. Found a few things, commit by commit.
The existing tests were bogus. Because the test grammars were edited and then restored in a try-with-resources, but then not actually recompiled after being restored, there were false-positives. Specifically the nth variant in the test suite could think that the checksum changed, but it might have changed because of undoing the prior test rather than the current test.
So I fixed this by rerunning state after every try-with-resources and then asserting that the checksums were equal to how they were the first time.
This exposed bugs in the lexer-changes test. First, we were adding a newline to the file which did not change the output checksum. Second, the timestamp modification check (at least on my computer) failed because it was only second-level granularity. Fix both these issues in the next two commits.
For some reason, the dependency analysis was only checking the first import, or assuming only one imported grammar per import statement. It should be checking all imports!
Add tests of the latter.
and of course, add myself to the contributors.
This builds on the really nice work by @marcohu in #1353. Maybe you want to take a look and sanity check me / add color? In any case, thanks for starting this off!
Please take a look and let me know what I can improve!
Thanks! @parrt