[gentoo-portage-dev] [PATCH] econf: update configure/config.{sub,guess} atomically to avoid races

2013-12-17 Thread Mike Frysinger 
Use $BASHPID which will be unique even in subshells.

URL: https://bugs.gentoo.org/487478
---
 bin/phase-helpers.sh | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
index ec48c94..1a7ae03 100644
--- a/bin/phase-helpers.sh
+++ b/bin/phase-helpers.sh
@@ -469,6 +469,7 @@ unpack() {
 
 econf() {
local x
+   local pid=${BASHPID}
 
if ! ___eapi_has_prefix_variables; then
local EPREFIX=
@@ -501,18 +502,22 @@ econf() {
if [[ -n $CONFIG_SHELL  \
$(head -n1 $ECONF_SOURCE/configure) =~ 
^'#!'[[:space:]]*/bin/sh([[:space:]]|$) ]] ; then
# preserve timestamp, see bug #440304
-   touch -r $ECONF_SOURCE/configure 
$ECONF_SOURCE/configure._portage_tmp_.$$ || die
-   sed -e 1s:^#![[:space:]]*/bin/sh:#!$CONFIG_SHELL: -i 
$ECONF_SOURCE/configure || \
-   die Substition of shebang in 
'$ECONF_SOURCE/configure' failed
-   touch -r $ECONF_SOURCE/configure._portage_tmp_.$$ 
$ECONF_SOURCE/configure || die
-   rm -f $ECONF_SOURCE/configure._portage_tmp_.$$
+   touch -r ${ECONF_SOURCE}/configure 
${ECONF_SOURCE}/configure._portage_tmp_.${pid} || die
+   sed -i \
+   -e 1s:^#![[:space:]]*/bin/sh:#!$CONFIG_SHELL: 
\
+   ${ECONF_SOURCE}/configure \
+   || die Substition of shebang in 
'${ECONF_SOURCE}/configure' failed
+   touch -r 
${ECONF_SOURCE}/configure._portage_tmp_.${pid} ${ECONF_SOURCE}/configure || 
die
+   rm -f ${ECONF_SOURCE}/configure._portage_tmp_.${pid}
fi
if [ -e ${EPREFIX}/usr/share/gnuconfig/ ]; then
find ${WORKDIR} -type f '(' \
-name config.guess -o -name config.sub ')' -print0 | \
while read -r -d $'\0' x ; do
__vecho  * econf: updating ${x/${WORKDIR}\/} 
with ${EPREFIX}/usr/share/gnuconfig/${x##*/}
-   cp -f 
${EPREFIX}/usr/share/gnuconfig/${x##*/} ${x}
+   # Make sure we do this atomically incase we're 
run in parallel. #487478
+   cp -f 
${EPREFIX}/usr/share/gnuconfig/${x##*/} ${x}.${pid}
+   mv -f ${x}.${pid} ${x}
done
fi
 
-- 
1.8.4.3

Re: [gentoo-portage-dev] [PATCH] econf: update configure/config.{sub,guess} atomically to avoid races

2013-12-17 Thread Brian Dolbec 
On Tue, 2013-12-17 at 18:28 -0500, Mike Frysinger wrote:
 Use $BASHPID which will be unique even in subshells.
 
 URL: https://bugs.gentoo.org/487478
 ---
  bin/phase-helpers.sh | 17 +++--
  1 file changed, 11 insertions(+), 6 deletions(-)
 
 diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
 index ec48c94..1a7ae03 100644
 --- a/bin/phase-helpers.sh
 +++ b/bin/phase-helpers.sh
 @@ -469,6 +469,7 @@ unpack() {
  
  econf() {
   local x
 + local pid=${BASHPID}
  
   if ! ___eapi_has_prefix_variables; then
   local EPREFIX=
 @@ -501,18 +502,22 @@ econf() {
   if [[ -n $CONFIG_SHELL  \
   $(head -n1 $ECONF_SOURCE/configure) =~ 
 ^'#!'[[:space:]]*/bin/sh([[:space:]]|$) ]] ; then
   # preserve timestamp, see bug #440304
 - touch -r $ECONF_SOURCE/configure 
 $ECONF_SOURCE/configure._portage_tmp_.$$ || die
 - sed -e 1s:^#![[:space:]]*/bin/sh:#!$CONFIG_SHELL: -i 
 $ECONF_SOURCE/configure || \
 - die Substition of shebang in 
 '$ECONF_SOURCE/configure' failed
 - touch -r $ECONF_SOURCE/configure._portage_tmp_.$$ 
 $ECONF_SOURCE/configure || die
 - rm -f $ECONF_SOURCE/configure._portage_tmp_.$$
 + touch -r ${ECONF_SOURCE}/configure 
 ${ECONF_SOURCE}/configure._portage_tmp_.${pid} || die
 + sed -i \
 + -e 1s:^#![[:space:]]*/bin/sh:#!$CONFIG_SHELL: 
 \
 + ${ECONF_SOURCE}/configure \
 + || die Substition of shebang in 
 '${ECONF_SOURCE}/configure' failed
 + touch -r 
 ${ECONF_SOURCE}/configure._portage_tmp_.${pid} ${ECONF_SOURCE}/configure 
 || die
 + rm -f ${ECONF_SOURCE}/configure._portage_tmp_.${pid}
   fi
   if [ -e ${EPREFIX}/usr/share/gnuconfig/ ]; then
   find ${WORKDIR} -type f '(' \
   -name config.guess -o -name config.sub ')' -print0 | \
   while read -r -d $'\0' x ; do
   __vecho  * econf: updating ${x/${WORKDIR}\/} 
 with ${EPREFIX}/usr/share/gnuconfig/${x##*/}
 - cp -f 
 ${EPREFIX}/usr/share/gnuconfig/${x##*/} ${x}
 + # Make sure we do this atomically incase we're 
 run in parallel. #487478
 + cp -f 
 ${EPREFIX}/usr/share/gnuconfig/${x##*/} ${x}.${pid}
 + mv -f ${x}.${pid} ${x}
   done
   fi
  


Sorry, my bash skills are not enough to review this stuff. Others will
have to reply :)
-- 
Brian Dolbec dol...@gentoo.org


signature.asc
Description: This is a digitally signed message part

Re: [gentoo-portage-dev] [PATCH] econf: update configure/config.{sub,guess} atomically to avoid races

2013-12-17 Thread Alec Warner 
LGTM


On Tue, Dec 17, 2013 at 4:26 PM, Brian Dolbec dol...@gentoo.org wrote:

 On Tue, 2013-12-17 at 18:28 -0500, Mike Frysinger wrote:
  Use $BASHPID which will be unique even in subshells.
 
  URL: https://bugs.gentoo.org/487478
  ---
   bin/phase-helpers.sh | 17 +++--
   1 file changed, 11 insertions(+), 6 deletions(-)
 
  diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
  index ec48c94..1a7ae03 100644
  --- a/bin/phase-helpers.sh
  +++ b/bin/phase-helpers.sh
  @@ -469,6 +469,7 @@ unpack() {
 
   econf() {
local x
  + local pid=${BASHPID}
 
if ! ___eapi_has_prefix_variables; then
local EPREFIX=
  @@ -501,18 +502,22 @@ econf() {
if [[ -n $CONFIG_SHELL  \
$(head -n1 $ECONF_SOURCE/configure) =~
 ^'#!'[[:space:]]*/bin/sh([[:space:]]|$) ]] ; then
# preserve timestamp, see bug #440304
  - touch -r $ECONF_SOURCE/configure
 $ECONF_SOURCE/configure._portage_tmp_.$$ || die
  - sed -e
 1s:^#![[:space:]]*/bin/sh:#!$CONFIG_SHELL: -i $ECONF_SOURCE/configure
 || \
  - die Substition of shebang in
 '$ECONF_SOURCE/configure' failed
  - touch -r
 $ECONF_SOURCE/configure._portage_tmp_.$$ $ECONF_SOURCE/configure || die
  - rm -f $ECONF_SOURCE/configure._portage_tmp_.$$
  + touch -r ${ECONF_SOURCE}/configure
 ${ECONF_SOURCE}/configure._portage_tmp_.${pid} || die
  + sed -i \
  + -e
 1s:^#![[:space:]]*/bin/sh:#!$CONFIG_SHELL: \
  + ${ECONF_SOURCE}/configure \
  + || die Substition of shebang in
 '${ECONF_SOURCE}/configure' failed
  + touch -r
 ${ECONF_SOURCE}/configure._portage_tmp_.${pid}
 ${ECONF_SOURCE}/configure || die
  + rm -f
 ${ECONF_SOURCE}/configure._portage_tmp_.${pid}
fi
if [ -e ${EPREFIX}/usr/share/gnuconfig/ ]; then
find ${WORKDIR} -type f '(' \
-name config.guess -o -name config.sub ')' -print0
 | \
while read -r -d $'\0' x ; do
__vecho  * econf: updating
 ${x/${WORKDIR}\/} with ${EPREFIX}/usr/share/gnuconfig/${x##*/}
  - cp -f
 ${EPREFIX}/usr/share/gnuconfig/${x##*/} ${x}
  + # Make sure we do this atomically incase
 we're run in parallel. #487478
  + cp -f
 ${EPREFIX}/usr/share/gnuconfig/${x##*/} ${x}.${pid}
  + mv -f ${x}.${pid} ${x}
done
fi
 


 Sorry, my bash skills are not enough to review this stuff. Others will
 have to reply :)
 --
 Brian Dolbec dol...@gentoo.org

Re: [gentoo-portage-dev] [PATCH] econf: update configure/config.{sub,guess} atomically to avoid races

2013-12-17 Thread Greg Turner 
On Tue, Dec 17, 2013 at 3:28 PM, Mike Frysinger vap...@gentoo.org wrote:
 +   sed -i \
 +   -e 
 1s:^#![[:space:]]*/bin/sh:#!$CONFIG_SHELL: \
 +   ${ECONF_SOURCE}/configure \
 +   || die Substition of shebang in 
 '${ECONF_SOURCE}/configure' failed

Shouldn't we take the same copy, ${pid}-suffix, move approach, here
that we've done with config.{sub,guess}?  Otherwise, what's to stop
these sed's from trampling each other's work-in-progress (the fact
that ebuilds don't crash left and right does suggest that some
mechanism actually does prevent this, already -- but it's a mystery to
me, and I worry it could be some kind of linux-specific filesystem
quirk).

-gmt

Re: [gentoo-portage-dev] [PATCH] econf: update configure/config.{sub,guess} atomically to avoid races

2013-12-17 Thread Alec Warner 
On Tue, Dec 17, 2013 at 5:41 PM, Greg Turner g...@malth.us wrote:

 On Tue, Dec 17, 2013 at 3:28 PM, Mike Frysinger vap...@gentoo.org wrote:
  +   sed -i \
  +   -e
 1s:^#![[:space:]]*/bin/sh:#!$CONFIG_SHELL: \
  +   ${ECONF_SOURCE}/configure \
  +   || die Substition of shebang in
 '${ECONF_SOURCE}/configure' failed

 Shouldn't we take the same copy, ${pid}-suffix, move approach, here
 that we've done with config.{sub,guess}?  Otherwise, what's to stop
 these sed's from trampling each other's work-in-progress (the fact
 that ebuilds don't crash left and right does suggest that some
 mechanism actually does prevent this, already -- but it's a mystery to
 me, and I worry it could be some kind of linux-specific filesystem
 quirk).


Sed is already atomic

antarus@goats5 /tmp/test $ cat foo
Debian Rocks!
antarus@goats5 /tmp/test $ strace -e trace=file sed -i -e
's/Debian/Gentoo/g' foo
execve(/bin/sed, [sed, -i, -e, s/Debian/Gentoo/g, foo], [/* 39
vars */]) = 0
access(/etc/ld.so.nohwcap, F_OK)  = -1 ENOENT (No such file or
directory)
access(/etc/ld.so.preload, R_OK)  = -1 ENOENT (No such file or
directory)
open(/etc/ld.so.cache, O_RDONLY|O_CLOEXEC) = 3
access(/etc/ld.so.nohwcap, F_OK)  = -1 ENOENT (No such file or
directory)
open(/lib/x86_64-linux-gnu/libselinux.so.1, O_RDONLY|O_CLOEXEC) = 3
access(/etc/ld.so.nohwcap, F_OK)  = -1 ENOENT (No such file or
directory)
open(/lib/x86_64-linux-gnu/libc.so.6, O_RDONLY|O_CLOEXEC) = 3
access(/etc/ld.so.nohwcap, F_OK)  = -1 ENOENT (No such file or
directory)
open(/lib/x86_64-linux-gnu/libdl.so.2, O_RDONLY|O_CLOEXEC) = 3
statfs(/selinux, {f_type=EXT2_SUPER_MAGIC, f_bsize=4096,
f_blocks=5419717, f_bfree=920598, f_bavail=645285, f_files=1379040,
f_ffree=885151, f_fsid={-495840576, 2082046975}, f_namelen=255,
f_frsize=4096}) = 0
open(/proc/filesystems, O_RDONLY) = 3
open(/usr/lib/locale/locale-archive, O_RDONLY|O_CLOEXEC) = 3
open(//lib/charset.alias, O_RDONLY)   = -1 ENOENT (No such file or
directory)
open(/usr/lib/x86_64-linux-gnu/gconv/gconv-modules.cache, O_RDONLY) = 3
open(foo, O_RDONLY)   = 3
open(/proc/filesystems, O_RDONLY) = 4
open(./sedfDxBxl, O_RDWR|O_CREAT|O_EXCL, 0600) = 4
rename(./sedfDxBxl, foo)= 0

-A


 -gmt

Re: [gentoo-portage-dev] [PATCH] econf: update configure/config.{sub,guess} atomically to avoid races

2013-12-17 Thread Alec Warner 
On Tue, Dec 17, 2013 at 6:53 PM, Greg Turner g...@malth.us wrote:

 On Tue, Dec 17, 2013 at 5:58 PM, Alec Warner anta...@gentoo.org wrote:
  Sed is already atomic
 
  antarus@goats5 /tmp/test $ cat foo
  Debian Rocks!
  antarus@goats5 /tmp/test $ strace -e trace=file sed -i -e
  's/Debian/Gentoo/g' foo
 [snip]
  open(foo, O_RDONLY)   = 3
  open(/proc/filesystems, O_RDONLY) = 4
  open(./sedfDxBxl, O_RDWR|O_CREAT|O_EXCL, 0600) = 4
  rename(./sedfDxBxl, foo)= 0

 Nice demonstration!  Been wondering about that...


I originally opened the source code and tried to find the actual rename
code...but gave up after about 2 minutes figuring strace would be quicker.
I was right ;)

-A



 -gmt