All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christoph Bonitz <ml.christophbonitz@gmail.com>
Cc: Johannes Sixt <j6t@kdbg.org>, Pete Wyckoff <pw@padd.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] git p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?
Date: Wed, 30 Jul 2014 12:09:48 -0700	[thread overview]
Message-ID: <xmqq4mxyzmrm.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CABUJjW_tnf2SRCyjRw1n+UbzLzTbmqW6XAJSJnZ5qszdpJ3SYA@mail.gmail.com> (Christoph Bonitz's message of "Wed, 30 Jul 2014 09:08:26 +0200")

Christoph Bonitz <ml.christophbonitz@gmail.com> writes:

> Apart from your change and the word wrap adjustments suggested by
> Pete, would the following also make sense, to fix the other flaw
> Johannes pointed out? With regards to failing, git diff-tree should be
> idempotent. I think those are the two occurrences in this file:

As a band-aid, that might be OK, but I think these pipelines are
unnecessarily and overly wasteful in the first place.

All the "sed 1d" you see here is only because the upstream uses
the one-tree form "diff-tree <options> $commit"; by comparing two,
i.e. "diff-tree <options> $commit^ $commit", they can be dropped.

All the "cut -f2" is to grab the pathname; we have "--name-only"
these days.

I.e.

>  src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&

should become

    src=$(git diff-tree --name-only -r -C --find-copies-harder HEAD^ HEAD) &&

I would think.

Extracting C[0-9]* manually with sed is bad, and expecting that the
score is within certain range is even worse, because there is no
formal guarantee that the definition of similarity indices will not
improve in the future.  --diff-filter=C to limit the output to only
copied paths, without looking at the similarity index, would be more
appropriate, e.g. 

    git diff-tree --name-only --diff-filter=C -r -C HEAD^ HEAD

or something along those lines.

Otherwise, run them outside $(), keep the result in a temporary
file, and process the temporary file with the pipeline.  That has an
added benefit that lets you inspect the file when something goes
wrong.  I.e.

	git diff-tree ... >diff-tree-out &&
        level=$( sed 1d <diff-tree-out | cut -f1 | ... )

  reply	other threads:[~2014-07-30 19:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-23 21:28 [PATCH] git p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test? Christoph Bonitz
2014-07-23 23:07 ` Pete Wyckoff
2014-07-24 18:45 ` Johannes Sixt
2014-07-24 22:05   ` Junio C Hamano
2014-07-30  7:08     ` Christoph Bonitz
2014-07-30 19:09       ` Junio C Hamano [this message]
2014-07-30  7:07   ` Christoph Bonitz

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=xmqq4mxyzmrm.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=ml.christophbonitz@gmail.com \
    --cc=pw@padd.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 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.