include buildid in LOCPATH

This avoids the possibility that the bundle could be updated in place,
leading to LOCPATH existing but containing locales for the old version,
which needed to be tested for with code that was not race-free.

LOCPATH/buildid is still written and checked when cleaning up stale caches.
That is not actually necessary, except old versions of the standalone
bundle expect to see it, and this prevents them cleaning up the locale
cache of a new version. And still checking it prevents the new version
cleaning up the locale cache of the old version while the old version is
still in use.

Added explicit tests before creating LOCPATH and the base and buildid files.

The buildid file no longer needs to be updated every time, because it's
stable for the given LOCPATH directory.

And the base file actually did not need to be updated every time,
because the LOCPATH is derived from base, so if the bundle is moved
elsewhere, a different LOCPATH will be used.

Transitioning to this will mean that two git-annex builds that otherwise
have the same buildid -- the same git-annex md5sum -- will use different
LOCPATH values, but that's handled fine by the cache cleanup code, so at
most it will mean one extra generation of the locale files.
This commit is contained in:
Joey Hess 2020-10-05 13:53:43 -04:00
parent cb487e7417
commit f0ec725234
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
2 changed files with 52 additions and 17 deletions

View file

@ -0,0 +1,37 @@
[[!comment format=mdwn
username="joey"
subject="""comment 4"""
date="2020-10-05T17:04:45Z"
content="""
Hmm, that rm -rf is idempotent itself once the stderr is hidden;
if a race causes one rm to fail the other one will probably succeed;
if both manage to fail it ignores the nonzero exit code and,
the next run will clean up after it by running it again. AFAICS the cache
eventually gets cleaned up in all circumstances.
rm: cannot remove '/home/yoh/.cache/git-annex/locales/bb375eb6ec0d2706f1723307f068911a': Directory not empty
I think what happened there is, the first process created
that cache directory, but had not yet written the base file
when the second process ran. So the second process sees what looks like a
cache directory with no base file, so it decides to clean it up. In the
meantime the first process has written base and other files and so the rm
fails. Also, the first process may succeed and end up running git-annex
with some locale files missing (if the rm happened to delete those),
resulting in incompatable system locales being used.
So, it ought to defer cleaning up old caches until after it's made sure its
current cache is all set up. Then that race goes away.
But, there's a problem with this, that comes right after the main cache
cleanup:
# If the locale cache for this bundle is out of date, refresh it.
if [ -e "$LOCPATH/buildid" ] && ! cmp "$LOCPATH/buildid" "$base/buildid" >/dev/null 2>&1 ; then
rm -rf "$LOCPATH"
fi
That runs if the git-annex.linux directory gets overwritten with a new version,
so the cache is the same. And it's also prone to similar races.
I think, to fix this, it probably needs to use the buildid in the LOCPATH.
"""]]

View file

@ -133,7 +133,7 @@ unset LD_PRELOAD
ORIG_LOCPATH="$LOCPATH" ORIG_LOCPATH="$LOCPATH"
export ORIG_LOCPATH export ORIG_LOCPATH
if [ -z "${LOCPATH+set}" ] && [ -z "$GIT_ANNEX_PACKAGE_INSTALL" ]; then if [ -z "${LOCPATH+set}" ] && [ -z "$GIT_ANNEX_PACKAGE_INSTALL" ]; then
LOCPATH="$HOME/.cache/git-annex/locales/$(echo "$base" | tr / _ )" LOCPATH="$HOME/.cache/git-annex/locales/$(cat "$base/buildid")_$(echo "$base" | tr / _ )"
# try to generate a short version, if md5sum is available # try to generate a short version, if md5sum is available
locpathmd5=$( (echo "$LOCPATH" | md5sum | cut -d ' ' -f 1 2>/dev/null) || true) locpathmd5=$( (echo "$LOCPATH" | md5sum | cut -d ' ' -f 1 2>/dev/null) || true)
if [ -n "$locpathmd5" ]; then if [ -n "$locpathmd5" ]; then
@ -149,24 +149,22 @@ if [ -z "${LOCPATH+set}" ] && [ -z "$GIT_ANNEX_PACKAGE_INSTALL" ]; then
fi fi
done done
# If the locale cache for this bundle is out of date, refresh it. if [ ! -d "$LOCPATH" ]; then
if [ -e "$LOCPATH/buildid" ] && ! cmp "$LOCPATH/buildid" "$base/buildid" >/dev/null 2>&1 ; then if ! mkdir -p "$LOCPATH"; then
rm -rf "$LOCPATH" echo "Unable to write to $LOCPATH; can't continue!" >&2
exit 1
fi
fi fi
if ! mkdir -p "$LOCPATH"; then if [ ! -e "$LOCPATH/base" ]; then
echo "Unable to write to $LOCPATH; can't continue!" >&2 echo "$base" > "$LOCPATH/base.$$"
exit 1 mv -f "$LOCPATH/base.$$" "$LOCPATH/base"
fi
# Not using cp to avoid using the one bundled with git-annex
# before the environment is set up to run it.
if [ ! -e "$LOCPATH/buildid" ]; then
cat < "$base/buildid" > "$LOCPATH/buildid.$$"
mv -f "$LOCPATH/buildid.$$" "$LOCPATH/buildid"
fi fi
# This is updated each time, because the bundle could be moved to a
# different directory.
echo "$base" > "$LOCPATH/base.$$"
mv -f "$LOCPATH/base.$$" "$LOCPATH/base"
# This is updated each time, because the bundle could be updated
# in place to a new version.
# Not using cp to avoid using the one bundled with git-annex before
# the environment is set up to run it.
cat < "$base/buildid" > "$LOCPATH/buildid.$$"
mv -f "$LOCPATH/buildid.$$" "$LOCPATH/buildid"
# Generate locale definition files for the locales in use, # Generate locale definition files for the locales in use,
# using the localedef and locale files from the bundle. # using the localedef and locale files from the bundle.