| Message ID | 1673744037-3484-1-git-send-email-chuck.wolber@boeing.com |
|---|---|
| State | New |
| Headers | show |
| Series | Make oe-setup-layers efficiently idempotent | expand |
Thanks. Do you think the tests for the script should as well be adjusted so that they fail without the change, and pass with it? (test_bitbakelayers_setup in meta/lib/oeqa/selftest/cases/bblayers.py) This is clearly fixing a problem you saw in local testing, and it would be good to ensure the problem does not silently reoccur in the future. Alex On Sun, 15 Jan 2023 at 01:54, 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. > > It is also worth noting that, where project level policy cannot be > asserted on layers with the bbappend mechanism (e.g. bbclass files), a > patch set can be checked in to the project repository and automatically > managed by the wrapper script. Patch management against local layer > clones would be superficial in nature (not pushed upstream). This > enables projects to "pull in" functionality and manage bug fixes on an > as-needed basis. > > Left unstated, but acknowledged here, are a number of nuances required > to successfully implement this strategy e.g. setup-layers does not work > so well if your layer clone is dirty from locally applied patches, etc. > 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(-) > > diff --git a/scripts/oe-setup-layers b/scripts/oe-setup-layers > index 7fe250c..a042a6c 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): > oesetupbuild = None > layers = json['sources'] > @@ -37,23 +67,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) > - > if os.path.exists(os.path.join(layerdir, 'scripts/oe-setup-build')): > oesetupbuild = os.path.join(layerdir, 'scripts/oe-setup-build') > > -- > 1.8.3.1 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#175861): https://lists.openembedded.org/g/openembedded-core/message/175861 > Mute This Topic: https://lists.openembedded.org/mt/96278464/1686489 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On Sat, 2023-01-14 at 16:53 -0800, Chuck Wolber 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. > > It is also worth noting that, where project level policy cannot be > asserted on layers with the bbappend mechanism (e.g. bbclass files), a > patch set can be checked in to the project repository and automatically > managed by the wrapper script. Patch management against local layer > clones would be superficial in nature (not pushed upstream). This > enables projects to "pull in" functionality and manage bug fixes on an > as-needed basis. > > Left unstated, but acknowledged here, are a number of nuances required > to successfully implement this strategy e.g. setup-layers does not work > so well if your layer clone is dirty from locally applied patches, etc. > 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(-) Thanks for the patch! Making the script rerun successfully seems sensible so the change itself is reasonable. I was a little confused by some of the comments in the commit message as oe-setup-layers doesn't apply patches as far as I know. Is that something you're thinking of adding? One small cosmetic issue is that the area of code the patch changes should be as a prefix in the shortlog, e.g. "scripts/oe-setup-layers:" in this case. It means people can understand the rough scope of the patch from a glance. Whilst not essential, I do like Alex's suggestion about improving the tests too, not sure if that is possible. I also noticed the patch is against master-next, which usually would be fine except I did drop a patch there which means this one no longer applies. If you could rebase it for the v2 with the shortlog fix that would be much appreciated. Cheers, Richard
diff --git a/scripts/oe-setup-layers b/scripts/oe-setup-layers index 7fe250c..a042a6c 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): oesetupbuild = None layers = json['sources'] @@ -37,23 +67,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) - if os.path.exists(os.path.join(layerdir, 'scripts/oe-setup-build')): oesetupbuild = os.path.join(layerdir, 'scripts/oe-setup-build')
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. It is also worth noting that, where project level policy cannot be asserted on layers with the bbappend mechanism (e.g. bbclass files), a patch set can be checked in to the project repository and automatically managed by the wrapper script. Patch management against local layer clones would be superficial in nature (not pushed upstream). This enables projects to "pull in" functionality and manage bug fixes on an as-needed basis. Left unstated, but acknowledged here, are a number of nuances required to successfully implement this strategy e.g. setup-layers does not work so well if your layer clone is dirty from locally applied patches, etc. 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(-)