git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] name-rev: use generation numbers if available

From
Derrick Stolee <derrickstolee@github.com>
Date
Mar 7, 2022, 20:26 UTC
Message-ID
<39f08f98-be0f-bba7-c41a-1c9eb6182a67@github.com>
In-Reply-To
<CA+P7+xo=UwUQ422o36_8XGNWoYjROGi5wBT4=jy4ThJBs_z=Xw@mail.gmail.com>
On 3/7/2022 3:22 PM, Jacob Keller wrote:
Show 41 quoted lines
> On Wed, Mar 2, 2022 at 5:10 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "Keller, Jacob E" <jacob.e.keller@intel.com> writes:
>>
>>> Let's clarify. There are two versions of the test in this version:
>>>
>>> 1) test which enables commit graph and tests that it does the right behavior.
>>>
>>> 2) test which removes commit graph and tests that it behaves the old way.
>>>
>>>
>>> test (1) checks the flow with the commit graph enabled, and verifies that with a commit graph the new behavior is used. This test will fail if you revert the name-rev commit-graph support.
>>>
>>> test (2) always performs the way we don't like. (since we disable the commit graph and the new flow doesn't kick in) This is the test I think I will eliminate in the next revision.
>>>
>>>
>>> I will remove the 2nd test, since the first test covers the change
>>> in behavior just fine, and I think I agree we don't need to set
>>> in-stone the implementation without commit graph.
>>>
>>> I will also look at adding a test which performs a count of which
>>> revisions get inspected and makes sure that we actually are doing
>>> the optimization.
>>
>> Sounds like a sensible thing to do.
>>
>> In any case, in the current patch, #2 is not working in
>> linux-TEST-vars job at CI.  You can visit this URL
>>
>> https://github.com/git/git/runs/5400048732?check_suite_focus=true#step:4:68062
>>
>> while logged into your GitHub account for details.
> 
> Looks like this job sets all the TEST variables including
> GIT_TEST_COMMIT_GRAPH=1? The negative test passes because the commit
> graph is enforced on and we then succeed even though we were trying to
> test the negative case.
> 
> I'm going to remove that test in v3 anyways, so I don't think it is a
> big deal. However, I wonder is there some way to mark a test as
> explicitely "don't run if GIT_TEST_COMMIT_GRAPH is set"?

Typically, we try to keep them compatible in both cases. However, you can set GIT_TEST_COMMIT_GRAPH=0 for the test, if you want. Be careful to only change it locally to the single test, not "globally" to the full test script.

Thanks, -Stolee

Previous: Jacob KellerNext: Keller, Jacob E
Message 14 of 18 in “name-rev: use generation numbers if available”
  1. 0/1 name-rev: use generation numbers if availableJacob Keller, Feb 28, 2022
  2. name-rev: use generation numbers if availableJacob Keller, Feb 28, 2022
  3. Junio C HamanoMar 1, 2022
  4. Jacob KellerMar 1, 2022
  5. Jacob KellerMar 1, 2022
  6. Junio C HamanoMar 1, 2022
  7. Derrick StoleeMar 1, 2022
  8. Keller, Jacob EMar 1, 2022
  9. Derrick StoleeMar 1, 2022
  10. Junio C HamanoMar 1, 2022
  11. Keller, Jacob EMar 1, 2022
  12. Junio C HamanoMar 3, 2022
  13. Jacob KellerMar 7, 2022
  14. Derrick StoleeMar 7, 2022
  15. Keller, Jacob EMar 7, 2022
  16. Derrick StoleeMar 7, 2022
  17. Junio C HamanoMar 7, 2022
  18. 1/1 name-rev: use generation numbers if availableJacob Keller, Feb 28, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.