[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`

[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`

[Mike Frysinger]

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 tools/Makefile |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/tools/Makefile b/tools/Makefile
index af0de47..22d9dae 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -203,19 +203,16 @@ else
 endif
 
 $(obj)environment.c:
-		@rm -f $(obj)environment.c
-		ln -s $(src)../common/environment.c $(obj)environment.c
+		ln -s -f $(src)../common/environment.c $(obj)environment.c
 
 $(obj)environment.o:	$(obj)environment.c
 		$(CC) -g $(HOST_ENVIRO_CFLAGS) $(CPPFLAGS) -c -o $@ $<
 
 $(obj)crc32.c:
-		@rm -f $(obj)crc32.c
-		ln -s $(src)../lib_generic/crc32.c $(obj)crc32.c
+		ln -s -f $(src)../lib_generic/crc32.c $(obj)crc32.c
 
 $(obj)sha1.c:
-		@rm -f $(obj)sha1.c
-		ln -s $(src)../lib_generic/sha1.c $(obj)sha1.c
+		ln -s -f $(src)../lib_generic/sha1.c $(obj)sha1.c
 
 $(LOGO_H):	$(obj)bmp_logo $(LOGO_BMP)
 		$(obj)./bmp_logo $(LOGO_BMP) >$@
-- 
1.5.3.8

[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`

[Mike Frysinger]

odd ... git-send-email ate the explanatory text ...
---
The -f option to `ln` should give the same behavior as the -f option to the
`rm` command.  It is better to do this in one shot so as to avoid race
conditions when building in parallel.  I build on a quad G5 and without this
change, it isn't uncommon for the build to fail when using -j8 due to this
small window where the files don't actually exist.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 tools/Makefile |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/tools/Makefile b/tools/Makefile
index af0de47..22d9dae 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -203,19 +203,16 @@ else
 endif
 
 $(obj)environment.c:
-		@rm -f $(obj)environment.c
-		ln -s $(src)../common/environment.c $(obj)environment.c
+		ln -s -f $(src)../common/environment.c $(obj)environment.c
 
 $(obj)environment.o:	$(obj)environment.c
 		$(CC) -g $(HOST_ENVIRO_CFLAGS) $(CPPFLAGS) -c -o $@ $<
 
 $(obj)crc32.c:
-		@rm -f $(obj)crc32.c
-		ln -s $(src)../lib_generic/crc32.c $(obj)crc32.c
+		ln -s -f $(src)../lib_generic/crc32.c $(obj)crc32.c
 
 $(obj)sha1.c:
-		@rm -f $(obj)sha1.c
-		ln -s $(src)../lib_generic/sha1.c $(obj)sha1.c
+		ln -s -f $(src)../lib_generic/sha1.c $(obj)sha1.c
 
 $(LOGO_H):	$(obj)bmp_logo $(LOGO_BMP)
 		$(obj)./bmp_logo $(LOGO_BMP) >$@
-- 
1.5.3.8

[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`

[Wolfgang Denk]

In message <1201520254-25593-1-git-send-email-vapier@gentoo.org> you wrote:
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  tools/Makefile |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index af0de47..22d9dae 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -203,19 +203,16 @@ else
>  endif
>  
>  $(obj)environment.c:
> -		@rm -f $(obj)environment.c
> -		ln -s $(src)../common/environment.c $(obj)environment.c
> +		ln -s -f $(src)../common/environment.c $(obj)environment.c

Be careful here. Are you 100% sure that all systems in the field will
behave exactly as your's is doing?

What is the exact problem you are trying to solve? If it's not a bug,
I recomment to leave the code as is.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If you can't beat it or corrupt it, you pretend it was your  idea  in
the first place.                 - Terry Pratchett, _Guards! Guards!_

[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`

[Wolfgang Denk]

In message <200801280638.59012.vapier@gentoo.org> you wrote:
> odd ... git-send-email ate the explanatory text ...
> ---
> The -f option to `ln` should give the same behavior as the -f option to the
> `rm` command.  It is better to do this in one shot so as to avoid race
> conditions when building in parallel.  I build on a quad G5 and without this
> change, it isn't uncommon for the build to fail when using -j8 due to this
> small window where the files don't actually exist.

Note that "ln -s -f" will come down to two separate system calls as
well:

	...
	unlink();
	symlink();
	...

So you don't avoid the race condition; you're just making it a little
less likely at the cost of reduced portability.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Software suppliers are trying to make their  software  packages  more
``user-friendly''.  .  .  .  Their best approach, so far, has been to
take all the old brochures, and stamp the words, ``user-friendly'' on
the cover.                                               - Bill Gates

[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`

[Mike Frysinger]

On Monday 28 January 2008, Wolfgang Denk wrote:
> In message <1201520254-25593-1-git-send-email-vapier@gentoo.org> you wrote:
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > ---
> >  tools/Makefile |    9 +++------
> >  1 files changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/Makefile b/tools/Makefile
> > index af0de47..22d9dae 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -203,19 +203,16 @@ else
> >  endif
> >
> >  $(obj)environment.c:
> > -		@rm -f $(obj)environment.c
> > -		ln -s $(src)../common/environment.c $(obj)environment.c
> > +		ln -s -f $(src)../common/environment.c $(obj)environment.c
>
> Be careful here. Are you 100% sure that all systems in the field will
> behave exactly as your's is doing?

i really dont understand what you mean.  any POSIX compliant system will 
behave exactly as it's supposed to: if the destination exists already, unlink 
it just before creating the link.

> What is the exact problem you are trying to solve? If it's not a bug,
> I recomment to leave the code as is.

i wouldnt be posting a change if it werent causing me a problem.  as i said, 
the time between executing `rm` and `ln` is too long and can cause problems 
when building u-boot in parallel.  moving the unlink() from `rm` to `ln` just 
shortens this window drastically.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/d5583c8c/attachment.pgp 

[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`

[Mike Frysinger]

On Monday 28 January 2008, Wolfgang Denk wrote:
> In message <200801280638.59012.vapier@gentoo.org> you wrote:
> > odd ... git-send-email ate the explanatory text ...
> > ---
> > The -f option to `ln` should give the same behavior as the -f option to
> > the `rm` command.  It is better to do this in one shot so as to avoid
> > race conditions when building in parallel.  I build on a quad G5 and
> > without this change, it isn't uncommon for the build to fail when using
> > -j8 due to this small window where the files don't actually exist.
>
> Note that "ln -s -f" will come down to two separate system calls as
> well:
>
> 	...
> 	unlink();
> 	symlink();
> 	...
>
> So you don't avoid the race condition; you're just making it a little
> less likely at the cost of reduced portability.

yes, i know it's much less likely, but that difference on my system has been 
from 1-in-20 build failures to 1-in-none-so-far-out-of-hundreds.  the real 
fix is to overhaul the u-boot build system, but that'll take quite a bit of 
time and this change is for all practical purposes, Good Enough.  i dont know 
what portability issues you refer to considering the -f flag is in POSIX and 
has been supported on all Linux systems since before that (pre-2000).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/8f778de0/attachment.pgp 

[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`

[J. William Campbell]

Mike Frysinger wrote:
> On Monday 28 January 2008, Wolfgang Denk wrote:
>   
>> In message <200801280638.59012.vapier@gentoo.org> you wrote:
>>     
>>> odd ... git-send-email ate the explanatory text ...
>>> ---
>>> The -f option to `ln` should give the same behavior as the -f option to
>>> the `rm` command.  It is better to do this in one shot so as to avoid
>>> race conditions when building in parallel.  I build on a quad G5 and
>>> without this change, it isn't uncommon for the build to fail when using
>>> -j8 due to this small window where the files don't actually exist.
>>>       
>> Note that "ln -s -f" will come down to two separate system calls as
>> well:
>>
>> 	...
>> 	unlink();
>> 	symlink();
>> 	...
>>
>> So you don't avoid the race condition; you're just making it a little
>> less likely at the cost of reduced portability.
>>     
>
> yes, i know it's much less likely, but that difference on my system has been 
> from 1-in-20 build failures to 1-in-none-so-far-out-of-hundreds.  the real 
> fix is to overhaul the u-boot build system, but that'll take quite a bit of 
> time and this change is for all practical purposes, Good Enough.  i dont know 
> what portability issues you refer to considering the -f flag is in POSIX and 
> has been supported on all Linux systems since before that (pre-2000).
> -mike
>   
>   
I don't think there is a race condition at all in the "ln -s -f" case. 
The race condition exists in the first approach because the shell is 
allowed to execute the rm and the ln in parallel, while in the "ln -s 
-f" case the two system calls are sequential in the same thread of 
execution (inside ln) and therefore happen in a defined order. It is not 
just that the window is small, there is no window.
-Bill
> ------------------------------------------------------------------------
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2008.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> ------------------------------------------------------------------------
>
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>   

[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`

[Johannes Stezenbach]

On Mon, Jan 28, 2008 at 06:38:58AM -0500, Mike Frysinger wrote:
> The -f option to `ln` should give the same behavior as the -f option to the
> `rm` command.  It is better to do this in one shot so as to avoid race
> conditions when building in parallel.  I build on a quad G5 and without this
> change, it isn't uncommon for the build to fail when using -j8 due to this
> small window where the files don't actually exist.

Since there is a clear dependency of environment.o on environment.c
there is no way for make to run the rm/ln and $(CC) commands
in parallel, so there should be no race condition.
For illustrative purposes: you should be able to put a sleep
between the rm and the ln -s, and it should still build.

It seems the real problem is one level above, the commands
were run twice in parallel, once for "depend" and once
for the SUBDIRS "all". Please test the patch below.

The

  $(obj)u-boot:		depend $(SUBDIRS) $(OBJS) $(LIBS) $(LDSCRIPT)

line tells make it can run commands to update each dependency
in parallel, unless there are further dependencies which restrict this.


Thanks,
Johannes

---
Add dependencies to avoid race conditions with parallel make.

Signed-off-by: Johannes Stezenbach <js@sig21.net>

diff --git a/Makefile b/Makefile
index 0f6cc59..464db34 100644
--- a/Makefile
+++ b/Makefile
@@ -317,13 +317,13 @@ $(obj)u-boot:		depend $(SUBDIRS) $(OBJS) $(LIBS) $(LDSCRIPT)
 			--start-group $(__LIBS) --end-group $(PLATFORM_LIBS) \
 			-Map u-boot.map -o u-boot
 
-$(OBJS):	$(obj)include/autoconf.mk
+$(OBJS):	depend $(obj)include/autoconf.mk
 		$(MAKE) -C cpu/$(CPU) $(if $(REMOTE_BUILD),$@,$(notdir $@))
 
-$(LIBS):	$(obj)include/autoconf.mk
+$(LIBS):	depend $(obj)include/autoconf.mk
 		$(MAKE) -C $(dir $(subst $(obj),,$@))
 
-$(SUBDIRS):	$(obj)include/autoconf.mk
+$(SUBDIRS):	depend $(obj)include/autoconf.mk
 		$(MAKE) -C $@ all
 
 $(NAND_SPL):	$(VERSION_FILE)	$(obj)include/autoconf.mk

[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`

[Mike Frysinger]

On Monday 28 January 2008, Johannes Stezenbach wrote:
> On Mon, Jan 28, 2008 at 06:38:58AM -0500, Mike Frysinger wrote:
> > The -f option to `ln` should give the same behavior as the -f option to
> > the `rm` command.  It is better to do this in one shot so as to avoid
> > race conditions when building in parallel.  I build on a quad G5 and
> > without this change, it isn't uncommon for the build to fail when using
> > -j8 due to this small window where the files don't actually exist.
>
> Since there is a clear dependency of environment.o on environment.c
> there is no way for make to run the rm/ln and $(CC) commands
> in parallel, so there should be no race condition.
> For illustrative purposes: you should be able to put a sleep
> between the rm and the ln -s, and it should still build.

right, i know the issue isnt between the link and the compile, it was that the 
rm/ln was run multiple times.  but i never dug deeper into the issue to find 
the true cause of the problem.

> It seems the real problem is one level above, the commands
> were run twice in parallel, once for "depend" and once
> for the SUBDIRS "all". Please test the patch below.

thanks for poking deeper

> -$(OBJS):	$(obj)include/autoconf.mk
> +$(OBJS):	depend $(obj)include/autoconf.mk
>  		$(MAKE) -C cpu/$(CPU) $(if $(REMOTE_BUILD),$@,$(notdir $@))
>
> -$(LIBS):	$(obj)include/autoconf.mk
> +$(LIBS):	depend $(obj)include/autoconf.mk
>  		$(MAKE) -C $(dir $(subst $(obj),,$@))
>
> -$(SUBDIRS):	$(obj)include/autoconf.mk
> +$(SUBDIRS):	depend $(obj)include/autoconf.mk
>  		$(MAKE) -C $@ all

perhaps depend should get its own line ?
u-boot $(OBJS) $(LIBS) $(SUBDIRS): depend
or not, doesnt matter to me ... as long as it works :)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/1687ce1f/attachment.pgp 

[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`

[Wolfgang Denk]

In message <20080128231125.GA27063@sig21.net> you wrote:
> On Mon, Jan 28, 2008 at 06:38:58AM -0500, Mike Frysinger wrote:
> > The -f option to `ln` should give the same behavior as the -f option to the
> > `rm` command.  It is better to do this in one shot so as to avoid race
> > conditions when building in parallel.  I build on a quad G5 and without this
> > change, it isn't uncommon for the build to fail when using -j8 due to this
> > small window where the files don't actually exist.
> 
> Since there is a clear dependency of environment.o on environment.c
> there is no way for make to run the rm/ln and $(CC) commands
> in parallel, so there should be no race condition.
> For illustrative purposes: you should be able to put a sleep
> between the rm and the ln -s, and it should still build.
> 
> It seems the real problem is one level above, the commands
> were run twice in parallel, once for "depend" and once
> for the SUBDIRS "all". Please test the patch below.
> 
> The
> 
>   $(obj)u-boot:		depend $(SUBDIRS) $(OBJS) $(LIBS) $(LDSCRIPT)
> 
> line tells make it can run commands to update each dependency
> in parallel, unless there are further dependencies which restrict this.

Applied, thanks.

> 
> Thanks,
> Johannes
> 
> ---
> Add dependencies to avoid race conditions with parallel make.
> 
> Signed-off-by: Johannes Stezenbach <js@sig21.net>

Note that these 3 lines belong *above* the "---" line.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There are three ways to get something  done:  do  it  yourself,  hire
someone, or forbid your kids to do it.

[U-Boot-Users] [PATCH] Use `ln -sf` rather than `rm -f && ln -s`

[Wolfgang Denk]

In message <200801281557.55976.vapier@gentoo.org> you wrote:
>
> > > -		@rm -f $(obj)environment.c
> > > -		ln -s $(src)../common/environment.c $(obj)environment.c
> > > +		ln -s -f $(src)../common/environment.c $(obj)environment.c
> >
> > Be careful here. Are you 100% sure that all systems in the field will
> > behave exactly as your's is doing?
> 
> i really dont understand what you mean.  any POSIX compliant system will 
> behave exactly as it's supposed to: if the destination exists already, unlink 
> it just before creating the link.

I finally fund such a situation again.

# cat /proc/version 
Linux version 2.6.22.14-72.fc6 (brewbuilder at ls20-bc1-14.build.redhat.com) (gcc version 4.1.2 20070626 (Red Hat 4.1.2-13)) #1 SMP Wed Nov 21 14:10:25 EST 2007
# cat /etc/issue
Fedora Core release 6 (Zod)
Kernel \r on an \m

# id
uid=0(root) gid=0(root) groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel)
# cd /opt
# ls -ld eldk eldk-4.2-2008-03-27 eldk-4.2-2008-04-01
lrwxrwxrwx  1 root root   19 Apr  4 04:01 eldk -> eldk-4.2-2008-03-27
drwxr-xr-x 14 root root 4096 Apr  8 23:37 eldk-4.2-2008-03-27
drwxr-xr-x 14 root root 4096 Apr  3 13:38 eldk-4.2-2008-04-01


Now we try:

# ln -sf eldk-4.2-2008-04-01 eldk
# echo $?
0

And what do you think we got?

# ls -ld eldk eldk-4.2-2008-03-27 eldk-4.2-2008-04-01
lrwxrwxrwx  1 root root   19 Apr  4 04:01 eldk -> eldk-4.2-2008-03-27
drwxr-xr-x 14 root root 4096 Apr 14 21:27 eldk-4.2-2008-03-27
drwxr-xr-x 14 root root 4096 Apr  3 13:38 eldk-4.2-2008-04-01


This is what I mean when I say that "rm -f foo ; ln -s bar foo" is
*NOT* exactly equivalent to "ln -sf bar foo". The former will always
work, the latter won't.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Wagner's music is better than it sounds."               - Mark Twain