[PATCH 0/2] gitsm fetcher, fix submodule unpack issues

[PATCH 0/2] gitsm fetcher, fix submodule unpack issues

[Mark Hatle]

Khem Raj discovered that a number of recipes using gitsm were failing.  This
was tied back to two items.  A mistake in the way the relative URLs was
being handled after the prior patch set, and the way the modules directory
of the various submodule components was being constructed.

This set fixes both issues, and additional tests for the specific cases
that Khem mentioned were also added to the test suite.


Mark Hatle (2):
  gitsm.py: Fix relative URLs
  gitsmy.py: Fix unpack of submodules of submodules

 lib/bb/fetch2/gitsm.py | 20 ++++++++++++--------
 lib/bb/tests/fetch.py  | 30 ++++++++++++++++++++++++++++--
 2 files changed, 40 insertions(+), 10 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] gitsm.py: Fix relative URLs

[Mark Hatle]

Prior code happened to work with relative URLs, when the code was recently
restructured it caused all relative urls to no longer work.  Restore the prior
code flow for relative support and better comment why that code is there.

Signed-off-by: Mark Hatle <mark.hatle@windriver.com>
---
 lib/bb/fetch2/gitsm.py | 11 ++++++-----
 lib/bb/tests/fetch.py  |  4 ++--
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/bb/fetch2/gitsm.py b/lib/bb/fetch2/gitsm.py
index 86198ee..faffb4c 100644
--- a/lib/bb/fetch2/gitsm.py
+++ b/lib/bb/fetch2/gitsm.py
@@ -98,6 +98,12 @@ class GitSM(Git):
                 uris[m] = md['url']
                 subrevision[m] = module_hash.split()[2]
 
+                # Convert relative to absolute uri based on parent uri
+                if uris[m].startswith('..'):
+                    newud = copy.copy(ud)
+                    newud.path = os.path.realpath(os.path.join(newud.path, uris[m]))
+                    uris[m] = Git._get_repo_url(self, newud)
+
         for module in submodules:
             # Translate the module url into a SRC_URI
 
@@ -120,11 +126,6 @@ class GitSM(Git):
                     # This has to be a file reference
                     proto = "file"
                     url = "gitsm://" + uris[module]
-                    if uris[module].startswith('..'):
-                        # Local on disk relative reference
-                        newud = copy.copy(ud)
-                        newud.path = os.path.realpath(os.path.join(newud.path, md['url']))
-                        url = "gitsm://" + Git._get_repo_url(self, newud)
 
             url += ';protocol=%s' % proto
             url += ";name=%s" % module
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 1497a3c..de3b0ce 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -894,9 +894,9 @@ class FetcherNetworkTest(FetcherTest):
     @skipIfNoNetwork()
     def test_git_submodule(self):
         # URL with ssh submodules
-        url = "gitsm://git.yoctoproject.org/git-submodule-test;branch=ssh-gitsm-tests;rev=f53765f515e0eeca569ed385bb1c89ce008bb058"
+        url = "gitsm://git.yoctoproject.org/git-submodule-test;branch=ssh-gitsm-tests;rev=049da4a6cb198d7c0302e9e8b243a1443cb809a7"
         # Original URL (comment this if you have ssh access to git.yoctoproject.org)
-        url = "gitsm://git.yoctoproject.org/git-submodule-test;branch=master;rev=132fea6e4dee56b61bcf5721c94e8b2445c6a017"
+        url = "gitsm://git.yoctoproject.org/git-submodule-test;branch=master;rev=a2885dd7d25380d23627e7544b7bbb55014b16ee"
         fetcher = bb.fetch.Fetch([url], self.d)
         fetcher.download()
         # Previous cwd has been deleted
-- 
1.8.3.1

[PATCH 2/2] gitsmy.py: Fix unpack of submodules of submodules

[Mark Hatle]

If the submodule is in a subdirectory, it needs to have that structure
preserved.  This means the unpack path needs to be in the 'dirname' of the
final path -- since the unpack directory name is specified in the URI.

Additional specific test cases were added to ensure this is working properly
based on two recent error reports.

Signed-off-by: Mark Hatle <mark.hatle@windriver.com>
---
 lib/bb/fetch2/gitsm.py |  9 ++++++---
 lib/bb/tests/fetch.py  | 26 ++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/lib/bb/fetch2/gitsm.py b/lib/bb/fetch2/gitsm.py
index faffb4c..f45546b 100644
--- a/lib/bb/fetch2/gitsm.py
+++ b/lib/bb/fetch2/gitsm.py
@@ -176,12 +176,11 @@ class GitSM(Git):
 
             try:
                 newfetch = Fetch([url], d, cache=False)
-                newfetch.unpack(root=os.path.join(repo_conf, 'modules'))
+                newfetch.unpack(root=os.path.dirname(os.path.join(repo_conf, 'modules', modpath)))
             except Exception as e:
                 logger.error('gitsm: submodule unpack failed: %s %s' % (type(e).__name__, str(e)))
                 raise
 
-            newfetch = Fetch([url], d, cache=False)
             local_path = newfetch.localpath(url)
 
             # Correct the submodule references to the local download version...
@@ -191,7 +190,11 @@ class GitSM(Git):
                 runfetchcmd("%(basecmd)s config submodule.%(module)s.shallow true" % {'basecmd': ud.basecmd, 'module': module}, d, workdir=ud.destdir)
 
             # Ensure the submodule repository is NOT set to bare, since we're checking it out...
-            runfetchcmd("%s config core.bare false" % (ud.basecmd), d, quiet=True, workdir=os.path.join(repo_conf, 'modules', modpath))
+            try:
+                runfetchcmd("%s config core.bare false" % (ud.basecmd), d, quiet=True, workdir=os.path.join(repo_conf, 'modules', modpath))
+            except:
+                logger.error("Unable to set git config core.bare to false for %s" % os.path.join(repo_conf, 'modules', modpath))
+                raise
 
         Git.unpack(self, ud, destdir, d)
 
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index de3b0ce..e9ad807 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -912,6 +912,32 @@ class FetcherNetworkTest(FetcherTest):
         if os.path.exists(os.path.join(repo_path, 'bitbake-gitsm-test1')):
             self.assertTrue(os.path.exists(os.path.join(repo_path, 'bitbake-gitsm-test1', 'bitbake')), msg='submodule of submodule missing')
 
+        # The following external repositories have show failures in fetch and unpack operations
+        # We want to avoid regressions!
+        url = "gitsm://github.com/bus1/dbus-broker;protocol=git;rev=fc874afa0992d0c75ec25acb43d344679f0ee7d2"
+        fetcher = bb.fetch.Fetch([url], self.d)
+        fetcher.download()
+        # Previous cwd has been deleted
+        os.chdir(os.path.dirname(self.unpackdir))
+        fetcher.unpack(self.unpackdir)
+
+        repo_path = os.path.join(self.tempdir, 'unpacked', 'git')
+        self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/subprojects/c-dvar/config')), msg='Missing submodule config "subprojects/c-dvar"')
+        self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/subprojects/c-list/config')), msg='Missing submodule config "subprojects/c-list"')
+        self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/subprojects/c-rbtree/config')), msg='Missing submodule config "subprojects/c-rbtree"')
+        self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/subprojects/c-sundry/config')), msg='Missing submodule config "subprojects/c-sundry"')
+        self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/subprojects/c-utf8/config')), msg='Missing submodule config "subprojects/c-utf8"')
+
+        url = "gitsm://github.com/CLIUtils/CLI11;protocol=git;rev=bd4dc911847d0cde7a6b41dfa626a85aab213baf"
+        fetcher = bb.fetch.Fetch([url], self.d)
+        fetcher.download()
+        # Previous cwd has been deleted
+        os.chdir(os.path.dirname(self.unpackdir))
+        fetcher.unpack(self.unpackdir)
+        self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/extern/googletest/config')), msg='Missing submodule config "extern/googletest"')
+        self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/extern/json/config')), msg='Missing submodule config "extern/json"')
+        self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/extern/sanitizers/config')), msg='Missing submodule config "extern/sanitizers"')
+
 class TrustedNetworksTest(FetcherTest):
     def test_trusted_network(self):
         # Ensure trusted_network returns False when the host IS in the list.
-- 
1.8.3.1