Patrick Steinhardt <ps@xxxxxx> writes:
> +repositories at all and most users aren't even aware of these mechanisms. They
> +have been deprecated for almost 20 years and 14 years respectively, and we are
> +not aware of any reason why anybody would want to use these mechanisms.
I am aware of one reason why some folks may prefer being able to say
$ ls .git/branches/*pattern*
$ echo "$URL#branch" >".git/branches/$shortname"
$ git fetch $shortname
over the configuration file based mechanism, especially when they
have to deal with dozens of remotes that change the branch name to
be pulled from. And as I already said the above while reviewing the
previous round of this series, _we_ are now aware of it.
I however am in favor of deprecating and removing the support, but
that is not because I am not aware how useful they could be. I am
and we are aware, but we haven't heard anybody jumping up and down
to advocate for its undeprecation for a long time, and that is why
I am personally OK with this removal.
> branches::
> - A slightly deprecated way to store shorthands to be used
> + A deprecated way to store shorthands to be used
> to specify a URL to 'git fetch', 'git pull' and 'git push'.
> A file can be stored as `branches/<name>` and then
> 'name' can be given to these commands in place of
> @@ -162,7 +162,8 @@ branches::
> and not likely to be found in modern repositories. This
> directory is ignored if $GIT_COMMON_DIR is set and
> "$GIT_COMMON_DIR/branches" will be used instead.
> -
> ++
> +Git will stop reading remotes from this directory in Git 3.0.
>
> hooks::
> Hooks are customization scripts used by various Git
> @@ -238,6 +239,8 @@ remotes::
> and not likely to be found in modern repositories. This
> directory is ignored if $GIT_COMMON_DIR is set and
> "$GIT_COMMON_DIR/remotes" will be used instead.
> ++
> +Git will stop reading remotes from this directory in Git 3.0.
OK.
> 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?
> 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.
> + "you cannot, please let us know you still use it by sending\n"
I do not think we care to receive a piece of e-mail that only says
"I still use it". We may want to learn _why_ they cannot switch
away, though.
The same comment applies to the other side.
Everything else in this patch looked superb.
Thanks.
