Re: [PATCH] fix simple deepening of a repo

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> wrote:
> "Shawn O. Pearce" <spearce@xxxxxxxxxxx> writes:
> > The client knows the *name* of the ref, but not the SHA-1 the ref is
> > currently valued at.  Thus when the client knows it wants a certain
> > ref by name, it needs to send a "want " line to the server that would
> > give it whatever that ref currently points at.  Unfortunately since we
> > have not obtained that value yet, we are stuck.
> 
> That could be something you can fix in the out-of-band procedure Gerrit
> uses (you let the client learn both name and value offline, and then the
> client uses that value on "want" line).

Well, we're trying to reduce out-of-band things with Gerrit.

Its bad enough that Gerrit doesn't use git am and git send-email
to slug around changes for discussion.  As it is we're an island
among the git world, *despite* the fact that Gerrit speaks the git
protocol natively and you can push directly to it, avoiding the
send-email SMTP nonsense many folks run into.
 
> However, even if we limit the discussion to Gerrit, you would need an
> updated client that can be called with the out-of-band information
> (i.e. "we know that changes/88/4488/2 points at X, so use X when
> requesting") when talking with such an updated server.

Yes, exactly.  Existing clients wouldn't send an arbitrary want
request, even if the server had a whitelist of objects it would
allow to be requested.

One reason why Gerrit publishes pending changes with ref names is to
make it easier for any user to obtain the proposed change locally.
Its hard to beat `git fetch URL blah`, that's even easier than
"save to mbox, git am mbox".
 
> So I think that expand-refs is a much nicer general solution than just
> "server side is configured to hide but still allow certain refs", and
> client updates cannot be avoided.

Yes, I agree.  Given 20/20 hindsight, its the way the protocol
should have been implemented:

  C: 0014expand refs/heads/*
  C: 0013expand refs/tags/*
  C: 0000

  S: ...refs/heads/master
  S: ...refs/heads/next
  S: ...refs/tags/v1.0
  S: 0000

This would have permitted clients doing `git pull URL for-linus` to say:

  C: 0011expand for-linus
  C: 0000

  S: ...refs/heads/for-linus
  S: ...refs/remotes/k26/for-linus
  S: 0000

and thus significantly narrow the scope of what they are shown when
they connect for a given ref.

> > The problem with this is servers which are sending this expand-refs
> > tag have hidden certain namespaces from older clients.  Those names
> > can't be seen by older git clients, unless the user does an upgrade.
> 
> I do not think "generally hidden, but if you need to know you are allowed
> to peek" is much of a problem.  You do not do that for regular refs, only
> for "on-demand-as-needed" type things.  If we are going to make extensive
> use of notes on commits to give richer annotations, I suspect notes
> hierarchy could be hidden by default in a similar way.

After sleeping on it, I'm OK with hiding some refs from older clients.

Sometimes things evolve, and you should just update your software
to keep up with them.  If you really want the "hidden refs" that
Gerrit advertises, you should install a newer client.

We could consider supporting a legacy option through upload-pack,
such as:

  git fetch --upload-pack='git-upload-pack --expand refs/changes/' URL

which tells the remote side to additionally expand those refs during
the initial advertisement.  Then users have an escape hatch if:

* They know the remote is new enough to hide refs;
* They suspect the remote is hiding refs;
* They received an out-of-band notification telling this;
* They have an older client which doesn't support expanding refs;
* They cannot upgrade said client yet;

I'm thinking about writing an RFC patch for this today for git.git.
I think the expand refs feature neatly solves a number of problems
for me in Gerrit.  But I'm really hoping its not the only set of
repositories that would benefit from such a feature, because if so,
its not worth the headache of the protocol change.

-- 
Shawn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]