Re: [RFD] making separate-remote layout easier to use

4 messages, 3 authors, 2016-08-11 · open the first message on its own page

Re: [RFD] making separate-remote layout easier to use

From: Junio C Hamano <hidden>
Date: 2016-08-11 20:24:25

Shawn Pearce [off-list ref] writes:
So now we're at a point of:

 * how do we get this branch information from the remote?
 * how does the remote store this branch information?

I'm leaning towards the repo config for the latter, with say:

	[branch "pu"]
		rewinds = true
		description = This branch contains some proposed updates to git.\n\
	It rewinds often, as different updates are proposed or merged into 'next'.

but looking at that think its rather horrible looking.  :-)

However for the former it may be useful if the client can download
the repo config from the remote repository, such as to fetch
the branch description data.
For the former, I was hoping that we could do a new action
against remote repository that is "get remote information". Just
like we have "ls-remote" as a protocol neutral front-end to get
what git native protocol would give "peek-remote", git native
protocol would have get-repository-info action to transfer this
information, and http and dumb transports do a file download
from a known file, just like they download files prepared with
update-server-info.  Most likely, that get-repository-info is
just a glorified single file transfer and nothing more.

As an implementation detail, I think it is one possibility to
store this information in the config of the repository being
cloned from.  We should be able to use repo-config to
parse that file in config-like syntax to extract what we would
want, so we do not have to come up with a new syntax nor a new
parser, which is a big plus.

	Side note: repo-config, especially the updating side of
	it, is one of the programs I find the most brittle in
	the whole system, and it always makes me hesitate when
	somebody proposes to use config file in any important
	way read-write for this reason.  But this "reading out
	per-branch information from the file" is read-only so I
	would not worry too much about it.

Strictly speaking, however, the config file is a wrong place to
store it.  For one thing it has core.sharedrepository and
receive.denynonfastforwards that are true configuration to
control the behaviour of git _at_ _the_ _repository_ the
configuration is at.  The new "branch property" are primarily to
help the other end, and the "filtering rewinding ones" we want
at the clone/fetch side wants that information only and not
interested in the true configuration information at that
repository.

So my preference is:

 * store it in a fixed filename under $GIT_DIR/$something, in a
   format that is similar to the true config file.

 * Dumb transports would just do a file transfer, and we add a
   new command (like "upload-pack", "upload-archive") for git
   native transport to read that file.  We might want to have a
   protocol neutral wrapper (just like "ls-remote").

 * Cloners and fetchers would do the file transfer and use
   repo-config to read from that transferred file to find out
   which are rewinding branches.



Re: [RFD] making separate-remote layout easier to use

From: Jakub Narebski <hidden>
Date: 2016-08-11 19:18:06

Junio C Hamano wrote:
 * Subsequent fetch, when the glob is not '+' variant, would
   download the config from the remote in order to see which are
   marked as "rewinds", and ignores the branches that are marked
   as such.

I am a bit unhappy that subsequent fetches have to re-read the
remote config every time.  I can sort-of-see we can cram the
"this is expected to be rewound" information as part of
peek-remote exchange to avoid the overhead, but I do not think
it is easily doable for dumb transports without breaking the
backward compatibility.
Perhaps below the "Pull: refs/heads/*:refs/remotes/origin/*" (or config
equivalent) we should list (in some format) branches which are not to be
fetched becaus they don't fast forward. One option, namely
  Pull:-refs/heads/pu:refs/remotes/origin/pu
(or config equivalent)comes to mind. It has the advantage that when we
decide to fetch this branch, it would be as easy as changing '-' to '+'.
Or we can use '!' instead of '-' if you think it is more readable (probably
is).
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [RFD] making separate-remote layout easier to use

From: Shawn Pearce <hidden>
Date: 2016-08-11 19:45:30

Junio C Hamano [off-list ref] wrote:
Shawn Pearce [off-list ref] writes:
quoted
So now we're at a point of:

 * how do we get this branch information from the remote?
 * how does the remote store this branch information?
For the former, I was hoping that we could do a new action
against remote repository that is "get remote information".
Yes, that is completely sane.  :)
 
As an implementation detail, I think it is one possibility to
store this information in the config of the repository being
cloned from.  We should be able to use repo-config to
parse that file in config-like syntax to extract what we would
want, so we do not have to come up with a new syntax nor a new
parser, which is a big plus.
Also quite sane.  Nobody wants to write new code if we don't really
have to. :)
	Side note: repo-config, especially the updating side of
	it, is one of the programs I find the most brittle in
	the whole system, and it always makes me hesitate when
	somebody proposes to use config file in any important
	way read-write for this reason.  But this "reading out
	per-branch information from the file" is read-only so I
	would not worry too much about it.
Now you tell me.  git-gui uses repo-config for all of its option
data.  It writes back to the config file to save user configured
options, including window layout at quit.  It also uses both the
user global and the local repository config files. :)

We can safely update the index, symrefs and refs.  Why can't we
also safely update the config file?  The biggest problem is its
also a hand-modified file complete with support for comments,
so users expect the file to stay intact during edits.  Aside from
that little issue its really a fairly simple format...
Strictly speaking, however, the config file is a wrong place to
store it.  For one thing it has core.sharedrepository and
receive.denynonfastforwards that are true configuration to
control the behaviour of git _at_ _the_ _repository_ the
configuration is at.  The new "branch property" are primarily to
help the other end, and the "filtering rewinding ones" we want
at the clone/fetch side wants that information only and not
interested in the true configuration information at that
repository.
Yes, of course.  I never suggested copy the entire configuration
as is.  What we really want to do is take the branch data from the
remote and rewrite it for every branch we fetched into a ref, e.g.:

  remote:
    [branch "master"]
        rewinds = false
        description = blah blah

  local:
    [branch "remotes/origin/master"]
        rewinds = false
        description = blah blah

while skipping other configuration sections and options that have
no meaning for the local repository.  core.sharedrepository is just
one of *many* configuration options that the local repository should
not inherit.  remote.<name>.url is another...  etc.
So my preference is:

 * store it in a fixed filename under $GIT_DIR/$something, in a
   format that is similar to the true config file.
At which point you might as well put it into $GIT_DIR/config.
People are going to use git-repo-config to edit that file, even if
you say its brittle and maybe shouldn't be used for heavy editing.
Its just damn handy.
 
Besides of which, lets not forget that something like:

  [branch "master"]
      rewinds = false
  [branch "pu"]
      rewinds = true

is not only data for the client to examine.  It can be useful in
say git-receive-pack as a much more fine-grained alternative to
receive.denynonfastforwards.  If the server's policy is to not
rewind a branch then receive-pack shouldn't let a remote user
rewind it.  At which point its useful to have that branch data
in $GIT_DIR/config.  :-)

-- 

Re: [RFD] making separate-remote layout easier to use

From: Junio C Hamano <hidden>
Date: 2016-08-11 20:33:28

Shawn Pearce [off-list ref] writes:
quoted
Strictly speaking, however, the config file is a wrong place to
store it.  For one thing it has core.sharedrepository and
receive.denynonfastforwards that are true configuration to
control the behaviour of git _at_ _the_ _repository_ the
configuration is at.  The new "branch property" are primarily to
help the other end, and the "filtering rewinding ones" we want
at the clone/fetch side wants that information only and not
interested in the true configuration information at that
repository.
Yes, of course.  I never suggested copy the entire configuration
as is.
I never misunderstood your plan as such.  I was more worried
about a case where you clone from a not-so-public repository and
the config file has some information of sensitive nature.
Currently we do not have any such variable, so I am probably
being a bit paranoid here, but using the config for the purpose
of what we are discussing right now closes the door to store
sensitive information and declare that the config file at the
remote is "none of the cloner's business".

However,...
Besides of which, lets not forget that something like:

  [branch "master"]
      rewinds = false
  [branch "pu"]
      rewinds = true

is not only data for the client to examine.  It can be useful in
say git-receive-pack as a much more fine-grained alternative to
receive.denynonfastforwards.  If the server's policy is to not
rewind a branch then receive-pack shouldn't let a remote user
rewind it.  At which point its useful to have that branch data
in $GIT_DIR/config.  :-)
This is a good point.  If we want to ever have a 'not to be
leaked' configuration per repository, we can have a separate,
private, configuration file to store that, so I probably should
not worry too much about this at this point.  I'm Ok with having
this in the usual config -- the convenience outweighs the
purity.

To summarize:

 * A repository can have branch."foo".rewinds configuration to
   mark the "foo" branch to be subject to rewinding.  This
   configuration variable defaults to false.

 * Receive.denynonfastforwards is currently a boolean but a new
   value, "per-branch", is also allowed.  When it is set to
   "per-branch", non fast-forward update is allowed only when
   branch."foo".rewinds is true for the branch.

 * A cloner creates "Pull: refs/heads/*:refs/remotes/origin/*"
   (or config equivalent) upon cloning.  When it clones, it
   downloads the config from the remote in order to see which
   are marked as "rewinds".  It makes tracking branches for only
   the ones that are not marked as "rewinds" by default.  We
   might have an option to do '+' variant glob, and clone all
   branches.

 * Subsequent fetch, when the glob is not '+' variant, would
   download the config from the remote in order to see which are
   marked as "rewinds", and ignores the branches that are marked
   as such.

I am a bit unhappy that subsequent fetches have to re-read the
remote config every time.  I can sort-of-see we can cram the
"this is expected to be rewound" information as part of
peek-remote exchange to avoid the overhead, but I do not think
it is easily doable for dumb transports without breaking the
backward compatibility.

To avoid this, we need to remember what remote branches we have
seen but decided not to track, perhaps because they were marked
as "rewinds".  Subsequent fetches first learn what are on the
remote side, and only when it sees a branch that we do not track
and we have not seen, it needs to re-read the remote config to
see if that is to be tracked (in which case we would create a
new tracking branch) or ignored (in which case we would remember
that we will ignore this branch in the future fetches).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help