This looks good, though if we're creating the temporary file in the same directory as the resulting file, which seems to be the case, could we not just os.rename() rather than calling a subprocess?
There is a potential race over the mirror tarballs where a partial git repo
could be extracted causing fetcher failures if the tarball is being rewritten
whilst another build accesses it.
Create the mirror tarball atomically to avoid this.
[YOCTO #14441]
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
lib/bb/fetch2/git.py | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index 488f4c7414..3643a491d7 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -68,6 +68,7 @@ import subprocess
import tempfile
import bb
import bb.progress
+from contextlib import contextmanager
from bb.fetch2 import FetchMethod
from bb.fetch2 import runfetchcmd
from bb.fetch2 import logger
@@ -418,6 +419,20 @@ class Git(FetchMethod):
bb.utils.remove(tmpdir, recurse=True)
def build_mirror_data(self, ud, d):
+
+ # Create as a temp file and move atomically into position to avoid races
+ @contextmanager
+ def create_atomic(filename, d):
+ fd, tfile = tempfile.mkstemp(dir=os.path.dirname(filename))
+ try:
+ yield tfile
+ umask = os.umask(0o666)
+ os.umask(umask)
+ os.chmod(tfile, (0o666 & ~umask))
+ runfetchcmd("mv %s %s" % (tfile, filename), d)
+ finally:
+ os.close(fd)
+
if ud.shallow and ud.write_shallow_tarballs:
if not os.path.exists(ud.fullshallow):
if os.path.islink(ud.fullshallow):
@@ -428,7 +443,8 @@ class Git(FetchMethod):
self.clone_shallow_local(ud, shallowclone, d)
logger.info("Creating tarball of git repository")
- runfetchcmd("tar -czf %s ." % ud.fullshallow, d, workdir=shallowclone)
+ with create_atomic(ud.fullshallow, d) as tfile:
+ runfetchcmd("tar -czf %s ." % tfile, d, workdir=shallowclone)
runfetchcmd("touch %s.done" % ud.fullshallow, d)
finally:
bb.utils.remove(tempdir, recurse=True)
@@ -437,7 +453,8 @@ class Git(FetchMethod):
os.unlink(ud.fullmirror)
logger.info("Creating tarball of git repository")
- runfetchcmd("tar -czf %s ." % ud.fullmirror, d, workdir=ud.clonedir)
+ with create_atomic(ud.fullmirror, d) as tfile:
+ runfetchcmd("tar -czf %s ." % tfile, d, workdir=ud.clonedir)
runfetchcmd("touch %s.done" % ud.fullmirror, d)
def clone_shallow_local(self, ud, dest, d):
--
2.32.0
--
Christopher Larson
kergoth at gmail dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Senior Software Engineer, Mentor Graphics