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

[Kieran Bingham]

Quoting Laurent Pinchart (2022-09-30 22:00:00)
> 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

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.


> 
> 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