Re: [PATCH 0/7] limit the usage of the default remote "origin" to the minimum

From: Jeff King <peff@peff.net>
Date: Apr 29, 2008, 19:35 UTC
Message-ID: <20080429193536.GA19340@sigill.intra.peff.net>
In-Reply-To: <cover.1209391614.git.bonzini@gnu.org>

On Mon, Apr 28, 2008 at 05:23:53PM +0200, Paolo Bonzini wrote:

Show 7 quoted lines

> Patches 1 to 4 deal with "git push".  The problem here is that the
> "magic" default operation ("git push sends to origin all refs that
> are already present there"), as of new, cannot be explained in the
> configuration, and this is fixed by patches 1 and 2.  Patch 1 adds a
> special push refspec meaning "consider all refs already on the remote",
> and patch 2 makes git-clone add one such line when it creates the
> origin remote.

I like patches 1 and 2. The principle of "remove defaults from code, and put them into the automatically generated config file" makes sense to me. It gives users an easy place to look to understand and change such behavior. So even without the rest of the patches, I think this is an improvement.

> Patch 3 could also be split out of this series.  It adds to "git-remote
> add" a new --push option, that sets up a push refspec for the newly
> created remote.

This feels a little wrong, since we treat push and fetch lines differently. That is, I can add just a fetch ("git remote add"), or both ("git remote add --push"), but not just a push. It seems like the concepts should be orthogonal to "git remote" (as they are in the config file).

Show 12 quoted lines

> Patch 4 is a reworking of my previous patch.  Instead of having "git
> push" push to "all mirrors plus the magic origin branch", it will
> push to "all remotes having a push refspec".  In the future, this
> will always include the origin branch because of patch 2, while
> right now the origin branch is still used if no remote has a
> push refspec (for backwards compatibility -- more discussion in the
> patch log message).
>
> This patch may cause incompatibilities to be warned about in
> the release notes.  Luckily, these incompatibilities only affect
> users that already know their way in .git/config, because no porcelain
> command creates push refspecs.

I think you understand the compatibility issues, but I think it needs to be not "warned in the release notes" but "warned in the release notes, followed by a period of adjustment, and then the change". And maybe it would even make sense to wait for a larger-number version change (like 1.6.0).

Show 6 quoted lines

> Patches 5 and 6 affect "git pull".  Pulling uses the magic origin branch
> less often, because branch.*.remote already establishes a saner default.
> Therefore, here we can actually warn and deprecate the misfeature for
> real (patch 6).  Patch 5 however is still needed, so that "git pull"
> itself decides when to use the magic "origin" branch instead of making
> the decision in "git fetch".

I agree that patch 5 is a reasonable cleanup, regardless.

I'm not sure about patch 6. What are the cases that can trigger this? I assume people still with ancient .git/remotes files? Are those actually deprecated?

Show 6 quoted lines

> Finally, patch 7 affects "git fetch".  As hinted earlier, I chose to
> duplicate the functionality already present in "git remote update"
> (for the case where remotes are not grouped), but the code is much
> simpler so there is no incentive (at least for now) in borrowing code
> from "git remote update" into "git fetch"---actually, as a cleanup
> it is maybe possible to borrow code in the other direction.

Again, I like this behavior just fine, but it probably needs a warning period and a major version bump.

> "BTW, I find myself decreasingly using "git fetch" in favor of "git
>  remote update" which handles fetching from multiple remotes."
> 	The series remove the need to switch from "git fetch" to
> 	"git remote update" as one learns the power of DVCS.

This was from me, and I do like this behavior better.

-Peff