Re: [PATCH] merge-recursive: use fspathcmp() in path_hashmap_cmp()

From: Jeff King <peff@peff.net>
Date: Sep 13, 2021, 17:09 UTC
Message-ID: <YT+FzPT0RCP2PdNL@coredump.intra.peff.net>
In-Reply-To: <nycvar.QRO.7.76.6.2109131335260.55@tvgsbejvaqbjf.bet>

On Mon, Sep 13, 2021 at 01:37:48PM +0200, Johannes Schindelin wrote:

Show 10 quoted lines

> > Good point.  So fspathcmp() and friends would need a repo parameter. :-|
> 
> Yes, we will eventually have to pass `struct repository *r` into a _lot_
> of call chains. It'll be a disruptive change, yet if the submodule folks
> truly want to aim for in-process recursive treatment of submodules, there
> is no alternative.
> 
> FWIW on Windows there are other potentially repository-specific settings
> that are relevant in similar situations. For example, there is
> `core.symlinks`.

Another approach is to stuff the appropriate globals into the repository struct, and then "push" onto the global the_repository pointer, treating it like a stack. And then low-level code is free to use that global context, even if it wasn't passed in.

That helps the primary use case of "now I need to do something in a sub-module, but I'd like to do it in-process". But it's not without challenges:

  - code which acts at the boundary of a submodule and a superproject
    may be more awkward (since only one of them can be "the current
    repository" at a time).

  - it's a challenge with threading (an obvious problem would be a
    multi-threaded grep which wanted to descend into a submodule). Using
    a thread-local global for the_repository might solve that.

It's possible that this is a terrible direction to go, so I'm not necessarily endorsing it, but just offering it as a possibility to think about. The trickiest thing is that any devil would likely be in the details, and we wouldn't know until proceeding for a while along that path. Whereas passing around a context struct, while verbose and annoying, is a well-understood construct.

-Peff