Skip to content

Conversation

@giospada
Copy link
Contributor

@giospada giospada commented Dec 3, 2025

  • Create unified setup_antlr.sh script for ANTLR dependency
  • Add setup-antlr make target for initialization
  • Update Dockerfile to use unified setup script
  • Add pre_push make target with all required checks
  • Update CONTRIBUTING.md with pre-push instructions
  • moved gen_proto.sh and update_proto.sh

antlr: setup-antlr
cd third_party/substrait/grammar \
&& java -jar ${ANTLR_JAR} -o ../../../src/substrait/gen/antlr -Dlanguage=Python3 SubstraitType.g4 \
&& java -jar ../../../lib/antlr-complete.jar -o ../../../src/substrait/gen/antlr -Dlanguage=Python3 SubstraitType.g4 \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand why this hardcoded path will always work..

Copy link
Contributor Author

@giospada giospada Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script is designed to download the ANTLR JAR file to that specific, project dir/lib. This approach is consistent with the relative path used for the ../../../src directory.

@tokoko
Copy link
Contributor

tokoko commented Dec 25, 2025

@giospada this looks mergeable at the moment, but the thing is I'm thinking of changing the whole codegen setup to use pixi to avoid running the devcontainer in ci for codegen check. I checked and all the components we care about are available as conda packages (openjdk, libprotobuf, antlr, buf).

I can merge this as-is before that happens if it's causing you any headache though.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants