bitbake-devel.lists.openembedded.org archive mirror
 help / color / mirror / Atom feed
* discussion: git fetcher now needs either file:// or -s
@ 2023-02-28 13:35 Sam Liddicott
  2023-03-13 13:35 ` [bitbake-devel] " Theodore A. Roth
  0 siblings, 1 reply; 3+ messages in thread
From: Sam Liddicott @ 2023-02-28 13:35 UTC (permalink / raw)
  To: bitbake-devel

These recent security changes to git prevent git.py cloning of local
repos that contain symlinks in the .git directories (e.g. a repo-tool
manifest checkout)

* https://github.com/git/git/commit/36596fd2dfa473cf1069d23776e62cc156e7b5c6
   clone: better handle symlinked files at .git/objects/
* https://github.com/git/git/commit/6f054f9fb3a501c35b55c65e547a244f14c38d56
   builtin/clone.c: disallow --local clones with symlinks
* https://github.com/git/git/commit/bffc762f87ae8d18c6001bf0044a76004245754c
   dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS

To prevent some local clones from failing, the bitbake git fetcher
could use the -s like this:

-            clone_cmd = "LANG=C %s clone --bare --mirror \"%s\" %s
--progress" % (ud.basecmd, repourl, ud.clonedir)
+            clone_cmd = "LANG=C %s clone -s --bare --mirror \"%s\" %s
--progress" % (ud.basecmd, repourl, ud.clonedir)

Instead of "-s", others may prefer to restore the url prefix file://
which also works,

-            if repourl.startswith("file://"):
-                repourl = repourl[7:]

I don't know which bests represents the original intention behind
removing file://, but "-s" gives a quicker clone of what for me is a
symlinked source repo anyway, and for the lifetime of a build I'm not
expecting the local original source repo to vanish in a way that will
affect the build.

Sam


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bitbake-devel] discussion: git fetcher now needs either file:// or -s
  2023-02-28 13:35 discussion: git fetcher now needs either file:// or -s Sam Liddicott
@ 2023-03-13 13:35 ` Theodore A. Roth
  2023-03-24  7:22   ` Robert Yang
  0 siblings, 1 reply; 3+ messages in thread
From: Theodore A. Roth @ 2023-03-13 13:35 UTC (permalink / raw)
  To: Sam Liddicott; +Cc: bitbake-devel

On Tue, Feb 28, 2023 at 6:36 AM Sam Liddicott <sam@liddicott.com> wrote:
>
> These recent security changes to git prevent git.py cloning of local
> repos that contain symlinks in the .git directories (e.g. a repo-tool
> manifest checkout)
>
> * https://github.com/git/git/commit/36596fd2dfa473cf1069d23776e62cc156e7b5c6
>    clone: better handle symlinked files at .git/objects/
> * https://github.com/git/git/commit/6f054f9fb3a501c35b55c65e547a244f14c38d56
>    builtin/clone.c: disallow --local clones with symlinks
> * https://github.com/git/git/commit/bffc762f87ae8d18c6001bf0044a76004245754c
>    dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS
>
> To prevent some local clones from failing, the bitbake git fetcher
> could use the -s like this:
>
> -            clone_cmd = "LANG=C %s clone --bare --mirror \"%s\" %s
> --progress" % (ud.basecmd, repourl, ud.clonedir)
> +            clone_cmd = "LANG=C %s clone -s --bare --mirror \"%s\" %s
> --progress" % (ud.basecmd, repourl, ud.clonedir)
>
> Instead of "-s", others may prefer to restore the url prefix file://
> which also works,
>
> -            if repourl.startswith("file://"):
> -                repourl = repourl[7:]
>
> I don't know which bests represents the original intention behind
> removing file://, but "-s" gives a quicker clone of what for me is a
> symlinked source repo anyway, and for the lifetime of a build I'm not
> expecting the local original source repo to vanish in a way that will
> affect the build.
>
> Sam

My organisation is also experiencing this issue with recipes that
contain SRC_URI values
pointing to `file://` source that was checked out using the
google-repo tool (which happens
to create symlinks to `.git` living outside of the SRC_URI referenced
git repository).

Our work around for this, is to have each developer impacted by this
append `--no-local`
to the `clone_cmd` as such:

diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
index 5bb8393133..17c4b8ca18 100644
--- a/bitbake/lib/bb/fetch2/git.py
+++ b/bitbake/lib/bb/fetch2/git.py
@@ -373,6 +373,8 @@ class Git(FetchMethod):
             clone_cmd = "LANG=C %s clone --bare --mirror %s %s
--progress" % (ud.basecmd, shlex.quote(repourl), ud.clonedir)
             if ud.proto.lower() != 'file':
                 bb.fetch2.check_network_access(d, clone_cmd, ud.url)
+            else:
+                clone_cmd += " --no-local"
             progresshandler = GitProgressHandler(d)
             runfetchcmd(clone_cmd, d, log=progresshandler)

Digging into the git history for this functionality, it appears to
have been introduced way back in 2012 to speed up cloning from local
source repos (e.g. things like the kernel source). The work arounds
provided above, would probably have a negative impact on that
particular user case though.

Would really love to see a fix for this included in bitbake upstream
so we don't have to manually patch.

Ted Roth


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [bitbake-devel] discussion: git fetcher now needs either file:// or -s
  2023-03-13 13:35 ` [bitbake-devel] " Theodore A. Roth
@ 2023-03-24  7:22   ` Robert Yang
  0 siblings, 0 replies; 3+ messages in thread
From: Robert Yang @ 2023-03-24  7:22 UTC (permalink / raw)
  To: troth, Sam Liddicott; +Cc: bitbake-devel

Hello,

I've sent a patch for it and added you to the CC list:

fetch/git: Fix local clone url to make it work with repo

// Robert


On 3/13/23 9:35 PM, Theodore A. Roth via lists.openembedded.org wrote:
> On Tue, Feb 28, 2023 at 6:36 AM Sam Liddicott <sam@liddicott.com> wrote:
>>
>> These recent security changes to git prevent git.py cloning of local
>> repos that contain symlinks in the .git directories (e.g. a repo-tool
>> manifest checkout)
>>
>> * https://github.com/git/git/commit/36596fd2dfa473cf1069d23776e62cc156e7b5c6
>>     clone: better handle symlinked files at .git/objects/
>> * https://github.com/git/git/commit/6f054f9fb3a501c35b55c65e547a244f14c38d56
>>     builtin/clone.c: disallow --local clones with symlinks
>> * https://github.com/git/git/commit/bffc762f87ae8d18c6001bf0044a76004245754c
>>     dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS
>>
>> To prevent some local clones from failing, the bitbake git fetcher
>> could use the -s like this:
>>
>> -            clone_cmd = "LANG=C %s clone --bare --mirror \"%s\" %s
>> --progress" % (ud.basecmd, repourl, ud.clonedir)
>> +            clone_cmd = "LANG=C %s clone -s --bare --mirror \"%s\" %s
>> --progress" % (ud.basecmd, repourl, ud.clonedir)
>>
>> Instead of "-s", others may prefer to restore the url prefix file://
>> which also works,
>>
>> -            if repourl.startswith("file://"):
>> -                repourl = repourl[7:]
>>
>> I don't know which bests represents the original intention behind
>> removing file://, but "-s" gives a quicker clone of what for me is a
>> symlinked source repo anyway, and for the lifetime of a build I'm not
>> expecting the local original source repo to vanish in a way that will
>> affect the build.
>>
>> Sam
> 
> My organisation is also experiencing this issue with recipes that
> contain SRC_URI values
> pointing to `file://` source that was checked out using the
> google-repo tool (which happens
> to create symlinks to `.git` living outside of the SRC_URI referenced
> git repository).
> 
> Our work around for this, is to have each developer impacted by this
> append `--no-local`
> to the `clone_cmd` as such:
> 
> diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
> index 5bb8393133..17c4b8ca18 100644
> --- a/bitbake/lib/bb/fetch2/git.py
> +++ b/bitbake/lib/bb/fetch2/git.py
> @@ -373,6 +373,8 @@ class Git(FetchMethod):
>               clone_cmd = "LANG=C %s clone --bare --mirror %s %s
> --progress" % (ud.basecmd, shlex.quote(repourl), ud.clonedir)
>               if ud.proto.lower() != 'file':
>                   bb.fetch2.check_network_access(d, clone_cmd, ud.url)
> +            else:
> +                clone_cmd += " --no-local"
>               progresshandler = GitProgressHandler(d)
>               runfetchcmd(clone_cmd, d, log=progresshandler)
> 
> Digging into the git history for this functionality, it appears to
> have been introduced way back in 2012 to speed up cloning from local
> source repos (e.g. things like the kernel source). The work arounds
> provided above, would probably have a negative impact on that
> particular user case though.
> 
> Would really love to see a fix for this included in bitbake upstream
> so we don't have to manually patch.
> 
> Ted Roth
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#14551): https://lists.openembedded.org/g/bitbake-devel/message/14551
> Mute This Topic: https://lists.openembedded.org/mt/97289801/3616940
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [liezhi.yang@windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-03-24  7:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-28 13:35 discussion: git fetcher now needs either file:// or -s Sam Liddicott
2023-03-13 13:35 ` [bitbake-devel] " Theodore A. Roth
2023-03-24  7:22   ` Robert Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).