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

Autometrics doesn't compile with Bazel due to CARGO_PKG_REPOSITORY not defined at compile time #176

Closed
marvin-hansen opened this issue May 10, 2024 · 4 comments

Comments

@marvin-hansen
Copy link
Contributor

marvin-hansen commented May 10, 2024

I've a larger mono-repo that builds with Bazel and when adding autometrics as dependency,
compile fails with the error below.

Apparently, the issue is caused because the repo_url in the setting file cannot access the environment variable.
Unlike Cargo, Bazel operates in a hermetic environment meaning Bazel cannot access host environment variables so this is actually expected behavior for hermetic builds.

The file in question:

let repo_url = self

I had similar issues in the past and, by experience, relying on settings stored in an env variables that is expected on the host was almost always problematic, so there are roughly three ways to resolve this issue:

  1. Ensure repo url is initialized during construction. The RAII pattern does makes sense here unless the URL somehow isn't invariant.

  2. Modify the recovery strategy. Currently, the implementation tries to read the env variable like so:

     let repo_url = self
            .repo_url
            .or_else(|| env::var("AUTOMETRICS_REPOSITORY_URL").ok())
            .unwrap_or_else(|| env!("CARGO_PKG_REPOSITORY").to_string());

The problem is that repo_url isn't initialized, but the env variable is inaccessible either from within Bazel.
Therefore, if the repo_url is invariant, you can store it in a constant and use that as a fallback.
If it's not invariant, for whatever reason, you would need to add a small util that constructs the correct repo_url.

  1. The error message suggests to switch to "std::env::var" to read env variables generated at runtime. Give it a try and see if this fixes anything, although I don't think that it resolves the problem that the autometrics repo url is still not initialized prior to accessing it.

Personally, in my project, I rely on configuration as code and only use one single env variable "ENV" to determine in which environment the system runs in order to determine which configuration to load.

I don't necessarily want to write a patch & fill a PR at this time because, quite frankly, I have a hard time understanding how settings configuration works in autometrics let alone how cargo config works in this context. However, If necessary, I can provide a self-contained code example with some instructions how to run the Bazel build if it helps to get this fixed.

The full error log:

 exec env - \
    CARGO_CFG_TARGET_ARCH=aarch64 \
    CARGO_CFG_TARGET_OS=darwin \
    CARGO_CRATE_NAME=autometrics \
    CARGO_MANIFEST_DIR='${pwd}/external/crate_index__autometrics-1.0.1' \
    CARGO_PKG_AUTHORS='' \
    CARGO_PKG_DESCRIPTION='' \
    CARGO_PKG_HOMEPAGE='' \
    CARGO_PKG_NAME=autometrics \
    CARGO_PKG_VERSION=1.0.1 \
    CARGO_PKG_VERSION_MAJOR=1 \
    CARGO_PKG_VERSION_MINOR=0 \
    CARGO_PKG_VERSION_PATCH=1 \
    CARGO_PKG_VERSION_PRE='' \
    OUT_DIR='${pwd}/bazel-out/darwin_arm64-fastbuild/bin/external/crate_index__autometrics-1.0.1/autometrics_bs.out_dir' \
... 
# Execution platform: @@local_config_platform//:host

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
error: environment variable `CARGO_PKG_REPOSITORY` not defined at compile time
   --> external/crate_index__autometrics-1.0.1/src/settings.rs:204:32
    |
204 |             .unwrap_or_else(|| env!("CARGO_PKG_REPOSITORY").to_string());
    |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: Cargo sets build script variables at run time. Use `std::env::var("CARGO_PKG_REPOSITORY")` instead
    = note: this error originates in the macro `env` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 1 previous error
@mellowagain
Copy link
Member

Hi, thanks for the issue! We talked internally and decided that we'll change the code to instead use env_optional! with a empty string as a fallback. In such a situation however, we recommend calling the AutometricsSettingsBuilder to set a repo_url for usage during runtime:

fn main() {
    AutometricsSettings::builder()
        .repo_url("https://github.com/autometrics-dev/autometrics-rs/")
        .init();
    
    // ...
}

In the meantime while we add a fallback, I suggest as a workaround that you define CARGO_PKG_REPOSITORY (can also just be an empty string) in your Bazel build.

@gagbo
Copy link
Member

gagbo commented May 13, 2024

For the record, the reason we thought it was fine was the Cargo book:

Cargo exposes these environment variables to your crate when it is compiled

That’s why we were kind of expecting the variables to be always accessible if cargo is used to compile, even for hermetic builds. The more we know

@marvin-hansen
Copy link
Contributor Author

marvin-hansen commented May 13, 2024 via email

@marvin-hansen
Copy link
Contributor Author

Closing this issue as its currently impossible to compile autometrics with Bazel.

After having worked around the aforementioned environment variable issue, I was running into
99 errors related to conflicting feature flags and versions. It's not exactly clear to me how that is even possible because the crates_repository index in Bazel usually resolves all feature flags correctly so I am really out of my depth here.

However, as a word of warning, whenever you change build environment variables in Bazel, you actually invalidate the cache and trigger a full rebuild plus you have to carry over those variables to test and run targets meaning if your repo is large, you spent the rest of the day watching the compiler....

That is to say, if this ever get touched again, please add a custom constructor and pass in the repo_url as argument, as shown in the example above, to prevent any reliance on env variables as these only cause 99 unnecessary problems at scale.

it's a pity because this is an outstanding crate that adds real value.

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

No branches or pull requests

3 participants