From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:36276)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1ZOPah-0005YV-OU
	for qemu-devel@nongnu.org; Sun, 09 Aug 2015 08:20:40 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1ZOPad-0001tM-Mr
	for qemu-devel@nongnu.org; Sun, 09 Aug 2015 08:20:39 -0400
Received: from mx1.redhat.com ([209.132.183.28]:40391)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1ZOPad-0001tG-BN
	for qemu-devel@nongnu.org; Sun, 09 Aug 2015 08:20:35 -0400
Date: Sun, 9 Aug 2015 15:20:30 +0300
From: "Michael S. Tsirkin" <mst@redhat.com>
Message-ID: <20150809151159-mutt-send-email-mst@redhat.com>
References: <1439113148-21937-1-git-send-email-victork@redhat.com>
	<1439113148-21937-3-git-send-email-victork@redhat.com>
	<20150809113923.GA3912@redhat.com>
	<CAFEAcA_JUZaW9ndA31bHemwV3=1Xf=jCJcqwLFtdRTVvVAh3+A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAFEAcA_JUZaW9ndA31bHemwV3=1Xf=jCJcqwLFtdRTVvVAh3+A@mail.gmail.com>
Subject: Re: [Qemu-devel] [PATCH v3 2/2] make: load only required dependency
	files.
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Victor Kaplansky <victork@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Stefan Weil <sw@weilnetz.de>, QEMU Developers <qemu-devel@nongnu.org>, Paolo Bonzini <pbonzini@redhat.com>, Alex =?iso-8859-1?Q?Benn=E9e?= <alex.bennee@linaro.org>, Richard Henderson <rth@twiddle.net>

On Sun, Aug 09, 2015 at 12:54:37PM +0100, Peter Maydell wrote:
> On 9 August 2015 at 12:39, Michael S. Tsirkin <mst@redhat.com> 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