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 673cacd633
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.
This commit is contained in:
Colin Walters 2020-09-16 00:35:33 +00:00
parent 6950a98099
commit 558720e7aa
5 changed files with 23 additions and 17 deletions

View File

@ -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>

View File

@ -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)

View File

@ -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" },

View File

@ -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} 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} -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
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"
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

View File

@ -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"