Re: [PATCH v3 1/8] t5004: skip SHA-1-only test in SHA-256 repository

[Patrick Steinhardt <ps@pks.im>]

On Wed, Oct 14, 2026 at 08:34:09PM +0530, Kaartic Sivaraam wrote:
> On 10/14/26 16:04, Patrick Steinhardt wrote:
> > On Wed, Oct 14, 2026 at 05:07:46AM -0400, Jeff King wrote:
> > > On Wed, Oct 14, 2026 at 10:30:27AM +0200, Patrick Steinhardt wrote:
> > > 
> > > > We're about to fix that typo in the next commit, but this will also
> > > > uncover that the above test case is broken when running in SHA-256
> > > > repositories. The extracted objects are SHA-1 objects, so extracting
> > > > them into a SHA-256 repository is not going to yield anything good. So
> > > > once we fix the Fedora-based job to enable breaking changes, which will
> > > > make tests use SHA-256 by default, the test will break.
> > > > 
> > > > Fix this issue by adding the SHA1 prerequisite.
> > > 
> > > Does that mean we'll basically never run the test once Git 3.0 shifts to
> > > sha256 by default? I guess we'll have a sha1 ci job (like we have sha256
> > > today), but would that job actually hit all the other prereqs?
> > > 
> > > If we instead teach it to use its own repo, independent of the defaults,
> > > then we could run it everywhere (that allows all of the other prereqs).
> > > 
> 
> That indeed sounds like a good idea.
> 
> I also wonder, though. How many CI jobs actually satisfy the prerequisite
> after the change? From a quick look and guessing via the time it took the
> t5004 jobs to run[1] it seems only the macOS jobs and on AlmaLinux 8 jobs
> run the test. The linux jobs don't seem to run it. I suppose this is
> expected behaviour.
> 
> I wonder if it is worth mentioning in the commit message or not. FWIW, the
> "not a lot of jobs even exercise this." in the commit message made me want
> to confirm if this job runs at all after the change.
> 
> [1]: https://github.com/git/git/actions/runs/38526140649?pr=2445

I guess it's less interesting which exact jobs run this, so I think I'll
keep this information out of the commit message for now.

Patrick