Re: [RFC/PATCH] remote: add new sync command

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Nov 12, 2011, 22:07 UTC
Message-ID: <CAMP44s06p+KyJAu4ddiCa8CFRq5eogbqxxJU16Z-SUb3GSp67Q@mail.gmail.com>
In-Reply-To: <20111111181352.GA16055@sigill.intra.peff.net>

On Fri, Nov 11, 2011 at 8:13 PM, Jeff King <peff@peff.net> wrote:

Show 14 quoted lines

> On Fri, Nov 11, 2011 at 02:30:48PM +0200, Felipe Contreras wrote:
>
>> > Doesn't --all mean all refs, including tags and branches?
>>
>> Nope, only branches, try it. I also found it strange. And what is
>> more, you can't use --all and --tags at the same time. Totally
>> strange.
>
> Yeah, you're right. Sorry for my confusion.
>
> So in that sense, it is poorly named, and "--branches" (or "--heads")
> would be more accurate. At the same time, it is probably more likely
> what the user wants to do (you almost never want to push "refs/remotes",
> for example).

But you do want to push tags, and --all --tags doesn't sound right; if I'm pushing everything, why do I specify I want to push more stuff. And then, why it --all --tags disallowed?

> So I am a little hesitant to suggest changing it, even
> with a warning and deprecation period.

It is confusing and wrong, what more reason do you need?

Show 8 quoted lines

>> And yes, in this particular use-case that's what I am trying to avoid,
>> but in other use-cases (like creating a new repo and pushing
>> *everything*), a *true* --all would be nice.
>
> Right. It looks like that is just spelled "--mirror" (which gives you
> pruning also), or "refs/*:refs/*" (without pruning). The latter is even
> more flexible, as you could do "refs/*:refs/foo/*" to keep several
> related backups in one upstream repo.

So, we agree that --all is the same as 'refs/heads/*'. Therefore we already have this mixture of refspecs and options.

> Just to get back to the original patch for a second: are we in agreement
> that what you want to do with "sync" is almost possible now (modulo a
> separate --prune option), and that there is a separate issue of making
> friendlier syntax for a few common refspecs?

Yes.

Show 18 quoted lines

>> > We could add syntactic sugar to make
>> > --branches mean "refs/heads/*". But I do worry that pseudo-options like
>> > that just end up creating more confusion (I seem to recall there being a
>> > lot of confusion about "--tags", which is more or less the same thing).
>>
>> But it's not, that could explain part of the confusion. I think these
>> would be totally intuitive.
>>
>>  --branches
>>  --tags
>>  --other
>>  --all
>>  --update
>>  --prune
>
> My problem with them syntactically is that you have option-looking
> things that are really not flags, but rather syntactic placeholders for
> refspecs.

We already have those: --all -> 'ref/heads/*', --tags -> 'refs/tags/*'.

Show 18 quoted lines

> So you have:
>
>  git push --prune remote --branches
>
> which looks wrong from the perspective of usual option-parsing rules.
> And does:
>
>  git push remote --prune --branches
>
> work? Or:
>
>  git push --prune --branches remote
>
> ?
>
> I think we could make them all work if we want. But somehow the syntaxes
> just look wrong to me. Using a non-dashed placeholder for special
> refspecs makes more sense to me like:

I don't see any problem with making them all work.

Show 11 quoted lines

>  git push --prune remote BRANCHES
>
> and then it really is just a special way of spelling "refs/heads/*". But
> then, I also think it's good for users to understand that the options
> are refspecs, and what that means. It's not a hard concept, and then
> when they inevitably say "how can I do BRANCHES, except put the result
> somewhere else in the remote namespace", it's a very natural extension
> to learn about the right-hand side o the refspec.
>
> Of course I also think BRANCHES looks ugly, and people should just learn
> "refs/heads/*".

Look, I'm all in favor of people learning stuff, but I have been involved in Git since basically day 1, and up to this day I was (am?) not familiar with refspecs, I don't use them regularly, and never really had a need to, and that's fine. People are already complaining about the learning curve of git, and what you are suggesting is that:

Instead of doing: % git push remote --branches --tags

They should do: % git push remote 'refs/heads/*' 'refs/tags/*'

I disagree, I think refspecs should remain as plumbing, and there must be a porcelain way to achieve the options I proposed.

> I dunno. Maybe my brain is fried from knowing too much about how git
> works. I don't have much of an "ordinary user" perspective anymore.

Maybe :)

Show 18 quoted lines

>> But what about 'git fetch'? You didn't comment anything. I think we
>> should try to be consistent in these imaginary future options, maybe
>> to devise a transition, or at least to identify good names for the new
>> options.
>
> Yeah, fetch makes it harder because the options may have subtly
> different meanings. Using non-option placeholders would work around
> that. You would still have options for pruning, which to me is not
> really a refspec, but an option that acts on the refspecs.
>
> From the list in your previous email, you wrote:
>
>> --prune -> rename to --prune-tracking?
>> --prune-local (new; prune local branches that don't exist on the remote)
>
> I think --prune wouldn't need renaming. If you fetch into tracking
> branches, then pruning would prune tracking branches. If you fetch as a
> mirror (refs/*:refs/*), then you would prune everything.

I'm not going to investigate the subtleties of these different setups, I'm going to put my common user hat and ask; how do I fetch as a mirror?

> And "--prune-local" doesn't seem like a fetch operation to me. Either
> you are mirroring, and --prune already handles it as above. Or you are
> interested in getting rid of branches whose upstream has gone away. But
> that's not a fetch operation; that's a branch operation.

This would make things more confusing to the user.

Say on one side I do this push? % git push test --prune 'refs/heads/*' 'refs/tags/*'

What do I do in the other side to synchronize the repo? % git fetch test --prune-local 'refs/heads/*:refs/heads/*' 'refs/tags/*:refs/tags/*'

I would prefer this of course: % git fetch test --all --prune-local

But you are saying it should be: % git fetch test 'refs/heads/*:refs/heads/*' 'refs/tags/*:refs/tags/*' % git branch --prune-remote test

That doesn't sound right to me; mixing branch operations with a specific remote?

Again, I think conceptually it's much easier to think about these operations to be related to a specific remote, sure, fetch and push mostly deal with specific remotes, but even more 'git remote'.

-- 
Felipe Contreras