On Tue, Jan 21, 2025 at 01:25:56PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
> > diff --git a/builtin/remote.c b/builtin/remote.c
> > index 1ad3e70a6b..e565b2b3fe 100644
> > --- a/builtin/remote.c
> > +++ b/builtin/remote.c
> > @@ -640,10 +640,12 @@ static int migrate_file(struct remote *remote)
> > strbuf_addf(&buf, "remote.%s.fetch", remote->name);
> > for (i = 0; i < remote->fetch.nr; i++)
> > git_config_set_multivar(buf.buf, remote->fetch.items[i].raw, "^$", 0);
> > +#ifndef WITH_BREAKING_CHANGES
> > if (remote->origin == REMOTE_REMOTES)
> > unlink_or_warn(git_path("remotes/%s", remote->name));
> > else if (remote->origin == REMOTE_BRANCHES)
> > unlink_or_warn(git_path("branches/%s", remote->name));
> > +#endif /* WITH_BREAKING_CHANGES */
> > strbuf_release(&buf);
>
> Interesting. I wonder if our new warning should talk about whatever
> end-user facing interface that triggers this code path. It would
> help them wean themselves away from the old interface, no?
Not quite sure that I understand what you're saying. Is it that we
should tell whether we were reading from "branches/" or "remotes/"? If
so we already do that.
> > diff --git a/remote.c b/remote.c
> > index 10104d11e3..5feb0ae886 100644
> > --- a/remote.c
> > +++ b/remote.c
> > @@ -293,6 +293,7 @@ static void add_instead_of(struct rewrite *rewrite, const char *instead_of)
> > rewrite->instead_of_nr++;
> > }
> >
> > +#ifndef WITH_BREAKING_CHANGES
> > static const char *skip_spaces(const char *s)
> > {
> > while (isspace(*s))
> > @@ -308,6 +309,13 @@ static void read_remotes_file(struct remote_state *remote_state,
> >
> > if (!f)
> > return;
> > +
> > + warning(_("Reading remote from \"remotes/%s\", which is nominated\n"
> > + "for removal. If you still use the \"remotes/\" directory\n"
> > + "it is recommended to migrate to config-based remotes. If\n"
>
> Do we have a way to concisely say "how" to do this? If I am reading
> the caller of migrate_file() in builtin/remote.c, it would be
>
> $ git remote mv foo foo
>
> for any foo in .git/remotes/* or .git/branches/* hierarchy?
>
> Of course they may be an ancient leftover file that the user even no
> longer is aware of having, in which case
>
> $ rm .git/remotes/foo
>
> might be an OK answer, but even then
>
> $ git remote rm foo
>
> would probably be more appropriate.
Very good idea indeed. We also have tests that exercise this behaviour
in t5505 (with the slight exception that it is `git remote rename`, not
`git remote mv`).
Patrick
