git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] remote: prefetch config

From
Shubham Kanodia <shubham.kanodia10@gmail.com>
Date
Sep 5, 2024, 02:51 UTC
Message-ID
<CAG=Um+1P8+SJa5aYhuuQ0fVgB0kis0MzMA94oRoBm=SNtROi+w@mail.gmail.com>
In-Reply-To
<52f6da10-e39a-4a2d-bd9a-5739a49235c2@gmail.com>
On Thu, Sep 5, 2024 at 7:38 AM Derrick Stolee <stolee@gmail.com> wrote:
Show 52 quoted lines
>
> On 9/4/24 4:55 PM, Junio C Hamano wrote:
> > Derrick Stolee <stolee@gmail.com> writes:
>
> >>> +           # Run maintenance prefetch task
> >>> +           GIT_TRACE2_EVENT="$(pwd)/prefetch.txt" git maintenance run --task=prefetch 2>/dev/null &&
> >>> +
> >>> +           # Check that remote1 was not fetched (prefetch=false)
> >>> +           test_subcommand ! git fetch remote1 --prefetch --prune --no-tags \
> >>> +                   --no-write-fetch-head --recurse-submodules=no --quiet \
> >>> +                   <prefetch.txt &&
> >>
> >> I'm happy to see this use of test_subcommand to validate the behavior
> >> of this patch!
> >
> > I found it a bit disturbing that the pattern is overly specific.
> >
> > The only thing we are interested in is that we are not fetching from
> > remote1, so it _should_ suffice if we could write
> >
> >       test_subcommand ! git fetch remote1 <prefetch.txt &&
> >
> > to avoid being tied to how the current version of Git happens to
> > pass these command line option flags and the order it does so.
> >
> > Looking at the implementation of test_subcommand, it seems that we
> > cannot quite do that (it assumes that the pattern it assembles out
> > of the parameters are to match the full argument list used in
> > invocation, enclosing them in a single [] pair and without giving
> > the caller an easy way to sneak wildcards like ".*" in), which is
> > sad.
> I agree the ergonomics of the test_subcommand helper is a bit poor
> (and not this patch author's fault). The trickiest part is the
> negative case, as in this highlighted one. It's hard to read from
> this if the subcommand wasn't found because the argument list is
> too specific and doesn't match the exact arguments.
>
> It helps that the same options are given for the other, positive
> tests. But maybe that could be a hint as to how to make this test
> a bit cleaner: make a variable describing the "uninteresting"
> arguments. Something like...
>
>         args="--prefetch --prune --no-tags \
>               --no-write-fetch-head --recurse-submodules=no --quiet" &&
>
>         test_subcommand ! git fetch remote1 $args <prefetch.txt &&
>         test_subcommand git fetch remote2 $args <prefetch.txt &&
>         test_subcommand git fetch remote3 $args <prefetch.txt &&
>
> Thanks,
> -Stolee
>
Agree with both the suggestions here. Updated my patch.
Previous: Derrick StoleeNext: Junio C Hamano
Message 5 of 16 in “remote: prefetch config”
  1. remote: prefetch configShubham Kanodia via GitGitGadget, Sep 4, 2024
  2. Derrick StoleeSep 4, 2024
  3. Junio C HamanoSep 4, 2024
  4. Derrick StoleeSep 5, 2024
  5. Shubham KanodiaSep 5, 2024
  6. Junio C HamanoSep 5, 2024
  7. remote: prefetch configShubham Kanodia via GitGitGadget, Sep 5, 2024
  8. Junio C HamanoSep 5, 2024
  9. Shubham KanodiaSep 5, 2024
  10. Junio C HamanoSep 5, 2024
  11. Shubham KanodiaSep 5, 2024
  12. Junio C HamanoSep 5, 2024
  13. remote: prefetch configShubham Kanodia via GitGitGadget, Sep 5, 2024
  14. remote: prefetch configShubham Kanodia via GitGitGadget, Sep 5, 2024
  15. Junio C HamanoSep 5, 2024
  16. Shubham KanodiaSep 6, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.