All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Kirill Smelkov <kirr@navytux.spb.ru>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] t/Makefile: Use $(sort ...) explicitly where needed
Date: Thu, 19 Jan 2012 23:14:18 -0800	[thread overview]
Message-ID: <7vbopyhmlx.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20120120063450.GA15371@mini.zxlink> (Kirill Smelkov's message of "Fri, 20 Jan 2012 10:34:50 +0400")

Kirill Smelkov <kirr@navytux.spb.ru> writes:

>> I do not necessarily buy your "so we HAVE TO, OR ELSE".
>> 
>> Even though I can understand "We can sort the list of tests _if_ we do not
>> want them executed in seemingly random order when running 'make -j1'", I
>> tend to think that *if* is a big one.  Aren't these tests designed not to
>> depend on each other anyway?
>
> Yes, they don't depend on each other, but what's the point in not
> sorting them? I usually watch test progress visually, and if tests are
> sorted, even with make -j4 they go more or less incrementally by their t
> number.
>
> On my netbook, adding $(sort ...) adds approximately 0.008s to make
> startup, so imho there is no performance penalty to adding that sort.

Heh, who said anything about performance?

I was pointing out that your justification "we HAVE TO" was wrong.

If you are doing this for perceived prettyness and not as a fix for any
correctness issue, I want to see the patch honestly described as such;
that's all.

By the way, if I recall correctly, $(sort) in GNU make not just sorts but
as a nice side effect removes duplicates. So if we used a(n fictional)
construct in our Makefile like this:

    T = $(wildcard *.sh a.*)

that might produce duplicates (i.e. "a.sh" might appear twice), which
might leave us two identical pathnames in $T and cause us trouble.  Even
if we do not have such a use currently, rewriting $(wildcard) like your
patch does using $(sort $(wildcard ...)) may be a good way to future-proof
our Makefile, and if you justify your patch that way, it would be a
possible correctness hardening, not just cosmetics, and phrasing it with
"HAVE TO" may be justifiable.

Care to try if $(wildcard *.sh a.*) give you duplicated output with newer
GNU make? I am lazy but am a bit curious ;-)

  reply	other threads:[~2012-01-20  7:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-19 20:17 [PATCH] t/Makefile: Use $(sort ...) explicitly where needed Kirill Smelkov
2012-01-19 22:01 ` Junio C Hamano
2012-01-20  6:34   ` Kirill Smelkov
2012-01-20  7:14     ` Junio C Hamano [this message]
2012-01-20  7:19       ` Kirill Smelkov
2012-01-22 19:17         ` Kirill Smelkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vbopyhmlx.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=kirr@navytux.spb.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.