Skip to content
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
10 changes: 10 additions & 0 deletions src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ async fn install_prebuilt(config: &Config, args: &Cli) -> Result<()> {
}

async fn install_from_local(config: &Config, local_path: &Path, args: &Cli) -> Result<()> {
need_cmd("cargo")?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this still runs after startup side effects: main has already migrated config, started the update check, printed the banner, checked running bins, and install::run has created dirs. if the goal is to fail before doing any work, this check likely needs to be hoisted to the top-level dispatch.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Confirmed live: with cargo missing, foundryup --path ... still prints the banner and creates .foundry/{bin,versions,share/man/man1} before need 'cargo'; with git missing, foundryup --branch ... does the same before need 'git'. So this thread is still relevant.

Tests: targeted tests pass; full suite passes when foundry_bins is run serially.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think thats fine


if args.repo.is_some() || args.branch.is_some() || args.version.is_some() {
warn!("--branch, --install, --use, and --repo arguments are ignored during local install");
}
Expand Down Expand Up @@ -134,6 +136,9 @@ fn profile_target_dir(profile: &str) -> &str {
}

async fn install_from_source(config: &Config, repo: &str, args: &Cli) -> Result<()> {
need_cmd("git")?;
need_cmd("cargo")?;

let branch = if let Some(pr) = args.pr {
format!("refs/pull/{pr}/head")
} else {
Expand Down Expand Up @@ -721,6 +726,11 @@ fn bin_name(name: &str) -> String {
if cfg!(windows) { format!("{name}.exe") } else { name.to_string() }
}

/// Fails with a clear error if `cmd` is not available on `PATH`.
fn need_cmd(cmd: &str) -> Result<()> {
which::which(cmd).map(|_| ()).map_err(|_| eyre::eyre!("need '{cmd}' (command not found)"))
}

/// Removes `path` if it exists, including dangling symlinks.
///
/// Uses `symlink_metadata` so a broken symlink is still detected and removed;
Expand Down
46 changes: 23 additions & 23 deletions src/platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,26 +56,21 @@ pub(crate) enum Arch {
}

impl Arch {
pub(crate) fn detect() -> Result<Self> {
let arch = std::env::consts::ARCH;
match arch {
"x86_64" => {
if is_rosetta() {
Ok(Self::Arm64)
} else {
Ok(Self::Amd64)
}
}
"aarch64" | "arm64" => Ok(Self::Arm64),
_ => bail!("unsupported architecture: {arch}"),
}
pub(crate) fn detect() -> Self {
Self::normalize(std::env::consts::ARCH)
}

pub(crate) fn from_str(s: &str) -> Result<Self> {
pub(crate) fn from_str(s: &str) -> Self {
Self::normalize(s)
}

/// Normalizes an arch name, defaulting to `amd64` for anything unrecognized.
/// A literal `x86_64` resolves to `arm64` when running under Rosetta.
fn normalize(s: &str) -> Self {
match s.to_lowercase().as_str() {
"amd64" | "x86_64" | "x64" => Ok(Self::Amd64),
"arm64" | "aarch64" => Ok(Self::Arm64),
_ => bail!("unsupported architecture: {s}"),
"x86_64" if is_rosetta() => Self::Arm64,
"arm64" | "aarch64" => Self::Arm64,
_ => Self::Amd64,
}
}

Expand Down Expand Up @@ -103,8 +98,8 @@ impl Target {
None => Platform::detect()?,
};
let arch = match arch_override {
Some(a) => Arch::from_str(a)?,
None => Arch::detect()?,
Some(a) => Arch::from_str(a),
None => Arch::detect(),
};
Ok(Self { platform, arch })
}
Expand Down Expand Up @@ -155,9 +150,14 @@ mod tests {

#[test]
fn arch_from_str_cases() {
assert_eq!(Arch::from_str("amd64").unwrap(), Arch::Amd64);
assert_eq!(Arch::from_str("x86_64").unwrap(), Arch::Amd64);
assert_eq!(Arch::from_str("arm64").unwrap(), Arch::Arm64);
assert_eq!(Arch::from_str("aarch64").unwrap(), Arch::Arm64);
assert_eq!(Arch::from_str("amd64"), Arch::Amd64);
assert_eq!(Arch::from_str("arm64"), Arch::Arm64);
assert_eq!(Arch::from_str("aarch64"), Arch::Arm64);
// Unknown values fall back to amd64 rather than erroring.
assert_eq!(Arch::from_str("riscv64"), Arch::Amd64);
// `x86_64` is amd64 unless running under Rosetta (not the case in tests).
if !super::is_rosetta() {
assert_eq!(Arch::from_str("x86_64"), Arch::Amd64);
}
}
}
Loading