-
Notifications
You must be signed in to change notification settings - Fork 25
feat: add submodule version+data hash info to --version --debug output #87
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
base: main
Are you sure you want to change the base?
Conversation
| iam_policy_autopilot_policy_generation::api::get_botocore_version_info()?; | ||
| println!("{}", crate_version!()); | ||
| println!( | ||
| "boto3 version: commit_id={}, commit_tag={}, data_hash={}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: is this something we'd want to expose to customers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could have a separate command which we hide, that is easier to parse, it could output JSON.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, i'll make a new --debug hidden flag to do this.
i'm against JSON for now (easier to parse line-by-line with sed and | head -n X | tail -n 1), but if we want JSON we can revisit later.
Cargo.toml
Outdated
| chrono = { version = "0.4", features = ["serde"] } | ||
| uuid = { version = "1.8", features = ["v4"] } | ||
| sha2 = "0.10" | ||
| sha2 = "0.10.9" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is causing the OpenSSL build issue. We use rustls-tls in the reqwest dependency to work around a similar issue. I wonder if using use https://docs.rs/aws-lc-rs/1.15.2/aws_lc_rs/digest/fn.digest.html here would resolve the error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like aws-lc-rs also failed the build for the same reason 💀
i resolved the build failure by adding the following to Cargo.toml depencies:
openssl = { version = "0.10", features = ["vendored"] }
|
The PR title will need a |
| #[derive(RustEmbed)] | ||
| #[folder = "target/submodule-version-info"] | ||
| #[include = "*.json"] | ||
| struct GitSubmoduleVersionInfoRaw; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could avoid embedding this data and parsing it in JSON by using the rustc-env instruction in the build script: https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-env - You'd only need to set the environment variables (e.g., BOTO3_SUBMODULE_TAG`) to whatever you compute in the script, then extract them here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i see, that's good to know for the future. I'm opposed to doing that here since there's six such tag/hash/commit values, which might justify putting them in an embed struct instead.
…mments to structs in build.rs
| let mut dir_entry_list = fs::read_dir(cwd)? | ||
| .map(|res| res.map(|e| e.path())) | ||
| .collect::<Result<Vec<_>, io::Error>>()?; | ||
| dir_entry_list.sort(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: why is this necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sha computation recurses the contents of the simplified boto3 and botocore subdirectories, and the ordering of this recursion affects the computed checksum. So the ordering of the recursion needs to be deterministic to make the hash deterministic.
don't forget about this |
Issue #, if available:
#82
Description of changes:
added custom
--versionoutput to publish boto3, botocore git tag+commit+data hash (data hash is the hash of the simplified data files). This information will only be exposed if the hidden--debugflag is set; current --version behaviour without the--deubgflag will remain unchanged.example output
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.