GNU bug report logs - #10470
MSYS: race in directory access?

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 10470 in the body.
You can then email your comments to 10470 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Peter Rosin <peda <at> lysator.liu.se>
To: bug-automake <at> gnu.org
Subject: MSYS: race in directory access?
Date: Tue, 10 Jan 2012 00:07:54 +0100

Hi!

distcheck-configure-flags-am.test fails on MSYS.  I think the cause is
that some process lingers with CWD in the _build directory which some
other process tries to remove.  On MSYS, it's not possible to remove a
directory that is "in use" like that.

Ugly patch coming up shortly.

Cheers,
Peter



Running from installcheck: no
Using TAP: no
PATH = /home/peda/automake/tests:/c/Program Files (x86)/Microsoft F#/v4.0/:/c/Program Files (x86)/Microsoft Visual Studio 10.0/VSTSDB/Deploy:/c/Program Files (x86)/Microsoft Visual Studio 10.0/Common7/IDE/:/c/Program Files (x86)/Microsoft Visual Studio 10.0/VC/BIN:/c/Program Files (x86)/Microsoft Visual Studio 10.0/Common7/Tools/:/c/Windows/Microsoft.NET/Framework/v4.0.30319:/c/Windows/Microsoft.NET/Framework/v3.5:/c/Program Files (x86)/Microsoft Visual Studio 10.0/VC/VCPackages:/c/Program Files (x86)/HTML Help Workshop:/c/Program Files (x86)/Microsoft Visual Studio 10.0/Team Tools/Performance Tools:/c/Program Files (x86)/Microsoft SDKs/Windows/v7.0A/bin/NETFX 4.0 Tools:.:/usr/local/bin:/mingw/bin:/bin:/c/Program Files/Common Files/Microsoft Shared/Windows Live:/c/Program Files (x86)/Common Files/Microsoft Shared/Windows Live:/c/Windows/system32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0/
++ pwd
/home/peda/automake/tests/distcheck-configure-flags-am.dir
+ cat
+ unset sentence
+ cat
+ aclocal-1.11a -Werror
+ automake-1.11a --foreign -Werror -Wall
+ /bin/grep -F '$(DISTCHECK_CONFIGURE_FLAGS)' Makefile.in
	    $(DISTCHECK_CONFIGURE_FLAGS) \
+ /bin/grep -F '$(AM_DISTCHECK_CONFIGURE_FLAGS)' Makefile.in
	    $(AM_DISTCHECK_CONFIGURE_FLAGS) \
+ grep 'DISTCHECK_CONFIGURE_FLAGS.*AM_DISTCHECK_CONFIGURE_FLAGS' Makefile.in
+ autoconf -B /no/such/dir
+ ./configure --enable-success 'sentence=it works :-)'
checking for a BSD-compatible install... /bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
+ make distcheck
make  dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir'
{ test ! -d "distcheck-configure-flags-am-1.0" || { find "distcheck-configure-flags-am-1.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -fr "distcheck-configure-flags-am-1.0"; }; }
test -d "distcheck-configure-flags-am-1.0" || mkdir "distcheck-configure-flags-am-1.0"
test -n "" \
	|| find "distcheck-configure-flags-am-1.0" -type d ! -perm -755 \
		-exec chmod u+rwx,go+rx {} \; -o \
	  ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
	  ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
	  ! -type d ! -perm -444 -exec /bin/sh /home/peda/automake/tests/distcheck-configure-flags-am.dir/install-sh -c -m a+r {} {} \; \
	|| chmod -R a+r "distcheck-configure-flags-am-1.0"
tardir=distcheck-configure-flags-am-1.0 && ${TAR-tar} chof - "$tardir" | GZIP=--best gzip -c >distcheck-configure-flags-am-1.0.tar.gz
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir'
{ test ! -d "distcheck-configure-flags-am-1.0" || { find "distcheck-configure-flags-am-1.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -fr "distcheck-configure-flags-am-1.0"; }; }
case 'distcheck-configure-flags-am-1.0.tar.gz' in \
	*.tar.gz*) \
	  GZIP=--best gzip -dc distcheck-configure-flags-am-1.0.tar.gz | ${TAR-tar} xf - ;;\
	*.tar.bz2*) \
	  bzip2 -dc distcheck-configure-flags-am-1.0.tar.bz2 | ${TAR-tar} xf - ;;\
	*.tar.lz*) \
	  lzip -dc distcheck-configure-flags-am-1.0.tar.lz | ${TAR-tar} xf - ;;\
	*.tar.xz*) \
	  xz -dc distcheck-configure-flags-am-1.0.tar.xz | ${TAR-tar} xf - ;;\
	*.tar.Z*) \
	  uncompress -c distcheck-configure-flags-am-1.0.tar.Z | ${TAR-tar} xf - ;;\
	*.shar.gz*) \
	  GZIP=--best gzip -dc distcheck-configure-flags-am-1.0.shar.gz | unshar ;;\
	*.zip*) \
	  unzip distcheck-configure-flags-am-1.0.zip ;;\
	esac
chmod -R a-w distcheck-configure-flags-am-1.0; chmod a+w distcheck-configure-flags-am-1.0
mkdir distcheck-configure-flags-am-1.0/_build
mkdir distcheck-configure-flags-am-1.0/_inst
chmod a-w distcheck-configure-flags-am-1.0
test -d distcheck-configure-flags-am-1.0/_build || exit 0; \
	dc_install_base=`CDPATH="${ZSH_VERSION+.}:" && cd distcheck-configure-flags-am-1.0/_inst && pwd | sed -e 's,^[^:\\/]:[\\/],/,'` \
	  && dc_destdir="${TMPDIR-/tmp}/am-dc-$$/" \
	  && am__cwd=`pwd` \
	  && CDPATH="${ZSH_VERSION+.}:" && cd distcheck-configure-flags-am-1.0/_build \
	  && ../configure --srcdir=.. --prefix="$dc_install_base" \
	    --enable-success sentence='it works :-)'  \
	     \
	  && make  \
	  && make  dvi \
	  && make  check \
	  && make  install \
	  && make  installcheck \
	  && make  uninstall \
	  && make  distuninstallcheck_dir="$dc_install_base" \
	        distuninstallcheck \
	  && chmod -R a-w "$dc_install_base" \
	  && ({ \
	       (cd ../.. && umask 077 && mkdir "$dc_destdir") \
	       && make  DESTDIR="$dc_destdir" install \
	       && make  DESTDIR="$dc_destdir" uninstall \
	       && make  DESTDIR="$dc_destdir" \
	            distuninstallcheck_dir="$dc_destdir" distuninstallcheck; \
	      } || { rm -rf "$dc_destdir"; exit 1; }) \
	  && rm -rf "$dc_destdir" \
	  && make  dist \
	  && rm -rf distcheck-configure-flags-am-1.0.tar.gz \
	  && make  distcleancheck \
	  && cd "$am__cwd" \
	  || exit 1
checking for a BSD-compatible install... /bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Nothing to be done for `dvi'.
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Nothing to be done for `check'.
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[2]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[2]: Nothing to be done for `install-exec-am'.
make[2]: Nothing to be done for `install-data-am'.
make[2]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Nothing to be done for `installcheck'.
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Nothing to be done for `uninstall'.
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[2]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[2]: Nothing to be done for `install-exec-am'.
make[2]: Nothing to be done for `install-data-am'.
make[2]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Nothing to be done for `uninstall'.
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make  dist-gzip am__post_remove_distdir='@:'
make[2]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
{ test ! -d "distcheck-configure-flags-am-1.0" || { find "distcheck-configure-flags-am-1.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -fr "distcheck-configure-flags-am-1.0"; }; }
test -d "distcheck-configure-flags-am-1.0" || mkdir "distcheck-configure-flags-am-1.0"
test -n "" \
	|| find "distcheck-configure-flags-am-1.0" -type d ! -perm -755 \
		-exec chmod u+rwx,go+rx {} \; -o \
	  ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
	  ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
	  ! -type d ! -perm -444 -exec /bin/sh /home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/install-sh -c -m a+r {} {} \; \
	|| chmod -R a+r "distcheck-configure-flags-am-1.0"
tardir=distcheck-configure-flags-am-1.0 && ${TAR-tar} chof - "$tardir" | GZIP=--best gzip -c >distcheck-configure-flags-am-1.0.tar.gz
make[2]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
{ test ! -d "distcheck-configure-flags-am-1.0" || { find "distcheck-configure-flags-am-1.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -fr "distcheck-configure-flags-am-1.0"; }; }
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
test -z "" || rm -f 
test . = ".." || test -z "" || rm -f 
rm -f config.status config.cache config.log configure.lineno config.status.lineno
rm -f Makefile
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir/distcheck-configure-flags-am-1.0/_build'
{ test ! -d "distcheck-configure-flags-am-1.0" || { find "distcheck-configure-flags-am-1.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -fr "distcheck-configure-flags-am-1.0"; }; }
==================================================================
distcheck-configure-flags-am-1.0 archives ready for distribution: 
distcheck-configure-flags-am-1.0.tar.gz
==================================================================
+ cat
+ automake-1.11a --foreign -Werror -Wall Makefile
+ ./config.status Makefile
config.status: creating Makefile
+ make distcheck
+ cat output
make  dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir'
{ test ! -d "distcheck-configure-flags-am-1.0" || { find "distcheck-configure-flags-am-1.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -fr "distcheck-configure-flags-am-1.0"; }; }
test -d "distcheck-configure-flags-am-1.0" || mkdir "distcheck-configure-flags-am-1.0"
test -n "" \
	|| find "distcheck-configure-flags-am-1.0" -type d ! -perm -755 \
		-exec chmod u+rwx,go+rx {} \; -o \
	  ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
	  ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
	  ! -type d ! -perm -444 -exec /bin/sh /home/peda/automake/tests/distcheck-configure-flags-am.dir/install-sh -c -m a+r {} {} \; \
	|| chmod -R a+r "distcheck-configure-flags-am-1.0"
tardir=distcheck-configure-flags-am-1.0 && ${TAR-tar} chof - "$tardir" | GZIP=--best gzip -c >distcheck-configure-flags-am-1.0.tar.gz
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir'
{ test ! -d "distcheck-configure-flags-am-1.0" || { find "distcheck-configure-flags-am-1.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -fr "distcheck-configure-flags-am-1.0"; }; }
case 'distcheck-configure-flags-am-1.0.tar.gz' in \
	*.tar.gz*) \
	  GZIP=--best gzip -dc distcheck-configure-flags-am-1.0.tar.gz | ${TAR-tar} xf - ;;\
	*.tar.bz2*) \
	  bzip2 -dc distcheck-configure-flags-am-1.0.tar.bz2 | ${TAR-tar} xf - ;;\
	*.tar.lz*) \
	  lzip -dc distcheck-configure-flags-am-1.0.tar.lz | ${TAR-tar} xf - ;;\
	*.tar.xz*) \
	  xz -dc distcheck-configure-flags-am-1.0.tar.xz | ${TAR-tar} xf - ;;\
	*.tar.Z*) \
	  uncompress -c distcheck-configure-flags-am-1.0.tar.Z | ${TAR-tar} xf - ;;\
	*.shar.gz*) \
	  GZIP=--best gzip -dc distcheck-configure-flags-am-1.0.shar.gz | unshar ;;\
	*.zip*) \
	  unzip distcheck-configure-flags-am-1.0.zip ;;\
	esac
tar: distcheck-configure-flags-am-1.0: time stamp 2012-01-09 23:58:44 is 0.285 s in the future
chmod -R a-w distcheck-configure-flags-am-1.0; chmod a+w distcheck-configure-flags-am-1.0
mkdir distcheck-configure-flags-am-1.0/_build
mkdir distcheck-configure-flags-am-1.0/_inst
chmod a-w distcheck-configure-flags-am-1.0
test -d distcheck-configure-flags-am-1.0/_build || exit 0; \
	dc_install_base=`CDPATH="${ZSH_VERSION+.}:" && cd distcheck-configure-flags-am-1.0/_inst && pwd | sed -e 's,^[^:\\/]:[\\/],/,'` \
	  && dc_destdir="${TMPDIR-/tmp}/am-dc-$$/" \
	  && am__cwd=`pwd` \
	  && CDPATH="${ZSH_VERSION+.}:" && cd distcheck-configure-flags-am-1.0/_build \
	  && ../configure --srcdir=.. --prefix="$dc_install_base" \
	    --enable-success sentence='it works :-)' --disable-success \
	     \
	  && make  \
	  && make  dvi \
	  && make  check \
	  && make  install \
	  && make  installcheck \
	  && make  uninstall \
	  && make  distuninstallcheck_dir="$dc_install_base" \
	        distuninstallcheck \
	  && chmod -R a-w "$dc_install_base" \
	  && ({ \
	       (cd ../.. && umask 077 && mkdir "$dc_destdir") \
	       && make  DESTDIR="$dc_destdir" install \
	       && make  DESTDIR="$dc_destdir" uninstall \
	       && make  DESTDIR="$dc_destdir" \
	            distuninstallcheck_dir="$dc_destdir" distuninstallcheck; \
	      } || { rm -rf "$dc_destdir"; exit 1; }) \
	  && rm -rf "$dc_destdir" \
	  && make  dist \
	  && rm -rf distcheck-configure-flags-am-1.0.tar.gz \
	  && make  distcleancheck \
	  && cd "$am__cwd" \
	  || exit 1
checking for a BSD-compatible install... /bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
configure: error: success='no', sentence='it works :-)'
make: *** [distcheck] Error 1
+ grep '^configure:.* success='\''no'\'', sentence='\''it works :-)'\''' output
configure: error: success='no', sentence='it works :-)'
+ make distcheck DISTCHECK_CONFIGURE_FLAGS=--enable-success=yes
make  dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir'
{ test ! -d "distcheck-configure-flags-am-1.0" || { find "distcheck-configure-flags-am-1.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -fr "distcheck-configure-flags-am-1.0"; }; }
rm: cannot remove directory `distcheck-configure-flags-am-1.0/_build': Permission denied
make[1]: *** [distdir] Error 1
make[1]: Leaving directory `/home/peda/automake/tests/distcheck-configure-flags-am.dir'
make: *** [dist] Error 2
+ exit_status=2
+ set +e
+ cd /home/peda/automake/tests
+ test no = yes
+ case $am_explicit_skips in
+ test 2 -eq 0
+ keep_testdirs=yes
+ am_keeping_testdirs
+ case $keep_testdirs in
+ return 0
+ set +x
distcheck-configure-flags-am: exit 2

Message #8 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Peter Rosin <peda <at> lysator.liu.se>
To: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Tue, 10 Jan 2012 00:15:55 +0100

Peter Rosin skrev 2012-01-10 00:07:
> Hi!
> 
> distcheck-configure-flags-am.test fails on MSYS.  I think the cause is
> that some process lingers with CWD in the _build directory which some
> other process tries to remove.  On MSYS, it's not possible to remove a
> directory that is "in use" like that.
> 
> Ugly patch coming up shortly.

As promised.

Cheers,
Peter


From 8ba67ab9885c8987d7a09ad650bcda69675b8285 Mon Sep 17 00:00:00 2001
From: Peter Rosin <peda <at> lysator.liu.se>
Date: Tue, 10 Jan 2012 00:14:08 +0100
Subject: [PATCH] dist: try to avoid race on MSYS when removing the distdir

Fixes automake bug#10470.

lib/am/distdir.am (am__remove_distdir): Add a short sleep to make old
processes release the directory before an attempt to remove it is made.
On MSYS, it is not possible to remove a directory that is "in use".
---
 lib/am/distdir.am |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/lib/am/distdir.am b/lib/am/distdir.am
index d8c1a89..e56d33b 100644
--- a/lib/am/distdir.am
+++ b/lib/am/distdir.am
@@ -22,10 +22,13 @@ if %?TOPDIR_P%
 distdir = $(PACKAGE)-$(VERSION)
 top_distdir = $(distdir)
 
+## On MSYS (1.0.17) it is not possible to remove a directory that is
+## in use, hence the sleep call, which gives processes time to exit
+## and "release" the directory before it is removed.
 am__remove_distdir = \
   { test ! -d "$(distdir)" \
     || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
-         && rm -fr "$(distdir)"; }; }
+         && sleep 1 && rm -fr "$(distdir)"; }; }
 am__post_remove_distdir = $(am__remove_distdir)
 endif %?TOPDIR_P%
 
-- 
1.7.5.1

Message #11 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Peter Rosin <peda <at> lysator.liu.se>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Tue, 10 Jan 2012 10:50:33 +0100

Hi Peter.

On 01/10/2012 12:15 AM, Peter Rosin wrote:
> Peter Rosin skrev 2012-01-10 00:07:
>> Hi!
>>
>> distcheck-configure-flags-am.test fails on MSYS.
>
Is this failure consistently reproducible, or racy?  If racy, do you have
an estimate of its frequency?

>> I think the cause is
>> that some process lingers with CWD in the _build directory which some
>> other process tries to remove.  On MSYS, it's not possible to remove a
>> directory that is "in use" like that.
>>
>> Ugly patch coming up shortly.
> 
> As promised.
> 
> Cheers,
> Peter
> 
> From 8ba67ab9885c8987d7a09ad650bcda69675b8285 Mon Sep 17 00:00:00 2001
> From: Peter Rosin <peda <at> lysator.liu.se>
> Date: Tue, 10 Jan 2012 00:14:08 +0100
> Subject: [PATCH] dist: try to avoid race on MSYS when removing the distdir
> 
> Fixes automake bug#10470.
> 
> lib/am/distdir.am (am__remove_distdir): Add a short sleep to make old
> processes release the directory before an attempt to remove it is made.
> On MSYS, it is not possible to remove a directory that is "in use".
> ---
>  lib/am/distdir.am |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/am/distdir.am b/lib/am/distdir.am
> index d8c1a89..e56d33b 100644
> --- a/lib/am/distdir.am
> +++ b/lib/am/distdir.am
> @@ -22,10 +22,13 @@ if %?TOPDIR_P%
>  distdir = $(PACKAGE)-$(VERSION)
>  top_distdir = $(distdir)
>  
> +## On MSYS (1.0.17) it is not possible to remove a directory that is
> +## in use, hence the sleep call, which gives processes time to exit
> +## and "release" the directory before it is removed.
>  am__remove_distdir = \
>    { test ! -d "$(distdir)" \
>      || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
> -         && rm -fr "$(distdir)"; }; }
> +         && sleep 1 && rm -fr "$(distdir)"; }; }
>  am__post_remove_distdir = $(am__remove_distdir)
>  endif %?TOPDIR_P%
>  
Yuck.  As you've warned, this is really ugly indeed!  I don't think this change
is acceptable in its current form simply to pacify a minor[1] testsuite error.

Before going ahead, I'd like to have a better understanding of the reason of
the failure you are seeing[2] -- even better, a testcase that embodies this
understanding by making the issue consistently reproducible.

I will give a shot at this later or tomorrow, unless someone beats me.

Thanks,
  Stefano

-*-*-

[1] Two reasons for which I see the error as minor:
     - failing to remove the $(distdir) right away is not a serious error, just
       a minor annoyance, and easily fixed by a hand-issued "rm -rf";
     - IMHO, one shouldn't be using a "peculiar" system like MSYS to build real
       distribution tarballs anyway.

[2] Now that I think of it, my knee-jerk guess is that the "( sleep 1 ) &"
    put in configure by AM_SANITY_CHECK is the likely culprit.

Message #14 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Peter Rosin <peda <at> lysator.liu.se>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Tue, 10 Jan 2012 11:12:40 +0100

Stefano Lattarini skrev 2012-01-10 10:50:
> Hi Peter.
> 
> On 01/10/2012 12:15 AM, Peter Rosin wrote:
>> Peter Rosin skrev 2012-01-10 00:07:
>>> Hi!
>>>
>>> distcheck-configure-flags-am.test fails on MSYS.
>>
> Is this failure consistently reproducible, or racy?  If racy, do you have
> an estimate of its frequency?

If the analysis is correct and it really is some other process that "sits"
in the to be removed directory, then it is bound to be a race.  And the
"locked directory" thing certainly appears to be the problem.  But apart
from that, it seems that the winner of the race is easy to predict, at
least going by from my limited runs (say 5 or 10 with sleep and 5 or so
without) as I can reproduce the problem by removing the sleep and work
around it by adding the sleep.

>>> I think the cause is
>>> that some process lingers with CWD in the _build directory which some
>>> other process tries to remove.  On MSYS, it's not possible to remove a
>>> directory that is "in use" like that.
>>>
>>> Ugly patch coming up shortly.
>>
>> As promised.
>>
>> Cheers,
>> Peter
>>
>> From 8ba67ab9885c8987d7a09ad650bcda69675b8285 Mon Sep 17 00:00:00 2001
>> From: Peter Rosin <peda <at> lysator.liu.se>
>> Date: Tue, 10 Jan 2012 00:14:08 +0100
>> Subject: [PATCH] dist: try to avoid race on MSYS when removing the distdir
>>
>> Fixes automake bug#10470.
>>
>> lib/am/distdir.am (am__remove_distdir): Add a short sleep to make old
>> processes release the directory before an attempt to remove it is made.
>> On MSYS, it is not possible to remove a directory that is "in use".
>> ---
>>  lib/am/distdir.am |    5 ++++-
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/lib/am/distdir.am b/lib/am/distdir.am
>> index d8c1a89..e56d33b 100644
>> --- a/lib/am/distdir.am
>> +++ b/lib/am/distdir.am
>> @@ -22,10 +22,13 @@ if %?TOPDIR_P%
>>  distdir = $(PACKAGE)-$(VERSION)
>>  top_distdir = $(distdir)
>>  
>> +## On MSYS (1.0.17) it is not possible to remove a directory that is
>> +## in use, hence the sleep call, which gives processes time to exit
>> +## and "release" the directory before it is removed.
>>  am__remove_distdir = \
>>    { test ! -d "$(distdir)" \
>>      || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
>> -         && rm -fr "$(distdir)"; }; }
>> +         && sleep 1 && rm -fr "$(distdir)"; }; }
>>  am__post_remove_distdir = $(am__remove_distdir)
>>  endif %?TOPDIR_P%
>>  
> Yuck.  As you've warned, this is really ugly indeed!  I don't think this change
> is acceptable in its current form simply to pacify a minor[1] testsuite error.

No problem.

> Before going ahead, I'd like to have a better understanding of the reason of
> the failure you are seeing[2] -- even better, a testcase that embodies this
> understanding by making the issue consistently reproducible.
> 
> I will give a shot at this later or tomorrow, unless someone beats me.
> 
> Thanks,
>   Stefano
> 
> -*-*-
> 
> [1] Two reasons for which I see the error as minor:
>      - failing to remove the $(distdir) right away is not a serious error, just
>        a minor annoyance, and easily fixed by a hand-issued "rm -rf";
>      - IMHO, one shouldn't be using a "peculiar" system like MSYS to build real
>        distribution tarballs anyway.
> 
> [2] Now that I think of it, my knee-jerk guess is that the "( sleep 1 ) &"
>     put in configure by AM_SANITY_CHECK is the likely culprit.

I tried that hypothesis and bumped this to "( sleep 2 ) &", and the test
case failed even with my ugly "sleep 1" workaround.  I then bumped "my" fix
to "sleep 2" and the test case succeeded again.  So they certainly seem
connected.

Cheers,
Peter

Message #17 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Peter Rosin <peda <at> lysator.liu.se>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Tue, 10 Jan 2012 12:35:11 +0100

Peter Rosin skrev 2012-01-10 11:12:
> Stefano Lattarini skrev 2012-01-10 10:50:
>> [2] Now that I think of it, my knee-jerk guess is that the "( sleep 1 ) &"
>>     put in configure by AM_SANITY_CHECK is the likely culprit.
> 
> I tried that hypothesis and bumped this to "( sleep 2 ) &", and the test
> case failed even with my ugly "sleep 1" workaround.  I then bumped "my" fix
> to "sleep 2" and the test case succeeded again.  So they certainly seem
> connected.

One further data point; changing that "( sleep 1 ) &" thing followed by a
wait just before AC_OUTPUT into an inlined sleep also fixes the test case.

As below, but as before, one unconditional second might be too expensive.

So, your knee-jerk is sound.  Question is what to do...

Cheers,
Peter


diff --git a/m4/sanity.m4 b/m4/sanity.m4
index b718ac4..689781a 100644
--- a/m4/sanity.m4
+++ b/m4/sanity.m4
@@ -69,17 +69,8 @@ fi
 AC_MSG_RESULT([yes])
 # If we didn't sleep, we still need to ensure time stamps of config.status and
 # generated files are strictly newer.
-am_sleep_pid=
 if grep 'slept: no' conftest.file >/dev/null 2>&1; then
-  ( sleep 1 ) &
-  am_sleep_pid=$!
+  sleep 1
 fi
-AC_CONFIG_COMMANDS_PRE(
-  [AC_MSG_CHECKING([that generated files are newer than configure])
-   if test -n "$am_sleep_pid"; then
-     # Hide warnings about reused PIDs.
-     wait $am_sleep_pid 2>/dev/null
-   fi
-   AC_MSG_RESULT([done])])
 rm -f conftest.file
 ])

Message #20 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Peter Rosin <peda <at> lysator.liu.se>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Tue, 10 Jan 2012 13:44:25 +0100

On 01/10/2012 12:35 PM, Peter Rosin wrote:
> Peter Rosin skrev 2012-01-10 11:12:
>> Stefano Lattarini skrev 2012-01-10 10:50:
>>> [2] Now that I think of it, my knee-jerk guess is that the "( sleep 1 ) &"
>>>     put in configure by AM_SANITY_CHECK is the likely culprit.
>>
>> I tried that hypothesis and bumped this to "( sleep 2 ) &", and the test
>> case failed even with my ugly "sleep 1" workaround.  I then bumped "my" fix
>> to "sleep 2" and the test case succeeded again.  So they certainly seem
>> connected.
> 
> One further data point; changing that "( sleep 1 ) &" thing followed by a
> wait just before AC_OUTPUT into an inlined sleep also fixes the test case.
>
Thanks for tracking this down.  So the question now becomes: why doesn't
the trick with background processes work on MSYS?  Does maybe MSYS lack
proper emulation of POSIX-ish PIDs?  If that is the case, can we easily
work around that in our situation?

> As below, but as before, one unconditional second might be too expensive.
>
As a last resort, we might enable this suboptimal-but-safer behaviour
only when the build system is recognized to be MinGW/MSYS...

... Or we could take advantage of the bash built-in special variable
$SECONDS, if available (that variable was already present in bash 3.0
BTW), and use that to decide whether enough time has elapsed ... WDYT?

Thanks,
  Stefano

Message #23 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: Peter Rosin <peda <at> lysator.liu.se>, 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Tue, 10 Jan 2012 06:02:11 -0700

[Message part 1 (text/plain, inline)]

On 01/10/2012 05:44 AM, Stefano Lattarini wrote:
> On 01/10/2012 12:35 PM, Peter Rosin wrote:
>> Peter Rosin skrev 2012-01-10 11:12:
>>> Stefano Lattarini skrev 2012-01-10 10:50:
>>>> [2] Now that I think of it, my knee-jerk guess is that the "( sleep 1 ) &"
>>>>     put in configure by AM_SANITY_CHECK is the likely culprit.

It IS the culprit.  Use of a directory as the cwd _IS_ what prevents
removal of said directory.

> Thanks for tracking this down.  So the question now becomes: why doesn't
> the trick with background processes work on MSYS?  Does maybe MSYS lack
> proper emulation of POSIX-ish PIDs?  If that is the case, can we easily
> work around that in our situation?

Background processes _do_ work, the problem here is that you cannot
rmdir an in-use directory.  So as long as the background process still
exists, the directory is still in use.

But that analysis points out what should be a valid workaround: instead
of doing '(sleep 1)&', do '(cd somewhere/safe; sleep 1)&'.  Then the cwd
of the sleep process is no longer tied to a directory that you plan on
removing, so you no longer have to wait for that sleep process to exit.
 As for what somewhere/safe should be, $(abs_builddir) is probably
reasonable.

> As a last resort, we might enable this suboptimal-but-safer behaviour
> only when the build system is recognized to be MinGW/MSYS...

Nah, doing a cd to somewhere safe before sleeping in the background
should be non-invasive and appropriate for all platforms.

-- 
Eric Blake   eblake <at> redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Message #26 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Peter Rosin <peda <at> lysator.liu.se>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Tue, 10 Jan 2012 14:13:44 +0100

Stefano Lattarini skrev 2012-01-10 13:44:
> On 01/10/2012 12:35 PM, Peter Rosin wrote:
>> Peter Rosin skrev 2012-01-10 11:12:
>>> Stefano Lattarini skrev 2012-01-10 10:50:
>>>> [2] Now that I think of it, my knee-jerk guess is that the "( sleep 1 ) &"
>>>>     put in configure by AM_SANITY_CHECK is the likely culprit.
>>>
>>> I tried that hypothesis and bumped this to "( sleep 2 ) &", and the test
>>> case failed even with my ugly "sleep 1" workaround.  I then bumped "my" fix
>>> to "sleep 2" and the test case succeeded again.  So they certainly seem
>>> connected.
>>
>> One further data point; changing that "( sleep 1 ) &" thing followed by a
>> wait just before AC_OUTPUT into an inlined sleep also fixes the test case.
>>
> Thanks for tracking this down.  So the question now becomes: why doesn't
> the trick with background processes work on MSYS?  Does maybe MSYS lack
> proper emulation of POSIX-ish PIDs?  If that is the case, can we easily
> work around that in our situation?

No no, that's not the problem.  In this case "make distcheck" runs
configure, but configure fails so it never reaches AC_OUTPUT and
the wait call never happens.  So, the ( sleep 1 ) subshell lives on
and "locks" the directory (but something else is also going on,
because changing the subshell to do ( cd /; sleep 1 ) doesn't help).

*pling* new mail, from Eric.

He explains the issue with different words and suggests:

"As for what somewhere/safe should be, $(abs_builddir) is probably
 reasonable."

So, testing that too, but neither cd /, nor cd "$(abs_builddir)" make
the test pass. Crap.

We could fix it in the testsuite only, by adding a trap to configure
that does the wait if it didn't happen before, but that would make
use of internals that would perhaps spread. Yuck again.

Cheers,
Peter

Message #29 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Peter Rosin <peda <at> lysator.liu.se>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Tue, 10 Jan 2012 16:02:57 +0100

On 01/10/2012 02:13 PM, Peter Rosin wrote:
> Stefano Lattarini skrev 2012-01-10 13:44:
>> On 01/10/2012 12:35 PM, Peter Rosin wrote:
>>>
>>> One further data point; changing that "( sleep 1 ) &" thing followed by a
>>> wait just before AC_OUTPUT into an inlined sleep also fixes the test case.
>>>
>> Thanks for tracking this down.  So the question now becomes: why doesn't
>> the trick with background processes work on MSYS?  Does maybe MSYS lack
>> proper emulation of POSIX-ish PIDs?  If that is the case, can we easily
>> work around that in our situation?
> 
> No no, that's not the problem.  In this case "make distcheck" runs
> configure, but configure fails so it never reaches AC_OUTPUT and
> the wait call never happens.
>
Ah, good catch.  So the problem is not limited to MinGW, but could potentially
hit also when we are building on (say) NFS mounts, right?

> So, the ( sleep 1 ) subshell lives on
> and "locks" the directory (but something else is also going on,
> because changing the subshell to do ( cd /; sleep 1 ) doesn't help).
> 
> *pling* new mail, from Eric.
> 
> He explains the issue with different words and suggests:
> 
> "As for what somewhere/safe should be, $(abs_builddir) is probably
>  reasonable."
> 
> So, testing that too, but neither cd /, nor cd "$(abs_builddir)" make
> the test pass. Crap.
>
It would probably be worthwhile to try to understand why this is the case.
As a wild guess, would this work?

  - ( sleep 1 ) &
  + sh -c "cd '$abs_builddir'; sleep 1" &

> We could fix it in the testsuite only, by adding a trap to configure
> that does the wait if it didn't happen before, but that would make
> use of internals that would perhaps spread. Yuck again.
>
Or we could enhance your original workaround like this:

 am__remove_distdir = \
   { test ! -d "$(distdir)" \
     || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
-         && rm -fr "$(distdir)"; }; }
+         && if rm -fr "$(distdir)"; then :; else \
+## On MSYS (1.0.17) it is not possible to remove a directory that is
+## in use; so, if the first rm fails, we sleep some seconds and retry,
+## to give pending processes some time to exit and "release" the
+## directory before we removed.  See automake bug#10470.
+              sleep 5 && rm -fr "$(distdir)"; fi; }; }
 am__post_remove_distdir = $(am__remove_distdir)
 endif %?TOPDIR_P%

Thanks,
  Stefano

Message #32 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Peter Rosin <peda <at> lysator.liu.se>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Tue, 10 Jan 2012 16:28:15 +0100

Stefano Lattarini skrev 2012-01-10 16:02:
> On 01/10/2012 02:13 PM, Peter Rosin wrote:
>> Stefano Lattarini skrev 2012-01-10 13:44:
>>> On 01/10/2012 12:35 PM, Peter Rosin wrote:
>>>>
>>>> One further data point; changing that "( sleep 1 ) &" thing followed by a
>>>> wait just before AC_OUTPUT into an inlined sleep also fixes the test case.
>>>>
>>> Thanks for tracking this down.  So the question now becomes: why doesn't
>>> the trick with background processes work on MSYS?  Does maybe MSYS lack
>>> proper emulation of POSIX-ish PIDs?  If that is the case, can we easily
>>> work around that in our situation?
>>
>> No no, that's not the problem.  In this case "make distcheck" runs
>> configure, but configure fails so it never reaches AC_OUTPUT and
>> the wait call never happens.
>>
> Ah, good catch.  So the problem is not limited to MinGW, but could potentially
> hit also when we are building on (say) NFS mounts, right?

I don't know how bad it is to try to remove the CWD of another process
for non-MSYS systems, so I can't offer any input on this one.

>> So, the ( sleep 1 ) subshell lives on
>> and "locks" the directory (but something else is also going on,
>> because changing the subshell to do ( cd /; sleep 1 ) doesn't help).
>>
>> *pling* new mail, from Eric.
>>
>> He explains the issue with different words and suggests:
>>
>> "As for what somewhere/safe should be, $(abs_builddir) is probably
>>  reasonable."
>>
>> So, testing that too, but neither cd /, nor cd "$(abs_builddir)" make
>> the test pass. Crap.
>>
> It would probably be worthwhile to try to understand why this is the case.
> As a wild guess, would this work?
> 
>   - ( sleep 1 ) &
>   + sh -c "cd '$abs_builddir'; sleep 1" &

No, fails in the same way as the original.

>> We could fix it in the testsuite only, by adding a trap to configure
>> that does the wait if it didn't happen before, but that would make
>> use of internals that would perhaps spread. Yuck again.
>>
> Or we could enhance your original workaround like this:
> 
>  am__remove_distdir = \
>    { test ! -d "$(distdir)" \
>      || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
> -         && rm -fr "$(distdir)"; }; }
> +         && if rm -fr "$(distdir)"; then :; else \
> +## On MSYS (1.0.17) it is not possible to remove a directory that is
> +## in use; so, if the first rm fails, we sleep some seconds and retry,
> +## to give pending processes some time to exit and "release" the
> +## directory before we removed.  See automake bug#10470.
> +              sleep 5 && rm -fr "$(distdir)"; fi; }; }
>  am__post_remove_distdir = $(am__remove_distdir)
>  endif %?TOPDIR_P%

This works, best so far!  Committable, if you ask me.

Cheers,
Peter

Message #35 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Peter Rosin <peda <at> lysator.liu.se>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Tue, 10 Jan 2012 18:03:09 +0100

>> It would probably be worthwhile to try to understand why this is the case.
>> As a wild guess, would this work?
>>
>>   - ( sleep 1 ) &
>>   + sh -c "cd '$abs_builddir'; sleep 1" &
> 
> No, fails in the same way as the original.
>
Sorry, by bad, I meant this:  sh -c "cd '$ac_abs_srcdir'; sleep 1"

>> We could enhance your original workaround like this:
>>
>>  am__remove_distdir = \
>>    { test ! -d "$(distdir)" \
>>      || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
>> -         && rm -fr "$(distdir)"; }; }
>> +         && if rm -fr "$(distdir)"; then :; else \
>> +## On MSYS (1.0.17) it is not possible to remove a directory that is
>> +## in use; so, if the first rm fails, we sleep some seconds and retry,
>> +## to give pending processes some time to exit and "release" the
>> +## directory before we removed.  See automake bug#10470.
>> +              sleep 5 && rm -fr "$(distdir)"; fi; }; }
>>  am__post_remove_distdir = $(am__remove_distdir)
>>  endif %?TOPDIR_P%
> 
> This works, best so far!  Committable, if you ask me.
>
I agree, but first I'd like to see if we manage to resolve the issue into
sanity.m4 itself.  If we fail, the above workaround will be good enough.

Thanks,
  Stefano

Message #38 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Peter Rosin <peda <at> lysator.liu.se>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Tue, 10 Jan 2012 19:37:47 +0100

Stefano Lattarini skrev 2012-01-10 18:03:
>>> It would probably be worthwhile to try to understand why this is the case.
>>> As a wild guess, would this work?
>>>
>>>   - ( sleep 1 ) &
>>>   + sh -c "cd '$abs_builddir'; sleep 1" &
>>
>> No, fails in the same way as the original.
>>
> Sorry, by bad, I meant this:  sh -c "cd '$ac_abs_srcdir'; sleep 1"

No, that doesn't work either, at least not if you meant the line to end
with &, as it would be an inline sleep if it wouldn't.  Side note,
$ac_abs_srcdir is empty (so it's not really "somewhere/safe", as Eric
put it).

>>> We could enhance your original workaround like this:
>>>
>>>  am__remove_distdir = \
>>>    { test ! -d "$(distdir)" \
>>>      || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
>>> -         && rm -fr "$(distdir)"; }; }
>>> +         && if rm -fr "$(distdir)"; then :; else \
>>> +## On MSYS (1.0.17) it is not possible to remove a directory that is
>>> +## in use; so, if the first rm fails, we sleep some seconds and retry,
>>> +## to give pending processes some time to exit and "release" the
>>> +## directory before we removed.  See automake bug#10470.
>>> +              sleep 5 && rm -fr "$(distdir)"; fi; }; }
>>>  am__post_remove_distdir = $(am__remove_distdir)
>>>  endif %?TOPDIR_P%
>>
>> This works, best so far!  Committable, if you ask me.
>>
> I agree, but first I'd like to see if we manage to resolve the issue into
> sanity.m4 itself.  If we fail, the above workaround will be good enough.

Yes, that would be best.  Otherwise we run the risk of having to add the
workaround for other places as well, ick.

I think (but I'm out on a limb here) that it might be the make process that
invoked the distcheck rule that lingers in the build-dir until configure
(and all its children terminates).  That make is locking the directory and
causes rm to fail. But as I said, I'm out on a limb and this is probably
just crap...

Cheers,
Peter

Message #43 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Peter Rosin <peda <at> lysator.liu.se>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Wed, 11 Jan 2012 18:31:06 +0100

[Message part 1 (text/plain, inline)]

Hi Peter, sorry for the delay.

>>>> We could enhance your original workaround like this:
>>>>
>>>>  am__remove_distdir = \
>>>>    { test ! -d "$(distdir)" \
>>>>      || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
>>>> -         && rm -fr "$(distdir)"; }; }
>>>> +         && if rm -fr "$(distdir)"; then :; else \
>>>> +## On MSYS (1.0.17) it is not possible to remove a directory that is
>>>> +## in use; so, if the first rm fails, we sleep some seconds and retry,
>>>> +## to give pending processes some time to exit and "release" the
>>>> +## directory before we removed.  See automake bug#10470.
>>>> +              sleep 5 && rm -fr "$(distdir)"; fi; }; }
>>>>  am__post_remove_distdir = $(am__remove_distdir)
>>>>  endif %?TOPDIR_P%
>>
>>> This works, best so far!  Committable, if you ask me.
>>>
> 
Could you try the attached test case to see if it can reliably expose the
problem on MSYS/MinGW?  If yes, I'll prepare a patch shortly.

Thanks,
  Stefano

[distcheck-pr10470.test (text/plain, attachment)]

Message #46 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Peter Rosin <peda <at> lysator.liu.se>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Wed, 11 Jan 2012 20:27:28 +0100

Stefano Lattarini skrev 2012-01-11 18:31:
> Hi Peter, sorry for the delay.

No rush!

>>>>> We could enhance your original workaround like this:
>>>>>
>>>>>  am__remove_distdir = \
>>>>>    { test ! -d "$(distdir)" \
>>>>>      || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
>>>>> -         && rm -fr "$(distdir)"; }; }
>>>>> +         && if rm -fr "$(distdir)"; then :; else \
>>>>> +## On MSYS (1.0.17) it is not possible to remove a directory that is
>>>>> +## in use; so, if the first rm fails, we sleep some seconds and retry,
>>>>> +## to give pending processes some time to exit and "release" the
>>>>> +## directory before we removed.  See automake bug#10470.
>>>>> +              sleep 5 && rm -fr "$(distdir)"; fi; }; }
>>>>>  am__post_remove_distdir = $(am__remove_distdir)
>>>>>  endif %?TOPDIR_P%
>>>
>>>> This works, best so far!  Committable, if you ask me.
>>>>
>>
> Could you try the attached test case to see if it can reliably expose the
> problem on MSYS/MinGW?  If yes, I'll prepare a patch shortly.

If I (try to) mend the race (the "rm -rf foo.d" reliably beats the "cd foo.d"
in the subshell) by adding a "sleep 1" before the "rm -rf foo.d" the test
passes (exit zero).  So, I guess no, the probelm is not exposed by the test.

Did you mean "$my_sleep &" in the foo.test script?  If add that &, the test
fails in much the same way as we've seen previously in this bug report.

...
rm: cannot remove directory `distcheck-pr10470-1.0/_build': Permission denied
make: *** [distcheck] Error 1
...

Cheers,
Peter

Message #49 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Peter Rosin <peda <at> lysator.liu.se>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Thu, 12 Jan 2012 10:15:36 +0100

[Message part 1 (text/plain, inline)]

On 01/11/2012 08:27 PM, Peter Rosin wrote:
> Stefano Lattarini skrev 2012-01-11 18:31:
>> Hi Peter, sorry for the delay.
> 
> No rush!
> 
>>>>>> We could enhance your original workaround like this:
>>>>>>
>>>>>>  am__remove_distdir = \
>>>>>>    { test ! -d "$(distdir)" \
>>>>>>      || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
>>>>>> -         && rm -fr "$(distdir)"; }; }
>>>>>> +         && if rm -fr "$(distdir)"; then :; else \
>>>>>> +## On MSYS (1.0.17) it is not possible to remove a directory that is
>>>>>> +## in use; so, if the first rm fails, we sleep some seconds and retry,
>>>>>> +## to give pending processes some time to exit and "release" the
>>>>>> +## directory before we removed.  See automake bug#10470.
>>>>>> +              sleep 5 && rm -fr "$(distdir)"; fi; }; }
>>>>>>  am__post_remove_distdir = $(am__remove_distdir)
>>>>>>  endif %?TOPDIR_P%
>>>>
>>>>> This works, best so far!  Committable, if you ask me.
>>>>>
>>>
>> Could you try the attached test case to see if it can reliably expose the
>> problem on MSYS/MinGW?  If yes, I'll prepare a patch shortly.
> 
> If I (try to) mend the race (the "rm -rf foo.d" reliably beats the "cd foo.d"
> in the subshell) by adding a "sleep 1" before the "rm -rf foo.d"
>
Thanks for the info, I've amend the test accordingly.

> So, I guess no, the probelm is not exposed by the test.
>
> Did you mean "$my_sleep &" in the foo.test script?
>
Yes, I did; sorry for the sloppiness.

> If add that &, the test fails in much the same way as we've seen previously
> in this bug report.
>
Good!

Attached is the proposed patch series: the first patch should expose the bug,
the second patch should fix it.  I will apply them once I have confirmation
the bug is correctly exposed and fixed.

Thanks,
  Stefano.

[0001-coverage-expose-automake-bug-10470-distcheck-related.patch (text/x-diff, attachment)]

[0002-dist-avoid-distdir-removal-failure-on-MSYS-MinGW.patch (text/x-diff, attachment)]

Message #52 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Peter Rosin <peda <at> lysator.liu.se>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Thu, 12 Jan 2012 11:39:51 +0100

Stefano Lattarini skrev 2012-01-12 10:15:
> On 01/11/2012 08:27 PM, Peter Rosin wrote:
>> Stefano Lattarini skrev 2012-01-11 18:31:
>>> Hi Peter, sorry for the delay.
>>
>> No rush!
>>
>>>>>>> We could enhance your original workaround like this:
>>>>>>>
>>>>>>>  am__remove_distdir = \
>>>>>>>    { test ! -d "$(distdir)" \
>>>>>>>      || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
>>>>>>> -         && rm -fr "$(distdir)"; }; }
>>>>>>> +         && if rm -fr "$(distdir)"; then :; else \
>>>>>>> +## On MSYS (1.0.17) it is not possible to remove a directory that is
>>>>>>> +## in use; so, if the first rm fails, we sleep some seconds and retry,
>>>>>>> +## to give pending processes some time to exit and "release" the
>>>>>>> +## directory before we removed.  See automake bug#10470.
>>>>>>> +              sleep 5 && rm -fr "$(distdir)"; fi; }; }
>>>>>>>  am__post_remove_distdir = $(am__remove_distdir)
>>>>>>>  endif %?TOPDIR_P%
>>>>>
>>>>>> This works, best so far!  Committable, if you ask me.
>>>>>>
>>>>
>>> Could you try the attached test case to see if it can reliably expose the
>>> problem on MSYS/MinGW?  If yes, I'll prepare a patch shortly.
>>
>> If I (try to) mend the race (the "rm -rf foo.d" reliably beats the "cd foo.d"
>> in the subshell) by adding a "sleep 1" before the "rm -rf foo.d"
>>
> Thanks for the info, I've amend the test accordingly.
> 
>> So, I guess no, the probelm is not exposed by the test.
>>
>> Did you mean "$my_sleep &" in the foo.test script?
>>
> Yes, I did; sorry for the sloppiness.
> 
>> If add that &, the test fails in much the same way as we've seen previously
>> in this bug report.
>>
> Good!
> 
> Attached is the proposed patch series: the first patch should expose the bug,
> the second patch should fix it.  I will apply them once I have confirmation
> the bug is correctly exposed and fixed.

After digging out that the patches are for maint, I managed to commit them,
but FYI this bugfix is only needed on master.  AM_SANITY_CHECK is not
aggressive enough on maint to cause problems, as can be seen below.  But ok
to commit wherever, it behaves as designed. Thanks!

I should also add that it's a POSIX violation to assume that directory
removal works when the dir is the CWD of a process.

	"If the directory is the root directory or the current working
	directory of any process, it is unspecified whether the function
	succeeds, or whether it shall fail and set errno to [EBUSY]."

Cheers,
Peter


On maint, with 1st patch:

$ make check TESTS="distcheck-pr10470.test distcheck-configure-flags-am.test"
XFAIL: distcheck-pr10470.test
PASS: distcheck-configure-flags-am.test

On maint, with both patches:

$ make check TESTS="distcheck-pr10470.test distcheck-configure-flags-am.test"
PASS: distcheck-pr10470.test
PASS: distcheck-configure-flags-am.test

Merging 1st patch into master:

$ make check TESTS="distcheck-pr10470.test distcheck-configure-flags-am.test"
XFAIL: distcheck-pr10470.test
FAIL: distcheck-configure-flags-am.test

Merging also the 2st patch into master:

$ make check TESTS="distcheck-pr10470.test distcheck-configure-flags-am.test"
PASS: distcheck-pr10470.test
PASS: distcheck-configure-flags-am.test

Message #55 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Peter Rosin <peda <at> lysator.liu.se>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Fri, 13 Jan 2012 10:05:55 +0100

On 01/12/2012 11:39 AM, Peter Rosin wrote:
>
> After digging out that the patches are for maint,
>
Oops, sorry I didn't state that explicitly!

> I managed to commit them,
> but FYI this bugfix is only needed on master.
>
True, but I'd rather avoid code divergences if possible and if that doesn't
cause performance regressions (conditions which both hold in this case).

> AM_SANITY_CHECK is not
> aggressive enough on maint to cause problems, as can be seen below.  But ok
> to commit wherever, it behaves as designed. Thanks!
>
Thanks to you for the patient analysis and testing.

> I should also add that it's a POSIX violation to assume that directory
> removal works when the dir is the CWD of a process.
> 
> 	"If the directory is the root directory or the current working
> 	directory of any process, it is unspecified whether the function
> 	succeeds, or whether it shall fail and set errno to [EBUSY]."
> 
Good to know.  I've amended the git commit message of the second patch
accordingly, to read as follows:

    dist: avoid $(distdir) removal failure on MSYS/MinGW

    This change fixes automake big#10470.

    On MSYS (1.0.17) it is not possible to remove a directory that is
    in use, and this, together with timing issues, could cause spurious
    failures in the cleanup code of the "distcheck" recipe.  In fact,
    it should be noted that assuming a directory can be removed while
    it the CWD of a running process is a POSIX violation:

      "If the directory is the root directory or the current working
       directory of any process, it is unspecified whether the function
       succeeds, or whether it shall fail and set errno to [EBUSY]."

    * lib/am/distdir.am (am__remove_distdir): If rm fails, sleep some
    seconds and retry, to give potential "pending" processes some time
    to exit and "release" the directory.
    * tests/Makefile.am (XFAIL_TESTS): Remove 'distcheck-pr10470.test'.

    Report and suggestions by Peter Rosin and Eric Blake.

I will push by this evening or tomorrow if there is no objection.

Thanks,
  Stefano

Message #58 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Peter Rosin <peda <at> lysator.liu.se>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Fri, 13 Jan 2012 11:11:04 +0100

Stefano Lattarini skrev 2012-01-13 10:05:
> Good to know.  I've amended the git commit message of the second patch
> accordingly, to read as follows:
> 
>     dist: avoid $(distdir) removal failure on MSYS/MinGW
> 
>     This change fixes automake big#10470.

s/big/bug/ (in case there is some automatic link creation or something)

Cheers,
Peter

Message #61 received at 10470 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Peter Rosin <peda <at> lysator.liu.se>
Cc: 10470 <at> debbugs.gnu.org
Subject: Re: bug#10470: MSYS: race in directory access?
Date: Fri, 13 Jan 2012 13:23:54 +0100

On 01/13/2012 11:11 AM, Peter Rosin wrote:
> Stefano Lattarini skrev 2012-01-13 10:05:
>> Good to know.  I've amended the git commit message of the second patch
>> accordingly, to read as follows:
>>
>>     dist: avoid $(distdir) removal failure on MSYS/MinGW
>>
>>     This change fixes automake big#10470.
> 
> s/big/bug/ (in case there is some automatic link creation or something)
> 
Thanks for catching that.  I've merged to patches into maint, merged maint
into master, and pushed.

Regards,
  Stefano

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.