diff mbox series

[v2] scripts/oe-setup-layers: Make efficiently idempotent

Message ID 1673920770-31378-1-git-send-email-chuck.wolber@boeing.com
State Accepted, archived
Commit da2e01cacd98715318a5307fe0618dbca0cf1fe7
Headers show
Series [v2] scripts/oe-setup-layers: Make efficiently idempotent | expand

Commit Message

Wolber (US), Chuck Jan. 17, 2023, 1:59 a.m. UTC
The effect of subsequent setup-layers executions is now either a NOOP
or the minimal set of changes required to ensure layers precisely match
the JSON configuration.

This change allows setup-layers to be incorporated into a team's
configuration management strategy. In particular, the configuration
JSON manages a "pinning policy" that documents the oversight of sources
of change (a requirement for embedded development in highly regulated
industries).

One model for this strategy would work as follows. Team level policy is
developed to regularly review upstream commits that occur between the
current upstream HEAD and the previously pinned revision. The JSON
configuration is periodically updated after a review, test, and approval
process. In the rare instance that an upstream change is considered
problematic, the bbappend mechanism can be used to make relevant
changes in the team's project repository. This approach also requires
that team developers regularly run the project repository copy of
setup-layers. This is most easily accomplished by including setup-layers
in a wrapper script that all team developers use to interact with the
bitbake tool suite (e.g. "bb bitbake foo-image"). Project level policy
and oversight is effectively "contained" within this wrapper script,
thereby reducing a significant source of human error.

Left unstated, but acknowledged here, are a number of nuances required
to successfully implement the above strategy.  The details are out of
scope for this explanation. What should be clear is that a larger
configuration management strategy can now benefit from the utility
provided by setup-layers.

Note: Neither the above configuration management strategy example nor
the change itself is intended to alter the original intent to use
"bitbake-layers create-layers-setup destdir" to keep pace with upstream
activity for those who wish to use it that way.

Signed-off-by: Chuck Wolber <chuck.wolber@boeing.com>
---
 scripts/oe-setup-layers | 55 +++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 46 insertions(+), 9 deletions(-)

Comments

Luca Ceresoli Jan. 17, 2023, 5:03 p.m. UTC | #1
Hello Chuck,

On Mon, 16 Jan 2023 17:59:30 -0800
"Chuck Wolber" <chuck.wolber@boeing.com> wrote:

> The effect of subsequent setup-layers executions is now either a NOOP
> or the minimal set of changes required to ensure layers precisely match
> the JSON configuration.
> 
> This change allows setup-layers to be incorporated into a team's
> configuration management strategy. In particular, the configuration
> JSON manages a "pinning policy" that documents the oversight of sources
> of change (a requirement for embedded development in highly regulated
> industries).
> 
> One model for this strategy would work as follows. Team level policy is
> developed to regularly review upstream commits that occur between the
> current upstream HEAD and the previously pinned revision. The JSON
> configuration is periodically updated after a review, test, and approval
> process. In the rare instance that an upstream change is considered
> problematic, the bbappend mechanism can be used to make relevant
> changes in the team's project repository. This approach also requires
> that team developers regularly run the project repository copy of
> setup-layers. This is most easily accomplished by including setup-layers
> in a wrapper script that all team developers use to interact with the
> bitbake tool suite (e.g. "bb bitbake foo-image"). Project level policy
> and oversight is effectively "contained" within this wrapper script,
> thereby reducing a significant source of human error.
> 
> Left unstated, but acknowledged here, are a number of nuances required
> to successfully implement the above strategy.  The details are out of
> scope for this explanation. What should be clear is that a larger
> configuration management strategy can now benefit from the utility
> provided by setup-layers.
> 
> Note: Neither the above configuration management strategy example nor
> the change itself is intended to alter the original intent to use
> "bitbake-layers create-layers-setup destdir" to keep pace with upstream
> activity for those who wish to use it that way.
> 
> Signed-off-by: Chuck Wolber <chuck.wolber@boeing.com>

I'm afraind I am unable to apply this patch on my testing branch as it
conflicts with another patch ("oe-setup-build: add a tool for
discovering config templates and setting up builds" by Alexander
Kanavin) which is already on my branch.

You might want to rework your patch on top of Alex's, or just wait for
it to be on master and then send a v3.

Kind regards,
Luca
Alexander Kanavin Jan. 17, 2023, 5:05 p.m. UTC | #2
On Tue, 17 Jan 2023 at 18:03, Luca Ceresoli via lists.openembedded.org
<luca.ceresoli=bootlin.com@lists.openembedded.org> wrote:

> I'm afraind I am unable to apply this patch on my testing branch as it
> conflicts with another patch ("oe-setup-build: add a tool for
> discovering config templates and setting up builds" by Alexander
> Kanavin) which is already on my branch.
>
> You might want to rework your patch on top of Alex's, or just wait for
> it to be on master and then send a v3.

I think RP is not going to accept the oe-setup-build patch in its
current form, so you can drop it for now.

Alex
Luca Ceresoli Jan. 17, 2023, 5:08 p.m. UTC | #3
Hi Alex,

On Tue, 17 Jan 2023 18:05:12 +0100
"Alexander Kanavin" <alex.kanavin@gmail.com> wrote:

> On Tue, 17 Jan 2023 at 18:03, Luca Ceresoli via lists.openembedded.org
> <luca.ceresoli=bootlin.com@lists.openembedded.org> wrote:
> 
> > I'm afraind I am unable to apply this patch on my testing branch as it
> > conflicts with another patch ("oe-setup-build: add a tool for
> > discovering config templates and setting up builds" by Alexander
> > Kanavin) which is already on my branch.
> >
> > You might want to rework your patch on top of Alex's, or just wait for
> > it to be on master and then send a v3.  
> 
> I think RP is not going to accept the oe-setup-build patch in its
> current form, so you can drop it for now.

Ah, sure, I had missed that, thanks for pointing out!

I removed Alex's patch from my branch and added Chuck's.
Wolber (US), Chuck Jan. 17, 2023, 5:25 p.m. UTC | #4
On 1/17/23, 9:05 AM, "Alexander Kanavin" <alex.kanavin@gmail.com <mailto:alex.kanavin@gmail.com>> wrote:

> On Tue, 17 Jan 2023 at 18:03, Luca Ceresoli via lists.openembedded.org <luca.ceresoli=bootlin.com@lists.openembedded.org <mailto:bootlin.com@lists.openembedded.org>> wrote:
>
> > I'm afraind I am unable to apply this patch on my testing branch as it
> > conflicts with another patch ("oe-setup-build: add a tool for
> > discovering config templates and setting up builds" by Alexander
> > Kanavin) which is already on my branch.
> >
> > You might want to rework your patch on top of Alex's, or just wait for
> > it to be on master and then send a v3.
>
> I think RP is not going to accept the oe-setup-build patch in its
> current form, so you can drop it for now.

Agreed, it sounds like RP is holding Alex's patch back, so you should
probably go with my v2. Alex should not have any problems working
against my v2 patch when he updates his own patch.

..Ch:W..
diff mbox series

Patch

diff --git a/scripts/oe-setup-layers b/scripts/oe-setup-layers
index 6ecaffe..d0bc9f1 100755
--- a/scripts/oe-setup-layers
+++ b/scripts/oe-setup-layers
@@ -10,12 +10,42 @@ 
 # bitbake-layers create-layers-setup destdir
 #
 # It is recommended that you do not modify this file directly, but rather re-run the above command to get the freshest upstream copy.
+#
+# This script is idempotent. Subsequent runs only change what is necessary to
+# ensure your layers match your configuration.
 
 import argparse
 import json
 import os
 import subprocess
 
+def _is_layer_git_repo(layerdir):
+    git_dir = os.path.join(layerdir, ".git")
+    if not os.access(git_dir, os.R_OK):
+        return False
+    try:
+        return subprocess.check_output("git -C %s rev-parse --is-inside-git-dir" % git_dir, shell=True, stderr=subprocess.DEVNULL)
+    except subprocess.CalledProcessError:
+        return False
+
+def _is_layer_at_rev(layerdir, rev):
+    try:
+        curr_rev = subprocess.check_output("git -C %s rev-parse HEAD" % layerdir, shell=True, stderr=subprocess.DEVNULL)
+        if curr_rev.strip().decode("utf-8") == rev:
+            return True
+    except subprocess.CalledProcessError:
+        pass
+    return False
+
+def _is_layer_at_remote_uri(layerdir, remote, uri):
+    try:
+        curr_uri = subprocess.check_output("git -C %s remote get-url %s" % (layerdir, remote), shell=True, stderr=subprocess.DEVNULL)
+        if curr_uri.strip().decode("utf-8") == uri:
+            return True
+    except subprocess.CalledProcessError:
+        pass
+    return False
+
 def _do_checkout(args, json):
     layers = json['sources']
     for l_name in layers:
@@ -36,23 +66,30 @@  def _do_checkout(args, json):
         remotes = l_remote['remotes']
 
         print('\nSetting up source {}, revision {}, branch {}'.format(l_name, desc, branch))
-        cmd = 'git init -q {}'.format(layerdir)
-        print("Running '{}'".format(cmd))
-        subprocess.check_output(cmd, shell=True)
+        if not _is_layer_git_repo(layerdir):
+            cmd = 'git init -q {}'.format(layerdir)
+            print("Running '{}'".format(cmd))
+            subprocess.check_output(cmd, shell=True)
 
         for remote in remotes:
-            cmd = "git remote remove {} > /dev/null 2>&1; git remote add {} {}".format(remote, remote, remotes[remote]['uri'])
+            if not _is_layer_at_remote_uri(layerdir, remote, remotes[remote]['uri']):
+                cmd = "git remote remove {} > /dev/null 2>&1; git remote add {} {}".format(remote, remote, remotes[remote]['uri'])
+                print("Running '{}' in {}".format(cmd, layerdir))
+                subprocess.check_output(cmd, shell=True, cwd=layerdir)
+
+                cmd = "git fetch -q {} || true".format(remote)
+                print("Running '{}' in {}".format(cmd, layerdir))
+                subprocess.check_output(cmd, shell=True, cwd=layerdir)
+
+        if not _is_layer_at_rev(layerdir, rev):
+            cmd = "git fetch -q --all || true"
             print("Running '{}' in {}".format(cmd, layerdir))
             subprocess.check_output(cmd, shell=True, cwd=layerdir)
 
-            cmd = "git fetch -q {} || true".format(remote)
+            cmd = 'git checkout -q {}'.format(rev)
             print("Running '{}' in {}".format(cmd, layerdir))
             subprocess.check_output(cmd, shell=True, cwd=layerdir)
 
-        cmd = 'git checkout -q {}'.format(rev)
-        print("Running '{}' in {}".format(cmd, layerdir))
-        subprocess.check_output(cmd, shell=True, cwd=layerdir)
-
 parser = argparse.ArgumentParser(description="A self contained python script that fetches all the needed layers and sets them to correct revisions using data in a json format from a separate file. The json data can be created from an active build directory with 'bitbake-layers create-layers-setup destdir' and there's a sample file and a schema in meta/files/")
 
 parser.add_argument('--force-bootstraplayer-checkout', action='store_true',