| Message ID | 20230913154645.3699632-1-JPEWhacker@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | fetch2: git: Use samestat() instead of path for repo check | expand |
Oops sorry I sent this to the wrong mailing list. Should be bitbake-devel not openembedded-core Anyway, this should fix your problem Mikko and Martin On Wed, Sep 13, 2023 at 9:46 AM Joshua Watt <jpewhacker@gmail.com> wrote: > > Using path prefixes to check if the git directory is a descendant of the > clone directory can be easily confused with symlinkes and bind mounts, > causing directories to be deleted unnecessarily. Instead, use > os.path.samestat() which is immune to the these sorts of problems. The > code needs manually check all parent directories until it has recursed > up to the root to make sure, but in the common case where the > directories do actually contain the correct git repository, this will > only run a single iteration. > > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com> > --- > bitbake/lib/bb/fetch2/git.py | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py > index e11271b757a..5703ba335e0 100644 > --- a/bitbake/lib/bb/fetch2/git.py > +++ b/bitbake/lib/bb/fetch2/git.py > @@ -374,19 +374,25 @@ class Git(FetchMethod): > # Since clones can be bare, use --absolute-git-dir instead of --show-toplevel > output = runfetchcmd("LANG=C %s rev-parse --absolute-git-dir" % ud.basecmd, d, workdir=ud.clonedir) > > - toplevel = os.path.abspath(output.rstrip()) > - abs_clonedir = os.path.abspath(ud.clonedir).rstrip('/') > - # The top level Git directory must either be the clone directory > - # or a child of the clone directory. Any ancestor directory of > - # the clone directory is not valid as the Git directory (and > - # probably belongs to some other unrelated repository), so a > - # clone is required > - if os.path.commonprefix([abs_clonedir, toplevel]) != abs_clonedir: > - logger.warning("Top level directory '%s' doesn't match expected '%s'. Re-cloning", toplevel, ud.clonedir) > + clonedir_stat = os.stat(ud.clonedir) > + toplevel = os.path.abspath(output.rstrip()).rstrip("/") > + check_dir = toplevel > + > + while check_dir: > + check_stat = os.stat(check_dir) > + if os.path.samestat(check_stat, clonedir_stat): > + break > + check_dir = os.path.dirname(check_dir).rstrip("/") > + > + if not check_dir: > + logger.warning("Top level directory '%s' is not a descendant of '%s'. Re-cloning", toplevel, ud.clonedir) > needs_clone = True > except bb.fetch2.FetchError as e: > logger.warning("Unable to get top level for %s (not a git directory?): %s", ud.clonedir, e) > needs_clone = True > + except FileNotFoundError as e: > + logger.warning("%s", e) > + needs_clone = True > > if needs_clone: > shutil.rmtree(ud.clonedir) > -- > 2.34.1 >
diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py index e11271b757a..5703ba335e0 100644 --- a/bitbake/lib/bb/fetch2/git.py +++ b/bitbake/lib/bb/fetch2/git.py @@ -374,19 +374,25 @@ class Git(FetchMethod): # Since clones can be bare, use --absolute-git-dir instead of --show-toplevel output = runfetchcmd("LANG=C %s rev-parse --absolute-git-dir" % ud.basecmd, d, workdir=ud.clonedir) - toplevel = os.path.abspath(output.rstrip()) - abs_clonedir = os.path.abspath(ud.clonedir).rstrip('/') - # The top level Git directory must either be the clone directory - # or a child of the clone directory. Any ancestor directory of - # the clone directory is not valid as the Git directory (and - # probably belongs to some other unrelated repository), so a - # clone is required - if os.path.commonprefix([abs_clonedir, toplevel]) != abs_clonedir: - logger.warning("Top level directory '%s' doesn't match expected '%s'. Re-cloning", toplevel, ud.clonedir) + clonedir_stat = os.stat(ud.clonedir) + toplevel = os.path.abspath(output.rstrip()).rstrip("/") + check_dir = toplevel + + while check_dir: + check_stat = os.stat(check_dir) + if os.path.samestat(check_stat, clonedir_stat): + break + check_dir = os.path.dirname(check_dir).rstrip("/") + + if not check_dir: + logger.warning("Top level directory '%s' is not a descendant of '%s'. Re-cloning", toplevel, ud.clonedir) needs_clone = True except bb.fetch2.FetchError as e: logger.warning("Unable to get top level for %s (not a git directory?): %s", ud.clonedir, e) needs_clone = True + except FileNotFoundError as e: + logger.warning("%s", e) + needs_clone = True if needs_clone: shutil.rmtree(ud.clonedir)
Using path prefixes to check if the git directory is a descendant of the clone directory can be easily confused with symlinkes and bind mounts, causing directories to be deleted unnecessarily. Instead, use os.path.samestat() which is immune to the these sorts of problems. The code needs manually check all parent directories until it has recursed up to the root to make sure, but in the common case where the directories do actually contain the correct git repository, this will only run a single iteration. Signed-off-by: Joshua Watt <JPEWhacker@gmail.com> --- bitbake/lib/bb/fetch2/git.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)