Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

maven plugin dependency tracking ignores imported files #1233

Closed
greedy opened this issue Jul 11, 2016 · 2 comments
Closed

maven plugin dependency tracking ignores imported files #1233

greedy opened this issue Jul 11, 2016 · 2 comments

Comments

@greedy
Copy link

greedy commented Jul 11, 2016

The implementation from #1094 doesn't consider the modification times of imported grammar files to determine if the grammar needs to be rebuilt.

I think properly tracking this would require that the antlr tool produce a dependency file that can be consumed by later runs. That's something that might take some time to get implemented, in the meantime it'd be good to have a configuration to the plugin to toggle the "smart" regeneration behavior on/off. It'd be my preference that it defaults to off since that will produce correct results in the default configuration but allow individuals to turn it on if they need faster builds and know it wouldn't cause them any problems.

@marcohu
Copy link
Contributor

marcohu commented Nov 8, 2016

A similar issue: If a grammar is split into lexer and parser grammars and the parser uses the vocab from the lexer (which is probably the norm?), changing only one part does not re-generate the other - possibly leading to errors because the tokens are out-of-sync.

I found this issue so annoying that I patched the plugin to detect non-combined grammars just by naming convention (search for NameLexer.g4 if NameParser.g4 is given and vice versa) and always re-generate both if either is modified.

But a more sophisticated approach like suggested by @greedy that actually analyses the files seems like the right approach.

@marcohu
Copy link
Contributor

marcohu commented Nov 14, 2016

I've added a PR for this that addresses both mentioned use cases: #1353

It seems the used test harness requires Java 8, though. I've ran into dependency issues with the maven test harness and therefore used a different plugin that seems to be better anyway.

As the JDK requirement for the build environment is about to be changed, maybe this is ok. Otherwise I might be able to adjust the unit test. Just let me know. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants