Follow-ups to the extensions work

docs/extensions.md

@@ -36,6 +36,9 @@ extensions:
   # This can be whatever name you'd like. The name itself
   # isn't used by rpm-ostree.
   sooper-dooper-tracers:
+    # Optional; defaults to `os-extension`. An OS extension
+    # is an extension intended to be `rpm-ostree install`ed.
+    kind: os-extension
     # List of packages for this extension
     packages:
         - strace
@@ -47,6 +50,11 @@ extensions:
         - x86_64
         - aarch64
   kernel-dev:
+    # A development extension lists packages useful for
+    # developing for the target OSTree, but won't be layered
+    # on top. A common example is kernel modules. No
+    # depsolving happens, packages listed are downloaded.
+    kind: development
     packages:
       - kernel-devel
       - kernel-headers

rust/src/extensions.rs

@@ -21,6 +21,8 @@ const RPMOSTREE_EXTENSIONS_STATE_FILE: &str = ".rpm-ostree-state-chksum";
 #[serde(rename_all = "kebab-case")]
 pub struct Extensions {
     extensions: HashMap<String, Extension>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    repos: Option<Vec<String>>,
 }
 
 #[derive(Serialize, Deserialize, Debug)]
@@ -31,6 +33,21 @@ pub struct Extension {
     architectures: Option<Vec<String>>,
     #[serde(skip_serializing_if = "Option::is_none")]
     match_base_evr: Option<String>,
+    #[serde(default)]
+    kind: ExtensionKind,
+}
+
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)]
+#[serde(rename_all = "kebab-case")]
+enum ExtensionKind {
+    OsExtension,
+    Development,
+}
+
+impl Default for ExtensionKind {
+    fn default() -> Self {
+        ExtensionKind::OsExtension
+    }
 }
 
 fn extensions_load_stream(
@@ -53,9 +70,11 @@ fn extensions_load_stream(
         .collect();
 
     for (_, ext) in parsed.extensions.iter_mut() {
-        for pkg in &ext.packages {
-            if base_pkgs.contains_key(pkg.as_str()) {
-                bail!("package {} already present in base", pkg);
+        if ext.kind == ExtensionKind::OsExtension {
+            for pkg in &ext.packages {
+                if base_pkgs.contains_key(pkg.as_str()) {
+                    bail!("package {} already present in base", pkg);
+                }
             }
         }
         if let Some(ref matched_base_pkg) = ext.match_base_evr {
@@ -78,16 +97,30 @@ pub(crate) fn extensions_load(
     path: &str,
     basearch: &str,
     base_pkgs: &Vec<StringMapping>,
-) -> Result<Box<Extensions>> {
+) -> CxxResult<Box<Extensions>> {
     let f = utils::open_file(path)?;
     let mut f = std::io::BufReader::new(f);
-    extensions_load_stream(&mut f, basearch, base_pkgs).with_context(|| format!("parsing {}", path))
+    Ok(extensions_load_stream(&mut f, basearch, base_pkgs)
+        .with_context(|| format!("parsing {}", path))?)
 }
 
 impl Extensions {
-    pub(crate) fn get_packages(&self) -> Vec<String> {
+    pub(crate) fn get_repos(&self) -> Vec<String> {
+        self.repos.as_ref().map(|v| v.clone()).unwrap_or_default()
+    }
+
+    pub(crate) fn get_os_extension_packages(&self) -> Vec<String> {
         self.extensions
             .iter()
+            .filter(|(_, ext)| ext.kind == ExtensionKind::OsExtension)
+            .flat_map(|(_, ext)| ext.packages.iter().cloned())
+            .collect()
+    }
+
+    pub(crate) fn get_development_packages(&self) -> Vec<String> {
+        self.extensions
+            .iter()
+            .filter(|(_, ext)| ext.kind == ExtensionKind::Development)
             .flat_map(|(_, ext)| ext.packages.iter().cloned())
             .collect()
     }
@@ -140,14 +173,18 @@ mod tests {
     #[test]
     fn basic() {
         let buf = r###"
+repos:
+    - my-repo
 extensions:
     bazboo:
         packages:
             - bazboo
 "###;
         let mut input = std::io::BufReader::new(buf.as_bytes());
         let extensions = extensions_load_stream(&mut input, "x86_64", &base_rpmdb()).unwrap();
-        assert!(extensions.get_packages() == vec!["bazboo"]);
+        assert!(extensions.get_repos() == vec!["my-repo"]);
+        assert!(extensions.get_os_extension_packages() == vec!["bazboo"]);
+        assert!(extensions.get_development_packages().is_empty());
     }
 
     #[test]
@@ -165,6 +202,21 @@ extensions:
         }
     }
 
+    #[test]
+    fn ext_in_devel() {
+        let buf = r###"
+extensions:
+    foobar:
+        packages:
+            - foobar
+        kind: development
+"###;
+        let mut input = std::io::BufReader::new(buf.as_bytes());
+        let extensions = extensions_load_stream(&mut input, "x86_64", &base_rpmdb()).unwrap();
+        assert!(extensions.get_os_extension_packages().is_empty());
+        assert!(extensions.get_development_packages() == vec!["foobar"]);
+    }
+
     #[test]
     fn basearch_filter() {
         let buf = r###"
@@ -183,10 +235,10 @@ extensions:
 "###;
         let mut input = std::io::BufReader::new(buf.as_bytes());
         let extensions = extensions_load_stream(&mut input, "x86_64", &base_rpmdb()).unwrap();
-        assert!(extensions.get_packages() == vec!["bazboo"]);
+        assert!(extensions.get_os_extension_packages() == vec!["bazboo"]);
         let mut input = std::io::BufReader::new(buf.as_bytes());
         let extensions = extensions_load_stream(&mut input, "s390x", &base_rpmdb()).unwrap();
-        assert!(extensions.get_packages() == vec!["dodo", "dada"]);
+        assert!(extensions.get_os_extension_packages() == vec!["dodo", "dada"]);
     }
 
     #[test]
@@ -197,9 +249,16 @@ extensions:
         packages:
             - foobar-ext
         match-base-evr: foobar
+        kind: os-extension
+    devel:
+        packages:
+            - foobar-devel
+        match-base-evr: foobar
+        kind: development
 "###;
         let mut input = std::io::BufReader::new(buf.as_bytes());
         let extensions = extensions_load_stream(&mut input, "x86_64", &base_rpmdb()).unwrap();
-        assert!(extensions.get_packages() == vec!["foobar-ext-1.2-3"]);
+        assert!(extensions.get_os_extension_packages() == vec!["foobar-ext-1.2-3"]);
+        assert!(extensions.get_development_packages() == vec!["foobar-devel-1.2-3"]);
     }
 }

rust/src/lib.rs

@@ -195,7 +195,9 @@ mod ffi {
             basearch: &str,
             base_pkgs: &Vec<StringMapping>,
         ) -> Result<Box<Extensions>>;
-        fn get_packages(&self) -> Vec<String>;
+        fn get_repos(&self) -> Vec<String>;
+        fn get_os_extension_packages(&self) -> Vec<String>;
+        fn get_development_packages(&self) -> Vec<String>;
         fn state_checksum_changed(&self, chksum: &str, output_dir: &str) -> Result<bool>;
         fn update_state_checksum(&self, chksum: &str, output_dir: &str) -> Result<()>;
         fn serialize_to_dir(&self, output_dir: &str) -> Result<()>;

rust/src/treefile.rs

@@ -864,9 +864,8 @@ struct TreeComposeConfig {
     #[serde(skip_serializing_if = "Option::is_none")]
     #[serde(rename = "add-commit-metadata")]
     add_commit_metadata: Option<BTreeMap<String, serde_json::Value>>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    #[serde(rename = "rpmdb")]
     // The database backend
+    #[serde(skip_serializing_if = "Option::is_none")]
     rpmdb: Option<RpmdbBackend>,
 
     #[serde(flatten)]

src/app/rpmostree-compose-builtin-tree.cxx

@@ -1532,6 +1532,11 @@ rpmostree_compose_builtin_extensions (int             argc,
 
   auto extensions = rpmostreecxx::extensions_load (extensions_path, basearch, *packages_mapping);
 
+  // notice we don't use a pkgcache repo here like in the treecompose path: we
+  // want RPMs, so having them already imported isn't useful to us (and anyway,
+  // for OS extensions by definition they're not expected to be cached since
+  // they're not in the base tree)
+
   g_autoptr(RpmOstreeContext) ctx =
       rpmostree_context_new_tree (cachedir_dfd, repo, cancellable, error);
   if (!ctx)
@@ -1556,16 +1561,24 @@ rpmostree_compose_builtin_extensions (int             argc,
 
   g_autoptr(RpmOstreeTreespec) spec = NULL;
   { g_autoptr(GPtrArray) gpkgs = g_ptr_array_new_with_free_func (g_free);
-    auto pkgs = extensions->get_packages();
+    auto pkgs = extensions->get_os_extension_packages();
     for (auto pkg : pkgs)
       g_ptr_array_add (gpkgs, (gpointer*) g_strdup (pkg.c_str()));
-    char **repos = ror_treefile_get_repos (treefile);
+
+    g_autoptr(GPtrArray) grepos = g_ptr_array_new_with_free_func (g_free);
+    auto repos = extensions->get_repos();
+    for (auto repo : repos)
+      g_ptr_array_add (grepos, (gpointer*) g_strdup (repo.c_str()));
+
+    char **treefile_repos = ror_treefile_get_repos (treefile);
+    for (char **it = treefile_repos; it && *it; it++)
+      g_ptr_array_add (grepos, (gpointer*) g_strdup (*it));
+
     g_autoptr(GKeyFile) treespec = g_key_file_new ();
     g_key_file_set_string_list (treespec, "tree", "packages",
                                 (const char* const*)gpkgs->pdata, gpkgs->len);
     g_key_file_set_string_list (treespec, "tree", "repos",
-                                (const char* const*)repos,
-                                g_strv_length (repos));
+                                (const char* const*)grepos->pdata, grepos->len);
     spec = rpmostree_treespec_new_from_keyfile (treespec, NULL);
   }
 
@@ -1609,6 +1622,44 @@ rpmostree_compose_builtin_extensions (int             argc,
         return FALSE;
     }
 
+  /* This is hacky: for "development" extensions, we don't want any depsolving
+   * against the base OS. Rather than awkwardly teach the core about this, we
+   * just reuse its sack and keep all the functionality here. */
+
+  DnfContext *dnfctx = rpmostree_context_get_dnf (ctx);
+  DnfSack *sack = dnf_context_get_sack (dnfctx);
+
+  /* disable the system repo; we always want to download, even if already in the base */
+  dnf_sack_repo_enabled (sack, HY_SYSTEM_REPO_NAME, 0);
+
+  auto pkgs = extensions->get_development_packages();
+  g_autoptr(GPtrArray) devel_pkgs_to_download =
+    g_ptr_array_new_with_free_func (g_object_unref);
+  for (auto pkg : pkgs)
+    {
+      g_autoptr(GPtrArray) matches = rpmostree_get_matching_packages (sack, pkg.c_str());
+      if (matches->len == 0)
+        return glnx_throw (error, "Package %s not found", pkg.c_str());
+      DnfPackage *found_pkg = (DnfPackage*)matches->pdata[0];
+      g_ptr_array_add (devel_pkgs_to_download, g_object_ref (found_pkg));
+    }
+
+  rpmostree_set_repos_on_packages (dnfctx, devel_pkgs_to_download);
+
+  if (!rpmostree_download_packages (devel_pkgs_to_download, cancellable, error))
+    return FALSE;
+
+  for (guint i = 0; i < devel_pkgs_to_download->len; i++)
+    {
+      DnfPackage *pkg = (DnfPackage*)devel_pkgs_to_download->pdata[i];
+      const char *src = dnf_package_get_filename (pkg);
+      const char *basename = glnx_basename (src);
+      if (!glnx_file_copy_at (AT_FDCWD, dnf_package_get_filename (pkg), NULL, output_dfd,
+                              basename, GLNX_FILE_COPY_NOXATTRS, cancellable, error))
+        return FALSE;
+    }
+
+  // XXX: account for development extensions
   extensions->update_state_checksum (state_checksum, opt_extensions_output_dir);
   extensions->serialize_to_dir (opt_extensions_output_dir);
   if (!process_touch_if_changed (error))