Re: 2.6.9-rc4-mm1: initramfs build fix [u]

[Martin Schlemmer [c]]

On Sun, 2004-10-17 at 23:33 +0200, Sam Ravnborg wrote:

Hiya,

> 
> Can you submit a new version based on Linus' tree with the following modifications:

It depends on gen_init_cpio-uses-external-file-list.patch from -mm, so
cannot until that is merged.

> 1) Propoer changelog

Should be with the other thread I guess.

> 2) Document use of .shipped somewhere
> 

If I remember correctly, Andrew asked Thayne Harbaugh to add docs for
the list format, etc.  I will thus have to wait for those to do changes
brought by my patch.

> And the following small comments.
> 
> 
> >  # or set INITRAMFS_LIST to another filename.
> > -INITRAMFS_LIST ?= $(obj)/initramfs_list
> > +INITRAMFS_LIST := $(obj)/initramfs_list
> 
> Kbuild style is to reser all-uppercase to external visible variables.
> 

Ok, so we need that in smaller caps.

> >  # initramfs_data.o contains the initramfs_data.cpio.gz image.
> >  # The image is included using .incbin, a dependency which is not
> > @@ -23,6 +23,23 @@ $(obj)/initramfs_data.o: $(obj)/initramf
> >  # Commented out for now
> >  # initramfs-y := $(obj)/root/hello
> >  
> > +quiet_cmd_gen_list = GEN_INITRAMFS_LIST $@
> Please aling output properly with rest of kbuild output.
> > +quiet_cmd_gen_list = GEN     $@
> Should be enough - the filename give some context as well
> 

Right, thanks.  I did change it at some time (second only being GEN),
but I guess I missed the first.

> > +      cmd_gen_list = $(shell \
> > +        if test -f "$(CONFIG_INITRAMFS_SOURCE)"; then \
> > +	  if [ "$(CONFIG_INITRAMFS_SOURCE)" != $@ ]; then \
> > +	    echo 'cp -f "$(CONFIG_INITRAMFS_SOURCE)" $@'; \
> > +	  else \
> > +	    echo 'cp -f "$(srctree)/$(INITRAMFS_LIST).shipped" $@'; \
> Test for .shipped to be present first?
> 

This needed even though its supposed to be there?  How should one handle
the event that its not .. just 'exit 1' ?

> 
> > +	  fi; \
> > +	elif test -d "$(CONFIG_INITRAMFS_SOURCE)"; then \
> > +	  echo 'scripts/gen_initramfs_list.sh "$(CONFIG_INITRAMFS_SOURCE)" > $@'; \
> > +	else \
> > +	  echo 'cp -f "$(srctree)/$(INITRAMFS_LIST).shipped" $@'; \
> Same here.
> 
> > +	fi)
> > +
> > +$(INITRAMFS_LIST): FORCE
> > +	$(call cmd,gen_list)
> 
> How do you secure that the list gets updated when some of the above logic changes?

Currently it will error if one of the commands called fails - is there a
more preferred error handling method?

> Likewise avoid the list to be generated unles required.
> 

Simply checking mtime should do?


Thanks,

-- 
Martin Schlemmer

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