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

Re: [PATCH 2/5] object.c: lookup_unknown_object() accept 'r' as parameter

From
Taylor Blau <me@ttaylorr.com>
Date
Feb 15, 2020, 00:00 UTC
Message-ID
<20200215000022.GB11400@syl.local>
In-Reply-To
<20200213185235.GA15252@coredump.intra.peff.net>
On Thu, Feb 13, 2020 at 01:52:35PM -0500, Jeff King wrote:
Show 33 quoted lines
> On Thu, Feb 13, 2020 at 10:10:45AM -0800, Junio C Hamano wrote:
>
> > > Right, but my suggestion was that this advice doesn't apply to this
> > > particular instance since I don't expect that we'd ever passing
> > > something other than 'the_repository'.
> > >
> > > Specifically, I was worried that we'd get bitten by re-assigning 'r' in
> > > the middle of the function and then end up in some odd broken state.
> >
> > "git fsck" works only in a single, "the", repository, so I guess you
> > are right to be worried about unnecessary complexity here.
>
> I think the end-game for this whole repository transition would be to
> get rid of the_repository, though. I.e., I'd envision the progression
> something like this:
>
>   1. Teach all of the library code to take (and operate on) "struct
>      repository".
>
>   2. Teach static local functions like this to pass in the_repository.
>
>   3. Teach top-level commands like cmd_fsck() to pass the_repository to
>      all of those static local helpers.
>
>   4. Teach top-level commands to get a real repository pointer, either
>      from the git.c wrapper (when RUN_SETUP is used) or by calling
>      setup_git_repository() themselves.
>
>   5. Grep for the_repository and drop it everywhere.
>
> Here we're at step 2 now, but declaring "r" makes moving to step 3 just
> a little easier. And I think the existence of steps 4 and 5 implies that
> it would eventually be worth going through step 3.

Ah, the transition to step 3 justifies this, I think. I wasn't aware that steps 3+ existed. If they didn't, I'd stand by my original advice, but given that they do, the approach here makes more sense long-term.

> Of course I just wrote those steps down for the first time, so maybe
> nobody else shares my vision. ;)

Thanks for writing it down. I'm sure that it has been loosely discussed over a while, but this is the first time that I've seen it all in one place.

> -Peff

Thanks, Taylor

Previous: Jeff KingNext: Parth Gala via GitGitGadget
Message 14 of 17 in “object.c: localize global the_repository variable into r”
  1. 0/5 object.c: localize global the_repository variable into rParth Gala via GitGitGadget, Feb 12, 2020
  2. 3/5 object.c: parse_object_or_die() accept 'r' as parameterParth Gala via GitGitGadget, Feb 12, 2020
  3. 1/5 object.c: get_max_object_index and get_indexed_object accept 'r' parameterParth Gala via GitGitGadget, Feb 12, 2020
  4. Taylor BlauFeb 12, 2020
  5. Eric SunshineFeb 12, 2020
  6. parth galaFeb 13, 2020
  7. 4/5 object.c: clear_object_flags() accept 'r' as parameterParth Gala via GitGitGadget, Feb 12, 2020
  8. 2/5 object.c: lookup_unknown_object() accept 'r' as parameterParth Gala via GitGitGadget, Feb 12, 2020
  9. Taylor BlauFeb 12, 2020
  10. Junio C HamanoFeb 12, 2020
  11. Taylor BlauFeb 13, 2020
  12. Junio C HamanoFeb 13, 2020
  13. Jeff KingFeb 13, 2020
  14. Taylor BlauFeb 15, 2020
  15. 5/5 object.c: clear_commit_marks_all() accept 'r' as parameterParth Gala via GitGitGadget, Feb 12, 2020
  16. Taylor BlauFeb 12, 2020
  17. parth galaFeb 13, 2020

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.