# [PATCH 0/2] Add OBJECT_INFO_NO_FETCH_IF_MISSING flag 10 messages from 2019-06-20 to 2019-06-21. Participants: Christian Couder, Jeff King, Derrick Stolee, Junio C Hamano. Thread: https://gitlist.dev/t/51348 ## Christian Couder, 2019-06-20 08:30 Subject: [PATCH 0/2] Add OBJECT_INFO_NO_FETCH_IF_MISSING flag Message-ID: <20190620083026.14524-1-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20190620083026.14524-1-chriscool%40tuxfamily.org ``` In a review[1] of my "many promisor remotes" patch series[2] and in the following thread, it was suggested that a flag should be passed to tell oid_object_info_extended() that it should not fetch objects from promisor remotes if they are missing, instead of using the ugly fetch_if_missing global. It looks like the OBJECT_INFO_FOR_PREFETCH flag already exists but unfortunately conflates 2 separate things. This patch series introduces OBJECT_INFO_NO_FETCH_IF_MISSING to disambiguate the different meanings and then uses it instead of OBJECT_INFO_FOR_PREFETCH where it makes sense. 1: https://public-inbox.org/git/b4d69d2b-dc0d-fffb-2909-c54060fe9cd1@gmail.com/ 2: https://public-inbox.org/git/20190409161116.30256-1-chriscool@tuxfamily.org/ Christian Couder (2): object-store: introduce OBJECT_INFO_NO_FETCH_IF_MISSING sha1-file: use OBJECT_INFO_NO_FETCH_IF_MISSING object-store.h | 9 +++++++-- sha1-file.c | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) -- 2.22.0 ``` ## Christian Couder, 2019-06-20 08:30 Subject: [PATCH 1/2] object-store: introduce OBJECT_INFO_NO_FETCH_IF_MISSING Message-ID: <20190620083026.14524-2-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20190620083026.14524-2-chriscool%40tuxfamily.org In-Reply-To: <20190620083026.14524-1-chriscool@tuxfamily.org> ``` The fetch_if_missing global variable should be replaced as much as possible by passing a flag to oid_object_info_extended(). The existing OBJECT_INFO_FOR_PREFETCH unfortunately conflates both a "no fetch if missing" meaning with OBJECT_INFO_QUICK which is about retrying packed storage. Let's disambiguate that by adding a new explicit OBJECT_INFO_NO_FETCH_IF_MISSING flag. Signed-off-by: Christian Couder --- object-store.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/object-store.h b/object-store.h index 272e01e452..02c1795d50 100644 --- a/object-store.h +++ b/object-store.h @@ -277,10 +277,15 @@ struct object_info { #define OBJECT_INFO_IGNORE_LOOSE 16 /* * Do not attempt to fetch the object if missing (even if fetch_is_missing is - * nonzero). This is meant for bulk prefetching of missing blobs in a partial + * nonzero). + */ +#define OBJECT_INFO_NO_FETCH_IF_MISSING 32 +/* + * This is meant for bulk prefetching of missing blobs in a partial * clone. Implies OBJECT_INFO_QUICK. */ -#define OBJECT_INFO_FOR_PREFETCH (32 + OBJECT_INFO_QUICK) +#define OBJECT_INFO_FOR_PREFETCH \ + (OBJECT_INFO_NO_FETCH_IF_MISSING + OBJECT_INFO_QUICK) int oid_object_info_extended(struct repository *r, const struct object_id *, -- 2.22.0 ``` ## Christian Couder, 2019-06-20 08:30 Subject: [PATCH 2/2] sha1-file: use OBJECT_INFO_NO_FETCH_IF_MISSING Message-ID: <20190620083026.14524-3-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20190620083026.14524-3-chriscool%40tuxfamily.org In-Reply-To: <20190620083026.14524-1-chriscool@tuxfamily.org> ``` Currently the OBJECT_INFO_FOR_PREFETCH flag is used to check if we should fetch objects from promisor remotes when we haven't found them elsewhere. Now that OBJECT_INFO_NO_FETCH_IF_MISSING exists, let's use it instead to be more correct in case this new flag is ever used without OBJECT_INFO_QUICK. Signed-off-by: Christian Couder --- sha1-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sha1-file.c b/sha1-file.c index ed5c50dac4..2116ff1e70 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -1379,7 +1379,7 @@ int oid_object_info_extended(struct repository *r, const struct object_id *oid, /* Check if it is a missing object */ if (fetch_if_missing && repository_format_partial_clone && !already_retried && r == the_repository && - !(flags & OBJECT_INFO_FOR_PREFETCH)) { + !(flags & OBJECT_INFO_NO_FETCH_IF_MISSING)) { /* * TODO Investigate having fetch_object() return * TODO error/success and stopping the music here. -- 2.22.0 ``` ## Jeff King, 2019-06-20 08:48 Subject: Re: [PATCH 1/2] object-store: introduce OBJECT_INFO_NO_FETCH_IF_MISSING Message-ID: <20190620084842.GB3952@sigill.intra.peff.net> URL: https://gitlist.dev/e/20190620084842.GB3952%40sigill.intra.peff.net In-Reply-To: <20190620083026.14524-2-chriscool@tuxfamily.org> ``` On Thu, Jun 20, 2019 at 10:30:25AM +0200, Christian Couder wrote: > The fetch_if_missing global variable should be replaced as much > as possible by passing a flag to oid_object_info_extended(). > > The existing OBJECT_INFO_FOR_PREFETCH unfortunately conflates > both a "no fetch if missing" meaning with OBJECT_INFO_QUICK > which is about retrying packed storage. > > Let's disambiguate that by adding a new explicit > OBJECT_INFO_NO_FETCH_IF_MISSING flag. I think this patch is obsoleted by Stolee's 31f5256c82 (sha1-file: split OBJECT_INFO_FOR_PREFETCH, 2019-05-28), which I think just made it to master in the last day or so. There it's called OBJECT_INFO_SKIP_FETCH_OBJECT, so your 2/2 will need to be tweaked. -Peff ``` ## Jeff King, 2019-06-20 08:50 Subject: Re: [PATCH 2/2] sha1-file: use OBJECT_INFO_NO_FETCH_IF_MISSING Message-ID: <20190620085009.GC3952@sigill.intra.peff.net> URL: https://gitlist.dev/e/20190620085009.GC3952%40sigill.intra.peff.net In-Reply-To: <20190620083026.14524-3-chriscool@tuxfamily.org> ``` On Thu, Jun 20, 2019 at 10:30:26AM +0200, Christian Couder wrote: > Currently the OBJECT_INFO_FOR_PREFETCH flag is used to check > if we should fetch objects from promisor remotes when we > haven't found them elsewhere. > > Now that OBJECT_INFO_NO_FETCH_IF_MISSING exists, let's use > it instead to be more correct in case this new flag is ever > used without OBJECT_INFO_QUICK. I said earlier that this one would need to be tweaked for the new upstream name. But actually, I think it is not necessary after Stolee's patch. -Peff ``` ## Derrick Stolee, 2019-06-20 12:39 Subject: Re: [PATCH 2/2] sha1-file: use OBJECT_INFO_NO_FETCH_IF_MISSING Message-ID: URL: https://gitlist.dev/e/d88ac28b-638b-89c0-2bfb-634a0bb6ca4c%40gmail.com In-Reply-To: <20190620085009.GC3952@sigill.intra.peff.net> ``` On 6/20/2019 4:50 AM, Jeff King wrote: > On Thu, Jun 20, 2019 at 10:30:26AM +0200, Christian Couder wrote: > >> Currently the OBJECT_INFO_FOR_PREFETCH flag is used to check >> if we should fetch objects from promisor remotes when we >> haven't found them elsewhere. >> >> Now that OBJECT_INFO_NO_FETCH_IF_MISSING exists, let's use >> it instead to be more correct in case this new flag is ever >> used without OBJECT_INFO_QUICK. > > I said earlier that this one would need to be tweaked for the new > upstream name. But actually, I think it is not necessary after Stolee's > patch. Yes, I believe that 31f5256c82 does an equivalent thing to the combination of these patches. Thanks, -Stolee ``` ## Christian Couder, 2019-06-20 14:08 Subject: Re: [PATCH 2/2] sha1-file: use OBJECT_INFO_NO_FETCH_IF_MISSING Message-ID: URL: https://gitlist.dev/e/CAP8UFD1-%3D%2B32G2XYXARcXm8%3DhnqZwxa1Ev9nmaS%3DqfqKS7GSWQ%40mail.gmail.com In-Reply-To: ``` On Thu, Jun 20, 2019 at 2:39 PM Derrick Stolee wrote: > > On 6/20/2019 4:50 AM, Jeff King wrote: > > On Thu, Jun 20, 2019 at 10:30:26AM +0200, Christian Couder wrote: > > > >> Currently the OBJECT_INFO_FOR_PREFETCH flag is used to check > >> if we should fetch objects from promisor remotes when we > >> haven't found them elsewhere. > >> > >> Now that OBJECT_INFO_NO_FETCH_IF_MISSING exists, let's use > >> it instead to be more correct in case this new flag is ever > >> used without OBJECT_INFO_QUICK. > > > > I said earlier that this one would need to be tweaked for the new > > upstream name. But actually, I think it is not necessary after Stolee's > > patch. > > Yes, I believe that 31f5256c82 does an equivalent thing to the > combination of these patches. Yeah, I agree. Thanks Stolee for having already fixed that, and sorry for bothering everyone with this. Christian. ``` ## Junio C Hamano, 2019-06-20 20:52 Subject: Re: [PATCH 0/2] Add OBJECT_INFO_NO_FETCH_IF_MISSING flag Message-ID: URL: https://gitlist.dev/e/xmqq36k4m12g.fsf%40gitster-ct.c.googlers.com In-Reply-To: <20190620083026.14524-1-chriscool@tuxfamily.org> ``` Christian Couder writes: > In a review[1] of my "many promisor remotes" patch series[2] and in > the following thread, it was suggested that a flag should be passed to > tell oid_object_info_extended() that it should not fetch objects from > promisor remotes if they are missing, instead of using the ugly > fetch_if_missing global. > > It looks like the OBJECT_INFO_FOR_PREFETCH flag already exists but > unfortunately conflates 2 separate things. > > This patch series introduces OBJECT_INFO_NO_FETCH_IF_MISSING to > disambiguate the different meanings and then uses it instead of > OBJECT_INFO_FOR_PREFETCH where it makes sense. > > 1: https://public-inbox.org/git/b4d69d2b-dc0d-fffb-2909-c54060fe9cd1@gmail.com/ > 2: https://public-inbox.org/git/20190409161116.30256-1-chriscool@tuxfamily.org/ What commit did you base these two patches on? > > Christian Couder (2): > object-store: introduce OBJECT_INFO_NO_FETCH_IF_MISSING > sha1-file: use OBJECT_INFO_NO_FETCH_IF_MISSING > > object-store.h | 9 +++++++-- > sha1-file.c | 2 +- > 2 files changed, 8 insertions(+), 3 deletions(-) ``` ## Junio C Hamano, 2019-06-20 20:57 Subject: Re: [PATCH 2/2] sha1-file: use OBJECT_INFO_NO_FETCH_IF_MISSING Message-ID: URL: https://gitlist.dev/e/xmqqy31wkm97.fsf%40gitster-ct.c.googlers.com In-Reply-To: ``` Christian Couder writes: > On Thu, Jun 20, 2019 at 2:39 PM Derrick Stolee wrote: >> >> On 6/20/2019 4:50 AM, Jeff King wrote: >> > On Thu, Jun 20, 2019 at 10:30:26AM +0200, Christian Couder wrote: >> > >> >> Currently the OBJECT_INFO_FOR_PREFETCH flag is used to check >> >> if we should fetch objects from promisor remotes when we >> >> haven't found them elsewhere. >> >> >> >> Now that OBJECT_INFO_NO_FETCH_IF_MISSING exists, let's use >> >> it instead to be more correct in case this new flag is ever >> >> used without OBJECT_INFO_QUICK. >> > >> > I said earlier that this one would need to be tweaked for the new >> > upstream name. But actually, I think it is not necessary after Stolee's >> > patch. >> >> Yes, I believe that 31f5256c82 does an equivalent thing to the >> combination of these patches. > > Yeah, I agree. Thanks Stolee for having already fixed that, and sorry > for bothering everyone with this. OK, I'll stop looking at this patch. ``` ## Christian Couder, 2019-06-21 10:47 Subject: Re: [PATCH 0/2] Add OBJECT_INFO_NO_FETCH_IF_MISSING flag Message-ID: URL: https://gitlist.dev/e/CAP8UFD0HsD%2Bq2Srd7iUkPhXMHzUN%3Dg0he3EHGd0m3n_TDxuZ1w%40mail.gmail.com In-Reply-To: ``` On Thu, Jun 20, 2019 at 10:52 PM Junio C Hamano wrote: > > Christian Couder writes: > > > In a review[1] of my "many promisor remotes" patch series[2] and in > > the following thread, it was suggested that a flag should be passed to > > tell oid_object_info_extended() that it should not fetch objects from > > promisor remotes if they are missing, instead of using the ugly > > fetch_if_missing global. > > > > It looks like the OBJECT_INFO_FOR_PREFETCH flag already exists but > > unfortunately conflates 2 separate things. > > > > This patch series introduces OBJECT_INFO_NO_FETCH_IF_MISSING to > > disambiguate the different meanings and then uses it instead of > > OBJECT_INFO_FOR_PREFETCH where it makes sense. > > > > 1: https://public-inbox.org/git/b4d69d2b-dc0d-fffb-2909-c54060fe9cd1@gmail.com/ > > 2: https://public-inbox.org/git/20190409161116.30256-1-chriscool@tuxfamily.org/ > > What commit did you base these two patches on? They were based on master at v2.22.0 when I worked on them, but I didn't send them right away. And I didn't rebase them before I later sent them. ```