[bitbake-devel] bitbake: fetch/git: add support for SRC_URI containing spaces in url

Submitted by Charlie Davies on Sept. 15, 2020, 9:12 p.m. | Patch ID: 176545

Details

Message ID 20200915211214.140434-1-charles.davies@whitetree.xyz
State New
Headers show

Commit Message

Charlie Davies Sept. 15, 2020, 9:12 p.m.
Microsoft's TFS VCS system allows for spaces in a git repository url.
An example of a valid url is:

ssh://tfs-my-company.org:22/tfs/My Projects/FooBar

This commit adds support for such urls by implementing two changes.
Firstly, when bitbake makes a git command line call the url is
surrounded by quotes so that the url, regardless of spaces, is
treated as one argument. Secondly, additional parsing of various
filepath variables, which are based off of the url, are now
completed with any spaces in the url replaced with underscores.

Signed-off-by: Charlie Davies <charles.davies@whitetree.xyz>
---
 bitbake/lib/bb/fetch2/git.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
index 644ba9238b..07064c694f 100644
--- a/bitbake/lib/bb/fetch2/git.py
+++ b/bitbake/lib/bb/fetch2/git.py
@@ -236,7 +236,7 @@  class Git(FetchMethod):
                     ud.unresolvedrev[name] = ud.revisions[name]
                 ud.revisions[name] = self.latest_revision(ud, d, name)
 
-        gitsrcname = '%s%s' % (ud.host.replace(':', '.'), ud.path.replace('/', '.').replace('*', '.'))
+        gitsrcname = '%s%s' % (ud.host.replace(':', '.'), ud.path.replace('/', '.').replace('*', '.').replace(' ','_'))
         if gitsrcname.startswith('.'):
             gitsrcname = gitsrcname[1:]
 
@@ -342,7 +342,7 @@  class Git(FetchMethod):
             # We do this since git will use a "-l" option automatically for local urls where possible
             if repourl.startswith("file://"):
                 repourl = repourl[7:]
-            clone_cmd = "LANG=C %s clone --bare --mirror %s %s --progress" % (ud.basecmd, repourl, ud.clonedir)
+            clone_cmd = "LANG=C %s clone --bare --mirror \"%s\" %s --progress" % (ud.basecmd, repourl, ud.clonedir)
             if ud.proto.lower() != 'file':
                 bb.fetch2.check_network_access(d, clone_cmd, ud.url)
             progresshandler = GitProgressHandler(d)
@@ -354,8 +354,8 @@  class Git(FetchMethod):
             if "origin" in output:
               runfetchcmd("%s remote rm origin" % ud.basecmd, d, workdir=ud.clonedir)
 
-            runfetchcmd("%s remote add --mirror=fetch origin %s" % (ud.basecmd, repourl), d, workdir=ud.clonedir)
-            fetch_cmd = "LANG=C %s fetch -f --progress %s refs/*:refs/*" % (ud.basecmd, repourl)
+            runfetchcmd("%s remote add --mirror=fetch origin \"%s\"" % (ud.basecmd, repourl), d, workdir=ud.clonedir)
+            fetch_cmd = "LANG=C %s fetch -f --progress \"%s\" refs/*:refs/*" % (ud.basecmd, repourl)
             if ud.proto.lower() != 'file':
                 bb.fetch2.check_network_access(d, fetch_cmd, ud.url)
             progresshandler = GitProgressHandler(d)
@@ -504,7 +504,7 @@  class Git(FetchMethod):
             raise bb.fetch2.UnpackError("No up to date source found: " + "; ".join(source_error), ud.url)
 
         repourl = self._get_repo_url(ud)
-        runfetchcmd("%s remote set-url origin %s" % (ud.basecmd, repourl), d, workdir=destdir)
+        runfetchcmd("%s remote set-url origin \"%s\"" % (ud.basecmd, repourl), d, workdir=destdir)
 
         if self._contains_lfs(ud, d, destdir):
             if need_lfs and not self._find_git_lfs(d):
@@ -623,7 +623,7 @@  class Git(FetchMethod):
         d.setVar('_BB_GIT_IN_LSREMOTE', '1')
         try:
             repourl = self._get_repo_url(ud)
-            cmd = "%s ls-remote %s %s" % \
+            cmd = "%s ls-remote \"%s\" %s" % \
                 (ud.basecmd, repourl, search)
             if ud.proto.lower() != 'file':
                 bb.fetch2.check_network_access(d, cmd, repourl)

Comments

Richard Purdie Sept. 16, 2020, 12:58 p.m.
On Tue, 2020-09-15 at 22:12 +0100, Charlie Davies wrote:
> Microsoft's TFS VCS system allows for spaces in a git repository url.
> An example of a valid url is:
> 
> ssh://tfs-my-company.org:22/tfs/My Projects/FooBar
> 
> This commit adds support for such urls by implementing two changes.
> Firstly, when bitbake makes a git command line call the url is
> surrounded by quotes so that the url, regardless of spaces, is
> treated as one argument. Secondly, additional parsing of various
> filepath variables, which are based off of the url, are now
> completed with any spaces in the url replaced with underscores.
> 
> Signed-off-by: Charlie Davies <charles.davies@whitetree.xyz>
> ---
>  bitbake/lib/bb/fetch2/git.py | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Hi,

Thanks for the patch. I wondered if you'd be able to add a test to
lib/bb/tests/fetch.py to cover this please? They're run by bitbake-
selftest.

I'd also caution that whilst this change is probably fine to support
the cloning of such repositories, the build doesn't work well for
repositories with spaces in the build paths. We're limited in what we
can do about it as autotools doesn't handle it well.

Cheers,
Richard
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#11698): https://lists.openembedded.org/g/bitbake-devel/message/11698
Mute This Topic: https://lists.openembedded.org/mt/76874814/3617530
Group Owner: bitbake-devel+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Charlie Davies Sept. 16, 2020, 9:28 p.m.
Hi,

I have added some unit tests and submitted v2 of the patch for your consideration.

I added unit tests to test the uri parsing and the removal of spaces in the filepaths. I didn't add a test for the addition of quotes in the git commands. The only method to test this that I could think of is to mock the runfetchmd. This seemed very invasive and looking at the rest of the tests not in keeping with what has been done before. However, I ran a full build of my project overnight and there weren't any issues with the patch applied.

Yes, I agree the purpose of this patch would only be to cope with spaces in the url of a repository.

Do I have to do anything else to have the patch merged into dunfell - if it is accepted - as this is the branch of poky I am working from?

Many Thanks,

Charlie
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#11702): https://lists.openembedded.org/g/bitbake-devel/message/11702
Mute This Topic: https://lists.openembedded.org/mt/76874814/3617530
Group Owner: bitbake-devel+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-