[bitbake-devel] [RESEND RFC PATCH] gitsm: Control submodule recursion

[Mark Hatle]

On 8/26/19 7:03 PM, Andrew Jeffery wrote:
> There exist some use-cases where repositories related by submodules do
> not require submodules be recursively initialised despite needing the
> first layer of submodules to be fetched. Enable this usecase in gitsm by
> implementing a `recurse=X` SRC_URI parameter. The current implementation
> of the recurse parameter defaults to enabling exhaustive recursion to
> match the current behaviour of gitsm, with the one alternative being no
> recursion. Enough rope is provided to allow specific recursion depths to
> be implemented if necessary via the same interface. Values currently
> unsupported for the recurse parameter raise a ValueError to ensure
> forward compatibility.

I was thinking about this patch and I'm not sure it's needed.  There are ways to
do this without using gitsm.

If you don't want recursion, then why are you specifying 'gitsm' at all?  The
'git' fetcher won't recurse and can be used to pull down a subset of what would
have been a recursive fetch.  The 'subpath' parameter can even be used to
combine the items in the same way as the gitsm fetcher if really needed.

(A few more comments below, as more of a technical review)

> Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> ---
>  lib/bb/fetch2/gitsm.py | 34 ++++++++++++++++++++++++++++------
>  1 file changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/bb/fetch2/gitsm.py b/lib/bb/fetch2/gitsm.py
> index c622771d21aa..5a8426b486e3 100644
> --- a/lib/bb/fetch2/gitsm.py
> +++ b/lib/bb/fetch2/gitsm.py
> @@ -6,7 +6,15 @@ after cloning.
>  
>  SRC_URI = "gitsm://<see Git fetcher for syntax>"
>  
> -See the Git fetcher, git://, for usage documentation.
> +Supported SRC_URI options are:
> +
> +- recurse
> +    Control recursive nature of submodule initialisation.
> +
> +    The default value is -1, which will recursively initialise all submodules.
> +    Set to 0 to disable recursiion.
> +
> +See the Git fetcher, git://, for further usage documentation.
>  
>  NOTE: Switching a SRC_URI from "git://" to "gitsm://" requires a clean of your recipe.
>  
> @@ -26,6 +34,15 @@ from   bb.fetch2 import logger
>  from   bb.fetch2 import Fetch
>  from   bb.fetch2 import BBFetchException
>  
> +def should_recurse(ud):
> +    recurse = ud.parm.get("recurse", "-1")
> +
> +    # Change this condition if we end up supporting specific maximum depths
> +    if recurse in ( "-1", "0" ):
> +        return recurse == "-1"
> +
> +    raise ValueError("Recursion control value not supported: %s" % (recurse))
> +
>  class GitSM(Git):
>      def supports(self, ud, d):
>          """
> @@ -95,25 +112,28 @@ class GitSM(Git):
>          for module in submodules:
>              # Translate the module url into a SRC_URI
>  
> +            recurse = should_recurse(ud)
>              if "://" in uris[module]:
>                  # Properly formated URL already
>                  proto = uris[module].split(':', 1)[0]
> -                url = uris[module].replace('%s:' % proto, 'gitsm:', 1)
> +                desired = "gitsm:" if recurse else "git:"
> +                url = uris[module].replace('%s:' % proto, desired, 1)

Above 'desired' is used, below 'scheme' is used.  You should use the same
variable for both, and set it before the first 'if'.  To me 'scheme' is a better
variable.

>              else:
> +                scheme = "gitsm://" if recurse else "git://"
>                  if ":" in uris[module]:
>                      # Most likely an SSH style reference
>                      proto = "ssh"
>                      if ":/" in uris[module]:
>                          # Absolute reference, easy to convert..
> -                        url = "gitsm://" + uris[module].replace(':/', '/', 1)
> +                        url = scheme + uris[module].replace(':/', '/', 1)
>                      else:
>                          # Relative reference, no way to know if this is right!
>                          logger.warning("Submodule included by %s refers to relative ssh reference %s.  References may fail if not absolute." % (ud.url, uris[module]))
> -                        url = "gitsm://" + uris[module].replace(':', '/', 1)
> +                        url = scheme + uris[module].replace(':', '/', 1)
>                  else:
>                      # This has to be a file reference
>                      proto = "file"
> -                    url = "gitsm://" + uris[module]
> +                    url = scheme + uris[module]
>  
>              url += ';protocol=%s' % proto
>              url += ";name=%s" % module
> @@ -209,7 +229,9 @@ class GitSM(Git):
>          ret = self.process_submodules(ud, ud.destdir, unpack_submodules, d)
>  
>          if not ud.bareclone and ret:
> +            recurse = should_recurse(ud)
>              # All submodules should already be downloaded and configured in the tree.  This simply sets
>              # up the configuration and checks out the files.  The main project config should remain
>              # unmodified, and no download from the internet should occur.
> -            runfetchcmd("%s submodule update --recursive --no-fetch" % (ud.basecmd), d, quiet=True, workdir=ud.destdir)
> +            runfetchcmd("%s submodule update %s --no-fetch" % (ud.basecmd, "--recursive" if recurse else ""),
> +                        d, quiet=True, workdir=ud.destdir)
>