From: Johannes Sixt Date: Tue, 07 Mar 2017 20:39:39 GMT Subject: Re: [PATCH] t*: avoid using pipes Message-ID: <3026648b-a26c-bc67-62dc-170217d6c2ca@kdbg.org> In-Reply-To: Am 07.03.2017 um 18:21 schrieb Stefan Beller: > On Tue, Mar 7, 2017 at 8:10 AM, Prathamesh Chavan 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 ... > > 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