| Message ID | 20250320014646.2899791-3-jltobler@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clone: suppress unexpected advice message during clone | expand |
On Wed, Mar 19, 2025 at 08:46:46PM -0500, Justin Tobler wrote: > diff --git a/builtin/clone.c b/builtin/clone.c > index 9eb66234bc..3b166b05e3 100644 > --- a/builtin/clone.c > +++ b/builtin/clone.c > @@ -1523,7 +1523,7 @@ int cmd_clone(int argc, > } > > remote_head = find_ref_by_name(refs, "HEAD"); > - remote_head_points_at = guess_remote_head(remote_head, mapped_refs, 0, 0); > + remote_head_points_at = guess_remote_head(remote_head, mapped_refs, 0, 1); > > if (option_branch) { > our_head_points_at = find_remote_branch(mapped_refs, option_branch); Makes sense. You don't have control over the branch name anyway when cloning, so it's nonsensical to print that advise. Another subsequent step could be to turn the `advise()` into `advise_if_enabled()`, but that change isn't really needed for git-clone(1) because there wouldn't ever be a reason to print it. Do we want to add a test somewhere that demonstrates that we don't print the advise anymore? Thanks for fixing this! Patrick
On 25/03/20 06:13AM, Patrick Steinhardt wrote: > On Wed, Mar 19, 2025 at 08:46:46PM -0500, Justin Tobler wrote: > > diff --git a/builtin/clone.c b/builtin/clone.c > > index 9eb66234bc..3b166b05e3 100644 > > --- a/builtin/clone.c > > +++ b/builtin/clone.c > > @@ -1523,7 +1523,7 @@ int cmd_clone(int argc, > > } > > > > remote_head = find_ref_by_name(refs, "HEAD"); > > - remote_head_points_at = guess_remote_head(remote_head, mapped_refs, 0, 0); > > + remote_head_points_at = guess_remote_head(remote_head, mapped_refs, 0, 1); > > > > if (option_branch) { > > our_head_points_at = find_remote_branch(mapped_refs, option_branch); > > Makes sense. You don't have control over the branch name anyway when > cloning, so it's nonsensical to print that advise. Another subsequent > step could be to turn the `advise()` into `advise_if_enabled()`, but > that change isn't really needed for git-clone(1) because there wouldn't > ever be a reason to print it. As you mentioned, in this specific situation printing the advice doesn't make much sense. It would probably be a good idea to allow this message to be suppressed from other call sites if requested to do so though. I'll add another patch that turns it into `advise_if_enabled()` to support this. > Do we want to add a test somewhere that demonstrates that we don't print > the advise anymore? Ya, I'll go ahead and include this in my next version. Thanks, -Justin
diff --git a/builtin/clone.c b/builtin/clone.c index 9eb66234bc..3b166b05e3 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1523,7 +1523,7 @@ int cmd_clone(int argc, } remote_head = find_ref_by_name(refs, "HEAD"); - remote_head_points_at = guess_remote_head(remote_head, mapped_refs, 0, 0); + remote_head_points_at = guess_remote_head(remote_head, mapped_refs, 0, 1); if (option_branch) { our_head_points_at = find_remote_branch(mapped_refs, option_branch);
In 199f44cb2ead (builtin/clone: allow remote helpers to detect repo, 2024-02-27), clones started partially initializing the refdb before executing the remote helpers by creating a HEAD file and "refs/" directory. This has resulted in some scenarios where git-clone(1) now prints the default branch name advice message where it previously did not. A side-effect of the HEAD file already existing, is that computation of the default branch name is handled later in execution. This matters because prior to 97abaab5f6 (refs: drop `git_default_branch_name()`, 2024-05-17), the default branch value would be computed during its first execution and cached. Subsequent invocations would simply return the cached value. Since the next `git_default_branch_name()` call site, which is invoked through `guess_remote_head()`, is not configured to suppress the advice message, computing the default branch name results in the advice message being printed. Configure `guess_remote_head()` to suppress the advice message, restoring the previous behavior. Signed-off-by: Justin Tobler <jltobler@gmail.com> --- builtin/clone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)