-
Notifications
You must be signed in to change notification settings - Fork 365
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
output configured targets #1240
base: main
Are you sure you want to change the base?
Conversation
Emilgardis
commented
Apr 3, 2023
•
edited
Loading
edited
dc4dfa5
to
4e9e6ca
Compare
@mcandre does this work for you? I can add more data to the array |
.iter() | ||
.filter_map(|i| { | ||
Some(Target { | ||
triplet: Some(i.name).filter(|i| *i != "zig")?.to_owned(), |
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 a nice usage of try trait :D
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.
triplet: Some(i.name).filter(|i| *i != "zig")?.to_owned(), | |
triplet: if *i.name != "zig" { i.name.to_owned() } else { return None }, |
.collect::<Vec<_>>() | ||
}) | ||
.filter(|v| !v.is_empty()) | ||
.unwrap_or_else(|| vec![docker::ImagePlatform::DEFAULT.to_string()]), |
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 meant to replicate what happens when we do https://github.com/Emilgardis/cross/blob/list-supported-targets/src/docker/image.rs#L30
#[derive(Args, Debug)] | ||
pub struct ListTargets { | ||
/// Format version | ||
#[clap(long)] | ||
format_version: Option<FormatVersion>, | ||
/// Coloring: auto, always, never | ||
#[clap(long)] | ||
pub color: Option<String>, | ||
} |
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.
should there be a way to only output targets in toml config? Maybe that could be data included in the output?
/// Format version | ||
#[clap(long)] | ||
format_version: Option<FormatVersion>, |
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.
do we really need this kind of stability?
For my purposes, I would like to see the support tier number included in the output. Perhaps a field like If possible, I would prefer a flatter array design, that doesn't restate some of the triple and platform information. I just want to be able to associate triple with support tier number. If we go with JSON, then I would like to see all cross data models exported as a stable Rust library. The current policy treats only the CLI as stable. Until the data models and schemas (e.g., the entire Cross.toml schema) enter a stable library, I would actually prefer to see non-JSON in cross CLI tool output. That is, |
The tier is not relevant to cross, all targets emitted are the targets that actually has been setup for the project. i.e not every tier 1 and tier 2 target are emitted, only targets in any tier, including custom targets when specified in a projects config, are emitted iff they are supported by cross or have been setup to work with cross in the project which i.e # Cross.toml
[target."my-custom-target.json"]
image = "my-image:latest"
build-std = false
[target.aarch64-apple-ios]
image = "aarch64-apple-ios-cross:local" would output with
That's the entire reason behind |