From: Phillip Wood Date: Tue, 30 Aug 2022 15:19:47 GMT Subject: Re: [PATCH 0/3] A couple of CI fixes regarding the built-in add --patch Message-ID: <26fdecfe-1715-1b12-2cd4-ec114f5b426b@gmail.com> In-Reply-To: Hi Dscho All three patches look good to me, thanks for working on them. Phillip On 30/08/2022 14:54, Johannes Schindelin via GitGitGadget wrote: > This is an attempt to address the concern Junio raised in > https://lore.kernel.org/git/xmqq7d3gm1bl.fsf@gitster.g/: originally > motivated by a test suite failure when running Git's test suite in Visual > Studio, this pulls out the signed vs unsigned fix whose implications are > potentially much wider than Visual C. > > While in that space, I spent the time (which took almost as long as I > expected > [https://lore.kernel.org/git/nrr2312s-q256-61n7-2843-7r0s817rp432@tzk.qr/]) > to craft a semantic patch to scrutinize Git's source code for similar issues > (narrator's voice: there were no other instances, what did ya expect?). > > To verify the fix, I then worked on a patch to exercise the built-in git add > -p in the test suite even when NO_PERL is set, and while developing this > patch and validating it, I got really puzzled that the add -p test case in > t6132 did not need to be guarded behind a PERL prereq. So this patch series > also includes a fix for that. > > The story arc that binds all of these patches together is that they all > revolve around NO_PERL and CI issues that involve git add --patch. > > Note: This patch series is based on ds/github-actions-use-newer-ubuntu (but > probably applies cleanly even on maint) because I tried to develop a > semantic patch to fix similar issues in the code base. However, I've since > run into what looks like a bug in Coccinelle > [https://github.com/coccinelle/coccinelle/issues/284]. My latest version of > that semantic patch looks like this, but I stopped when running it on Git's > source code triggered the bug for 66 of Git's .c files: > > @@ > type T = { unsigned int }; > T:n b; > type S != { unsigned int, size_t }; > S s; > binary operator o != { &&, || }; > @@ > -s o b > +s o (S)b > > @@ > type T = { unsigned int }; > T:n b; > type S != { unsigned int, size_t }; > S s; > binary operator o != { &&, || }; > @@ > -b o s > +(S)b o s > > > Johannes Schindelin (3): > add -p: avoid ambiguous signed/unsigned comparison > t3701: test the built-in `add -i` regardless of NO_PERL > t6132(NO_PERL): do not run the scripted `add -p` > > add-patch.c | 2 +- > t/t3701-add-interactive.sh | 4 ++-- > t/t6132-pathspec-exclude.sh | 6 +++++- > 3 files changed, 8 insertions(+), 4 deletions(-) > > > base-commit: ef46584831268a83591412a33783caf866867482 > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1340%2Fdscho%2Fbuilt-in-add-i-does-not-need-perl-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1340/dscho/built-in-add-i-does-not-need-perl-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/1340