curtin-dev team mailing list archive

[Olivier Gayot <mp+413637@xxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing

Hello,

A few observations on the code itself. Nothing serious.

also, there are other places where the changelog.trunk is mentioned, maybe we should update:
* if [ ! -d "$dir" ]; then
    # make-tarball will create the directory name based on the
    # contents of debian/changelog.trunk in the version provided.

*       echo "WARNING: git at '${uver}' had different version"
        echo "  in debian/changelog.trunk than your tree. version there"
        echo "  is '$d' working directory had $uver"

Thanks,
Olivier

Diff comments:

> diff --git a/tools/build-deb b/tools/build-deb
> index dbe364f..6355592 100755
> --- a/tools/build-deb
> +++ b/tools/build-deb
> @@ -1,7 +1,7 @@
>  #!/bin/sh
>  # This file is part of curtin. See LICENSE file for copyright and license info.
>  
> -set -e
> +set -eu

If we want to bring `set -u`, we need to check the value of $# before going through:

if [ "$1" = "-h" -o "$1" = "--help" ]; then
   ...

Otherwise, we lose the ability to run debuild with no argument.

>  
>  sourcename="curtin"
>  TEMP_D=""
> @@ -93,18 +87,8 @@ if [ ! -d "$dir" ]; then
>  fi
>  cd "$dir" || fail "failed cd $dir"
>  
> -# move files ending in .trunk to name without .trunk
> -# ie, this copies debian/changelog.trunk to debian/changelog
> -for f in debian/*.trunk; do
> -   mv "$f" "${f%.trunk}"
> -done
> -
> -# first line of debian/changelog looks like
> -#   curtin (<version>) UNRELEASED; urgency=low
> -# fix the version and UNRELEASED
> -sed -i -e "1s,([^)]*),(${clogver_new})," \
> -       -e "1s,UNRELEASED,${RELEASE}," debian/changelog ||
> -   fail "failed to write debian/changelog"
> +dch --create --package curtin --newversion $clogver_new \

Please use quotes around "$clogver_new" and "$RELEASE".

This should avoid unexpected behavior such as having "Development release" being bound to --distribution when $RELEASE is an empty string. Although currently there seems to be no path leading to $RELEASE being an empty string.

> +    --distribution $RELEASE "Development release"
>  debuild "$@" || fail "debuild failed"
>  
>  cd "$TEMP_D"


-- 
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/413637
Your team curtin developers is subscribed to branch curtin:master.