All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <bonzini@gnu.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Eric Blake <ebb9@byu.net>,
	git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>
Subject: Re: [PATCH 01/18] rebase -i: Make the condition for an   &quot;if&quot; more transparent
Date: Fri, 15 Jan 2010 08:20:27 +0100	[thread overview]
Message-ID: <4B50173B.5030802@gnu.org> (raw)
In-Reply-To: <7v8wc0fuqw.fsf@alter.siamese.dyndns.org>

On 01/14/2010 06:42 PM, Junio C Hamano wrote:
> Eric Blake<ebb9@byu.net>  writes:
>
>> Michael Haggerty<mhagger<at>  alum.mit.edu>  writes:
>>
>>>   	current_sha1=$(git rev-parse --verify HEAD)
>>> -	if test "$no_ff$current_sha1" = "$parent_sha1"; then
>>> +	if test -z "$no_ff" -a "$current_sha1" = "$parent_sha1"
>>
>> 'test cond1 -a cond2' is not portable.  Use 'test cond1&&  test cond2'.
>
> I avoid "test -a/-o" myself without even thinking (I am from old-school),
> but at the same time I thought the progress in the world made such caution
> obsolescent.
>
> Not so.  Even though POSIX.1 lists -a/-o as options to "test", they are
> marked "Obsolescent XSI" ("Strictly Conforming POSIX Applications and
> Strictly Conforming XSI Applications shall not use obsolescent features").

The reason for this is that the precedence rules were never well 
specified, and this made many sane-looking uses of "test -a/-o" problematic.

For example, if $x is "=", these work according to POSIX (it's not 
portable, but in practice it's okay):

   $ test -z "$x"
   $ test -z "$x" && test a = b

but this doesn't

   $ test -z "$x" -a a = b
   bash: test: too many arguments

because it groups "test -n = -a" and is left with "a = b".

Similarly, if $x is "-f", these

   $ test "$x"
   $ test "$x" || test c = d

correctly adds an implicit "-n", but this fails:

   $ test "$x" -o c = d
   bash: test: too many arguments

If anybody cleans up git's usage of test -a/-o, feel free to cut'n'paste 
the above into the commit messages.

Paolo

  reply	other threads:[~2010-01-15  7:20 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-14  5:54 [PATCH 00/18] rebase -i: For pure fixups, do not start log message editor Michael Haggerty
2010-01-14  5:54 ` [PATCH 01/18] rebase -i: Make the condition for an "if" more transparent Michael Haggerty
2010-01-14 15:42   ` [PATCH 01/18] rebase -i: Make the condition for an &quot;if&quot; " Eric Blake
2010-01-14 17:42     ` Junio C Hamano
2010-01-15  7:20       ` Paolo Bonzini [this message]
2010-01-25 18:28   ` [PATCH 01/18] rebase -i: Make the condition for an "if" " Johannes Schindelin
2010-01-26 12:38     ` Michael Haggerty
2010-01-26 13:11       ` Johannes Schindelin
2010-01-14  5:54 ` [PATCH 02/18] rebase -i: Remove dead code Michael Haggerty
2010-01-14  5:54 ` [PATCH 03/18] rebase -i: Inline expression Michael Haggerty
2010-01-14  5:54 ` [PATCH 04/18] rebase -i: Use "test -n" instead of "test ! -z" Michael Haggerty
2010-01-14  5:54 ` [PATCH 05/18] rebase -i: Use symbolic constant $MSG consistently Michael Haggerty
2010-01-14  5:54 ` [PATCH 06/18] rebase -i: Document how temporary files are used Michael Haggerty
2010-01-25  3:38   ` Greg Price
2010-01-14  5:54 ` [PATCH 07/18] rebase -i: Introduce a constant AUTHOR_SCRIPT Michael Haggerty
2010-01-14  5:54 ` [PATCH 08/18] rebase -i: Introduce a constant AMEND Michael Haggerty
2010-01-14  5:54 ` [PATCH 09/18] t3404: Test the commit count in commit messages generated by "rebase -i" Michael Haggerty
2010-01-14  7:44   ` Johannes Sixt
2010-01-14  8:59     ` Michael Haggerty
2010-01-14  9:16       ` Johannes Sixt
2010-01-14  5:54 ` [PATCH 10/18] rebase -i: Improve consistency of commit count in generated commit messages Michael Haggerty
2010-01-14  7:54   ` Johannes Sixt
2010-01-14  9:05     ` Michael Haggerty
2010-01-14  5:54 ` [PATCH 11/18] rebase -i: Simplify commit counting for " Michael Haggerty
2010-01-14  5:54 ` [PATCH 12/18] rebase -i: Extract a function "commit_message" Michael Haggerty
2010-01-14  5:54 ` [PATCH 13/18] rebase -i: Handle the author script all in one place in do_next Michael Haggerty
2010-01-14  5:54 ` [PATCH 14/18] rebase -i: Extract function do_with_author Michael Haggerty
2010-01-14  5:54 ` [PATCH 15/18] rebase -i: Change function make_squash_message into update_squash_message Michael Haggerty
2010-01-14  8:20   ` Johannes Sixt
2010-01-14  5:54 ` [PATCH 16/18] rebase -i: For fixup commands without squashes, do not start editor Michael Haggerty
2010-01-14  5:54 ` [PATCH 17/18] t3404: Set up more of the test repo in the "setup" step Michael Haggerty
2010-01-14  5:54 ` [PATCH 18/18] rebase -i: Retain user-edited commit messages after squash/fixup conflicts Michael Haggerty
2010-01-14  9:17 ` [PATCH 00/18] rebase -i: For pure fixups, do not start log message editor Junio C Hamano
2010-01-25 18:38 ` Johannes Schindelin

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=4B50173B.5030802@gnu.org \
    --to=bonzini@gnu.org \
    --cc=ebb9@byu.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    /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.