Re: [PATCH v2 3/8] fetch: fix missing from-reference when fetching HEAD:foo
- From
Glen Choo <chooglen@google.com>
- Date
- Apr 27, 2023, 17:26 UTC
- Message-ID
- <kl6lo7n9me29.fsf@chooglen-macbookpro.roam.corp.google.com>
- In-Reply-To
- <d1fb6eeae76bc550030475e40868b90a2ca6ce8c.1682593865.git.ps@pks.im>
For v2, I've only read through this patch for now. I'm still formulating my thoughts on the output format and the CLI option name.
Patrick Steinhardt <ps@pks.im> writes:
Show 25 quoted lines
> `store_updated_refs()` parses the remote reference for two purposes: > > - It gets used as a note when writing FETCH_HEAD. > > - It is passed through to `display_ref_update()` to display > updated references in the following format: > > ``` > * branch master -> master > ``` > > In most cases, the parsed remote reference is the prettified reference > name and can thus be used for both cases. But if the remote reference is > HEAD, the parsed remote reference becomes empty. This is intended when > we write the FETCH_HEAD, where we skip writing the note in that case. > But it is not intended when displaying the updated references and would > cause us to miss the left-hand side of the displayed reference update: > > [...] > > As the return value of `prettify_refname()` would do the correct thing > for us in all three cases, we can fix the bug by passing through the > full remote reference name to `display_ref_update()`, which learns to > call `prettify_refname()`. At the same time, this also simplifies the > code a bit.
This rewritten section explains the bug and the fix very nicely :) Thanks!
Show 21 quoted lines
> diff --git a/t/t5574-fetch-output.sh b/t/t5574-fetch-output.sh > index 0e45c27007..b9dcdade63 100755 > --- a/t/t5574-fetch-output.sh > +++ b/t/t5574-fetch-output.sh > @@ -54,6 +54,31 @@ test_expect_success 'fetch compact output' ' > test_cmp expect actual > ' > > +test_expect_success 'fetch output with HEAD and --dry-run' ' > + test_when_finished "rm -rf head" && > + git clone . head && > + > + git -C head fetch --dry-run origin HEAD >actual 2>&1 && > + cat >expect <<-EOF && > + From $(test-tool path-utils real_path .)/. > + * branch HEAD -> FETCH_HEAD > + EOF > + test_cmp expect actual && > + > + git -C head fetch origin HEAD >actual 2>&1 && > + test_cmp expect actual &&
Maybe rename the test to just 'fetch output with HEAD'? The 'and --dry-run' part seems completely obsolete.