[Qemu-trivial] [PATCH] configure: Fix creation of symbolic links for MinGW toolchain

[Qemu-trivial] [PATCH] configure: Fix creation of symbolic links for MinGW toolchain

[Stefan Weil]

The MinGW toolchain on w32/w64 hosts does not create symbolic links,
but implements 'ln -s' similar to 'cp -r'.

In incremental out of tree builds, this resulted in files which
were not updated when their counterparts in the QEMU source tree
changed. Especially for Makefile* this happened very often.

With this patch, the 'symlinked' files are now always updated for
out of tree builds. Similar code was already used for the symbolic
link of libcacard/Makefile.

The symlink macro always removes the target before it is created
again, therefore the rm command for libcacard/Makefile was redundant
and is removed now.

Macro symlink is also used with directories. To remove them on w32
hosts, a recursive rm is needed.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 configure |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index afe7395..acf63a9 100755
--- a/configure
+++ b/configure
@@ -41,7 +41,7 @@ compile_prog() {
 
 # symbolically link $1 to $2.  Portable version of "ln -sf".
 symlink() {
-  rm -f $2
+  rm -rf $2
   ln -s $1 $2
 }
 
@@ -3883,7 +3883,7 @@ do
 done
 mkdir -p $DIRS
 for f in $FILES ; do
-    if [ -e "$source_path/$f" ] && ! [ -e "$f" ]; then
+    if [ -e "$source_path/$f" ] && [ "$source_path" != `pwd` ]; then
         symlink "$source_path/$f" "$f"
     fi
 done
@@ -3914,7 +3914,6 @@ done
 if [ "$source_path" != `pwd` ]; then
     # out of tree build
     mkdir -p libcacard
-    rm -f libcacard/Makefile
     symlink "$source_path/libcacard/Makefile" libcacard/Makefile
 fi
 
-- 
1.7.9

Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Fix creation of symbolic links for MinGW toolchain

[Peter Maydell]

On 17 March 2012 15:31, Stefan Weil <sw@weilnetz.de> wrote:
> Macro symlink is also used with directories. To remove them on w32
> hosts, a recursive rm is needed.

Where do we symlink directories? (I exclude the setting up
of the linux headers because that will only happen on Linux
hosts where we know we have working symlinks.)

-- PMM

Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Fix creation of symbolic links for MinGW toolchain

[Stefan Weil]

Am 17.03.2012 16:49, schrieb Peter Maydell:
> On 17 March 2012 15:31, Stefan Weil<sw@weilnetz.de>  wrote:
>> Macro symlink is also used with directories. To remove them on w32
>> hosts, a recursive rm is needed.
> Where do we symlink directories? (I exclude the setting up
> of the linux headers because that will only happen on Linux
> hosts where we know we have working symlinks.)
>
> -- PMM

pc-bios/keymaps

Cheers,
Stefan W.

Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Fix creation of symbolic links for MinGW toolchain

[Peter Maydell]

On 17 March 2012 15:59, Stefan Weil <sw@weilnetz.de> wrote:
> Am 17.03.2012 16:49, schrieb Peter Maydell:
>> Where do we symlink directories? (I exclude the setting up
>> of the linux headers because that will only happen on Linux
>> hosts where we know we have working symlinks.)

> pc-bios/keymaps

Oh yes, missed that one (since it's a directory name cunningly
hidden in a variable named FILES :-)).

I'm just a little wary of 'rm -rf', especially here where
we're nuking whatever was passed as an argument and we
aren't getting the shell quoting right. In theory we could
use a wildcard for the keymaps the same way we do for a
bunch of the other pc-bios files; I'm not sure if it's worth
the effort, though.

-- PMM

Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Fix creation of symbolic links for MinGW toolchain

[Stefan Hajnoczi]

On Sat, Mar 17, 2012 at 05:24:08PM +0000, Peter Maydell wrote:
> On 17 March 2012 15:59, Stefan Weil <sw@weilnetz.de> wrote:
> > Am 17.03.2012 16:49, schrieb Peter Maydell:
> >> Where do we symlink directories? (I exclude the setting up
> >> of the linux headers because that will only happen on Linux
> >> hosts where we know we have working symlinks.)
> 
> > pc-bios/keymaps
> 
> Oh yes, missed that one (since it's a directory name cunningly
> hidden in a variable named FILES :-)).
> 
> I'm just a little wary of 'rm -rf', especially here where
> we're nuking whatever was passed as an argument and we
> aren't getting the shell quoting right. In theory we could
> use a wildcard for the keymaps the same way we do for a
> bunch of the other pc-bios files; I'm not sure if it's worth
> the effort, though.

Yes, I'm not comfortable with the 'rm -rf' either.  Please use "$2" so
that the path is at least quoted.

Stefan

[Qemu-trivial] [PATCH v2] configure: Fix creation of symbolic links for MinGW toolchain

[Stefan Weil]

The MinGW toolchain on w32/w64 hosts does not create symbolic links,
but implements 'ln -s' similar to 'cp -r'.

In incremental out of tree builds, this resulted in files which
were not updated when their counterparts in the QEMU source tree
changed. Especially for Makefile* this happened very often.

With this patch, the 'symlinked' files are now always updated for
out of tree builds. Similar code was already used for the symbolic
link of libcacard/Makefile.

The symlink macro always removes the target before it is created
again, therefore the rm command for libcacard/Makefile was redundant
and is removed now.

Macro symlink is also used with directories. To remove them on w32
hosts, a recursive rm is needed.

v2:
Quote arguments in shell function symlink, and also quote any argument
which is passed to symlink and which contains macros. This should reduce
the chance of accidents caused by rm -rf.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 configure |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/configure b/configure
index afe7395..53a8174 100755
--- a/configure
+++ b/configure
@@ -41,8 +41,8 @@ compile_prog() {
 
 # symbolically link $1 to $2.  Portable version of "ln -sf".
 symlink() {
-  rm -f $2
-  ln -s $1 $2
+  rm -rf "$2"
+  ln -s "$1" "$2"
 }
 
 # check whether a command is available to this shell (may be either an
@@ -3373,7 +3373,7 @@ fi
 
 for d in libdis libdis-user; do
     mkdir -p $d
-    symlink $source_path/Makefile.dis $d/Makefile
+    symlink "$source_path/Makefile.dis" "$d/Makefile"
     echo > $d/config.mak
 done
 
@@ -3382,13 +3382,13 @@ if test "$linux" = "yes" ; then
   mkdir -p linux-headers
   case "$cpu" in
   i386|x86_64)
-    symlink $source_path/linux-headers/asm-x86 linux-headers/asm
+    symlink "$source_path/linux-headers/asm-x86" linux-headers/asm
     ;;
   ppcemb|ppc|ppc64)
-    symlink $source_path/linux-headers/asm-powerpc linux-headers/asm
+    symlink "$source_path/linux-headers/asm-powerpc" linux-headers/asm
     ;;
   s390x)
-    symlink $source_path/linux-headers/asm-s390 linux-headers/asm
+    symlink "$source_path/linux-headers/asm-s390" linux-headers/asm
     ;;
   esac
 fi
@@ -3453,7 +3453,7 @@ mkdir -p $target_dir/kvm
 if test "$target" = "arm-linux-user" -o "$target" = "armeb-linux-user" -o "$target" = "arm-bsd-user" -o "$target" = "armeb-bsd-user" ; then
   mkdir -p $target_dir/nwfpe
 fi
-symlink $source_path/Makefile.target $target_dir/Makefile
+symlink "$source_path/Makefile.target" "$target_dir/Makefile"
 
 
 echo "# Automatically generated by configure - do not modify" > $config_target_mak
@@ -3883,7 +3883,7 @@ do
 done
 mkdir -p $DIRS
 for f in $FILES ; do
-    if [ -e "$source_path/$f" ] && ! [ -e "$f" ]; then
+    if [ -e "$source_path/$f" ] && [ "$source_path" != `pwd` ]; then
         symlink "$source_path/$f" "$f"
     fi
 done
@@ -3906,7 +3906,7 @@ for hwlib in 32 64; do
   mkdir -p $d
   mkdir -p $d/ide
   mkdir -p $d/usb
-  symlink $source_path/Makefile.hw $d/Makefile
+  symlink "$source_path/Makefile.hw" "$d/Makefile"
   mkdir -p $d/9pfs
   echo "QEMU_CFLAGS+=-DTARGET_PHYS_ADDR_BITS=$hwlib" > $d/config.mak
 done
@@ -3914,7 +3914,6 @@ done
 if [ "$source_path" != `pwd` ]; then
     # out of tree build
     mkdir -p libcacard
-    rm -f libcacard/Makefile
     symlink "$source_path/libcacard/Makefile" libcacard/Makefile
 fi
 
@@ -3922,7 +3921,7 @@ d=libuser
 mkdir -p $d
 mkdir -p $d/trace
 mkdir -p $d/qom
-symlink $source_path/Makefile.user $d/Makefile
+symlink "$source_path/Makefile.user" "$d/Makefile"
 
 if test "$docs" = "yes" ; then
   mkdir -p QMP
-- 
1.7.9

Re: [Qemu-trivial] [PATCH v2] configure: Fix creation of symbolic links for MinGW toolchain

[Stefan Weil]

Am 19.03.2012 13:20, schrieb Stefan Weil:
> The MinGW toolchain on w32/w64 hosts does not create symbolic links,
> but implements 'ln -s' similar to 'cp -r'.
>
> In incremental out of tree builds, this resulted in files which
> were not updated when their counterparts in the QEMU source tree
> changed. Especially for Makefile* this happened very often.
>
> With this patch, the 'symlinked' files are now always updated for
> out of tree builds. Similar code was already used for the symbolic
> link of libcacard/Makefile.
>
> The symlink macro always removes the target before it is created
> again, therefore the rm command for libcacard/Makefile was redundant
> and is removed now.
>
> Macro symlink is also used with directories. To remove them on w32
> hosts, a recursive rm is needed.
>
> v2:
> Quote arguments in shell function symlink, and also quote any argument
> which is passed to symlink and which contains macros. This should reduce
> the chance of accidents caused by rm -rf.
>
> Signed-off-by: Stefan Weil<sw@weilnetz.de>
> ---
>   configure |   21 ++++++++++-----------
>   1 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/configure b/configure
> index afe7395..53a8174 100755
> --- a/configure
> +++ b/configure
> @@ -41,8 +41,8 @@ compile_prog() {
>
>   # symbolically link $1 to $2.  Portable version of "ln -sf".
>   symlink() {
> -  rm -f $2
> -  ln -s $1 $2
> +  rm -rf "$2"
> +  ln -s "$1" "$2"
>   }
>
>   # check whether a command is available to this shell (may be either an
> @@ -3373,7 +3373,7 @@ fi
>
>   for d in libdis libdis-user; do
>       mkdir -p $d
> -    symlink $source_path/Makefile.dis $d/Makefile
> +    symlink "$source_path/Makefile.dis" "$d/Makefile"
>       echo>  $d/config.mak
>   done
>
> @@ -3382,13 +3382,13 @@ if test "$linux" = "yes" ; then
>     mkdir -p linux-headers
>     case "$cpu" in
>     i386|x86_64)
> -    symlink $source_path/linux-headers/asm-x86 linux-headers/asm
> +    symlink "$source_path/linux-headers/asm-x86" linux-headers/asm
>       ;;
>     ppcemb|ppc|ppc64)
> -    symlink $source_path/linux-headers/asm-powerpc linux-headers/asm
> +    symlink "$source_path/linux-headers/asm-powerpc" linux-headers/asm
>       ;;
>     s390x)
> -    symlink $source_path/linux-headers/asm-s390 linux-headers/asm
> +    symlink "$source_path/linux-headers/asm-s390" linux-headers/asm
>       ;;
>     esac
>   fi
> @@ -3453,7 +3453,7 @@ mkdir -p $target_dir/kvm
>   if test "$target" = "arm-linux-user" -o "$target" = "armeb-linux-user" -o "$target" = "arm-bsd-user" -o "$target" = "armeb-bsd-user" ; then
>     mkdir -p $target_dir/nwfpe
>   fi
> -symlink $source_path/Makefile.target $target_dir/Makefile
> +symlink "$source_path/Makefile.target" "$target_dir/Makefile"
>
>
>   echo "# Automatically generated by configure - do not modify">  $config_target_mak
> @@ -3883,7 +3883,7 @@ do
>   done
>   mkdir -p $DIRS
>   for f in $FILES ; do
> -    if [ -e "$source_path/$f" ]&&  ! [ -e "$f" ]; then
> +    if [ -e "$source_path/$f" ]&&  [ "$source_path" != `pwd` ]; then
>           symlink "$source_path/$f" "$f"
>       fi
>   done
> @@ -3906,7 +3906,7 @@ for hwlib in 32 64; do
>     mkdir -p $d
>     mkdir -p $d/ide
>     mkdir -p $d/usb
> -  symlink $source_path/Makefile.hw $d/Makefile
> +  symlink "$source_path/Makefile.hw" "$d/Makefile"
>     mkdir -p $d/9pfs
>     echo "QEMU_CFLAGS+=-DTARGET_PHYS_ADDR_BITS=$hwlib">  $d/config.mak
>   done
> @@ -3914,7 +3914,6 @@ done
>   if [ "$source_path" != `pwd` ]; then
>       # out of tree build
>       mkdir -p libcacard
> -    rm -f libcacard/Makefile
>       symlink "$source_path/libcacard/Makefile" libcacard/Makefile
>   fi
>
> @@ -3922,7 +3921,7 @@ d=libuser
>   mkdir -p $d
>   mkdir -p $d/trace
>   mkdir -p $d/qom
> -symlink $source_path/Makefile.user $d/Makefile
> +symlink "$source_path/Makefile.user" "$d/Makefile"
>
>   if test "$docs" = "yes" ; then
>     mkdir -p QMP
>    

Should I send a pull request for this patch, or can it be applied
through qemu-devel?

Regards,
Stefan W.

Re: [Qemu-trivial] [Qemu-devel] [PATCH v2] configure: Fix creation of symbolic links for MinGW toolchain

[Stefan Hajnoczi]

On Mon, Mar 19, 2012 at 01:20:47PM +0100, Stefan Weil wrote:
> The MinGW toolchain on w32/w64 hosts does not create symbolic links,
> but implements 'ln -s' similar to 'cp -r'.
> 
> In incremental out of tree builds, this resulted in files which
> were not updated when their counterparts in the QEMU source tree
> changed. Especially for Makefile* this happened very often.
> 
> With this patch, the 'symlinked' files are now always updated for
> out of tree builds. Similar code was already used for the symbolic
> link of libcacard/Makefile.
> 
> The symlink macro always removes the target before it is created
> again, therefore the rm command for libcacard/Makefile was redundant
> and is removed now.
> 
> Macro symlink is also used with directories. To remove them on w32
> hosts, a recursive rm is needed.
> 
> v2:
> Quote arguments in shell function symlink, and also quote any argument
> which is passed to symlink and which contains macros. This should reduce
> the chance of accidents caused by rm -rf.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  configure |   21 ++++++++++-----------
>  1 files changed, 10 insertions(+), 11 deletions(-)

Sorry, this got lost in my inbox.  If you send patch revisions as
top-level emails then it's easier to notice them.  I probably marked it
as read and forgot about it because being part of the original email
thread made it appear like the discussion was still continuing.

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan