RE: [PATCH] name-rev: test showing failure with non-monotonic commit dates

From: "Keller, Jacob E" <jacob.e.keller@intel.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
	Jacob Keller <jacob.keller@gmail.com>
Subject: RE: [PATCH] name-rev: test showing failure with non-monotonic commit dates
Date: Wed, 16 Feb 2022 01:16:30 +0000	[thread overview]
Message-ID: <CO1PR11MB5089008D502F964AF9425F86D6359@CO1PR11MB5089.namprd11.prod.outlook.com> (raw)
In-Reply-To: <220215.868rucy4t6.gmgdl@evledraar.gmail.com>

> -----Original Message-----
> From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Sent: Monday, February 14, 2022 11:16 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: git@vger.kernel.org; Jacob Keller <jacob.keller@gmail.com>
> Subject: Re: [PATCH] name-rev: test showing failure with non-monotonic commit
> dates
> 
> 
> Shorter & avoids the needless subshell as:
> 
>     git init repo &&
>     test_commit -C repo --date="2020-01-01 18:00" A &&
>     test_commit -C repo --date="2020-01-02 18:00" B &&
>     [...]
> 
>

Sure. I can pick these improvements up if if we end up actually wanting the test case. I think we're still discussing the core problem in this thread too.

> > +test_expect_failure 'name-rev commit timestamp prevents naming commits' '
> > +	(
> > +		cd non-monotonic &&
> > +
> > +		B=$(git rev-parse main~3) &&
> > +
> > +		echo "$B main~3" >expect &&
> > +		git name-rev $B >actual &&
> > +
> > +		test_cmp expect actual
> > +	)
> > +'
> 
> I haven't checked, but is the explicit peeling to $B really needed here,
> are the results different with a main~3 or main~3^{commit}?
> 
> I.e. the first column of the output will of course be, but will the
> result on the second column? I suspect not, but haven't run this. In any
> case I tihnk teh test/commit message could do with an explanation.
> 

The first column is the commit id, so we need to get that either way to compare the expected and actual output. As far as I know passing "main~3" instead of the commit id to name-rev doesn't change this.

Thanks,
Jake