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

Emil Velikov emil.l.velikov at gmail.com
Tue Jan 24 02:46:39 UTC 2017


On 24 January 2017 at 01:14, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> On Mon, Jan 23, 2017 at 12:51:31PM +0000, Emil Velikov 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 :-]
>
> ok, verified, sorry. clean clone of the repo seems to succeed, at least with
> --dry-run.
>
Ack, ty. Glad to hear I did not miss something dead obvious ;-)

>
>> 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.
>
> why not? the only time INSTALL should be generated is when releaseing and
> hopefully that runs from a git directory. The case of someone unzipping a
> tarball, initializing it as git directory and then hoping for the correct
> INSTALL should be ignorable, right? (even though we do exactly that in
> Fedora, minus the INSTALL expectatation :)
>
> Aside from that, I do wonder if INSTALL is a write-only document anyway...
>
Barring the WO comment - thanks to the .PHONY rule the INSTALL_CMD  is
always invoked, and afaics the file is already generated. With the CMD
being

macros_datadir=`$PKG_CONFIG --print-errors --variable=pkgdatadir xorg-macros`
INSTALL_CMD="(cp -f "$macros_datadir/INSTALL" \$(top_srcdir)/.INSTALL.tmp && \
mv \$(top_srcdir)/.INSTALL.tmp \$(top_srcdir)/INSTALL) \
|| (rm -f \$(top_srcdir)/.INSTALL.tmp; touch \$(top_srcdir)/INSTALL; \
echo 'util-macros \"pkgdatadir\" from xorg-macros.pc not found:
installing possibly empty INSTALL.' >&2)"

The former (mv) will be picked during make dist, while the latter will
fail during make distcheck since either a) scrdir RO and touch will
bail, or (if we change to builddir) make clean will fail.

[1] https://cgit.freedesktop.org/xorg/util/macros/tree/xorg-macros.m4.in#n1840

>> 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 ?
>
> wouldn't you risk potentially having the changelog wrong then if a ChangeLog
> file is leftover? so you have to work around that then, etc. seems like
> leaving the current bits in place is sufficient
>
>> - 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 ?
>
> you mean when no version is specified? fixing that would be easy by
> specifying a version :)
>
>> Completely unrelated:
>> Can I ask you/anyone else to cherry-pick [1] throughout the xorg repositories ?
>
> yep, see the message I just sent regarding the 3 patches to sync autogen.sh
> across all repos.
>
Great, tyvm.

-Emil


More information about the xorg-devel mailing list