Re: [PATCH] pull: abort by default if fast-forwarding is impossible

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: Alex Henrie <alexhenrie24@gmail.com>,
	Elijah Newren <newren@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] pull: abort by default if fast-forwarding is impossible
Date: Mon, 28 Jun 2021 12:31:52 +0200	[thread overview]
Message-ID: <87k0megtlo.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <60d8edbb3803f_70e20886@natae.notmuch>

On Sun, Jun 27 2021, Felipe Contreras wrote:

> Alex Henrie wrote:
>
>> On Sat, Jun 26, 2021 at 10:12 PM Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>> >
>> > Also, a bunch of tests are broken after this change:
>> >
>> >   t4013-diff-various.sh
>> >   t5521-pull-options.sh
>> >   t5524-pull-msg.sh
>> >   t5520-pull.sh
>> >   t5553-set-upstream.sh
>> >   t5604-clone-reference.sh
>> >   t6409-merge-subtree.sh
>> >   t6402-merge-rename.sh
>> >   t6417-merge-ours-theirs.sh
>> >   t7601-merge-pull-config.sh
>> >   t7603-merge-reduce-heads.sh
>> >
>> > If you didn't mean this patch to be applied then perhaps add the RFC
>> > prefix.
>> 
>> I actually did run `make test` before sending the patch, but when so
>> many seemingly unrelated tests failed, I foolishly assumed that they
>> were pre-existing failures. I should have run the tests on master for
>> comparison, sorry. Or at least put "RFC" in the subject instead of
>> "PATCH" as you suggest. I sincerely apologize for my lack of due
>> diligence and I know that I need to do better at self-reviewing
>> patches before sending them.
> I personally don't see any need for apologies, we all make mistakes,
> just keep it in mind for the future.

Yes, for someone joining the project it's not obvious what the status of
the tests is. No problem.

Alex: To elaborate, our exists tests should pass, and should pass on
every commit (both as a matter of fact and future coding practice). We
also have CI, so if you e.g. have a fork of git/git and push to your
fork you'll find that CI is run for you on several platforms.

See below for a one-liner to possibly speed up the testing for you.

> Personally I prefer to run prove instead, because the output is less
> verbose, and there's a nice summary at the end:
>
>   prove t[0-9][0-9][0-9][0-9]-*.sh

I also like "prove" better (well, I added the support for it, so
...). It's generally better to use e.g.:

    make test DEFAULT_TEST_TARGET=prove GIT_PROVE_OPTS="--jobs $(nproc)"

Since we do some basic checking via the Makefile that effectively form a
part of our tests.

FWIW for your one-liner it can be just:

    prove t[0-9]*.sh

Alex: You might also find that if you specify --root as the path to a
ramdisk the tests are much faster, e.g. on my Linux boxes I set
--root=/run/user/`id -u`/git.