[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freedesktop-xorg-devel
Subject:    Re: [PATCH util-modular 4/4] release.sh: implement around git worktree
From:       Emil Velikov <emil.l.velikov () gmail ! com>
Date:       2017-01-26 18:04:00
Message-ID: CACvgo50sKzi0eHbYAuq4kyk1kB4zOLY2jFccoc8CRaPn-Ny6mA () mail ! gmail ! com
[Download RAW message or body]

On 23 January 2017 at 12:51, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 23 January 2017 at 04:03, Peter Hutterer <peter.hutterer@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@who-t.net> wrote:
> > > > On Thu, Jan 19, 2017 at 07:30:10PM +0000, Emil Velikov wrote:
> > > > > From: Emil Velikov <emil.velikov@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@who-t.net>
> > > > > Signed-off-by: Emil Velikov <emil.velikov@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@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
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic