From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id C548C1F9FD for ; Tue, 16 Feb 2021 21:44:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230209AbhBPVnU (ORCPT ); Tue, 16 Feb 2021 16:43:20 -0500 Received: from cloud.peff.net ([104.130.231.41]:35004 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229533AbhBPVnU (ORCPT ); Tue, 16 Feb 2021 16:43:20 -0500 Received: (qmail 15046 invoked by uid 109); 16 Feb 2021 21:42:39 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 16 Feb 2021 21:42:39 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 29737 invoked by uid 111); 16 Feb 2021 21:42:38 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 16 Feb 2021 16:42:38 -0500 Authentication-Results: peff.net; auth=none Date: Tue, 16 Feb 2021 16:42:38 -0500 From: Jeff King To: Taylor Blau Cc: git@vger.kernel.org, dstolee@microsoft.com, gitster@pobox.com Subject: Re: [PATCH v2 1/8] packfile: introduce 'find_kept_pack_entry()' Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Wed, Feb 03, 2021 at 10:58:50PM -0500, Taylor Blau wrote: > Future callers will want a function to fill a 'struct pack_entry' for a > given object id but _only_ from its position in any kept pack(s). > > In particular, an new 'git repack' mode which ensures the resulting Nit (not worth re-rolling): s/an new/a new/ > There is a gotcha when looking up objects that are duplicated in kept > and non-kept packs, particularly when the MIDX stores the non-kept > version and the caller asked for kept objects only. This could be > resolved by teaching the MIDX to resolve duplicates by always favoring > the kept pack (if one exists), but this breaks an assumption in existing > MIDXs, and so it would require a format change. I don't think this would be possible without a major rethink of how midxs work. The "keep" property of a pack is not set in stone when the midx is created. You could add a ".keep" file to one of its packs later, or even mark one as an in-core keep on the fly. But the duplicate resolution happens at creation. So maybe your "breaks an assumption" is the notion that we do not store duplicate information at all in the midx. If so, then I agree. :) But I'd also call fixing that more than just a format change. (None of which changes your point, which isn't that it isn't worth pursuing that direction). -Peff