Merge pull request #2418 from lucab/ups/lib-commit-xattrs

commit: automatically skip xattrs in bare-user-only mode

6 files changed, 27 insertions, 34 deletions

diff --git a/src/libostree/ostree-diff.c b/src/libostree/ostree-diff.c
index e2d68d0a..a5c02b7b 100644
--- a/src/libostree/ostree-diff.c
+++ b/src/libostree/ostree-diff.c
@@ -271,13 +271,13 @@ ostree_diff_dirs_with_options (OstreeDiffFlags        flags,
   if (OSTREE_IS_REPO_FILE (a))
     {
       OstreeRepo *repo = ostree_repo_file_get_repo ((OstreeRepoFile*)a);
-      if (repo->disable_xattrs)
+      if (repo->disable_xattrs || repo->mode == OSTREE_REPO_MODE_BARE_USER_ONLY)
         flags |= OSTREE_DIFF_FLAGS_IGNORE_XATTRS;
     }
   if (OSTREE_IS_REPO_FILE (b))
     {
       OstreeRepo *repo = ostree_repo_file_get_repo ((OstreeRepoFile*)b);
-      if (repo->disable_xattrs)
+      if (repo->disable_xattrs || repo->mode == OSTREE_REPO_MODE_BARE_USER_ONLY)
         flags |= OSTREE_DIFF_FLAGS_IGNORE_XATTRS;
     }
 
diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
index 249e792c..d5ab57a2 100644
--- a/src/libostree/ostree-repo-commit.c
+++ b/src/libostree/ostree-repo-commit.c
@@ -3382,8 +3382,9 @@ get_final_xattrs (OstreeRepo                       *self,
   /* track whether the returned xattrs differ from the file on disk */
   gboolean modified = TRUE;
   const gboolean skip_xattrs = (modifier &&
-      modifier->flags & (OSTREE_REPO_COMMIT_MODIFIER_FLAGS_SKIP_XATTRS |
-                         OSTREE_REPO_COMMIT_MODIFIER_FLAGS_CANONICAL_PERMISSIONS)) > 0;
+      (modifier->flags & (OSTREE_REPO_COMMIT_MODIFIER_FLAGS_SKIP_XATTRS |
+                          OSTREE_REPO_COMMIT_MODIFIER_FLAGS_CANONICAL_PERMISSIONS)) > 0) ||
+      self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY;
 
   /* fetch on-disk xattrs if needed & not disabled */
   g_autoptr(GVariant) original_xattrs = NULL;
diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h
index bbec1a37..962fa8cc 100644
--- a/src/libostree/ostree-repo.h
+++ b/src/libostree/ostree-repo.h
@@ -684,7 +684,7 @@ typedef OstreeRepoCommitFilterResult (*OstreeRepoCommitFilter) (OstreeRepo    *r
  * @OSTREE_REPO_COMMIT_MODIFIER_FLAGS_DEVINO_CANONICAL: If a devino cache hit is found, skip modifier filters (non-directories only); Since: 2017.14
  *
  * Flags modifying commit behavior. In bare-user-only mode, @OSTREE_REPO_COMMIT_MODIFIER_FLAGS_CANONICAL_PERMISSIONS
- * is automatically enabled.
+ * and @OSTREE_REPO_COMMIT_MODIFIER_FLAGS_SKIP_XATTRS are automatically enabled.
  *
  */
 typedef enum {
diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c
index 3698a2f4..370e085c 100644
--- a/src/ostree/ot-builtin-commit.c
+++ b/src/ostree/ot-builtin-commit.c
@@ -572,7 +572,9 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio
       goto out;
     }
 
-  if (opt_no_xattrs)
+  if (opt_canonical_permissions || repo->mode == OSTREE_REPO_MODE_BARE_USER_ONLY)
+    flags |= OSTREE_REPO_COMMIT_MODIFIER_FLAGS_CANONICAL_PERMISSIONS;
+  if (opt_no_xattrs || repo->mode == OSTREE_REPO_MODE_BARE_USER_ONLY)
     flags |= OSTREE_REPO_COMMIT_MODIFIER_FLAGS_SKIP_XATTRS;
   if (opt_consume)
     flags |= OSTREE_REPO_COMMIT_MODIFIER_FLAGS_CONSUME;
@@ -581,8 +583,6 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio
       opt_link_checkout_speedup = TRUE; /* Imply this */
       flags |= OSTREE_REPO_COMMIT_MODIFIER_FLAGS_DEVINO_CANONICAL;
     }
-  if (opt_canonical_permissions)
-    flags |= OSTREE_REPO_COMMIT_MODIFIER_FLAGS_CANONICAL_PERMISSIONS;
   if (opt_generate_sizes)
     flags |= OSTREE_REPO_COMMIT_MODIFIER_FLAGS_GENERATE_SIZES;
   if (opt_disable_fsync)
diff --git a/tests/basic-test.sh b/tests/basic-test.sh
index 7946ffa3..935544d9 100644
--- a/tests/basic-test.sh
+++ b/tests/basic-test.sh
@@ -29,10 +29,8 @@ COMMIT_ARGS=""
 DIFF_ARGS=""
 if is_bare_user_only_repo repo; then
     # In bare-user-only repos we can only represent files with uid/gid 0, no
-    # xattrs and canonical permissions, so we need to commit them as such, or
-    # we end up with repos that don't pass fsck
-    COMMIT_ARGS="--no-xattrs"
-    DIFF_ARGS="--owner-uid=0 --owner-gid=0 --no-xattrs"
+    # xattrs and canonical permissions.
+    DIFF_ARGS="--owner-uid=0 --owner-gid=0"
     # Also, since we can't check out uid=0 files we need to check out in user mode
     CHECKOUT_U_ARG="-U"
     CHECKOUT_H_ARGS="-U -H"
@@ -314,7 +312,7 @@ echo "ok diff revisions"
 
 cd ${test_tmpdir}/checkout-test2-4
 echo afile > oh-look-a-file
-$OSTREE diff test2 ./ > ${test_tmpdir}/diff-test2-2
+$OSTREE diff ${DIFF_ARGS} test2 ./ > ${test_tmpdir}/diff-test2-2
 rm oh-look-a-file
 cd ${test_tmpdir}
 assert_file_has_content diff-test2-2 'A *oh-look-a-file$'
@@ -787,11 +785,14 @@ cd ${test_tmpdir}
 rm files -rf && mkdir files
 mkdir files/worldwritable-dir
 chmod a+w files/worldwritable-dir
-$CMD_PREFIX ostree --repo=repo commit -b content-with-dir-world-writable --tree=dir=files
+$OSTREE commit ${COMMIT_ARGS} -b content-with-dir-world-writable --tree=dir=files
+$OSTREE fsck
 rm dir-co -rf
-$CMD_PREFIX ostree --repo=repo checkout -U -H -M content-with-dir-world-writable dir-co
-assert_file_has_mode dir-co/worldwritable-dir 775
-if ! is_bare_user_only_repo repo; then
+$OSTREE checkout -U -H -M content-with-dir-world-writable dir-co
+if is_bare_user_only_repo repo; then
+    assert_file_has_mode dir-co/worldwritable-dir 755
+else
+    assert_file_has_mode dir-co/worldwritable-dir 775
     rm dir-co -rf
     $CMD_PREFIX ostree --repo=repo checkout -U -H content-with-dir-world-writable dir-co
     assert_file_has_mode dir-co/worldwritable-dir 777
diff --git a/tests/test-basic-user-only.sh b/tests/test-basic-user-only.sh
index 7ef153c3..568f9e95 100755
--- a/tests/test-basic-user-only.sh
+++ b/tests/test-basic-user-only.sh
@@ -25,7 +25,7 @@ set -euo pipefail
 
 mode="bare-user-only"
 setup_test_repository "$mode"
-extra_basic_tests=7
+extra_basic_tests=6
 . $(dirname $0)/basic-test.sh
 
 $CMD_PREFIX ostree --version > version.yaml
@@ -63,8 +63,8 @@ rm files -rf && mkdir files
 echo "a group writable file" > files/some-group-writable
 chmod 0664 files/some-group-writable
 $CMD_PREFIX ostree --repo=repo-input commit -b content-with-group-writable --tree=dir=files
-$CMD_PREFIX ostree pull-local --repo=repo repo-input
-$CMD_PREFIX ostree --repo=repo checkout -U -H content-with-group-writable groupwritable-co
+$OSTREE pull-local repo-input
+$OSTREE checkout -U -H content-with-group-writable groupwritable-co
 assert_file_has_mode groupwritable-co/some-group-writable 664
 echo "ok supported group writable"
 
@@ -75,8 +75,8 @@ rm files -rf && mkdir files
 mkdir files/worldwritable-dir
 chmod a+w files/worldwritable-dir
 $CMD_PREFIX ostree --repo=repo-input commit -b content-with-dir-world-writable --tree=dir=files
-$CMD_PREFIX ostree pull-local --repo=repo repo-input
-$CMD_PREFIX ostree --repo=repo checkout -U -H content-with-dir-world-writable dir-co
+$OSTREE pull-local repo-input
+$OSTREE checkout -U -H content-with-dir-world-writable dir-co
 assert_file_has_mode dir-co/worldwritable-dir 775
 echo "ok didn't make world-writable dir"
 
@@ -106,21 +106,12 @@ rm repo -rf
 ostree_repo_init repo init --mode=bare-user-only
 rm files -rf && mkdir files
 echo afile > files/afile
+chmod 0777 files/afile
 $OSTREE commit ${COMMIT_ARGS} -b perms files
-rm out -rf
-$OSTREE checkout --force-copy perms out
-$OSTREE checkout ${CHECKOUT_H_ARGS} --union-identical perms out
 $OSTREE fsck
-echo "ok checkout checksum with canonical perms"
-
-cd ${test_tmpdir}
-rm repo -rf
-ostree_repo_init repo init --mode=bare-user-only
-rm files -rf && mkdir files
-echo afile > files/afile
-$OSTREE commit ${COMMIT_ARGS} -b perms files
 rm out -rf
 $OSTREE checkout --force-copy perms out
+assert_file_has_mode out/afile 755
 $OSTREE checkout ${CHECKOUT_H_ARGS} --union-identical perms out
-$OSTREE fsck
+assert_file_has_mode out/afile 755
 echo "ok automatic canonical perms for bare-user-only"