diff --git a/README.md b/README.md index 1af0224..d87f089 100644 --- a/README.md +++ b/README.md @@ -164,6 +164,10 @@ By default this field is enabled. *The /etc/os-release file can also be included by adding the distro's `-release` package to the packages array: this field exists to ensure the /etc/os-release file is included by default.* +#### Weak dependencies + +rpmoci does not install [weak dependencies](https://docs.fedoraproject.org/en-US/packaging-guidelines/WeakDependencies/#:~:text=Weak%20dependencies%20should%20be%20used%20where%20possible%20to,require%20the%20full%20feature%20set%20of%20the%20package.), optimizing for small container image sizes. + ### Image building Running `rpmoci build --image foo --tag bar` will build a container image in OCI format. diff --git a/src/config.rs b/src/config.rs index 5bdd41f..5bcab9e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -101,7 +101,7 @@ pub(crate) enum Repository { #[derive(Debug, Serialize, Deserialize, Clone)] #[serde(deny_unknown_fields)] pub(crate) struct RepositoryDefinition { - id: Option, + pub(crate) id: Option, // The base url of the repository pub(crate) url: Url, /// Additional repository options. diff --git a/src/lib.rs b/src/lib.rs index 23930b9..6fc3186 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -65,7 +65,7 @@ pub fn main(command: Command) -> anyhow::Result<()> { let (cfg, lockfile_path, existing_lockfile) = load_config_and_lock_file(manifest_path)?; let lockfile = if let Ok(Some(lockfile)) = &existing_lockfile { - if lockfile.is_compatible(&cfg) && from_lockfile { + if lockfile.is_compatible_excluding_local_rpms(&cfg) && from_lockfile { lockfile.resolve_from_previous(&cfg)? } else { if from_lockfile { @@ -96,7 +96,9 @@ pub fn main(command: Command) -> anyhow::Result<()> { let (cfg, lockfile_path, existing_lockfile) = load_config_and_lock_file(manifest_path)?; let lockfile = match (existing_lockfile, locked) { (Ok(Some(lockfile)), true) => { - if !lockfile.is_compatible(&cfg) { + // TODO: consider whether this can move to including local RPMs. (Subtlety here is that may + // break scenarios where the user is using local RPMs that have a subset of the locked local RPM dependencies.) + if !lockfile.is_compatible_excluding_local_rpms(&cfg) { bail!(format!( "the lock file {} needs to be updated but --locked was passed to prevent this", lockfile_path.display() @@ -105,7 +107,7 @@ pub fn main(command: Command) -> anyhow::Result<()> { lockfile } (Ok(Some(lockfile)), false) => { - if lockfile.all_local_deps_compatible(&cfg)? { + if lockfile.is_compatible_including_local_rpms(&cfg)? { // Compatible lockfile, use it lockfile } else { @@ -184,7 +186,7 @@ pub fn main(command: Command) -> anyhow::Result<()> { load_config_and_lock_file(manifest_path)?; if let Ok(Some(lockfile)) = existing_lockfile { - if lockfile.is_compatible(&cfg) { + if lockfile.is_compatible_excluding_local_rpms(&cfg) { lockfile.download_rpms(&cfg, &out_dir)?; lockfile.check_gpg_keys(&out_dir)?; } else { diff --git a/src/lockfile/mod.rs b/src/lockfile/mod.rs index a4269d8..1ebcc85 100644 --- a/src/lockfile/mod.rs +++ b/src/lockfile/mod.rs @@ -33,11 +33,11 @@ mod resolve; pub struct Lockfile { pkg_specs: Vec, packages: BTreeSet, - #[serde(default)] + #[serde(default, skip_serializing_if = "BTreeSet::is_empty")] local_packages: BTreeSet, - #[serde(default)] + #[serde(default, skip_serializing_if = "HashMap::is_empty")] repo_gpg_config: HashMap, - #[serde(default)] + #[serde(default, skip_serializing_if = "Vec::is_empty")] global_key_specs: Vec, } @@ -117,14 +117,20 @@ pub enum Algorithm { impl Lockfile { /// Returns true if the lockfile is compatible with the /// given configuration, false otherwise + /// + /// Local RPMs are not considered in this check, so this check can be performed + /// without them being present #[must_use] - pub fn is_compatible(&self, cfg: &Config) -> bool { + pub fn is_compatible_excluding_local_rpms(&self, cfg: &Config) -> bool { self.pkg_specs == cfg.contents.packages && self.global_key_specs == cfg.contents.gpgkeys } - /// Returns true if the lockfile is compatible with the specified configuration - /// and if all rpm packages required by local dependencies are included in the lockfile. - pub fn all_local_deps_compatible(&self, cfg: &Config) -> Result { + /// Returns true if the lockfile is compatible with the + /// given configuration, false otherwise + /// + /// This check also verifies that dependencies of local RPMs match those in the lockfile, + /// so requires the local RPMs to be present + pub fn is_compatible_including_local_rpms(&self, cfg: &Config) -> Result { let local_package_deps: BTreeSet = self .local_packages .clone() @@ -132,8 +138,7 @@ impl Lockfile { .flat_map(|p| p.requires) .collect(); - Ok(self.is_compatible(cfg) - // Verify dependencies of all local packages + Ok(self.is_compatible_excluding_local_rpms(cfg) && Self::read_local_rpm_deps(cfg)? == local_package_deps) } diff --git a/src/lockfile/resolve.py b/src/lockfile/resolve.py index bd1a2fc..ce4d3ab 100644 --- a/src/lockfile/resolve.py +++ b/src/lockfile/resolve.py @@ -1,4 +1,5 @@ """A dependency resolver for rpmoci""" + # Copyright (C) Microsoft Corporation. # # This program is free software: you can redistribute it and/or modify @@ -32,7 +33,7 @@ def resolve(base, packages): for pkg in pkgs: goal.install(pkg) - if not goal.run(): + if not goal.run(ignore_weak_deps=True): msg = dnf.util._format_resolve_problems(goal.problem_rules()) raise dnf.exceptions.DepsolveError(msg) diff --git a/src/lockfile/resolve.rs b/src/lockfile/resolve.rs index 06f41c2..1d0599f 100644 --- a/src/lockfile/resolve.rs +++ b/src/lockfile/resolve.rs @@ -360,3 +360,40 @@ fn repo_password(repo_id: &str) -> Option { )) .ok() } + +#[cfg(test)] +mod tests { + use std::{collections::HashMap, str::FromStr}; + + use url::Url; + + use crate::{ + config::{Repository, RepositoryDefinition}, + lockfile::Lockfile, + }; + + #[test] + fn test_weak_deps() { + // prce2-tools in mariner recommends pcre2-docs. use this to test weak dep behaviour + let mut options = HashMap::new(); + options.insert("gpgcheck".to_string(), "True".to_string()); + options.insert("gpgkey".to_string(), "https://raw.githubusercontent.com/microsoft/CBL-Mariner/2.0/SPECS/mariner-repos/MICROSOFT-RPM-GPG-KEY,https://packages.microsoft.com/keys/microsoft.asc".to_string()); + + let mariner_repository = Repository::Definition(RepositoryDefinition { + id: Some("marinertest".to_string()), + url: Url::from_str("https://packages.microsoft.com/cbl-mariner/2.0/prod/base/x86_64") + .unwrap(), + options, + }); + let repositories = vec![mariner_repository]; + + let lock = Lockfile::resolve( + vec!["pcre2-tools".to_string()], + &repositories, + Vec::new(), + true, + ) + .unwrap(); + assert!(!lock.packages.iter().any(|p| p.name == "pcre2-doc")); + } +} diff --git a/tests/fixtures/weakdeps/rpmoci.toml b/tests/fixtures/weakdeps/rpmoci.toml new file mode 100644 index 0000000..d40cf1c --- /dev/null +++ b/tests/fixtures/weakdeps/rpmoci.toml @@ -0,0 +1,10 @@ +[contents] +repositories = [ + "https://packages.microsoft.com/cbl-mariner/2.0/prod/base/x86_64", +] +gpgkeys = [ + "https://raw.githubusercontent.com/microsoft/CBL-Mariner/2.0/SPECS/mariner-repos/MICROSOFT-RPM-GPG-KEY", + "https://packages.microsoft.com/keys/microsoft.asc", +] +# This has a weak dependency on pcre2-docs +packages = ["pcre2-tools"] diff --git a/tests/it.rs b/tests/it.rs index 3928198..15037fa 100644 --- a/tests/it.rs +++ b/tests/it.rs @@ -318,5 +318,18 @@ fn test_explicit_etc_os_release() { .unwrap(); let stderr = std::str::from_utf8(&output.stderr).unwrap(); eprintln!("stderr: {}. {}. {}", stderr, root.display(), EXE); +} + +#[test] +fn test_weak_deps() { + // Verify a build without weak dependencies succeeds + let root = setup_test("weakdeps"); + let output = Command::new(EXE) + .arg("build") + .arg("--image=weak") + .arg("--tag=deps") + .current_dir(&root) + .output() + .unwrap(); assert!(output.status.success()); }