Skip to content

Conversation

@marshallpierce
Copy link
Contributor

If the pdlc binary is not available, use pdl_compiler as a library instead.

Also do some misc tidying of the build script, since it looks like the output path was being derived in different ways with the same result.

@marshallpierce
Copy link
Contributor Author

May not be needed after google/pdl#61.

- name: Install dependencies
run: |
cargo install pdl-compiler --version 0.2.2
cargo install pdl-compiler --version 0.2.3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm assuming it's desirable for pdl-compiler to be up to date -- if you'd rather stick with 0.2.2 that's fine too of course


// Find the pdl tool. Expecting it at CARGO_HOME/bin
let pdl = match env::var("CARGO_HOME") {
let pdlc = match env::var("CARGO_HOME") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it even desirable to maintain this code path? If someone is using Cargo vs the Blaze build, when would they want to use the external pdlc? Or perhaps should it be present to allow hypothetical debugging of issues specific to a pdlc binary, but only used if an env var is set?

@marshallpierce marshallpierce marked this pull request as ready for review February 26, 2024 16:52

- name: Check Cargo build
run: |
cargo test
Copy link
Contributor Author

Choose a reason for hiding this comment

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

might as well check that the cargo build works 🤷

If the `pdlc` binary is not available, use `pdl_compiler` as a library
instead.

Also do some misc tidying of the build script, since it looks like the
output path was being derived in different ways with the same result.
@hchataing
Copy link
Collaborator

This change is obsolete, has been applied independently on AOSP.
Another change to update the bazel build to remove the dependency on pdlc in the environment would be the logical follow up.

@hchataing hchataing closed this Oct 16, 2025
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