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

Environment values should not be VERGEN_IDEMPOTENT_OUTPUT on failure #299

Open
holly-hacker opened this issue Jan 19, 2024 · 5 comments
Open

Comments

@holly-hacker
Copy link

I am using the following configuration:

pub fn main() {
    vergen::EmitBuilder::builder()
        .idempotent()
        .quiet()
        .all_cargo()
        .all_git()
        .git_describe(true, false, None)
        .all_rustc()
        .emit()
        .unwrap();
}

Due to an error on my end, there was no .git folder in my build environment which means the git env vars could not be populated correctly. However, the value of each git environment value was VERGEN_IDEMPOTENT_OUTPUT which is very unexpected and cost me a lot of time troubleshooting in the wrong direction. Ideally, these should be filled with another value to indicate the error.

I understand that the .quiet() call and lack of .fail_on_error() make this error harder to spot, but I still think the current behavior is very weird.

@CraZySacX
Copy link
Member

This behavior is intentional. As a build tool, I don't want vergen to block your build if something has failed internally, unless specifically configured. So by default vergen will populate with those values unless you turn on fail_on_error. With quiet off, you would get warnings that those values had been replaced. I recommend developing with fail_on_error turned on and quiet off. When you reach a stable state, flip them off.

@holly-hacker
Copy link
Author

I understand that it is intentional to fill in some value rather than fail by default, but this issue is to report that it should be a different value that indicates that the value is filled in because of an error, not because the value is not idempotent. It doesn't make sense for VERGEN_GIT_SHA to ever be VERGEN_IDEMPOTENT_OUTPUT because the value should always be stable.

@teythoon
Copy link

teythoon commented May 6, 2024

I also find the behavior not helpful. We use VERGEN_GIT_SHA to compute a version string. If we build from a tarball, with .git being absent, we get a string like 1.8.1-VERGEN_IDEMPOTENT_OUTPUT+sequoia-openpgp-1.19.0.

In previous versions of vergen, the environment variable was simply absent. I think I'll try to go back to this behavior by opting in to the errors, and ignoring them.

@kingwingfly
Copy link

I think that leaving env variable unset and only showing a warning may be better. In my case, if there's no git tree, all I could do is to panic during building or got a meaningless VERGEN_IDEMPOTENT_OUTPUT.

Currently, my solution is separating EmitBuilder apart:

EmitBuilder::builder()
        .fail_on_error()
        .git_describe(false, true, None)
        .git_commit_timestamp()
        .emit()
        .ok();
    EmitBuilder::builder()
        .rustc_host_triple()
        .rustc_channel()
        .rustc_semver()
        .emit()
        .unwrap();

And got version str by option_env!:

const VERSION: &str = const_format::formatcp!(
    "{} {}\nRUSCT {} {} {}",
    match option_env!("VERGEN_GIT_DESCRIBE") {
        Some(var) => var,
        _ => concat!(env!("CARGO_PKG_VERSION"), "+"),
    },
    match option_env!("VERGEN_GIT_COMMIT_TIMESTAMP") {
        Some(var) => var,
        _ => "unknown-timestamp",
    },
    env!("VERGEN_RUSTC_HOST_TRIPLE"),
    env!("VERGEN_RUSTC_CHANNEL"),
    env!("VERGEN_RUSTC_SEMVER")
);

@CraZySacX
Copy link
Member

Here is some additional background on this "feature". There was an issue with packages built from tarballs where the user didn't have control over the source code (in this particular case a Linux package maintainer) and the git variables were causing failures because they weren't building from a repository. In parallel, there was a request to be able to force idempotent (or as close as possible with Rust) builds that didn't include non-idempotent items such as timestamps every time the code was built. This solution covered both bases.

I can see how this fix of two issues with one type of output is causing some confusion in the regular use case of the tool. I'll probably tweak the "don't fail unless I've requested it" output to be different from the force idempotent output.

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

4 participants