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

Improve artifact compatibility by adding a fallback mechanism #38

Merged
merged 4 commits into from
Jul 14, 2024
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
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