[PATCH util-modular 4/4] release.sh: implement around git worktree

Thu Jan 26 18:04:00 UTC 2017

On 23 January 2017 at 12:51, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 23 January 2017 at 04:03, Peter Hutterer <peter.hutterer at who-t.net> wrote:
>> On Fri, Jan 20, 2017 at 02:19:06PM +0000, Emil Velikov wrote:
>>> On 20 January 2017 at 02:49, Peter Hutterer <peter.hutterer at who-t.net> wrote:
>>> > On Thu, Jan 19, 2017 at 07:30:10PM +0000, Emil Velikov wrote:
>>> >> From: Emil Velikov <emil.velikov at collabora.com>
>>> >>
>>> >> Months ago, before my work in here there the script had a number of
>>> >> assumptions:
>>> >>  - autoreconf or alike is ran by the user - slight nuisance esp. when
>>> >> releasing multiple packages.
>>> >>  - the generated files were compatible with the ones on the server -
>>> >> rarely and issue, but still
>>> >>  - config.status means 'all the autotools bits were generated correctly'
>>> >>  - failed to consider if user has multiple config.status - using
>>> >> ./build-64bit/ and ./build-32bit/ anyone ?
>>> >>  - ...
>>> >>
>>> >> With 663364cda5e316a0509ff5869293e3a815b9945f we mitigated a lot of that
>>> >> but did not consider
>>> >>  - forcing people to `git clean' is annoying and if not careful one can
>>> >> purge local files that they want to keep
>>> >>  - the files generated by autotools were left behind
>>> >>
>>> >> In order to mitigate those use git worktree. This is more efficient
>>> >> [both bandwidth and storage wise] than pulling/generating git tarballs,
>>> >> git clone and friends.
>>> >>
>>> >> Cc: Peter Hutterer <peter.hutterer at who-t.net>
>>> >> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>>> >> ---
>>> >> There's a git of noise produced by git worktree and autoreconf. I'm fine
>>> >> with 2>/dev/null but I'd leave that to others to decide
>>> >> ---
>>> >>  release.sh | 34 +++++++++++++++++++---------------
>>> >>  1 file changed, 19 insertions(+), 15 deletions(-)
>>> >>
>>> >> diff --git a/release.sh b/release.sh
>>> >> index 8ed179d..0b3fcbf 100755
>>> >> --- a/release.sh
>>> >> +++ b/release.sh
>>> >> @@ -357,25 +357,24 @@ process_module() {
>>> >>       return 1
>>> >>      fi
>>> >>
>>> >> -    if [ -e configure ]; then
>>> >> -        echo "Error: the git repository contains configure"
>>> >> -        echo "Did you forget to run git clean -fXd (and git clean -fxd) ?"
>>> >> -        return 1
>>> >> -    fi
>>> >> -
>>> >> -    # Create tmpdir for the build
>>> >> +    # Create tmpdir for the release
>>> >>      build_dir=`mktemp -d -p . build.XXXXXXXXXX`
>>> >>      if [ $? -ne 0 ]; then
>>> >> -        echo "Error: could not create a temporary directory for the build."
>>> >> +        echo "Error: could not create a temporary directory for the release"
>>> >>          echo "Do you have coreutils' mktemp ?"
>>> >>          return 1
>>> >>      fi
>>> >>
>>> >> -    echo "Info: generating configure."
>>> >> -    autoreconf --force --install >/dev/null
>>> >> +    # Worktree removal is intentionally left to the user, due to:
>>> >> +    #  - currently we cannot select only one worktree to prune
>>> >> +    #  - requires to removal of $build_dir which might contradict with the
>>> >> +    # user decision to keep some artefacts like tarballs or other
>>> >> +    echo "Info: creating new git worktree."
>>> >> +    git worktree add $build_dir
>>> >>      if [ $? -ne 0 ]; then
>>> >> -        echo "Error: failed to generate configure."
>>> >> -        return 1
>>> >> +     echo "Error: failed to create a git worktree."
>>> >> +     cd $top_src
>>> >> +     return 1
>>> >
>>> > there's an indentation error,
>>> Funky indentation was there (it was a code movement) - we still have a
>>> handful of tabs that are meant to be 8 spaces.
>>>
>>> > but otherwise the series looks good and is
>>> > Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
>>> >
>>> Note: after another thought I've went with v2 for 3/4 (having a
>>> version check and building from there).
>>>
>>> > I guess I'll test this on the next release ;)
>>> >
>>> Thank you very much. Please let me know if you spot anything odd.
>>
>> Ok, tested it on the libXi release and something is off, but haven't found
>> out what it is yet. You can reproduce it by running a --dry-run on the libXi
>> repo, for some reason make distcleancheck fails with
>>
>> ERROR: files left in build directory after distclean:
>> ./config.sub
>> ./ltmain.sh
>> ./config.guess
>> ./install-sh
>> ./missing
>> ./depcomp
>> ./compile
>> make[1]: *** [distcleancheck] Error 1
>>
>> since these are standard files, it's not a libXi issue, must be something to
>> do with the worktrees or how we invoke configure. Reverting this patch
>> makes it work again.
>>
> That's strange I haven't seen issues like the ones you mentioned.
> Fetching a clean copy of libXi and doing "../path/to/release.sh .
> --dry-run" brings up other issues :-]
> Which version of autotools are you using ? See libtool --help
>
> Using Arch here, packages patched as mentioned:
>        version:        libtool (GNU libtool) 2.4.6
>        automake:       automake (GNU automake) 1.15
>        autoconf:       autoconf (GNU Autoconf) 2.69
>
> https://git.archlinux.org/svntogit/packages.git/tree/trunk/automake-1.15-perl-regex.patch?h=packages/automake
> - fix perl regular expression
> https://git.archlinux.org/svntogit/packages.git/tree/trunk/autoconf-2.69-perl-5.22-autoscan.patch?h=packages/autoconf
> - fix perl regular expression
>
>
> The other 'fun' experiences - seemingly all bugs in xorg-macros
>
> - INSTALL_CMD and CHANGELOG_CMD assume that you can write tmp files in
> $top_srcdir, which afaict is a violation.
> FIXED: Patched xorg-macros to honour top_builddir
>
> - ChangeLog seem iffy - always creating the file in BUILDDIR,
> contradicts with MAINTAINERCLEANFILES
> FIXED: Creating tarball -> use $(top_builddir)/ChangeLog, building
> from tarball "touch $(top_srcdir)/ChangeLog"
>  - INSTALL... as above
> Unlike the above we cannot use xorg-macros (.git) to detect if we're
> creating or building from tarballs.
> TODO: How do we fix this properly ? Simplest workaround is to continue
> if INSTALL is already available... should be opt for the same with
> ChangeLog ?
>
The above ChangeLog/INSTALL issues should be resolved with
https://patchwork.freedesktop.org/series/18629/

> - xmlto complains loudly [in the xorg-macros test] when the input is
> empty and thus detection fails
> TODO: Perhaps we should provide a non-empty file ?
>
While this one is already addressed upstream.
Can you/others please give those a bash and let me know if you spot any issues.

Thanks !
Emil