| Message ID | 20230913190425.3761792-3-JPEWhacker@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | fetch2: git: Fix path check for git repos | expand |
Hi, On Wed, Sep 13, 2023 at 01:04:25PM -0600, Joshua Watt 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 > bb.utils.path_is_descendant() which is immune to the these sorts of > problems. I'm confused by this. What is the original problem? Corrupt download cache git repo being confused with poky git tree when building in "poky/build" directory and downloads are in "poky/build/downloads"? And at the time of check we don't know if git commit, branch, or git remote have changed and we want to avoid full clone if possible? Cheers, -Mikko > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com> > --- > bitbake/lib/bb/fetch2/git.py | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py > index e11271b757a..4385d0b37af 100644 > --- a/bitbake/lib/bb/fetch2/git.py > +++ b/bitbake/lib/bb/fetch2/git.py > @@ -373,20 +373,17 @@ class Git(FetchMethod): > try: > # 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 = output.rstrip() > > - 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) > + if not bb.utils.path_is_descendant(toplevel, ud.clonedir): > + 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 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#15057): https://lists.openembedded.org/g/bitbake-devel/message/15057 > Mute This Topic: https://lists.openembedded.org/mt/101344082/7159507 > Group Owner: bitbake-devel+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [mikko.rapeli@linaro.org] > -=-=-=-=-=-=-=-=-=-=-=- >
diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py index e11271b757a..4385d0b37af 100644 --- a/bitbake/lib/bb/fetch2/git.py +++ b/bitbake/lib/bb/fetch2/git.py @@ -373,20 +373,17 @@ class Git(FetchMethod): try: # 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 = output.rstrip() - 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) + if not bb.utils.path_is_descendant(toplevel, ud.clonedir): + 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 bb.utils.path_is_descendant() which is immune to the these sorts of problems. Signed-off-by: Joshua Watt <JPEWhacker@gmail.com> --- bitbake/lib/bb/fetch2/git.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)