[PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[Matt Hoosier]

Although the submodules' histories have been fetched during the
do_fetch() phase, the mechanics used to clone the workdir copy
of the repo haven't been transferring the actual .git/modules
directory from the repo fetched into downloads/ during the
fetch task.

Fix that, and for good measure also explicitly tell Git to avoid
hitting the network during do_unpack() of the submodules.

[YOCTO #12739]

Signed-off-by: Matt Hoosier <matt.hoosier@gmail.com>
---
 lib/bb/fetch2/gitsm.py | 84 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 76 insertions(+), 8 deletions(-)

diff --git a/lib/bb/fetch2/gitsm.py b/lib/bb/fetch2/gitsm.py
index 0aff1008..7ac0dbb1 100644
--- a/lib/bb/fetch2/gitsm.py
+++ b/lib/bb/fetch2/gitsm.py
@@ -98,7 +98,7 @@ class GitSM(Git):
                         for line in lines:
                             f.write(line)
 
-    def update_submodules(self, ud, d):
+    def update_submodules(self, ud, d, allow_network):
         # We have to convert bare -> full repo, do the submodule bit, then convert back
         tmpclonedir = ud.clonedir + ".tmp"
         gitdir = tmpclonedir + os.sep + ".git"
@@ -108,11 +108,47 @@ class GitSM(Git):
         runfetchcmd("sed " + gitdir + "/config -i -e 's/bare.*=.*true/bare = false/'", d)
         runfetchcmd(ud.basecmd + " reset --hard", d, workdir=tmpclonedir)
         runfetchcmd(ud.basecmd + " checkout -f " + ud.revisions[ud.names[0]], d, workdir=tmpclonedir)
-        runfetchcmd(ud.basecmd + " submodule update --init --recursive", d, workdir=tmpclonedir)
-        self._set_relative_paths(tmpclonedir)
-        runfetchcmd("sed " + gitdir + "/config -i -e 's/bare.*=.*false/bare = true/'", d, workdir=tmpclonedir)
-        os.rename(gitdir, ud.clonedir,)
-        bb.utils.remove(tmpclonedir, True)
+
+        try:
+            if allow_network:
+                fetch_flags = ""
+            else:
+                fetch_flags = "--no-fetch"
+
+            # The 'git submodule sync' sandwiched between two successive 'git submodule update' commands is
+            # intentional. See the notes on the similar construction in download() for an explanation.
+            runfetchcmd("%(basecmd)s submodule update --init --recursive %(fetch_flags)s || (%(basecmd)s submodule sync --recursive && %(basecmd)s submodule update --init --recursive %(fetch_flags)s)" % {'basecmd': ud.basecmd, 'fetch_flags' : fetch_flags}, d, workdir=tmpclonedir)
+        except bb.fetch.FetchError:
+            if allow_network:
+                raise
+            else:
+                # This method was called as a probe to see whether the submodule history
+                # is complete enough to allow the current working copy to have its
+                # modules filled in. It's not, so swallow up the exception and report
+                # the negative result.
+                return False
+        finally:
+            self._set_relative_paths(tmpclonedir)
+            runfetchcmd(ud.basecmd + " submodule deinit -f --all", d, workdir=tmpclonedir)
+            runfetchcmd("sed " + gitdir + "/config -i -e 's/bare.*=.*false/bare = true/'", d, workdir=tmpclonedir)
+            os.rename(gitdir, ud.clonedir,)
+            bb.utils.remove(tmpclonedir, True)
+
+        return True
+
+    def need_update(self, ud, d):
+        main_repo_needs_update = Git.need_update(self, ud, d)
+
+        # First check that the main repository has enough history fetched. If it doesn't, then we don't
+        # even have the .gitmodules and gitlinks for the submodules to attempt asking whether the
+        # submodules' histories are recent enough.
+        if main_repo_needs_update:
+            return True
+
+        # Now check that the submodule histories are new enough. The git-submodule command doesn't have
+        # any clean interface for doing this aside from just attempting the checkout (with network
+        # fetched disabled).
+        return not self.update_submodules(ud, d, allow_network=False)
 
     def download(self, ud, d):
         Git.download(self, ud, d)
@@ -120,7 +156,7 @@ class GitSM(Git):
         if not ud.shallow or ud.localpath != ud.fullshallow:
             submodules = self.uses_submodules(ud, d, ud.clonedir)
             if submodules:
-                self.update_submodules(ud, d)
+                self.update_submodules(ud, d, allow_network=True)
 
     def clone_shallow_local(self, ud, dest, d):
         super(GitSM, self).clone_shallow_local(ud, dest, d)
@@ -132,4 +168,36 @@ class GitSM(Git):
 
         if self.uses_submodules(ud, d, ud.destdir):
             runfetchcmd(ud.basecmd + " checkout " + ud.revisions[ud.names[0]], d, workdir=ud.destdir)
-            runfetchcmd(ud.basecmd + " submodule update --init --recursive", d, workdir=ud.destdir)
+
+            # Copy over the submodules' fetched histories too.
+            if ud.bareclone:
+                repo_conf = ud.destdir
+            else:
+                repo_conf = os.path.join(ud.destdir, '.git')
+
+            if os.path.exists(ud.clonedir):
+                # This is not a copy unpacked from a shallow mirror clone. So
+                # the manual intervention to populate the .git/modules done
+                # in clone_shallow_local() won't have been done yet.
+                runfetchcmd("cp -fpPRH %s %s" % (os.path.join(ud.clonedir, 'modules'), repo_conf), d)
+                fetch_flags = "--no-fetch"
+            elif os.path.exists(os.path.join(repo_conf, 'modules')):
+                # Unpacked from a shallow mirror clone. Manual population of
+                # .git/modules is already done.
+                fetch_flags = "--no-fetch"
+            else:
+                # This isn't fatal; git-submodule will just fetch it
+                # during do_unpack().
+                fetch_flags = ""
+                bb.error("submodule history not retrieved during do_fetch()")
+
+            # Careful not to hit the network during unpacking; all history should already
+            # be fetched.
+            #
+            # The repeated attempts to do the submodule initialization sandwiched around a sync to
+            # install the correct remote URLs into the submodules' .git/config metadata are deliberate.
+            # Bad remote URLs are leftover in the modules' .git/config files from the unpack of bare
+            # clone tarballs and an initial 'git submodule update' is necessary to prod them back to
+            # enough life so that the 'git submodule sync' realizes the existing module .git/config
+            # files exist to be updated.
+            runfetchcmd("%(basecmd)s submodule update --init --recursive %(fetch_flags)s || (%(basecmd)s submodule sync --recursive && %(basecmd)s submodule update --init --recursive %(fetch_flags)s)" % {'basecmd': ud.basecmd, 'fetch_flags': fetch_flags}, d, workdir=ud.destdir)
-- 
2.13.6

Re: [PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[Matt Hoosier]

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

Hi;

I was just wondering what the expected procedure is here among the bitbake
devs. Should I be contacting the specific people that have historically
been the committers for the particular code affected by the change?

Thanks,
Matt

On Fri, May 25, 2018 at 8:45 AM Matt Hoosier <matt.hoosier@gmail.com> wrote:

> Although the submodules' histories have been fetched during the
> do_fetch() phase, the mechanics used to clone the workdir copy
> of the repo haven't been transferring the actual .git/modules
> directory from the repo fetched into downloads/ during the
> fetch task.
>
> Fix that, and for good measure also explicitly tell Git to avoid
> hitting the network during do_unpack() of the submodules.
>
> [YOCTO #12739]
>
> Signed-off-by: Matt Hoosier <matt.hoosier@gmail.com>
> ---
>  lib/bb/fetch2/gitsm.py | 84
> +++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 76 insertions(+), 8 deletions(-)
>
> diff --git a/lib/bb/fetch2/gitsm.py b/lib/bb/fetch2/gitsm.py
> index 0aff1008..7ac0dbb1 100644
> --- a/lib/bb/fetch2/gitsm.py
> +++ b/lib/bb/fetch2/gitsm.py
> @@ -98,7 +98,7 @@ class GitSM(Git):
>                          for line in lines:
>                              f.write(line)
>
> -    def update_submodules(self, ud, d):
> +    def update_submodules(self, ud, d, allow_network):
>          # We have to convert bare -> full repo, do the submodule bit,
> then convert back
>          tmpclonedir = ud.clonedir + ".tmp"
>          gitdir = tmpclonedir + os.sep + ".git"
> @@ -108,11 +108,47 @@ class GitSM(Git):
>          runfetchcmd("sed " + gitdir + "/config -i -e
> 's/bare.*=.*true/bare = false/'", d)
>          runfetchcmd(ud.basecmd + " reset --hard", d, workdir=tmpclonedir)
>          runfetchcmd(ud.basecmd + " checkout -f " +
> ud.revisions[ud.names[0]], d, workdir=tmpclonedir)
> -        runfetchcmd(ud.basecmd + " submodule update --init --recursive",
> d, workdir=tmpclonedir)
> -        self._set_relative_paths(tmpclonedir)
> -        runfetchcmd("sed " + gitdir + "/config -i -e
> 's/bare.*=.*false/bare = true/'", d, workdir=tmpclonedir)
> -        os.rename(gitdir, ud.clonedir,)
> -        bb.utils.remove(tmpclonedir, True)
> +
> +        try:
> +            if allow_network:
> +                fetch_flags = ""
> +            else:
> +                fetch_flags = "--no-fetch"
> +
> +            # The 'git submodule sync' sandwiched between two successive
> 'git submodule update' commands is
> +            # intentional. See the notes on the similar construction in
> download() for an explanation.
> +            runfetchcmd("%(basecmd)s submodule update --init --recursive
> %(fetch_flags)s || (%(basecmd)s submodule sync --recursive && %(basecmd)s
> submodule update --init --recursive %(fetch_flags)s)" % {'basecmd':
> ud.basecmd, 'fetch_flags' : fetch_flags}, d, workdir=tmpclonedir)
> +        except bb.fetch.FetchError:
> +            if allow_network:
> +                raise
> +            else:
> +                # This method was called as a probe to see whether the
> submodule history
> +                # is complete enough to allow the current working copy to
> have its
> +                # modules filled in. It's not, so swallow up the
> exception and report
> +                # the negative result.
> +                return False
> +        finally:
> +            self._set_relative_paths(tmpclonedir)
> +            runfetchcmd(ud.basecmd + " submodule deinit -f --all", d,
> workdir=tmpclonedir)
> +            runfetchcmd("sed " + gitdir + "/config -i -e
> 's/bare.*=.*false/bare = true/'", d, workdir=tmpclonedir)
> +            os.rename(gitdir, ud.clonedir,)
> +            bb.utils.remove(tmpclonedir, True)
> +
> +        return True
> +
> +    def need_update(self, ud, d):
> +        main_repo_needs_update = Git.need_update(self, ud, d)
> +
> +        # First check that the main repository has enough history
> fetched. If it doesn't, then we don't
> +        # even have the .gitmodules and gitlinks for the submodules to
> attempt asking whether the
> +        # submodules' histories are recent enough.
> +        if main_repo_needs_update:
> +            return True
> +
> +        # Now check that the submodule histories are new enough. The
> git-submodule command doesn't have
> +        # any clean interface for doing this aside from just attempting
> the checkout (with network
> +        # fetched disabled).
> +        return not self.update_submodules(ud, d, allow_network=False)
>
>      def download(self, ud, d):
>          Git.download(self, ud, d)
> @@ -120,7 +156,7 @@ class GitSM(Git):
>          if not ud.shallow or ud.localpath != ud.fullshallow:
>              submodules = self.uses_submodules(ud, d, ud.clonedir)
>              if submodules:
> -                self.update_submodules(ud, d)
> +                self.update_submodules(ud, d, allow_network=True)
>
>      def clone_shallow_local(self, ud, dest, d):
>          super(GitSM, self).clone_shallow_local(ud, dest, d)
> @@ -132,4 +168,36 @@ class GitSM(Git):
>
>          if self.uses_submodules(ud, d, ud.destdir):
>              runfetchcmd(ud.basecmd + " checkout " +
> ud.revisions[ud.names[0]], d, workdir=ud.destdir)
> -            runfetchcmd(ud.basecmd + " submodule update --init
> --recursive", d, workdir=ud.destdir)
> +
> +            # Copy over the submodules' fetched histories too.
> +            if ud.bareclone:
> +                repo_conf = ud.destdir
> +            else:
> +                repo_conf = os.path.join(ud.destdir, '.git')
> +
> +            if os.path.exists(ud.clonedir):
> +                # This is not a copy unpacked from a shallow mirror
> clone. So
> +                # the manual intervention to populate the .git/modules
> done
> +                # in clone_shallow_local() won't have been done yet.
> +                runfetchcmd("cp -fpPRH %s %s" %
> (os.path.join(ud.clonedir, 'modules'), repo_conf), d)
> +                fetch_flags = "--no-fetch"
> +            elif os.path.exists(os.path.join(repo_conf, 'modules')):
> +                # Unpacked from a shallow mirror clone. Manual population
> of
> +                # .git/modules is already done.
> +                fetch_flags = "--no-fetch"
> +            else:
> +                # This isn't fatal; git-submodule will just fetch it
> +                # during do_unpack().
> +                fetch_flags = ""
> +                bb.error("submodule history not retrieved during
> do_fetch()")
> +
> +            # Careful not to hit the network during unpacking; all
> history should already
> +            # be fetched.
> +            #
> +            # The repeated attempts to do the submodule initialization
> sandwiched around a sync to
> +            # install the correct remote URLs into the submodules'
> .git/config metadata are deliberate.
> +            # Bad remote URLs are leftover in the modules' .git/config
> files from the unpack of bare
> +            # clone tarballs and an initial 'git submodule update' is
> necessary to prod them back to
> +            # enough life so that the 'git submodule sync' realizes the
> existing module .git/config
> +            # files exist to be updated.
> +            runfetchcmd("%(basecmd)s submodule update --init --recursive
> %(fetch_flags)s || (%(basecmd)s submodule sync --recursive && %(basecmd)s
> submodule update --init --recursive %(fetch_flags)s)" % {'basecmd':
> ud.basecmd, 'fetch_flags': fetch_flags}, d, workdir=ud.destdir)
> --
> 2.13.6
>
>

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

Re: [PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[Alexander Kanavin]

On 06/01/2018 05:02 PM, Matt Hoosier wrote:
> I was just wondering what the expected procedure is here among the 
> bitbake devs. Should I be contacting the specific people that have 
> historically been the committers for the particular code affected by the 
> change?

Ross is on holiday this week, so some patience is advised.

Alex

Re: [PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[Richard Purdie]

On Fri, 2018-05-25 at 08:45 -0500, Matt Hoosier wrote:
> Although the submodules' histories have been fetched during the
> do_fetch() phase, the mechanics used to clone the workdir copy
> of the repo haven't been transferring the actual .git/modules
> directory from the repo fetched into downloads/ during the
> fetch task.
> 
> Fix that, and for good measure also explicitly tell Git to avoid
> hitting the network during do_unpack() of the submodules.
> 
> [YOCTO #12739]
> 
> Signed-off-by: Matt Hoosier <matt.hoosier@gmail.com>
> ---
>  lib/bb/fetch2/gitsm.py | 84
> +++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 76 insertions(+), 8 deletions(-)

I hate to say this but we saw another failure from this :(

https://autobuilder.yocto.io/builders/nightly-oe-selftest/builds/1099/steps/Running%20oe-selftest/logs/stdio

(the second one, grep for submodule deinit)

This was running on the centos 7 worker so I suspect older git versions
don't have the options you're using.

Cheers,

Richard

Re: [PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[Matt Hoosier]

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

On Wed, Jun 6, 2018 at 5:19 AM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Fri, 2018-05-25 at 08:45 -0500, Matt Hoosier wrote:
> > Although the submodules' histories have been fetched during the
> > do_fetch() phase, the mechanics used to clone the workdir copy
> > of the repo haven't been transferring the actual .git/modules
> > directory from the repo fetched into downloads/ during the
> > fetch task.
> >
> > Fix that, and for good measure also explicitly tell Git to avoid
> > hitting the network during do_unpack() of the submodules.
> >
> > [YOCTO #12739]
> >
> > Signed-off-by: Matt Hoosier <matt.hoosier@gmail.com>
> > ---
> >  lib/bb/fetch2/gitsm.py | 84
> > +++++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 76 insertions(+), 8 deletions(-)
>
> I hate to say this but we saw another failure from this :(
>
>
> https://autobuilder.yocto.io/builders/nightly-oe-selftest/builds/1099/steps/Running%20oe-selftest/logs/stdio
>
> (the second one, grep for submodule deinit)
>
> This was running on the centos 7 worker so I suspect older git versions
> don't have the options you're using.
>
> Cheers,
>
> Richard
>
>
>
Okay, I'll see what's going on with that. I'm still interested to know if
there's a listing of all the various automated regimens that are used as CI
before accepting Bitbake/Poky patches -- I don't want to keep wasting the
maintainers' time with failures that I could have uncovered independently.

Any comments on the aims and overall technique of this change? So far all
I've really gotten back are copy/pastes of automated builder failures.

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

Re: [PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[Richard Purdie]

On Wed, 2018-06-06 at 08:36 -0500, Matt Hoosier wrote:
> Okay, I'll see what's going on with that. I'm still interested to
> know if there's a listing of all the various automated regimens that
> are used as CI before accepting Bitbake/Poky patches -- I don't want
> to keep wasting the maintainers' time with failures that I could have
> uncovered independently.

The test matrix is basically encoded into:

http://git.yoctoproject.org/cgit.cgi/yocto-autobuilder-helper/tree/config.json

run across the various autobuilder workers on https://autobuilder.yocto
.io/buildslaves, i.e. centos7, debian8, debian9, fedora26, fedora27,
opensuse423, opensuse tumbleweed, ubuntu 1604 and ubuntu 1704

I'm not sure its reasonable to expect everyone to test across all those
combinations!

> Any comments on the aims and overall technique of this change? So far
> all I've really gotten back are copy/pastes of automated builder
> failures.

I strongly believe we should only have network access in do_fetch and
that our source mirroring should be functional and I therefore very
much support what the patch is doing and appreciate this getting fixed.

Sorry for the terse nature of the replies, we're just seeing a lot of
patches which are breaking the regression tests and its hard to root
cause them back to the underlying patches and help people debug them.

I do appreciate the help in getting this issue fixed as it will be a
good improvement to the fetcher.

Cheers,

Richard



 

Re: [PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[Matt Hoosier]

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

On Wed, Jun 6, 2018 at 9:03 AM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Wed, 2018-06-06 at 08:36 -0500, Matt Hoosier wrote:
> > Okay, I'll see what's going on with that.


I've posted an updated copy at
http://lists.openembedded.org/pipermail/bitbake-devel/2018-June/009365.html
that address the usage of the submodule commands that were too new for
CentOS. Fortunately, their usage was basically unnecessary -- I just
deleted them outright.


> I'm still interested to
> > know if there's a listing of all the various automated regimens that
> > are used as CI before accepting Bitbake/Poky patches -- I don't want
> > to keep wasting the maintainers' time with failures that I could have
> > uncovered independently.
>
> The test matrix is basically encoded into:
>
>
> http://git.yoctoproject.org/cgit.cgi/yocto-autobuilder-helper/tree/config.json
>
> run across the various autobuilder workers on https://autobuilder.yocto
> .io/buildslaves, i.e. centos7, debian8, debian9, fedora26, fedora27,
> opensuse423, opensuse tumbleweed, ubuntu 1604 and ubuntu 1704
>
> I'm not sure its reasonable to expect everyone to test across all those
> combinations!
>

Okay, I agree. :)

I think I've managed to test at the new and old ends of each of the major
distro families there, so I'll be content let the autobuilder sniff out any
more corner cases.


>
> > Any comments on the aims and overall technique of this change? So far
> > all I've really gotten back are copy/pastes of automated builder
> > failures.
>
> I strongly believe we should only have network access in do_fetch and
> that our source mirroring should be functional and I therefore very
> much support what the patch is doing and appreciate this getting fixed.
>
> Sorry for the terse nature of the replies, we're just seeing a lot of
> patches which are breaking the regression tests and its hard to root
> cause them back to the underlying patches and help people debug them.
>
> I do appreciate the help in getting this issue fixed as it will be a
> good improvement to the fetcher.
>
> Cheers,
>
> Richard
>

Sounds great; thanks, Richard.

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

Re: [PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[Matt Hoosier]

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

Thanks for the help getting this landed. With this change being part of
Bitbake, what's the procedure for attempting to get it backported to the
final MR of Yocto 2.2? Do I just backport it to a certain maintenance
branch of Bitbake (announced somehow with a prefix on the git-send-email
subject) and it would magically pop up out the corresponding branch of poky
if accepted to Bitbake?

On Wed, Jun 6, 2018 at 12:00 PM Matt Hoosier <matt.hoosier@gmail.com> wrote:

> On Wed, Jun 6, 2018 at 9:03 AM Richard Purdie <
> richard.purdie@linuxfoundation.org> wrote:
>
>> On Wed, 2018-06-06 at 08:36 -0500, Matt Hoosier wrote:
>> > Okay, I'll see what's going on with that.
>
>
> I've posted an updated copy at
> http://lists.openembedded.org/pipermail/bitbake-devel/2018-June/009365.html
> that address the usage of the submodule commands that were too new for
> CentOS. Fortunately, their usage was basically unnecessary -- I just
> deleted them outright.
>
>
>> I'm still interested to
>> > know if there's a listing of all the various automated regimens that
>> > are used as CI before accepting Bitbake/Poky patches -- I don't want
>> > to keep wasting the maintainers' time with failures that I could have
>> > uncovered independently.
>>
>> The test matrix is basically encoded into:
>>
>>
>> http://git.yoctoproject.org/cgit.cgi/yocto-autobuilder-helper/tree/config.json
>>
>> run across the various autobuilder workers on https://autobuilder.yocto
>> .io/buildslaves, i.e. centos7, debian8, debian9, fedora26, fedora27,
>> opensuse423, opensuse tumbleweed, ubuntu 1604 and ubuntu 1704
>>
>> I'm not sure its reasonable to expect everyone to test across all those
>> combinations!
>>
>
> Okay, I agree. :)
>
> I think I've managed to test at the new and old ends of each of the major
> distro families there, so I'll be content let the autobuilder sniff out any
> more corner cases.
>
>
>>
>> > Any comments on the aims and overall technique of this change? So far
>> > all I've really gotten back are copy/pastes of automated builder
>> > failures.
>>
>> I strongly believe we should only have network access in do_fetch and
>> that our source mirroring should be functional and I therefore very
>> much support what the patch is doing and appreciate this getting fixed.
>>
>> Sorry for the terse nature of the replies, we're just seeing a lot of
>> patches which are breaking the regression tests and its hard to root
>> cause them back to the underlying patches and help people debug them.
>>
>> I do appreciate the help in getting this issue fixed as it will be a
>> good improvement to the fetcher.
>>
>> Cheers,
>>
>> Richard
>>
>
> Sounds great; thanks, Richard.
>

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

Re: [PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[Richard Purdie]

On Thu, 2018-06-07 at 08:47 -0500, Matt Hoosier wrote:
> Thanks for the help getting this landed. With this change being part
> of Bitbake, what's the procedure for attempting to get it backported
> to the final MR of Yocto 2.2? Do I just backport it to a certain
> maintenance branch of Bitbake (announced somehow with a prefix on the
> git-send-email subject) and it would magically pop up out the
> corresponding branch of poky if accepted to Bitbake?

With bitbake its a case of requesting backports to the appropriate
stable release branches, either in the form of a patch with [1.3X] in
the subject, or just mention which revision to cherry-pick to which
branch if it cherry-picks cleanly.

I think to get back to 2.2, we'd have to backport to 1.38, 1.36, 1.34
and 1.32. I do get nervous about patches which land in master and then 
immediately get backported across so many releases. I'm less nervous if
the patches cleanly cherry-pick as at least the code is the same. How
cleanly does it backport?

Cheers,

Richard

Re: [PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[Matt Hoosier]

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

On Fri, Jun 8, 2018 at 5:20 AM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Thu, 2018-06-07 at 08:47 -0500, Matt Hoosier wrote:
> > Thanks for the help getting this landed. With this change being part
> > of Bitbake, what's the procedure for attempting to get it backported
> > to the final MR of Yocto 2.2? Do I just backport it to a certain
> > maintenance branch of Bitbake (announced somehow with a prefix on the
> > git-send-email subject) and it would magically pop up out the
> > corresponding branch of poky if accepted to Bitbake?
>
> With bitbake its a case of requesting backports to the appropriate
> stable release branches, either in the form of a patch with [1.3X] in
> the subject, or just mention which revision to cherry-pick to which
> branch if it cherry-picks cleanly.
>
> I think to get back to 2.2, we'd have to backport to 1.38, 1.36, 1.34
> and 1.32. I do get nervous about patches which land in master and then
> immediately get backported across so many releases. I'm less nervous if
> the patches cleanly cherry-pick as at least the code is the same. How
> cleanly does it backport?
>
> Cheers,
>
> Richard
>
>
It's not a completely clean back-port; some of the context lines don't
match because of changes to the logic that detect when a repository uses
gitsubmodules to begin with. The actual logic changes lift pretty-much
straight in though.

I appreciate your point about being cautious when putting unproven code
straight into a branch that gets used for maintenance releases though. So I
could understand wanting to let it prove out on master for a while.
Conversely, there are no recipes in Poky that use gitsm to begin with, so I
don't know how much additional confidence would really be gained just
through time. Is that also an argument that the potential impact to the
stable branch of poky's own metadata is small?

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

Re: [PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[Joshua Watt]

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

On Fri, 2018-06-08 at 07:36 -0500, Matt Hoosier wrote:
> On Fri, Jun 8, 2018 at 5:20 AM Richard Purdie <richard.purdie@linuxfo
> undation.org> wrote:
> > On Thu, 2018-06-07 at 08:47 -0500, Matt Hoosier wrote:
> > 
> > > Thanks for the help getting this landed. With this change being
> > part
> > 
> > > of Bitbake, what's the procedure for attempting to get it
> > backported
> > 
> > > to the final MR of Yocto 2.2? Do I just backport it to a certain
> > 
> > > maintenance branch of Bitbake (announced somehow with a prefix on
> > the
> > 
> > > git-send-email subject) and it would magically pop up out the
> > 
> > > corresponding branch of poky if accepted to Bitbake?
> > 
> > 
> > 
> > With bitbake its a case of requesting backports to the appropriate
> > 
> > stable release branches, either in the form of a patch with [1.3X]
> > in
> > 
> > the subject, or just mention which revision to cherry-pick to which
> > 
> > branch if it cherry-picks cleanly.
> > 
> > 
> > 
> > I think to get back to 2.2, we'd have to backport to 1.38, 1.36,
> > 1.34
> > 
> > and 1.32. I do get nervous about patches which land in master and
> > then 
> > 
> > immediately get backported across so many releases. I'm less
> > nervous if
> > 
> > the patches cleanly cherry-pick as at least the code is the same.
> > How
> > 
> > cleanly does it backport?
> > 
> > 
> > 
> > Cheers,
> > 
> > 
> > 
> > Richard
> > 
> > 
> 
> It's not a completely clean back-port; some of the context lines
> don't match because of changes to the logic that detect when a
> repository uses gitsubmodules to begin with. The actual logic changes
> lift pretty-much straight in though.
> 
> I appreciate your point about being cautious when putting unproven
> code straight into a branch that gets used for maintenance releases
> though. So I could understand wanting to let it prove out on master
> for a while. Conversely, there are no recipes in Poky that use gitsm
> to begin with, so I don't know how much additional confidence would
> really be gained just through time. Is that also an argument that the
> potential impact to the stable branch of poky's own metadata is
> small?

It might be worth looking through meta-oe also, at least as a reference
point.


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

Re: [PATCH v5] fetch/gitsm: avoid live submodule fetching during unpack()

[Matt Hoosier]

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

On Fri, Jun 8, 2018 at 8:17 AM Joshua Watt <jpewhacker@gmail.com> wrote:

> On Fri, 2018-06-08 at 07:36 -0500, Matt Hoosier wrote:
>
> On Fri, Jun 8, 2018 at 5:20 AM Richard Purdie <
> richard.purdie@linuxfoundation.org> wrote:
>
> On Thu, 2018-06-07 at 08:47 -0500, Matt Hoosier wrote:
> > Thanks for the help getting this landed. With this change being part
> > of Bitbake, what's the procedure for attempting to get it backported
> > to the final MR of Yocto 2.2? Do I just backport it to a certain
> > maintenance branch of Bitbake (announced somehow with a prefix on the
> > git-send-email subject) and it would magically pop up out the
> > corresponding branch of poky if accepted to Bitbake?
>
> With bitbake its a case of requesting backports to the appropriate
> stable release branches, either in the form of a patch with [1.3X] in
> the subject, or just mention which revision to cherry-pick to which
> branch if it cherry-picks cleanly.
>
> I think to get back to 2.2, we'd have to backport to 1.38, 1.36, 1.34
> and 1.32. I do get nervous about patches which land in master and then
> immediately get backported across so many releases. I'm less nervous if
> the patches cleanly cherry-pick as at least the code is the same. How
> cleanly does it backport?
>
> Cheers,
>
> Richard
>
>
> It's not a completely clean back-port; some of the context lines don't
> match because of changes to the logic that detect when a repository uses
> gitsubmodules to begin with. The actual logic changes lift pretty-much
> straight in though.
>
> I appreciate your point about being cautious when putting unproven code
> straight into a branch that gets used for maintenance releases though. So I
> could understand wanting to let it prove out on master for a while.
> Conversely, there are no recipes in Poky that use gitsm to begin with, so I
> don't know how much additional confidence would really be gained just
> through time. Is that also an argument that the potential impact to the
> stable branch of poky's own metadata is small?
>
>
> It might be worth looking through meta-oe also, at least as a reference
> point.
>
>
I don't think there are any recipes using gitsm:// protocol in git://
git.openembedded.org/meta-openembedded either (at least, on the 2.2 branch).

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