[libcamera-devel] [PATCH 4/4] utils: Provide a release script

[Laurent Pinchart]

On Fri, Sep 30, 2022 at 11:12:30PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2022-09-30 22:00:00)
> > On Fri, Sep 30, 2022 at 11:37:23PM +0300, Laurent Pinchart via libcamera-devel wrote:
> > > On Fri, Sep 30, 2022 at 04:43:31PM +0100, Kieran Bingham via libcamera-devel wrote:
> > > > Quoting Jacopo Mondi (2022-09-30 16:35:28)
> > > > > On Thu, Sep 29, 2022 at 03:36:26PM +0100, Kieran Bingham via libcamera-devel wrote:
> > > > > > Support making releases of libcamera by introducing a helper script
> > > > > > which will facilitate the increment of any release version, along with
> > > > > > generating an associated tag.
> > > > > >
> > > > > > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > > > >
> > > > > > ---
> > > > > > This can later be extended to support or enforce adding an overview
> > > > > > changelog to the commit, and annotated tag.
> > > > > >
> > > > > >  utils/release.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 48 insertions(+)
> > > > > >  create mode 100755 utils/release.sh
> > > > > >
> > > > > > diff --git a/utils/release.sh b/utils/release.sh
> > > > > > new file mode 100755
> > > > > > index 000000000000..c1c35dacab8e
> > > > > > --- /dev/null
> > > > > > +++ b/utils/release.sh
> > > > > > @@ -0,0 +1,48 @@
> > > > > > +#!/bin/sh
> > > > > > +
> > > > > > +# SPDX-License-Identifier: GPL-2.0-or-later
> > > > > > +# Prepare a project release
> > > > > > +
> > > > > > +# Abort if we are not within the project root or the tree is not clean.
> > > > > > +if [ ! -e utils/gen-version.sh -o ! -e .git ]; then

By the way, I'm getting warnings from checkstyle.py:

------------------------------------------------------------------------
288cd54b98a5ff80a65022ce577081b0ebf9d12a utils: Provide a release script
------------------------------------------------------------------------
--- utils/release.sh
+++ utils/release.sh
#7:                                ^-- SC2166 (warning): Prefer [ p ] || [ q ] as [ p -o q ] is not well defined.
+if [ ! -e utils/gen-version.sh -o ! -e .git ]; then
#48:          ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting.
+git tag v$new_version -am "libcamera v$new_version"

> > > gen-version.sh uses
> > > 
> > > # Bail out if the directory isn't under git control
> > > git_dir=$(git rev-parse --git-dir 2>&1) || exit 1
> > > 
> > > which makes sure that the .git directory contains something valid.
> > > 
> > > > > > +     echo "This release script must be run from the root of libcamera git clone."
> > > 
> > > s/libcamera git clone/a libcamera git tree/
> > > 
> > > > > > +     exit 1
> > > > > > +fi
> > > > > > +
> > > > > > +if ! git diff-index --quiet HEAD; then
> > > > > 
> > > > > Took me a while to validate this as --quite implies --exit-code which
> > > > > is documented as:
> > > > > 
> > > > > Make the program exit with codes similar to diff(1). That is, it exits
> > > > > with 1 if there were differences and 0 means no differences.
> > > > > 
> > > > > But in bash an exit code 0 mean success, so it's right to negate it
> > > > 
> > > > Yes, we use this in utils/gen-version.sh
> > > > 
> > > > > > +     echo "Tree must be clean to release."
> > > > > > +     exit 1
> > > > > > +fi
> > > > > > +
> > > > > > +# Identify current version components
> > > > > > +version=$(./utils/gen-version.sh)
> > > 
> > > Hmmm... gen-version.sh retrieves the version from git tags only, and now
> > > we're also bumping the project version in the root meson.build. With the
> > > current implementation, the project version is used as a fallback only,
> > > if gen-version.sh fails. Is this something we need to revisit ? Do we
> > > need to worry about a mismatch between the two ? It can be handling on
> > > top, but I'd like to hear your opinion.
> > > 
> > > > > > +
> > > > > > +# Decide if we are here to bump major, minor, or patch release.
> > > > > > +case $1 in
> > > > > > +     major|minor|patch)
> > > > > > +             bump=$1;
> > > > > > +             ;;
> > > > > > +     *)
> > > > > > +             echo "You must specify the version bump level:"
> > > > > > +             echo " - major"
> > > > > > +             echo " - minor"
> > > > > > +             echo " - patch"
> > > 
> > > This could hold on one line:
> > > 
> > >               echo "You must specify the version bump level (major, minor, patch)"
> > > 
> > > Up to you.
> > > 
> > > > > > +             exit 1
> > > > > > +             ;;
> > > > > > +esac
> > > > > > +
> > > > > > +new_version=$(./utils/semver bump "$bump" "$version")
> > > > > > +
> > > > > > +echo "Bumping $bump"
> > > > > > +echo "  Existing version is: $version"
> > > > > > +echo "  New version is : $new_version"
> > > > > > +
> > > > > > +# Patch in the version to our meson.build
> > > > > > +sed -i -E "s/ version : '.*',/ version : '$new_version',/" meson.build
> > 
> > Maybe I worry too much, but this could be made safer with
> > 
> > sed -i -E "0,/ version : '.*',/{s/ version : '.*',/ version : '$new_version',/}" meson.build
> 
> If another " version : 'xxx'," string gets added to this top level
> meson.build then I'd expect it to be updated too. I can't see a reason
> to add such a line, and if one's added, I think it should be considered
> then. So I'd actually rather it matched right now.

It's hard to predict what those other matches for the regexp would be,
so both arguments can make sense. We'll see it anyway, so I'm fine with
this.

> > to only replace the first occurrence (credits go to
> > https://stackoverflow.com/questions/148451/how-to-use-sed-to-replace-only-the-first-occurrence-in-a-file).
> > 
> > > > > > +
> > > > > > +# Commit the update
> > > > > > +git add meson.build
> > > > > > +git commit meson.build -m "libcamera v$new_version"
> > > 
> > > You can drop meson.build here. I would also add -s to add a SoB line.
> > > 
> > > > > > +
> > > > > > +# Create a tag
> > > > > > +git tag v$new_version -am "libcamera v$new_version"
> > > 
> > > And here too, -s, to sign the tag.
> > > 
> > > > > The rest looks good
> > > > > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

-- 
Regards,

Laurent Pinchart