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?

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 30, 2014, 19:09 UTC
Message-ID: <xmqq4mxyzmrm.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CABUJjW_tnf2SRCyjRw1n+UbzLzTbmqW6XAJSJnZ5qszdpJ3SYA@mail.gmail.com>

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 | ... )