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

[Laurent Pinchart]

Another comment.

On Fri, Sep 30, 2022 at 11:37:23PM +0300, Laurent Pinchart via libcamera-devel wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> 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
> 
> 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

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