Re: [Qemu-devel] [PATCH v3 2/2] make: load only required dependency files.

[Michael S. Tsirkin]

On Sun, Aug 09, 2015 at 12:54:37PM +0100, Peter Maydell wrote:
> On 9 August 2015 at 12:39, Michael S. Tsirkin <address@hidden> wrote:
> > On Sun, Aug 09, 2015 at 12:39:59PM +0300, Victor Kaplansky wrote:
> >> -        $(eval -include $(addsuffix *.d, $(sort $(dir $($v)))))
> >> +        $(eval -include $(patsubst %.o,%.d,$(patsubst %.mo,%.d,$($v))))
> >>          $(eval $v := $(filter-out %/,$($v))))
> >>  endef
> >
> > Please add space after each comma.
> 
> (I thought we discussed this in v1 review?)
> 
> We don't seem to be completely consistent in our makefile
> style, but I would say that the majority of it is written
> in the same style used in the GNU Make docs, with no spaces
> after commas.
> 
> In particular, $(patsubst %.o,%.d,$(THING)) and
> $(patsubst %.o, %.d, $(THING)) do *not* expand to the same thing;
> Make syntax for functions says the delimiter is 'comma', not
> 'comma followed by whitespace':
> 
> FOO=bar.o baz.o boz.o
> 
> all:
>     @echo '$(patsubst %.o,%.d,$(FOO))'
>     @echo '$(patsubst %.o, %.d, $(FOO))'
> manooth$ make -f test.mak
> bar.d baz.d boz.d
>  bar.d  baz.d  boz.d
> 
> Admittedly most of the time the extra whitespace that ends
> up in the output is unlikely to be problematic, but there
> might be rare cases when it is, so the better style
> recommendation is to not have spaces after the commas,
> because that is always safe and always the behaviour
> the author actually wanted.
> 
> thanks
> -- PMM

OK, I'll merge this as is, we can tweak this with a patch on top if
desired.

Still - just corious about the motivation.
Extra whitespace in input -> extra whitespace in output, should
be harmless in both cases, should it not? E.g. do we prefer

    A\
B

or

        A \
        B
?

First option never adds whitespace, but second one is more readable.
I'd argue if you write code where extra whitespace breaks things you are
doing something wrong.


-- 
MST