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

[Laurent Pinchart]

Hi Kieran,

On Fri, Sep 30, 2022 at 11:08:46PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2022-09-30 21:37:23)
> > 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.
> 
> I can't use that here. (And maybe we need to update gen-version).
> 
> I want to be certain that we are within the git repository for
> libcamera. Not an un-tracked sources (for example an extracted tar ball)
> that has another parent directory which is controlled by git.

How wonder how far we need to go to catch such weird cases. What if
we're in a different directory, where someone has a different
utils/gen-version.sh ? :-) Let's not forget that the release script will
only be used by maintainers, so I don't think we need to have as many
safeguards in place as for situations that could affect users (such as
cloning the repository without tags, for instance).

> > > > > +     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.
> 
> That's why the meson version summary reports both.

If they're never supposed to be different, then we should fail at
configure time. If they can be different, we need to correctly support
those cases. Let's discuss that in 1/4.

> > > > > +
> > > > > +# 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
> > > > > +
> > > > > +# 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.
> 
> I wanted it to be explicit that /only/ meson.build is to be commited.

The script verifies that the tree is clean first, so nothing else will
be committed. I don't mind much, but if you want to keep the file name
in the git commit command, then you can drop git add.

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