Review request 6981484: Update development launcher

Kelly O'Hair kelly.ohair at oracle.com
Wed Sep 22 09:13:24 PDT 2010


On Sep 22, 2010, at 5:48 AM, Christian Thalinger wrote:

> On Wed, 2010-09-22 at 05:38 -0700, Staffan Larsen wrote:
>> Here we go again: http://cr.openjdk.java.net/~sla/6981484/webrev.03/
>>
>> Eventually, I'll get there...
>
> Good.  -- Christian
>

Just sticking my nose in a little.  I think this syntax is not a great  
idea:
   84 $(LAUNCHER_SCRIPT): $(LAUNCHERDIR)/launcher.script
   85         $(QUIETLY) { \
   86         sed -e 's/@@LIBARCH@@/$(LIBARCH)/g' $< > $@; \
   87         chmod +x $@; \
   88         }

I think that if the sed command fails, the rule could still be  
successful if $@ exists.
Using subshells I usually encourage things like "(command1 &&  
command2)" so that command2
will not be run if command1 fails, and the subshell exits with a non- 
zero exit.
But I don't think you can use && with this shell {} syntax.

I would think this logic could be more predictable with something like:
   84 $(LAUNCHER_SCRIPT): $(LAUNCHERDIR)/launcher.script
   85         $(QUIETLY) $(RM) $@
   86         $(QUIETLY) sed -e 's/@@LIBARCH@@/$(LIBARCH)/g' $< > $@
   87         $(QUIETLY)chmod +x $@

Maybe even:
   84 $(LAUNCHER_SCRIPT): $(LAUNCHERDIR)/launcher.script
   85         $(QUIETLY) $(RM) $@
   86         $(QUIETLY) $(MKDIR) -p $(@D)
   87         $(QUIETLY) sed -e 's/@@LIBARCH@@/$(LIBARCH)/g' $< > $@
   88         $(QUIETLY) chmod +x $@




More information about the hotspot-dev mailing list