Re: [PATCH v4 05/25] midx: clear auxiliary .rev after replacing the MIDX

From: Derrick Stolee <stolee@gmail.com>
Date: Aug 31, 2021, 01:21 UTC
Message-ID: <22366f81-65a6-55d1-706c-59f877127be0@gmail.com>
In-Reply-To: <YSwhNxqAS8JajA7p@nand.local>

On 8/29/21 8:07 PM, Taylor Blau wrote:

Show 7 quoted lines

> On Sun, Aug 29, 2021 at 03:56:31PM -0700, Junio C Hamano wrote:
>> My recollection is that "--object-dir" is mostly about the alternate
>> odb usecase---am I correct?
> 
> That matches my understanding. The documentation refers to the value of
> this flag as `<alt>`, making me think that supporting non-alternates is
> a historical accident.

Yes, supporting non-alternates is a historical accident. Supporting alternates that are not actually the core object database of a full repository is on purpose.

So, hopefully the remaining discussion that I am seeing can be solved by a decision such as:

  "If we add the restriction that the builtin always runs with a
   repository and --object-dir always points to its objects dir
   or one of its registered alternates, then we have access to a
   local config file to learn how to interpret that object directory."

Show 5 quoted lines

>> I wonder if it is safe to assume that in practice a directory given
>> to the "--object-dir" option is always the "objects" subdirectory in
>> a repository, and it is an error if there is no "config" file next
>> to the directory.  Then, we could check ../config relative to the
>> given directory and error out if they use different hash.

I would say that is not always the case, and we should not error out.

I think taking a look to see if ../config exists to use the data might be helpful for some cases, but should not be a blocker for completing the requested operation. The config from the non-alternate repo should be sufficient for this (somewhat strange) case.

Show 10 quoted lines

> I'm admittedly a little unsure of how to progress here. Given that this
> series has received positive review over the complicated parts, it seems
> that it is getting stuck on how to deal with `--object-dir`, especially
> when invoked outside of a Git repository. My inclination would be to
> send a new version that simply requires the MIDX builtin to be run from
> within a repository (as well as the cleanups from Johannes).
> 
> Does that seem like a good direction forward to you? If not, let me know
> if there's another issue that we should deal with first and I'd be happy
> to start there.

I think it is sensible to restrict 'git multi-pack-index' to run inside a repository on its own merits. It happens to also solve some tricky problems that have come up since its creation.

Sorry I'm so late to this thread. I gave most of the messages in this chain a quick read and this seemed like the best place to chime in. Hopefully this isn't too much of a re-tread of things covered elsewhere.

Thanks, -Stolee