-
Notifications
You must be signed in to change notification settings - Fork 2
Changes to be compatible with crate2nix #143
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
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.
Left some comments.
crate_description!(), | ||
crate_version!(), | ||
built_info::GIT_VERSION, | ||
built_info::TARGET, | ||
built_info::TARGET_PLATFORM.unwrap_or("unknown target"), |
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 print_startup_string function is superseded by tracing::info!
:
commons-operator/rust/operator-binary/src/main.rs
Lines 58 to 66 in c276cec
tracing::info!( | |
built_info.pkg_version = built_info::PKG_VERSION, | |
built_info.git_version = built_info::GIT_VERSION, | |
built_info.target = built_info::TARGET, | |
built_info.built_time_utc = built_info::BUILT_TIME_UTC, | |
built_info.rustc_version = built_info::RUSTC_VERSION, | |
"Starting {description}", | |
description = built_info::PKG_DESCRIPTION | |
); |
|
||
#[derive(Parser)] | ||
#[clap(about = built_info::PKG_DESCRIPTION, author = stackable_operator::cli::AUTHOR)] | ||
#[clap(about, author)] |
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.
This is already done:
#[clap(about, author)] |
|
||
mod built_info { | ||
include!(concat!(env!("OUT_DIR"), "/built.rs")); | ||
pub const TARGET_PLATFORM: Option<&str> = option_env!("TARGET"); |
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.
built_info seems to already export TARGET. But maybe that's different to what you want here?
See:
commons-operator/rust/operator-binary/src/main.rs
Lines 21 to 23 in c276cec
mod built_info { include!(concat!(env!("OUT_DIR"), "/built.rs")); } built_info.target = built_info::TARGET,
use stackable_operator::cli::{Command, ProductOperatorRun}; | ||
|
||
use clap::Parser; | ||
use clap::{crate_description, crate_version, Parser}; |
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 get this info from built_info. Is there a difference?
// built's env module depends on a whole bunch of variables that crate2nix doesn't provide | ||
// so we grab the specific env variables that we care about out ourselves instead. |
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.
Ohhh. This might make my comments useless. But who knows, maybe it works now?
see stackabletech/operator-templating#212 for more details