checkout: Don't hardlink zero sized files

Alternative to https://github.com/ostreedev/ostree/pull/2197

Python's (usually) zero-sized `__init__.py` files can provoke
us hitting the hardlink limits on some filesystems (`EMLINK`).
At least one Fedora rpm-ostree user hit this.

The benefits of hardlinking here are quite marginal; lots
of hardlinks can behave suboptimally in particular filesystems
like BTRFS too.

This builds on prior code which made this an option, introduced
in https://github.com/ostreedev/ostree/commit/673cacd633f9d6b653cdea530657d3e780a41bbd
Now we just do it uncondtionally.

Also this provoked a different bug in a very obscure user mode checkout
case; when the "real" permissions were different from the "physical"
permissions, we would still hardlink.  Fix the test case for this.

diff --git a/man/ostree-checkout.xml b/man/ostree-checkout.xml
index 3956e34faa375f58a56b88d2222937901c47f3c5..dfa2ce16b3208fc363487e17380012d203207d9f 100644 (file)
--- a/man/ostree-checkout.xml
+++ b/man/ostree-checkout.xml
@@ -163,7 +163,7 @@ Boston, MA 02111-1307, USA.
                 <option>-z</option></term>
 
                 <listitem><para>
-                    Do not hardlink zero-sized files.
+                    This option does nothing; the functionality is now always on by default.
                 </para></listitem>
             </varlistentry>
 

diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c
index 10535a3585c20c2e0f81b27daebdd8218e51f476..00c6a7732afd0aff811b54aa10d601bae13db4ab 100644 (file)
--- a/src/libostree/ostree-repo-checkout.c
+++ b/src/libostree/ostree-repo-checkout.c
@@ -637,8 +637,12 @@ checkout_one_file_at (OstreeRepo                        *repo,
 
       need_copy = FALSE;
     }
-  else if ((options->force_copy_zerosized && is_reg_zerosized) || override_user_unreadable)
+  else if (is_reg_zerosized || override_user_unreadable)
     {
+      /* In https://github.com/ostreedev/ostree/commit/673cacd633f9d6b653cdea530657d3e780a41bbd we
+       * made this an option, but in order to avoid hitting EMLINK, we now force copy zerosized
+       * files unconditionally.
+       */
       need_copy = TRUE;
     }
   else if (!options->force_copy)

diff --git a/src/ostree/ot-builtin-checkout.c b/src/ostree/ot-builtin-checkout.c
index adb763a905f77e02502a9143d8d51fc1b785da0c..fe9558c812721b417bf4cfbf2f366b90e6812222 100644 (file)
--- a/src/ostree/ot-builtin-checkout.c
+++ b/src/ostree/ot-builtin-checkout.c
@@ -84,7 +84,7 @@ static GOptionEntry options[] = {
   { "from-file", 0, 0, G_OPTION_ARG_STRING, &opt_from_file, "Process many checkouts from input file", "FILE" },
   { "fsync", 0, 0, G_OPTION_ARG_CALLBACK, parse_fsync_cb, "Specify how to invoke fsync()", "POLICY" },
   { "require-hardlinks", 'H', 0, G_OPTION_ARG_NONE, &opt_require_hardlinks, "Do not fall back to full copies if hardlinking fails", NULL },
-  { "force-copy-zerosized", 'z', 0, G_OPTION_ARG_NONE, &opt_force_copy_zerosized, "Do not hardlink zero-sized files", NULL },
+  { "force-copy-zerosized", 'z', G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, &opt_force_copy_zerosized, "Do not hardlink zero-sized files", NULL },
   { "force-copy", 'C', 0, G_OPTION_ARG_NONE, &opt_force_copy, "Never hardlink (but may reflink if available)", NULL },
   { "bareuseronly-dirs", 'M', 0, G_OPTION_ARG_NONE, &opt_bareuseronly_dirs, "Suppress mode bits outside of 0775 for directories (suid, world writable, etc.)", NULL },
   { "skip-list", 0, 0, G_OPTION_ARG_FILENAME, &opt_skiplist_file, "File containing list of files to skip", "FILE" },

diff --git a/tests/basic-test.sh b/tests/basic-test.sh
index fc193f4f9641768c1281824135cd01e4a08f1856..9227b0cdd0dc973facd82bb6bcdf4f66b9498dff 100644 (file)
--- a/tests/basic-test.sh
+++ b/tests/basic-test.sh
@@ -750,15 +750,17 @@ rm files -rf && mkdir files
 touch files/anemptyfile
 touch files/anotheremptyfile
 $CMD_PREFIX ostree --repo=repo commit --consume -b tree-with-empty-files --tree=dir=files
-$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} -z tree-with-empty-files tree-with-empty-files
+$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} tree-with-empty-files tree-with-empty-files
 if files_are_hardlinked tree-with-empty-files/an{,other}emptyfile; then
     fatal "--force-copy-zerosized failed"
 fi
+# And pass the now-defunct -z option to validate it does nothing
 rm tree-with-empty-files -rf
-$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} tree-with-empty-files tree-with-empty-files
-assert_files_hardlinked tree-with-empty-files/an{,other}emptyfile
-rm tree-with-empty-files -rf
-echo "ok checkout --force-copy-zerosized"
+$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} -z tree-with-empty-files tree-with-empty-files
+if files_are_hardlinked tree-with-empty-files/an{,other}emptyfile; then
+    fatal "--force-copy-zerosized failed"
+fi
+echo "ok checkout zero sized files are not hardlinked"
 
 # These should merge, they're identical
 $CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} --union-identical -z tree-with-empty-files tree-with-empty-files

diff --git a/tests/test-libarchive.sh b/tests/test-libarchive.sh
index 174be800a32f4bb1d5a8b5c7b7980a153722eea8..73a58ddd64374b33869a4604db8d65d4ecba876c 100755 (executable)
--- a/tests/test-libarchive.sh
+++ b/tests/test-libarchive.sh
@@ -37,7 +37,7 @@ mkdir foo
 cd foo
 mkdir -p usr/bin usr/lib
 echo contents > usr/bin/foo
-touch usr/bin/foo0
+echo foo0 > usr/bin/foo0
 ln usr/bin/foo usr/bin/bar
 ln usr/bin/foo0 usr/bin/bar0
 ln -s foo usr/bin/sl
@@ -45,8 +45,8 @@ mkdir -p usr/local/bin
 ln usr/bin/foo usr/local/bin/baz
 ln usr/bin/foo0 usr/local/bin/baz0
 ln usr/bin/sl usr/local/bin/slhl
-touch usr/bin/setuidme
-touch usr/bin/skipme
+echo setuidme > usr/bin/setuidme
+echo skipme > usr/bin/skipme
 echo "a library" > usr/lib/libfoo.so
 echo "another library" > usr/lib/libbar.so
 
@@ -102,9 +102,9 @@ assert_valid_content () {
   assert_file_has_content usr/bin/foo contents
   assert_file_has_content usr/bin/bar contents
   assert_file_has_content usr/local/bin/baz contents
-  assert_file_empty usr/bin/foo0
-  assert_file_empty usr/bin/bar0
-  assert_file_empty usr/local/bin/baz0
+  assert_file_has_content usr/bin/foo0 foo0
+  assert_file_has_content usr/bin/bar0 foo0
+  assert_file_has_content usr/local/bin/baz0 foo0
   assert_file_has_content usr/lib/libfoo.so 'a library'
   assert_file_has_content usr/lib/libbar.so 'another library'
 
@@ -244,7 +244,7 @@ ${CMD_PREFIX} ostree --repo=repo2 commit \
   --generate-sizes \
   --tree=tar=foo.tar.gz
 ${CMD_PREFIX} ostree --repo=repo2 show --print-sizes test-tar > sizes.txt
-assert_file_has_content sizes.txt 'Compressed size (needed/total): 0[  ]bytes/1.1[  ]kB'
-assert_file_has_content sizes.txt 'Unpacked size (needed/total): 0[  ]bytes/900[  ]bytes'
-assert_file_has_content sizes.txt 'Number of objects (needed/total): 0/12'
+assert_file_has_content sizes.txt 'Compressed size (needed/total): 0[  ]bytes/1.2[  ]kB'
+assert_file_has_content sizes.txt 'Unpacked size (needed/total): 0[  ]bytes/921[  ]bytes'
+assert_file_has_content sizes.txt 'Number of objects (needed/total): 0/14'
 echo "ok tar sizes metadata"