git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: William Chargin <wchargin@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust
Date: Sat, 4 Aug 2018 23:23:16 -0700	[thread overview]
Message-ID: <20180805062316.GB44140@aiede.svl.corp.google.com> (raw)
In-Reply-To: <xmqqftzts63d.fsf@gitster-ct.c.googlers.com>

Hi,

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> but $'' is too recent of a shell feature to count on (e.g., dash doesn't
>> support it).  See t/t3600-rm.sh for an example of a portable way to do
>
> Is that "too recent"?  I thought it was bashism, not even in POSIX,
> but I may be mistaken.

You're right.  I got a little ahead of myself: it's not part of POSIX
yet but is likely to be so once the details get ironed out:
http://austingroupbugs.net/view.php?id=249

> Quite honestly, our tests are still run inside a sort-of controlled
> environment, so if it _requires_ use of things we have avoided so
> far, like "ls -A" and "xargs -0", in order to be resistant to
> funnyly-named files like dot-LF-dot, I would say it is not worth
> worrying about them--instead we can simply refrain from using such a
> pathological name, can't we?

The "xargs -0" is a bit of a red herring.  That construct is
definitely not needed for the test it was used in.

For "ls -A", I agree with you that the benefit is not very high, so
the cost would have to be pretty low for this to be worth it.  But
given the lineage of "ls -A", I feel there's a chance that it's
widespread enough that it would meet that bar.

> "ls -A" may be in POSIX, but our attitude generally is to avoid
> saying things like "it is in POSIX so it's your platform's fault
> that it is not yet supported".  We instead say "even it may be in
> POSIX, in real life many people don't have it, so let's avoid it".
> And "xargs -0" I do not think is.

Indeed.

Thanks,
Jonathan

  reply	other threads:[~2018-08-05  6:34 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-05  2:20 [PATCH 0/1] t/test-lib: make `test_dir_is_empty` more robust William Chargin
2018-08-05  2:20 ` [PATCH 1/1] " William Chargin
2018-08-05  4:19   ` Jonathan Nieder
2018-08-05  5:23     ` Eric Sunshine
2018-08-05 20:52       ` Jeff King
2018-08-06 13:02         ` Jeff King
2018-08-06 17:52           ` Eric Sunshine
2018-08-12  4:06             ` [PATCH v3] test_dir_is_empty: properly detect files with newline in name William Chargin
2018-08-12  6:17               ` Eric Sunshine
2018-08-12  6:32                 ` William Chargin
2018-08-12  6:44                   ` Eric Sunshine
2018-09-12 18:35                     ` [PATCH v4] test_dir_is_empty: fix edge cases with newlines and hyphens William Chargin
2018-09-12 19:50                       ` Junio C Hamano
2018-09-12 18:37                     ` William Chargin
2018-08-12  4:06             ` [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust William Chargin
2018-08-05  5:24     ` William Chargin
2018-08-05  6:34       ` Jonathan Nieder
2018-08-05  6:03     ` Junio C Hamano
2018-08-05  6:23       ` Jonathan Nieder [this message]
2018-08-05  3:36 ` [PATCH 0/1] " Jonathan Nieder
2018-08-05  4:19   ` William Chargin
2018-08-05  4:20   ` [PATCH v2] " William Chargin
2018-08-05  8:34     ` Johannes Sixt

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=20180805062316.GB44140@aiede.svl.corp.google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=wchargin@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).