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

fix: trin cli flag displays wrong version info #1615

Merged
merged 2 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 51 additions & 81 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions ethportal-api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
name = "ethportal-api"
version = "0.4.0"
description = "Definitions for various Ethereum Portal Network JSONRPC APIs"
build = "build.rs"
authors.workspace = true
categories.workspace = true
edition.workspace = true
Expand Down Expand Up @@ -45,7 +44,6 @@ serde-this-or-that.workspace = true
serde_json.workspace = true
sha2 = "0.10.1"
sha3.workspace = true
shadow-rs = "0.27"
ssz_types.workspace = true
superstruct = "0.7.0"
thiserror.workspace = true
Expand All @@ -67,4 +65,4 @@ tracing.workspace = true
tracing-subscriber.workspace = true

[build-dependencies]
shadow-rs = "0.27"
vergen = { version = "8.0.0", features = ["build", "cargo", "git", "gitcl", "rustc"] }
47 changes: 13 additions & 34 deletions ethportal-api/build.rs
Original file line number Diff line number Diff line change
@@ -1,36 +1,15 @@
use std::{fs::File, io::Write};

use shadow_rs::SdResult;

fn main() -> SdResult<()> {
shadow_rs::new_hook(hook)?;

Ok(())
}

fn hook(mut file: &File) -> SdResult<()> {
let env_var_git_hash = std::env::var("GIT_HASH").unwrap_or_default();
writeln!(file, "const ENV_GIT_HASH: &str = \"{}\";", env_var_git_hash)?;

hook_method(file)?;

Ok(())
}

fn hook_method(mut file: &File) -> SdResult<()> {
let hook_fn = r#"
pub const fn short_commit() -> &'static str {
if shadow_rs::str_get!(SHORT_COMMIT, 0).is_some() {
return SHORT_COMMIT;
}
if shadow_rs::str_get!(ENV_GIT_HASH, 0).is_some() {
ENV_GIT_HASH
} else {
"unknown"
}
}"#;

writeln!(file, "{}", hook_fn)?;
use std::error::Error;

use vergen::EmitBuilder;

fn main() -> Result<(), Box<dyn Error>> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you're going to either have to set a GIT_HASH env var here, or update our 2 dockerfiles so that it'll pick up the env var here

Copy link
Member Author

Choose a reason for hiding this comment

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

https://github.com/sigp/lighthouse/blob/stable/Dockerfile
https://github.com/paradigmxyz/reth/blob/main/Dockerfile

both Lighthouse and Reth copy the whole folder, if we do that it would resolve this issue and we could remove

trin/docker/Dockerfile

Lines 8 to 11 in ddbd9ff

# Docker build command *SHOULD* include --build-arg GIT_HASH=...
# eg. --build-arg GIT_HASH=$(git rev-parse HEAD)
ARG GIT_HASH=unknown
ENV GIT_HASH=$GIT_HASH

and passing the git hash in with the CI,

As only copying in the crates would be problematic for vergen

Reth uses this https://github.com/LukeMathWalker/cargo-chef?tab=readme-ov-file#benefits-of-cargo-chef package, which I think will resolve our caching problems we were trying to solve by copying crates individually, this should also make it so we have to update our crates less so the code is more maintainable.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will make a PR updating our dockerfiles with what I said above in mind

Copy link
Member Author

Choose a reason for hiding this comment

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

#1619

Here is a PR which resolves your concern, once both of these PR's are merged it will work as expected

EmitBuilder::builder()
.git_sha(true)
.git_describe(false, true, None)
.build_timestamp()
.rustc_semver()
.cargo_features()
.cargo_target_triple()
.emit()?;
Ok(())
}
2 changes: 0 additions & 2 deletions ethportal-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,3 @@ pub use types::{
portal::{RawContentKey, RawContentValue},
};
pub use web3::{Web3ApiClient, Web3ApiServer};

shadow_rs::shadow!(build_info);
21 changes: 3 additions & 18 deletions ethportal-api/src/types/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use clap::{
use url::Url;

use crate::{
build_info,
types::{bootnodes::Bootnodes, network::Subnetwork},
version::FULL_VERSION,
};

pub const DEFAULT_WEB3_IPC_PATH: &str = "/tmp/trin-jsonrpc.ipc";
Expand Down Expand Up @@ -56,22 +56,6 @@ impl FromStr for Web3TransportType {
}

const APP_NAME: &str = "trin";
const VERSION: &str = const_format::formatcp!(
"{version}-{hash} {build_os} {rust_version}",
// Remove -alpha.1 versioning if it is present.
// This must be done as it can conflict with eth versioning
version = const_format::str_split!(build_info::PKG_VERSION, '-')[0],
hash = build_info::short_commit(),
build_os = build_info::BUILD_OS,
// the rust version looks like that:
// rustc 1.77.0 (aedd173a2 2024-03-17)
// we remove everything in the brackets and replace spaces with nothing
rust_version = const_format::str_replace!(
const_format::str_split!(build_info::RUST_VERSION, '(')[0],
' ',
""
)
);

/// The storage capacity configurtion.
#[derive(Debug, Clone, PartialEq, Eq)]
Expand All @@ -91,7 +75,8 @@ pub enum StorageCapacityConfig {
#[command(name = APP_NAME,
author = "https://github.com/ethereum/trin/graphs/contributors",
about = "Run an eth portal client",
version = VERSION)]
version = FULL_VERSION
)]
pub struct TrinConfig {
#[arg(
default_value = DEFAULT_WEB3_TRANSPORT,
Expand Down
29 changes: 28 additions & 1 deletion ethportal-api/src/version.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,31 @@
/// The latest git commit hash of the build.
pub const TRIN_FULL_COMMIT: &str = env!("VERGEN_GIT_SHA");
pub const TRIN_SHORT_COMMIT: &str = const_format::str_index!(TRIN_FULL_COMMIT, ..8);

/// Trin's version is the same as the git tag.
pub const TRIN_VERSION: &str = const_format::str_split!(env!("VERGEN_GIT_DESCRIBE"), '-')[0];

/// The operating system of the build, linux, macos, windows etc.
pub const BUILD_OPERATING_SYSTEM: &str =
const_format::str_split!(env!("VERGEN_CARGO_TARGET_TRIPLE"), "-")[2];

/// The architecture of the build, x86_64, aarch64, etc.
pub const BUILD_ARCHITECTURE: &str =
const_format::str_split!(env!("VERGEN_CARGO_TARGET_TRIPLE"), "-")[0];

// /// The version of the programming language used to build the binary.
pub const PROGRAMMING_LANGUAGE_VERSION: &str = env!("VERGEN_RUSTC_SEMVER");

pub const FULL_VERSION: &str = const_format::formatcp!(
"{version}-{hash} {build_os}-{build_arch} rustc{rust_version}",
version = TRIN_VERSION,
hash = TRIN_SHORT_COMMIT,
build_os = BUILD_OPERATING_SYSTEM,
build_arch = BUILD_ARCHITECTURE,
rust_version = PROGRAMMING_LANGUAGE_VERSION
);

/// Returns the trin version and git revision.
pub const fn get_trin_version() -> &'static str {
crate::build_info::short_commit()
TRIN_SHORT_COMMIT
}