[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 2/2] make: load only required dependency file
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v3 2/2] make: load only required dependency files. |
Date: |
Sun, 9 Aug 2015 15:20:30 +0300 |
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
Re: [Qemu-devel] [PATCH v3 0/2] make: Cleanup and fix of loading of dependency info, Michael S. Tsirkin, 2015/08/09