Re: [ptxdist] [PATCH 2/2] barebox_mlo: use images/barebox-*-mlo.img as images

[Alexander Aring <alex.aring@gmail.com>]

On Wed, May 14, 2014 at 11:04:01AM +0200, Uwe Kleine-König wrote:
> On Wed, May 14, 2014 at 10:47:38AM +0200, Alexander Aring wrote:
> > On Wed, May 14, 2014 at 09:54:32AM +0200, Teresa Gámez wrote:
> > > New barebox versions may create multiple MLO images.
> > > Adapted barebox_mlo rule to handle this.
> > > 
> > > Signed-off-by: Teresa Gámez <t.gamez@phytec.de>
> > > ---
> > >  rules/barebox_mlo.make |   17 ++++++++++++++---
> > >  1 files changed, 14 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/rules/barebox_mlo.make b/rules/barebox_mlo.make
> > > index 3c53575..8b2bce4 100644
> > > --- a/rules/barebox_mlo.make
> > > +++ b/rules/barebox_mlo.make
> > > @@ -76,9 +76,20 @@ $(STATEDIR)/barebox_mlo.install:
> > >  $(STATEDIR)/barebox_mlo.targetinstall:
> > >  	@$(call targetinfo)
> > >  #	#barebox renamed barebox.bin.ift to MLO, so fall back to barebox.bin.ift
> > > -	@ptxd_get_path "$(BAREBOX_MLO_DIR)/MLO" \
> > > -		"$(BAREBOX_MLO_DIR)/barebox.bin.ift" && \
> > > -	install -D -m644 "$${ptxd_reply}" "$(IMAGEDIR)/MLO"
> > > +	@rm -f $(IMAGEDIR)/MLO
> > > +	@for image in `ls $(BAREBOX_MLO_DIR)/images/barebox-*-mlo.img`; do \
> > > +		install -D -m644 $$image $(IMAGEDIR)/`basename $$image`; \
> > > +		if [ ! -e $(IMAGEDIR)/MLO ]; then \
> > > +			ln -sf `basename $$image` $(IMAGEDIR)/MLO; \
> > > +		fi; \
> > > +	done
> > > +	@if [ -e $(IMAGEDIR)/MLO ]; then \
> > > +		:; \
> > > +	else \
> > > +		ptxd_get_path "$(BAREBOX_MLO_DIR)/MLO" \
> > > +			"$(BAREBOX_MLO_DIR)/barebox.bin.ift" && \
> > > +		install -D -m644 "$${ptxd_reply}" "$(IMAGEDIR)/MLO"; \
> > > +	fi
> > negative logic here, without explicit do nothing [if -e $(IMAGEDIR)/MLO] ?
> > Or we should call "exit 1" here? I don't know if ptxdist can handle this
> > as error or this case can ever happen.
> > 
> > And why ":;"?
> You need a command in the then branch:
> 
> $ sh -c 'if true; then
>    # nothing
> else
>    echo jippie
> fi'
> sh: 3: Syntax error: "else" unexpected
> 
> Using ":" instead of "# nothing" works fine.

ok.

> 
> An option is [ ! -e $(IMAGEDIR)/MLO ];.
> ISTR that
> 
> 	if ! $somecommand; then ...
> 
> isn't portable, but I think [ supports ! everywhere.
> 
> Apart from that, there are missing quotes, better write
> 
> 	if [ ! -e "$(IMAGEDIR)/MLO" ]; then \
> 
> but the surrounding code seems affected, too.

Yea, you mean the line:

if [ ! -e $(IMAGEDIR)/MLO ]; then \

I never saw a test implementation builtin or coreutils implementation
which doesn't support a "!". But you can never be sure...

- Alex

-- 
ptxdist mailing list
ptxdist@pengutronix.de