importer: Error importing RPMs which install to /opt (outside of /usr)

[cgwalters]

See #233 - for RPMs which
place files in e.g. /opt, we have different behavior in the treecompose case
(silently drop it) versus package layering (does the wrong thing).

Since the unpacker right now is only used in the layering case, this just
ensures we'll get a consistent error there.

[jlebon]

@rh-atomic-bot r+ 59b1b5e

[rh-atomic-bot]

⌛ Testing commit 59b1b5e with merge c978d8b...

[rh-atomic-bot]

💔 Test failed - status-atomicjenkins

[cgwalters]

Confused...tests pass here.

@rh-atomic-bot retry

[rh-atomic-bot]

⌛ Testing commit 59b1b5e with merge bc10e5f...

[jlebon]

I was looking into this failure before backporting #622 it case it was wrong in some subtle way... It turned out to be a rather tricky issue.

When we go the second time around to commit the generated tmpfiles.d file, the compose filter gets called back for e.g. / and /usr. And of course, those do not pass the filter (and we also don't check for callback errors there).

This solved it for me:

diff --git a/src/libpriv/rpmostree-unpacker.c b/src/libpriv/rpmostree-unpacker.c
index 8d91c7c..4c3ff74 100644
--- a/src/libpriv/rpmostree-unpacker.c
+++ b/src/libpriv/rpmostree-unpacker.c
@@ -612,9 +612,12 @@ compose_filter_cb (OstreeRepo         *repo,
           return OSTREE_REPO_COMMIT_FILTER_SKIP;
         }
       /* And ensure the RPM installs into supported paths */
-      else if (!(g_str_has_prefix (path, "/usr/") || g_str_has_prefix (path, "/bin/") ||
-                 g_str_has_prefix (path, "/sbin/") || g_str_has_prefix (path, "/lib/") ||
-                 g_str_has_prefix (path, "/lib64/")))
+      else if (!(g_str_equal (path, "/") ||
+                 g_str_equal (path, "/usr")   || g_str_has_prefix (path, "/usr/")  ||
+                 g_str_equal (path, "/bin")   || g_str_has_prefix (path, "/bin/")  ||
+                 g_str_equal (path, "/sbin")  || g_str_has_prefix (path, "/sbin/") ||
+                 g_str_equal (path, "/lib")   || g_str_has_prefix (path, "/lib/")  ||
+                 g_str_equal (path, "/lib64") || g_str_has_prefix (path, "/lib64/")))
         {
           g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
                        "Unsupported path: %s; See %s",

(Or maybe a hashtable might be more appropriate at this point).

We could just not use a modifier on the second pass and lose selabeling (which is fine since those dirs will already exist). Though we'll probably want the above anyway when we switch over tree composes to use the unpacker.

[rh-atomic-bot]

💔 Test failed - status-atomicjenkins

[cgwalters]

Oooh, masking the errors was insidious 👺 🔪 🤕. I added a patch to fix that first.

[jlebon]

@rh-atomic-bot r+ 5e7cc94

[rh-atomic-bot]

⌛ Testing commit 5e7cc94 with merge ddbaf19...

[rh-atomic-bot]

☀️ Test successful - status-atomicjenkins
Approved by: jlebon
Pushing ddbaf19 to master...

[mattdm]

FWIW, according to FHS 3.0, directories of the form /opt/provider (where provider is a LANANA registered provider, like fedora or rhat or centos) are fair game for the distro: "Distributions may install and otherwise manage software in /opt under an appropriately registered subdirectory." (ref). It's a little ambiguous whether /opt/google is also distro-clear — google is "appropriately registered", but not registered to the distro provider. We should probably clear that up in the FHS, but I think since Google provides an RPM that uses that path, the intent is clear enough.