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

Update to vergen 5, add branch, commit time, and build target to the panic metadata, automatically update app version from crate version #2029

Merged
merged 8 commits into from
Apr 19, 2021

Conversation

fanatid
Copy link
Contributor

@fanatid fanatid commented Apr 18, 2021

Motivation

Add more metadata with vergen crate.

Solution

Update vergen to version 5.

Review

I wanted add rustc metadata but this requires sentry update for the new version of rustc_version.

Related Issues

Follow Up Work

Update sentry crate (require tokio 1 😢 + sentry-tracing update: kellpossible/sentry-tracing#1) and add rustc metadata.

("commit timestamp", env!("VERGEN_GIT_COMMIT_TIMESTAMP")),
("target triple", env!("VERGEN_CARGO_TARGET_TRIPLE")),
("branch", env!("VERGEN_GIT_BRANCH")),
("Zcash network", (&config.network.network).into()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks I've been waiting for these for a while.

@teor2345 teor2345 added A-dependencies Area: Dependency file updates A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup C-enhancement Category: This is an improvement P-Medium labels Apr 18, 2021
@teor2345 teor2345 added this to the 2021 Sprint 8 milestone Apr 18, 2021
@teor2345
Copy link
Contributor

I edited this line, because GitHub reads "partially resolve #ticket" as "resolve #ticket", and auto-closes the ticket.

@teor2345
Copy link
Contributor

Yeah we really do need to move to the latest cc crate version, it's blocking a bunch of changes.

@mpguerra mpguerra removed this from the 2021 Sprint 8 milestone Apr 19, 2021
The zebrad code automatically uses the crate version now.
zebrad/src/application.rs Outdated Show resolved Hide resolved
@teor2345 teor2345 changed the title More metadata with vergen Update to vergen 5, add branch, commit time, and build target to the panic metadata, automatically update app version command Apr 19, 2021
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Looks good, let's merge if CI passes.

@teor2345 teor2345 enabled auto-merge (squash) April 19, 2021 11:08
@teor2345 teor2345 changed the title Update to vergen 5, add branch, commit time, and build target to the panic metadata, automatically update app version command Update to vergen 5, add branch, commit time, and build target to the panic metadata, automatically update app version from crate version Apr 19, 2021
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Looks like we might need a metrics-exporter-prometheus update before we can merge this change:

error: failed to select a version for `cc`.
    ... required by package `libgit2-sys v0.12.18+1.1.0`
    ... which is depended on by `git2 v0.13.17`
    ... which is depended on by `vergen v5.1.1`
    ... which is depended on by `zebrad v1.0.0-alpha.6 (/zebra/zebrad)`
versions that meet the requirements `^1.0.43` are: 1.0.67, 1.0.66, 1.0.65, 1.0.64, 1.0.63, 1.0.62, 1.0.61, 1.0.60, 1.0.59, 1.0.58, 1.0.57, 1.0.56, 1.0.55, 1.0.54, 1.0.53, 1.0.52, 1.0.51, 1.0.50, 1.0.49, 1.0.48, 1.0.47, 1.0.46, 1.0.45, 1.0.44, 1.0.43

all possible versions conflict with previously selected packages.

  previously selected package `cc v1.0.41`
    ... which is depended on by `generator v0.6.23`
    ... which is depended on by `loom v0.3.6`
    ... which is depended on by `sharded-slab v0.1.0`
    ... which is depended on by `metrics v0.13.0-alpha.8 (https://github.com/ZcashFoundation/metrics?rev=971133128e5aebe3ad177acffc6154449736cfa2#97113312)`
    ... which is depended on by `metrics-exporter-prometheus v0.1.0-alpha.7 (https://github.com/ZcashFoundation/metrics?rev=971133128e5aebe3ad177acffc6154449736cfa2#97113312)`
    ... which is depended on by `zebrad v1.0.0-alpha.6 (/zebra/zebrad)`

auto-merge was automatically disabled April 19, 2021 12:17

Head branch was pushed to by a user without write access

@teor2345
Copy link
Contributor

Looks like we might need a metrics-exporter-prometheus update before we can merge this change:

Here's what I tried:

Merge the most recent version of metrics-exporter-prometheus that doesn't use tokio-1.0 with the tokio-0.3 changes in the Foundation's metrics fork:
https://github.com/teor2345/metrics/tree/tokio-0.3%2Bmetrics-exporter-prometheus-v0.1.0-alpha.12

Update Zebra to use that branch:
https://github.com/teor2345/zebra/tree/vergen

But I get a metrics exporter acceptance test failure - it doesn't provide any output.

We had this issue once before when there was a version mismatch in some dependencies, so it should be fixable. But I might not have time to look at it for a few days.

@teor2345
Copy link
Contributor

We had this issue once before when there was a version mismatch in some dependencies, so it should be fixable. But I might not have time to look at it for a few days.

It was #1349, duplicate metrics dependencies. Seems like my attempt had similar issues.

@fanatid
Copy link
Contributor Author

fanatid commented Apr 19, 2021

We do not need to update metrics-exporter-prometheus for the new vergen.

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Looks like merging the latest main branch fixed it all up.

@teor2345 teor2345 merged commit 43e792b into ZcashFoundation:main Apr 19, 2021
@fanatid fanatid deleted the vergen branch April 20, 2021 05:09
@dconnolly
Copy link
Contributor

dconnolly commented Apr 20, 2021

Looks like this is broken inside a gcloud build because the source is copied into a container image build and the git info injected as env vars, with no .git or associated env data present: #2037

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependencies Area: Dependency file updates A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to vergen 5
4 participants