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

Support arch-os-prefixed PROTOC in prost_build (e.g: AARCH64_LINUX_PROTOC) #1083

Open
daprilik opened this issue Jun 14, 2024 · 6 comments
Open

Comments

@daprilik
Copy link

#283 introduced the PROTOC env var, which our project uses to great effect via the [env] table in .cargo/config.toml, ensuring that all folks working on the project use a consistent pre-packaged version of protoc (that we clone as part of our repo dev-env setup).

Unfortunately, our project is multi-platform and multi-arch, and needs to build across both windows and linux, as well as x64 and aarch64.

This fact makes it a bit awkward to use PROTOC as we are using it, as until rust-lang/cargo#10273 is resolved, we are forced to use some sort of external script to symlink the correct protoc binary for a given build platform into a known-good directory, and then have PROTOC point at the symlink, which can get a bit finicky across multiple platforms.

Would it be possible to support arch-os-prefixed variants of of PROTOC in prost_build? i.e: if PROTOC isn't found, fall-back to checking for the existence of a $ARCH_$OS_PROTOC env-var?

This would allow us to do something like this in our [env] table:

[env]
X86_64_LINUX_PROTOC = { value = ".packages/protobuf/tools/x64-linux/protoc", relative = true }
X86_64_WINDOWS_PROTOC = { value = ".packages/protobuf/tools/x64-windows/protoc.exe", relative = true }
AARCH64_LINUX_PROTOC = { value = ".packages/protobuf/tools/aarch64-linux/protoc", relative = true }
AARCH64_WINDOWS_PROTOC = { value = ".packages/protobuf/tools/aarch64-windows/protoc.exe", relative = true }

...thereby avoiding the need to do any additional symlinking.

For some prior art: the openssl crate supports this exact feature: https://docs.rs/openssl/latest/openssl/#manual

@caspermeijn
Copy link
Collaborator

I believe the openssl example is different from that is needed for prost. The needed protoc binary is based on the host architecture and not the target architecture, right?

Could you solve your problem with a build.rs script? With cargo::rustc-env=VAR=VALUE you can set and environment variable. In the build script your can detect which protoc binary is needed.

@daprilik
Copy link
Author

daprilik commented Jul 3, 2024

Unfortunately, a build-script based approach doesn't work here, as if the prost dependency is in a downstream crate, setting the env as part of a upstream build.rs won't percolate down the dependency chain.

The needed protoc binary is based on the host architecture and not the target architecture, right?

Right, but that doesn't actually change the usefulness of this proposed solution. We still want some way to uniformly set a path to a host-specific pre-packaged protoc executable via the [env] table.

@caspermeijn
Copy link
Collaborator

Yeah, you are right. cargo::rustc-env=VAR=VALUE will not work for you.

If you call prost_build::Config::compile_protos() somewhere in your own codebase, then you can set the correct PROTOC env var before that. For example: std::env::set_var("PROTOC", "/tmp/hello_world");

@daprilik
Copy link
Author

@caspermeijn, unless I'm misunderstanding your suggestion, I don't think that's a valid solution either.

It suffers from the same issue as the rustc-env=VAR=VALUE issue, whereby the env::set_var would only ever apply to the single build.rs file that is invoking compile_protos. Even if I use set_var in one of the build.rs files I control, it won't propagate the var down the dependency chain.

@caspermeijn
Copy link
Collaborator

Sorry, now I understand your problem. You don't own all the crates that use prost in your dependency chain.

Personally, I would make sure the path of the development environment is set correctly. Then the PROTOC env var is not needed at all. But I understand your use case and there is a reason that the PROTOC variable exists.

I am open to the idea of checking another environment variable while determining the protoc executable path based on the compiler host arch. It needs to be well documented and use the same names as the rust compiler uses to indicate the targets. I have no idea how to retreive those identifiers.

Are you willing to do the work and open a PR for this feature?

@daprilik
Copy link
Author

Sorry, now I understand your problem. You don't own all the crates that use prost in your dependency chain.

Indeed...

I have no idea how to retrieve those identifiers.

It seems like it might be possible to use the HOST environment variable set during build.rs compilation to retrieve this info? https://doc.rust-lang.org/stable/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts

Are you willing to do the work and open a PR for this feature?

Unfortunately, since the symlink-based workaround does "work" for us at the moment, and I've got a veritable avalanche of work to churn thought nowadays, I can't commit to implementing this feature in prost at this time. My sincere apologies 🥲

By opening the issue, I wanted to draw attention to this potential feature gap, in case there were any other folks who were running into the same issue.

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

2 participants