[bitbake][dunfell][1.46][PATCH 0/2] Patch review

[bitbake][dunfell][1.46][PATCH 0/2] Patch review

[Steve Sakoman]

Please review this next set of patches for 1.46 (dunfell) and have comments
back by end of day Wednesday.

Passed a-full on autobuilder:

https://autobuilder.yoctoproject.org/typhoon/#/builders/83/builds/1466

The following changes since commit 18e1957337fd9f06bc673d28dd4f8277321d07bc:

  tests/fetch: Move away from problematic freedesktop.org urls (2020-09-17 09:13:32 -1000)

are available in the Git repository at:

  git://git.openembedded.org/bitbake-contrib stable/1.46-nut
  http://cgit.openembedded.org/bitbake-contrib/log/?h=stable/1.46-nut

Charlie Davies (2):
  bitbake: fetch/git: add support for SRC_URI containing spaces in url
  bitbake: tests/fetch: add unit tests for SRC_URI with spaces in url

 lib/bb/fetch2/git.py  | 12 +++++------
 lib/bb/tests/fetch.py | 47 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 6 deletions(-)

-- 
2.17.1

[bitbake][dunfell][1.46][PATCH 1/2] bitbake: fetch/git: add support for SRC_URI containing spaces in url

[Steve Sakoman]

From: Charlie Davies <charles.davies@whitetree.xyz>

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>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit eb38b6f0935763f7ba19e5618f376fcae1dac41a)
Signed-off-by: Steve Sakoman <steve@sakoman.com>
---
 lib/bb/fetch2/git.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index 2ce9395f..dcecff5d 100644
--- a/lib/bb/fetch2/git.py
+++ b/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)
@@ -501,7 +501,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):
@@ -613,7 +613,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)
-- 
2.17.1

[bitbake][dunfell][1.46][PATCH 2/2] bitbake: tests/fetch: add unit tests for SRC_URI with spaces in url

[Steve Sakoman]

From: Charlie Davies <charles.davies@whitetree.xyz>

Signed-off-by: Charlie Davies <charles.davies@whitetree.xyz>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit e7dab75c8d1923abcbbc7c9ac7de215d720ccf26)
Signed-off-by: Steve Sakoman <steve@sakoman.com>
---
 lib/bb/tests/fetch.py | 47 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index b1883541..f7fe78b4 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -223,6 +223,21 @@ class URITest(unittest.TestCase):
             'query': {},
             'relative': False
         },
+        "git://tfs-example.org:22/tfs/example%20path/example.git": {
+            'uri': 'git://tfs-example.org:22/tfs/example%20path/example.git',
+            'scheme': 'git',
+            'hostname': 'tfs-example.org',
+            'port': 22,
+            'hostport': 'tfs-example.org:22',
+            'path': '/tfs/example path/example.git',
+            'userinfo': '',
+            'userinfo': '',
+            'username': '',
+            'password': '',
+            'params': {},
+            'query': {},
+            'relative': False
+        },
         "http://somesite.net;someparam=1": {
             'uri': 'http://somesite.net;someparam=1',
             'scheme': 'http',
@@ -2080,6 +2095,38 @@ class GitLfsTest(FetcherTest):
         shutil.rmtree(self.gitdir, ignore_errors=True)
         fetcher.unpack(self.d.getVar('WORKDIR'))
 
+class GitURLWithSpacesTest(FetcherTest):
+    test_git_urls = {
+        "git://tfs-example.org:22/tfs/example%20path/example.git" : {
+            'url': 'git://tfs-example.org:22/tfs/example%20path/example.git',
+            'gitsrcname': 'tfs-example.org.22.tfs.example_path.example.git',
+            'path': '/tfs/example path/example.git'
+        },
+        "git://tfs-example.org:22/tfs/example%20path/example%20repo.git" : {
+            'url': 'git://tfs-example.org:22/tfs/example%20path/example%20repo.git',
+            'gitsrcname': 'tfs-example.org.22.tfs.example_path.example_repo.git',
+            'path': '/tfs/example path/example repo.git'
+        }
+    }
+
+    def test_urls(self):
+
+        # Set fake SRCREV to stop git fetcher from trying to contact non-existent git repo
+        self.d.setVar('SRCREV', '82ea737a0b42a8b53e11c9cde141e9e9c0bd8c40')
+
+        for test_git_url, ref in self.test_git_urls.items():
+
+            fetcher = bb.fetch.Fetch([test_git_url], self.d)
+            ud = fetcher.ud[fetcher.urls[0]]
+
+            self.assertEqual(ud.url, ref['url'])
+            self.assertEqual(ud.path, ref['path'])
+            self.assertEqual(ud.localfile, os.path.join(self.dldir, "git2", ref['gitsrcname']))
+            self.assertEqual(ud.localpath, os.path.join(self.dldir, "git2", ref['gitsrcname']))
+            self.assertEqual(ud.lockfile, os.path.join(self.dldir, "git2", ref['gitsrcname'] + '.lock'))
+            self.assertEqual(ud.clonedir, os.path.join(self.dldir, "git2", ref['gitsrcname']))
+            self.assertEqual(ud.fullmirror, os.path.join(self.dldir, "git2_" + ref['gitsrcname'] + '.tar.gz'))
+
 class NPMTest(FetcherTest):
     def skipIfNoNpm():
         import shutil
-- 
2.17.1

Re: [bitbake-devel] [bitbake][dunfell][1.46][PATCH 1/2] bitbake: fetch/git: add support for SRC_URI containing spaces in url

[Enrico Scholz]

"Steve Sakoman" <steve@sakoman.com> writes:

> 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.
>
> -            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)

Why do you quote it manually (and with double instead of single quote))
instead of using shlex.quote() or making the clone_cmd an array?


Enrico

Re: [bitbake-devel] [bitbake][dunfell][1.46][PATCH 1/2] bitbake: fetch/git: add support for SRC_URI containing spaces in url

[Steve Sakoman]

On Tue, Oct 13, 2020 at 12:51 AM Enrico Scholz
<enrico.scholz@sigma-chemnitz.de> wrote:
>
> "Steve Sakoman" <steve@sakoman.com> writes:
>
> > 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.
> >
> > -            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)
>
> Why do you quote it manually (and with double instead of single quote))
> instead of using shlex.quote() or making the clone_cmd an array?

This question is best addressed to the patch author (copied)

Note that this patch has already been accepted into the master branch,
and my request for comment was just for cherry-picking the commit into
the dunfell branch.

Steve

Re: [bitbake][dunfell][1.46][PATCH 1/2] bitbake: fetch/git: add support for SRC_URI containing spaces in url

[Charlie Davies]

[-- Attachment #1: Type: text/plain, Size: 360 bytes --]

Thank you for the feedback Enrico.

I agree that using shlex.quote() is a better method to implement this functionality.

I have written a patch using shlex.quote() instead of escaped double quotes. I will run this patch through my local setup tonight before submitting in the morning.

Steve, I will submit against master, dunfell and 1.46.

Charlie

[-- Attachment #2: Type: text/html, Size: 392 bytes --]