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-ASN: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-11.3 required=3.0 tests=AWL,BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by dcvr.yhbt.net (Postfix) with ESMTP id 82C831F51E for ; Thu, 29 Sep 2022 21:58:20 +0000 (UTC) Authentication-Results: dcvr.yhbt.net; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="L3YXCzms"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229532AbiI2V6N (ORCPT ); Thu, 29 Sep 2022 17:58:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50662 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229482AbiI2V6M (ORCPT ); Thu, 29 Sep 2022 17:58:12 -0400 Received: from mail-pl1-x64a.google.com (mail-pl1-x64a.google.com [IPv6:2607:f8b0:4864:20::64a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EB1C414F811 for ; Thu, 29 Sep 2022 14:58:11 -0700 (PDT) Received: by mail-pl1-x64a.google.com with SMTP id n1-20020a170902f60100b00179c0a5c51fso1820676plg.7 for ; Thu, 29 Sep 2022 14:58:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:mime-version:in-reply-to:date:from:to :cc:subject:date:message-id:reply-to; bh=bNJVmGuycnpUTrbk2mZ7qWFTvJYs0/4vpATsaQymfNw=; b=L3YXCzmsUJ8KXkqJ/3Jl6vnyJlhWpcKee+2tcjjiMSxa3Peotsq0kUKxvTjekYr4lN 02hSZ/KCIMLJnmGBM+a3Yd9yI5sJxj/TAVRncnQV65fQ7xaAe0Asfs6Hr8sr8N+DT10C qBfCUhFaeXlB9p3Drgbony9iNUBqCjwXM1gMnv0mAA3mydHLZURg2KxRBt26E+yCcbpu OloHQUkeQL/phf4266y1UAJPANYoRNH0JLlwhd2idcx44f2WQDueMzQRRTzpBRZVT2bT i4eNsc/ZiVnBQ229wK/udQkHVgmmdrlyB/cTlZCOPpvPiwrseSuzq62SJjwOkFFbb3WG 6dBQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:mime-version:in-reply-to:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=bNJVmGuycnpUTrbk2mZ7qWFTvJYs0/4vpATsaQymfNw=; b=lSy1OX3MRURYMo9wEXgIxr3KPKG8TJ8UcW8fntZKK0iDajm48PCD4On0Uhz9BAbKIb GDptCdEXyPgEIcrb3wIR/SPlPS4J3kQx+0GsX2ibhQzJ9T/URwg+d9AiKPWIcmpY9jUF lqNbvnLoakZ7EUlXiVuyeAG5Ri0kW6m4Cb4JEp8jwaCi0TZOJHLo1ms7biCSq74gYCV5 SlVBpd3wYocXPErx97OlYcsYPgPidz6hchCeFrTO551B9WpQFdExJzFPBAb6H/0p4es1 icPQ2QJO+cMraIWnHd7zM8tL+VGubxb+q5gb332cZU/+2Jo6CnUNNUzMxNZ/TronrI5E fXFA== X-Gm-Message-State: ACrzQf12nCvgW2owuWZ29Xv9dSuVKtwYwWtAOip80tVYNzlFYkmyGVQb gdMqQHlKpXZvkWlzkdqMXpaVIIGps9UkMzY8dJDg X-Google-Smtp-Source: AMsMyM7Cggr5Q1MlYFTk4HXddKAQg8uKLbEOscrFMeuRLuuDejsD2+5+B9GERXTWwB8cd20XaQs/MsWJ367CY0YjItU9 X-Received: from twelve4.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:437a]) (user=jonathantanmy job=sendgmr) by 2002:a17:90a:c986:b0:205:f08c:a82b with SMTP id w6-20020a17090ac98600b00205f08ca82bmr51056pjt.1.1664488691142; Thu, 29 Sep 2022 14:58:11 -0700 (PDT) Date: Thu, 29 Sep 2022 14:58:08 -0700 In-Reply-To: Mime-Version: 1.0 X-Mailer: git-send-email 2.38.0.rc1.362.ged0d419d3c-goog Message-ID: <20220929215808.1438418-1-jonathantanmy@google.com> Subject: Re: [PATCH v2 9/9] bundle-uri: fetch a list of bundles From: Jonathan Tan To: Derrick Stolee via GitGitGadget Cc: Jonathan Tan , git@vger.kernel.org, gitster@pobox.com, me@ttaylorr.com, newren@gmail.com, avarab@gmail.com, mjcheetham@outlook.com, steadmon@google.com, Glen Choo , Teng Long , Derrick Stolee Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org "Derrick Stolee via GitGitGadget" writes: > +static int download_bundle_to_file(struct remote_bundle_info *bundle, void *data) > +{ > + int res; > + struct bundle_list_context *ctx = data; > + > + if (ctx->mode == BUNDLE_MODE_ANY && ctx->count) > + return 0; > + > + res = fetch_bundle_uri_internal(ctx->r, bundle, ctx->depth + 1, ctx->list); > + > + /* > + * Only increment count if the download succeeded. If our mode is > + * BUNDLE_MODE_ANY, then we will want to try other URIs in the > + * list in case they work instead. > + */ > + if (!res) > + ctx->count++; > + return res; > +} So this returns nonzero if a download fails... > +static int download_bundle_list(struct repository *r, > + struct bundle_list *local_list, > + struct bundle_list *global_list, > + int depth) > +{ > + struct bundle_list_context ctx = { > + .r = r, > + .list = global_list, > + .depth = depth + 1, > + .mode = local_list->mode, > + }; > + > + return for_all_bundles_in_list(local_list, download_bundle_to_file, &ctx); > +} ...and for_all_bundles_in_list does not proceed with the rest of the loop if any callback invocation returns nonzero. Don't we need to continue retrying the others if the mode is ANY? > +static int attempt_unbundle(struct remote_bundle_info *info, void *data) > +{ > + struct attempt_unbundle_context *ctx = data; > + > + if (info->unbundled || !unbundle_from_file(ctx->r, info->file)) { > + ctx->success_count++; > + info->unbundled = 1; > + } else { > + ctx->failure_count++; > + } > + > + return 0; > +} Do we need to handle the case in which a file is missing but it's expected because the mode is ANY and another file was successfully downloaded? > +static int unbundle_all_bundles(struct repository *r, > + struct bundle_list *list) > +{ > + int last_success_count = -1; > + struct attempt_unbundle_context ctx = { > + .r = r, > + }; > + > + /* > + * Iterate through all bundles looking for ones that can > + * successfully unbundle. If any succeed, then perhaps another > + * will succeed in the next attempt. > + */ > + while (last_success_count < ctx.success_count) { > + last_success_count = ctx.success_count; > + > + ctx.success_count = 0; > + ctx.failure_count = 0; > + for_all_bundles_in_list(list, attempt_unbundle, &ctx); I think it would have been clearer if the invocation to for_all_bundles_in_list were to stop early if a bundle has been successfully unbundled, and then you can just run this loop n times, instead of needing to reset the success count each time in order to check that the latest count is more than the prior one. But this works too. [snip tests] I see that there are ALL tests, but could we have an ANY test as well?