Web lists-archives.com

Re: [PATCH] t*: avoid using pipes




Am 07.03.2017 um 18:21 schrieb Stefan Beller:
On Tue, Mar 7, 2017 at 8:10 AM, Prathamesh Chavan <pc44800@xxxxxxxxx> wrote:
I'm Prathamesh Chavan. As a part of my micropraoject I have been working on
"Avoid pipes for git related commands in test suites".

Welcome to the Git community!

* keep it micro first, e.g. just convert one file,
  send to list, wait for reviewers feedback and incorporate that
  (optional step after having done the full development cycle:
  convert all the other files; each as its own patch)
* split up this one patch into multiple patches, e.g. one
  file per patch, then send a patch series.

Actually, being a *micro* project, it should stay so. Not doing all of the changes would leave some tasks for other apprentices to get warm with our review process.

https://github.com/pratham-pc/git/tree/micro-proj

While I did look at that, not everyone here in the git community
does so. (Also for getting your change in, Junio seems to strongly prefer
patches on list instead of e.g. fetching and cherry-picking from your
github)

Thank you, Stefan, for digging out one particularly interesting case.

When looking at the content, the conversion seems a bit mechanical
(which is fine for a micro project), such as:
...
- test "$(git show --pretty=format:%s | head -n 1)" = "one"
+ test "$(git show --pretty=format:%s >out && head -n 1 <out)" = "one"
...

specifically for the "head" command I don't think it makes a
difference in correctness whether you pipe the file into the tool
or give the filename, i.e.  "head -n 1 out" would work just as fine.

True, but!

The intent of removing git invocations from the upstream of a pipe is that a failure exit code is able to stop a && chain.

The example above does not achieve this even after removal of the pipe. The reason is that exit code of process expansions $(...) is usually ignored. To meet the intent, further changes are necessary, for example to:

	git show --pretty=format:%s >out &&
	test "$(head -n 1 out)" = "one"

Side note: There is one exception where the exit code of $(...) is not ignored: when it occurs in the last variable assignment of a command that consists only of variable assignments.

-- Hannes