[PATCH 0/2] Fix gitsm issues

[PATCH 0/2] Fix gitsm issues

[Mark Hatle]

Two issues here:

1) Fix an issue where the submodule commit may not be on a branch of the
same name as the SRC_URI entry.

2) Recursive submodules may not properly unpacked 


Mark Hatle (2):
  fetch2/gitsm.py: Disable branch checking on submodules
  fetch2/gitsm.py: Fix the references when the module and path are
    different

 lib/bb/fetch2/gitsm.py | 71 +++++++++++++++++++++++++-------------------------
 1 file changed, 35 insertions(+), 36 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] fetch2/gitsm.py: Disable branch checking on submodules

[Mark Hatle]

Submodules by definition refer to a specific commit, not branch.  If we don't
ignore the branch, then any commits on a submodule on a branch different then
the original module will trigger a failure that the commit is not on the
branch.

Signed-off-by: Mark Hatle <mark.hatle@windriver.com>
---
 lib/bb/fetch2/gitsm.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bb/fetch2/gitsm.py b/lib/bb/fetch2/gitsm.py
index 0a982da..dbfa3a4 100644
--- a/lib/bb/fetch2/gitsm.py
+++ b/lib/bb/fetch2/gitsm.py
@@ -92,7 +92,7 @@ class GitSM(Git):
             url = uris[module].replace('%s:' % proto, 'gitsm:', 1)
             url += ';protocol=%s' % proto
             url += ";name=%s" % module
-            url += ";bareclone=1;nocheckout=1"
+            url += ";bareclone=1;nocheckout=1;nobranch=1"
 
             ld = d.createCopy()
             # Not necessary to set SRC_URI, since we're passing the URI to
-- 
1.8.3.1

[PATCH 2/2] fetch2/gitsm.py: Fix the references when the module and path are different

[Mark Hatle]

Git does not require the module and target path to be the same in the
.gitmodules file.  This incorrect assumption was being made previously
causing various unpack failures.

An example .gitmodule showing this issue:

   [submodule "plugins/WaveShaper/Libs/inih"]
        path = plugins/wolf-shaper/Libs/inih
        url = https://github.com/pdesaulniers/inih.git

The unpack function also needed to work in a loop on the overall
submodules_queue.  Before it could have missed items that were not in the
primary repository.

Signed-off-by: Mark Hatle <mark.hatle@windriver.com>
---
 lib/bb/fetch2/gitsm.py | 69 +++++++++++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 35 deletions(-)

diff --git a/lib/bb/fetch2/gitsm.py b/lib/bb/fetch2/gitsm.py
index dbfa3a4..35729db 100644
--- a/lib/bb/fetch2/gitsm.py
+++ b/lib/bb/fetch2/gitsm.py
@@ -152,9 +152,9 @@ class GitSM(Git):
         if submodules and not os.path.exists(os.path.join(repo_conf, 'modules')):
             os.mkdir(os.path.join(repo_conf, 'modules'))
 
-        for module in submodules:
-            srcpath = os.path.join(ud.clonedir, 'modules', module)
-            modpath = os.path.join(repo_conf, 'modules', module)
+        for module, md in submodules.items():
+            srcpath = os.path.join(ud.clonedir, 'modules', md['path'])
+            modpath = os.path.join(repo_conf, 'modules', md['path'])
 
             if os.path.exists(srcpath):
                 if os.path.exists(os.path.join(srcpath, '.git')):
@@ -187,9 +187,8 @@ class GitSM(Git):
                 # No submodules to update
                 continue
 
-            submodules = list(self.parse_gitmodules(gitmodules).keys())
-
-        self.copy_submodules(submodules, ud, dest, d)
+            submodules = self.parse_gitmodules(gitmodules)
+            self.copy_submodules(submodules, ud, dest, d)
 
     def unpack(self, ud, destdir, d):
         Git.unpack(self, ud, destdir, d)
@@ -200,7 +199,7 @@ class GitSM(Git):
         else:
             repo_conf = os.path.join(ud.destdir, '.git')
 
-        submodules = []
+        update_submodules = False
         paths = {}
         uris = {}
         local_paths = {}
@@ -211,41 +210,41 @@ class GitSM(Git):
                 # No submodules to update
                 continue
 
-            for m, md in self.parse_gitmodules(gitmodules).items():
-                submodules.append(m)
-                paths[m] = md['path']
-                uris[m] = md['url']
+            submodules = self.parse_gitmodules(gitmodules)
+            self.copy_submodules(submodules, ud, ud.destdir, d)
+
+            submodules_queue = [(module, os.path.join(repo_conf, 'modules', md['path'])) for module, md in submodules.items()]
+            while len(submodules_queue) != 0:
+                module, modpath = submodules_queue.pop()
 
-        self.copy_submodules(submodules, ud, ud.destdir, d)
+                # add submodule children recursively
+                try:
+                    gitmodules = runfetchcmd("%s show HEAD:.gitmodules" % (ud.basecmd), d, quiet=True, workdir=modpath)
+                    for m, md in self.parse_gitmodules(gitmodules).items():
+                        submodules_queue.append([m, os.path.join(modpath, 'modules', md['path'])])
+                except:
+                    # no children
+                    pass
 
-        submodules_queue = [(module, os.path.join(repo_conf, 'modules', module)) for module in submodules]
-        while len(submodules_queue) != 0:
-            module, modpath = submodules_queue.pop()
 
-            # add submodule children recursively
-            try:
-                gitmodules = runfetchcmd("%s show HEAD:.gitmodules" % (ud.basecmd), d, quiet=True, workdir=modpath)
-                for m, md in self.parse_gitmodules(gitmodules).items():
-                    submodules_queue.append([m, os.path.join(modpath, 'modules', m)])
-            except:
-                # no children
-                pass
+                # There are submodules to update
+                update_submodules = True
 
-            # Determine (from the submodule) the correct url to reference
-            try:
-                output = runfetchcmd("%(basecmd)s config remote.origin.url" % {'basecmd': ud.basecmd}, d, workdir=modpath)
-            except bb.fetch2.FetchError as e:
-                # No remote url defined in this submodule
-                continue
+                # Determine (from the submodule) the correct url to reference
+                try:
+                    output = runfetchcmd("%(basecmd)s config remote.origin.url" % {'basecmd': ud.basecmd}, d, workdir=modpath)
+                except bb.fetch2.FetchError as e:
+                    # No remote url defined in this submodule
+                    continue
 
-            local_paths[module] = output
+                local_paths[module] = output
 
-            # Setup the local URL properly (like git submodule init or sync would do...)
-            runfetchcmd("%(basecmd)s config submodule.%(module)s.url %(url)s" % {'basecmd': ud.basecmd, 'module': module, 'url' : local_paths[module]}, d, workdir=ud.destdir)
+                # Setup the local URL properly (like git submodule init or sync would do...)
+                runfetchcmd("%(basecmd)s config submodule.%(module)s.url %(url)s" % {'basecmd': ud.basecmd, 'module': module, 'url' : local_paths[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=modpath)
+                # 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=modpath)
 
-        if submodules:
+        if update_submodules:
             # Run submodule update, this sets up the directories -- without touching the config
             runfetchcmd("%s submodule update --recursive --no-fetch" % (ud.basecmd), d, quiet=True, workdir=ud.destdir)
-- 
1.8.3.1

Re: [PATCH 1/2] fetch2/gitsm.py: Disable branch checking on submodules

[Stefan Agner]

On 31.10.2018 20:21, Mark Hatle wrote:
> Submodules by definition refer to a specific commit, not branch.  If we don't
> ignore the branch, then any commits on a submodule on a branch different then
> the original module will trigger a failure that the commit is not on the
> branch.
> 
> Signed-off-by: Mark Hatle <mark.hatle@windriver.com>

Thanks Mark, this seems to also fix the regression I mentioned a couple
of days ago:
http://lists.openembedded.org/pipermail/bitbake-devel/2018-October/009691.html

Tested-by: Stefan Agner <stefan@agner.ch>

--
Stefan

> ---
>  lib/bb/fetch2/gitsm.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/bb/fetch2/gitsm.py b/lib/bb/fetch2/gitsm.py
> index 0a982da..dbfa3a4 100644
> --- a/lib/bb/fetch2/gitsm.py
> +++ b/lib/bb/fetch2/gitsm.py
> @@ -92,7 +92,7 @@ class GitSM(Git):
>              url = uris[module].replace('%s:' % proto, 'gitsm:', 1)
>              url += ';protocol=%s' % proto
>              url += ";name=%s" % module
> -            url += ";bareclone=1;nocheckout=1"
> +            url += ";bareclone=1;nocheckout=1;nobranch=1"
>  
>              ld = d.createCopy()
>              # Not necessary to set SRC_URI, since we're passing the URI to
> -- 
> 1.8.3.1