Skip to content

repository: Remove harmful `name` parameter in FetchRemote

Patrick Steinhardt requested to merge pks-repo-fetch-remote-drop-name into master

The FetchRemote RPC currently accepts either a remote name or a Remote message as input: in the first case, it will try to fetch from a preexisting remote, while in the latter case it will create an ad-hoc remote and fetch from that one. Confusingly though, the user is supposed to pass a remote name even for the ad-hoc repository: this is the name used to create the remote as.

This remote name conveys the sense that it's used as a persistent remote, but that's not the case: the remote is getting created and deleted in the same call. Which effectively means that it doesn't provide much of a benefit to the user in the first place, because why would he care for the remote name if it doesn't persist anyway? The only usecase one might think of is that it's done in order to set up the default fetchspec of refs/*:refs/remotes/$name/*. But quite the reverse: all refs which get fetched into the remote's namespace will get deleted after the call. This is completely unexpected and quite harmful because it may break mirror fetches it the remote happens to have refs in there, too. Furthermore, if a persistent remote previously existed from which we want to fetch, the caller will be sad because we simply delete it.

Fix the issue by ignoring the user-provided name: instead, we now generate a random remote name which is safe to get deleted afterwards.

Part of #1773 (closed)

Edited by Toon Claes

Merge request reports