Skip to content

Commit

Permalink
Improve artifact compatibility by adding a fallback mechanism (#38)
Browse files Browse the repository at this point in the history
  • Loading branch information
filiptibell authored Jul 14, 2024
1 parent 32437d3 commit 94b4c0e
Show file tree
Hide file tree
Showing 10 changed files with 153 additions and 55 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Failed to parse tool specifications in foreman.toml if they were an inline table ([#36])
- Fixed tool specifications failing to parse in `foreman.toml` when using inline tables ([#36])
- Fixed tools not specifying architectures (such as `wally-macos.zip`) failing to install ([#38])

[#36]: https://github.com/rojo-rbx/rokit/pull/36
[#38]: https://github.com/rojo-rbx/rokit/pull/38

## `0.1.4` - July 11th, 2024

Expand Down
46 changes: 23 additions & 23 deletions lib/descriptor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub enum DescriptionParseError {
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct Descriptor {
os: OS,
arch: Arch,
arch: Option<Arch>,
toolchain: Option<Toolchain>,
}

Expand All @@ -41,7 +41,7 @@ impl Descriptor {
pub fn current_system() -> Self {
Self {
os: OS::current_system(),
arch: Arch::current_system(),
arch: Some(Arch::current_system()),
toolchain: Toolchain::current_system(),
}
}
Expand All @@ -55,7 +55,7 @@ impl Descriptor {
let search_string = search_string.as_ref();

let os = OS::detect(search_string)?;
let arch = Arch::detect(search_string).unwrap_or_default();
let arch = Arch::detect(search_string);
let toolchain = Toolchain::detect(search_string);

Some(Self {
Expand All @@ -76,7 +76,7 @@ impl Descriptor {
let (os, arch) = parse_executable(binary_contents)?;
Some(Self {
os,
arch,
arch: Some(arch),
toolchain: None,
})
}
Expand All @@ -93,7 +93,7 @@ impl Descriptor {
Get the architecture of this description.
*/
#[must_use]
pub fn arch(&self) -> Arch {
pub fn arch(&self) -> Option<Arch> {
self.arch
}

Expand Down Expand Up @@ -125,9 +125,9 @@ impl Descriptor {
// ... or special cases for architecture compatibility
|| matches!(
(self.os, self.arch, other.arch),
(OS::Windows, Arch::X64, Arch::X86)
| (OS::Linux, Arch::X64, Arch::X86)
| (OS::MacOS, Arch::Arm64, Arch::X64)
(OS::Windows, Some(Arch::X64), Some(Arch::X86))
| (OS::Linux, Some(Arch::X64), Some(Arch::X86))
| (OS::MacOS, Some(Arch::Arm64), Some(Arch::X64))
)
)
}
Expand Down Expand Up @@ -170,7 +170,7 @@ impl FromStr for Descriptor {
type Err = DescriptionParseError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
let os = OS::detect(s).ok_or(DescriptionParseError::OS)?;
let arch = Arch::detect(s).unwrap_or_default();
let arch = Arch::detect(s);
let toolchain = Toolchain::detect(s);

Ok(Self {
Expand Down Expand Up @@ -201,7 +201,7 @@ mod tests {
fn current_description() {
let current = Descriptor::current_system();
assert_eq!(current.os, OS::current_system());
assert_eq!(current.arch, Arch::current_system());
assert_eq!(current.arch, Some(Arch::current_system()));
assert_eq!(current.toolchain, Toolchain::current_system());
}

Expand All @@ -212,39 +212,39 @@ mod tests {
"windows-x64-msvc",
Descriptor {
os: OS::Windows,
arch: Arch::X64,
arch: Some(Arch::X64),
toolchain: Some(Toolchain::Msvc),
},
);
check_desc(
"win64",
Descriptor {
os: OS::Windows,
arch: Arch::X64,
arch: Some(Arch::X64),
toolchain: None,
},
);
check_desc(
"windows-x86-gnu",
Descriptor {
os: OS::Windows,
arch: Arch::X86,
arch: Some(Arch::X86),
toolchain: Some(Toolchain::Gnu),
},
);
check_desc(
"windows-x86",
Descriptor {
os: OS::Windows,
arch: Arch::X86,
arch: Some(Arch::X86),
toolchain: None,
},
);
check_desc(
"win32",
Descriptor {
os: OS::Windows,
arch: Arch::X86,
arch: Some(Arch::X86),
toolchain: None,
},
);
Expand All @@ -253,23 +253,23 @@ mod tests {
"aarch64-macos",
Descriptor {
os: OS::MacOS,
arch: Arch::Arm64,
arch: Some(Arch::Arm64),
toolchain: None,
},
);
check_desc(
"macos-x64-gnu",
Descriptor {
os: OS::MacOS,
arch: Arch::X64,
arch: Some(Arch::X64),
toolchain: Some(Toolchain::Gnu),
},
);
check_desc(
"macos-x64",
Descriptor {
os: OS::MacOS,
arch: Arch::X64,
arch: Some(Arch::X64),
toolchain: None,
},
);
Expand All @@ -278,23 +278,23 @@ mod tests {
"linux-x86_64-gnu",
Descriptor {
os: OS::Linux,
arch: Arch::X64,
arch: Some(Arch::X64),
toolchain: Some(Toolchain::Gnu),
},
);
check_desc(
"linux-gnu-x86",
Descriptor {
os: OS::Linux,
arch: Arch::X86,
arch: Some(Arch::X86),
toolchain: Some(Toolchain::Gnu),
},
);
check_desc(
"armv7-linux-musl",
Descriptor {
os: OS::Linux,
arch: Arch::Arm32,
arch: Some(Arch::Arm32),
toolchain: Some(Toolchain::Musl),
},
);
Expand All @@ -307,15 +307,15 @@ mod tests {
"macos-universal",
Descriptor {
os: OS::MacOS,
arch: Arch::X64,
arch: Some(Arch::X64),
toolchain: None,
},
);
check_desc(
"darwin-universal",
Descriptor {
os: OS::MacOS,
arch: Arch::X64,
arch: Some(Arch::X64),
toolchain: None,
},
);
Expand Down
34 changes: 34 additions & 0 deletions lib/sources/artifact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,4 +245,38 @@ impl Artifact {
.map(|(_, artifact)| artifact.clone())
.collect()
}

/**
Tries to find a partially compatible artifact, to be used as a fallback
during artifact selection if [`Artifact::sort_by_system_compatibility`]
finds no system-compatible artifacts to use.
Returns `None` if more than one artifact is partially compatible.
*/
pub fn find_partially_compatible_fallback(artifacts: impl AsRef<[Self]>) -> Option<Self> {
let current_desc = Descriptor::current_system();

let os_compatible_artifacts = artifacts
.as_ref()
.iter()
.filter_map(|artifact| {
let name = artifact.name.as_deref()?;
if let Some(asset_desc) = Descriptor::detect(name) {
if current_desc.os() == asset_desc.os() {
Some(artifact)
} else {
None
}
} else {
None
}
})
.collect::<Vec<_>>();

if os_compatible_artifacts.len() == 1 {
Some(os_compatible_artifacts[0].clone())
} else {
None
}
}
}
15 changes: 5 additions & 10 deletions src/cli/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ use console::style;
use rokit::{
discovery::discover_all_manifests,
manifests::RokitManifest,
sources::Artifact,
storage::Home,
tool::{ToolAlias, ToolId},
};

use crate::util::{prompt_for_trust, CliProgressTracker, ToolIdOrSpec};
use crate::util::{
find_most_compatible_artifact, prompt_for_trust, CliProgressTracker, ToolIdOrSpec,
};

/// Adds a new tool to Rokit and installs it.
#[derive(Debug, Parser)]
Expand Down Expand Up @@ -85,18 +86,12 @@ impl AddSubcommand {
let (spec, artifact) = match self.tool.clone() {
ToolIdOrSpec::Spec(spec) => {
let artifacts = source.get_specific_release(&spec).await?;
let artifact = Artifact::sort_by_system_compatibility(&artifacts)
.first()
.cloned()
.with_context(|| format!("No compatible artifact found for {id}"))?;
let artifact = find_most_compatible_artifact(&artifacts, &id)?;
(spec, artifact)
}
ToolIdOrSpec::Id(id) => {
let artifacts = source.get_latest_release(&id).await?;
let artifact = Artifact::sort_by_system_compatibility(&artifacts)
.first()
.cloned()
.with_context(|| format!("No compatible artifact found for {id}"))?;
let artifact = find_most_compatible_artifact(&artifacts, &id)?;
(artifact.tool_spec.clone(), artifact)
}
};
Expand Down
9 changes: 3 additions & 6 deletions src/cli/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use clap::Parser;

use console::style;
use futures::{stream::FuturesUnordered, TryStreamExt};
use rokit::{discovery::discover_all_manifests, sources::Artifact, storage::Home};
use rokit::{discovery::discover_all_manifests, storage::Home};

use crate::util::{prompt_for_trust_specs, CliProgressTracker};
use crate::util::{find_most_compatible_artifact, prompt_for_trust_specs, CliProgressTracker};

/// Adds a new tool using Rokit and installs it.
#[derive(Debug, Parser)]
Expand Down Expand Up @@ -84,10 +84,7 @@ impl InstallSubcommand {
let artifacts = source.get_specific_release(&tool_spec).await?;
pt.subtask_completed();

let artifact = Artifact::sort_by_system_compatibility(&artifacts)
.first()
.cloned()
.with_context(|| format!("No compatible artifact found for {tool_spec}"))?;
let artifact = find_most_compatible_artifact(&artifacts, tool_spec.id())?;
pt.subtask_completed();

let contents = source
Expand Down
8 changes: 3 additions & 5 deletions src/cli/self_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ use clap::Parser;
use console::style;
use semver::Version;

use rokit::{sources::Artifact, storage::Home, tool::ToolId};
use rokit::{storage::Home, tool::ToolId};

use crate::util::CliProgressTracker;
use crate::util::{find_most_compatible_artifact, CliProgressTracker};

/// Updates Rokit to the latest version.
#[derive(Debug, Parser)]
Expand Down Expand Up @@ -55,9 +55,7 @@ impl SelfUpdateSubcommand {
pt.task_completed();
pt.update_message("Downloading");

let artifact = Artifact::sort_by_system_compatibility(&artifacts)
.first()
.cloned()
let artifact = find_most_compatible_artifact(&artifacts, &tool_id)
.context("No compatible Rokit artifact was found (WAT???)")?;
let artifact_contents = source
.download_artifact_contents(&artifact)
Expand Down
15 changes: 6 additions & 9 deletions src/cli/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ use clap::Parser;
use console::style;
use futures::{stream::FuturesUnordered, TryStreamExt};

use rokit::{
discovery::discover_all_manifests, manifests::RokitManifest, sources::Artifact, storage::Home,
};
use rokit::{discovery::discover_all_manifests, manifests::RokitManifest, storage::Home};

use crate::util::{CliProgressTracker, ToolAliasOrIdOrSpec, ToolIdOrSpec};
use crate::util::{
find_most_compatible_artifact, CliProgressTracker, ToolAliasOrIdOrSpec, ToolIdOrSpec,
};

/// Updates all tools, or specific tools, to the latest version.
#[derive(Debug, Parser)]
Expand Down Expand Up @@ -141,12 +141,9 @@ impl UpdateSubcommand {
}
};

let artifact = Artifact::sort_by_system_compatibility(&artifacts)
.first()
.cloned()
.with_context(|| format!("No compatible artifact found for {id}"))?;

let artifact = find_most_compatible_artifact(&artifacts, &id)?;
pt.subtask_completed();

Ok::<_, anyhow::Error>((alias, id, artifact))
})
.collect::<FuturesUnordered<_>>()
Expand Down
2 changes: 1 addition & 1 deletion src/runner/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn is_likely_rosetta2_error(e: &Error) -> bool {

let is_running_macos_aarch64 = {
let current = Descriptor::current_system();
matches!(current.os(), OS::MacOS) && matches!(current.arch(), Arch::Arm64)
matches!(current.os(), OS::MacOS) && matches!(current.arch(), Some(Arch::Arm64))
};

is_bad_cpu_type && is_running_macos_aarch64
Expand Down
Loading

0 comments on commit 94b4c0e

Please sign in to comment.