From: Jay Soffian <hidden> Date: 2016-06-15 22:46:08
There is currently no porcelain for dealing with remote HEADs (i.e.
$GIT_DIR/remotes/<remote>/HEAD). This series:
1) Teaches git remote a new "sethead" verb:
To set a remote HEAD explicitly:
$ git remote sethead <name> <branch>
To set a remote HEAD to match the upstream repo:
$ git remote sethead <name> -a
To delete a remote HEAD:
$ git remote sethead <name> -d
2) Teaches git remote show to display the remote HEAD:
$ git remote show origin
* remote origin
URL: git://git.kernel.org/pub/scm/git/git.git
HEAD: master
3) Documents the new sethead verb. I also correct the git remote man page
w/respect to the "-m <master>" option. The man page implied that the remote
HEAD was set automatically when adding a remote (a la git clone), but this is
not true. And, since I couldn't find anywhere else that the point of having a
remote HEAD is documented, I documented it here.
Jay Soffian (3):
builtin-remote: move duplicated cleanup code its own function
builtin-remote: teach show to display remote HEAD
builtin-remote: add sethead verb
Documentation/git-remote.txt | 20 +++++++-
builtin-remote.c | 108 ++++++++++++++++++++++++++++++++++++++---
2 files changed, 118 insertions(+), 10 deletions(-)
From: Jay Soffian <hidden> Date: 2016-06-15 22:46:08
Moved some identical lines of code into their own function in
preparation for adding additional functionality which will use this
function as well.
Signed-off-by: Jay Soffian <redacted>
---
builtin-remote.c | 18 ++++++++++--------
1 files changed, 10 insertions(+), 8 deletions(-)
From: Jay Soffian <hidden> Date: 2016-06-15 22:46:08
The remote HEAD cannot be determined unambiguosly, so use the same
heuristics as builtin-clone's locate_head().
Signed-off-by: Jay Soffian <redacted>
---
builtin-remote.c | 39 +++++++++++++++++++++++++++++++++++++++
1 files changed, 39 insertions(+), 0 deletions(-)
@@ -271,6 +272,38 @@ static int get_ref_states(const struct ref *ref, struct ref_states *states)return0;}+staticchar*get_head_name(conststructref*ref)+{+conststructref*remote_head=NULL;+conststructref*remote_master=NULL;+conststructref*r;+for(r=ref;r;r=r->next){+if(!strcmp(r->name,"HEAD"))+remote_head=r;+if(!strcmp(r->name,"refs/heads/master"))+remote_master=r;+}++/* If there's no HEAD value at all, never mind. */+if(!remote_head)+returnNULL;++/* If refs/heads/master could be right, it is. */+if(remote_master&&!hashcmp(remote_master->old_sha1,+remote_head->old_sha1))+returnxstrdup(abbrev_branch(remote_master->name));++/* Look for another ref that points there */+for(r=ref;r;r=r->next)+if(r!=remote_head&&+!hashcmp(r->old_sha1,remote_head->old_sha1)&&+!prefixcmp(r->name,"refs/heads/"))+returnxstrdup(abbrev_branch(r->name));++/* Nothing is the same */+returnNULL;+}+structknown_remote{structknown_remote*next;structremote*remote;
From: Jay Soffian <hidden> Date: 2016-06-15 22:46:08
Provide a porcelain command for setting/deleting
$GIT_DIR/remotes/<remote>/HEAD.
While we're at it, document what $GIT_DIR/remotes/<remote>/HEAD is all
about.
Signed-off-by: Jay Soffian <redacted>
---
Documentation/git-remote.txt | 20 ++++++++++++++-
builtin-remote.c | 51 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 69 insertions(+), 2 deletions(-)
@@ -53,8 +54,7 @@ is created. You can give more than one `-t <branch>` to track multiple branches without grabbing all branches. + With `-m <master>` option, `$GIT_DIR/remotes/<name>/HEAD` is set-up to point at remote's `<master>` branch instead of whatever-branch the `HEAD` at the remote repository actually points at.+up to point at remote's `<master>` branch. See also the sethead command. + In mirror mode, enabled with `\--mirror`, the refs will not be stored in the 'refs/remotes/' namespace, but in 'refs/heads/'. This option
@@ -76,6 +76,22 @@ the configuration file format. Remove the remote named <name>. All remote tracking branches and configuration settings for the remote are removed.+'sethead'::++Sets or deletes the default branch (`$GIT_DIR/remotes/<name>/HEAD`) for+the named remote. Having a default branch for a remote is not required,+but allows the name of the remote to be specified in lieu of a specific+branch. For example, if the default branch for `origin` is set to+`master`, then `origin` may be specified wherever you would normally+specify `origin/master`.+++With `-d`, `$GIT_DIR/remotes/<name>/HEAD` is deleted.+++With `-a`, the remote is queried to determine its `HEAD`, then+`$GIT_DIR/remotes/<name>/HEAD` is set to the same branch.+++Use `<branch>` to set `$GIT_DIR/remotes/<name>/HEAD` explicitly.+ 'show':: Gives some information about the remote <name>.
@@ -791,6 +792,54 @@ static int show(int argc, const char **argv)returnresult;}+staticintsethead(intargc,constchar**argv)+{+intopt_a=0,opt_d=0,result=0;+structstrbufbuf=STRBUF_INIT,buf2=STRBUF_INIT;+char*head_name=NULL;++structoptionoptions[]={+OPT_GROUP("sethead specific options"),+OPT_BOOLEAN('a',0,&opt_a,+"set refs/remotes/<name>/HEAD according to remote"),+OPT_BOOLEAN('d',0,&opt_d,"delete refs/remotes/<name>/HEAD"),+OPT_END()+};+argc=parse_options(argc,argv,options,builtin_remote_usage,0);+if((argc==1&&!(opt_a||opt_d))||+((argc==2&&(opt_a||opt_d)))||argc<1||argc>2)+usage_with_options(builtin_remote_usage,options);++strbuf_addf(&buf,"refs/remotes/%s/HEAD",argv[0]);++if(opt_d){+if(result|=delete_ref(buf.buf,NULL,REF_NODEREF))+error("Could not delete %s",buf.buf);+}elseif(opt_a){+structref_statesstates;+memset(&states,0,sizeof(states));+get_remote_ref_states(argv[0],&states,1);+head_name=xstrdup(states.head_name);+free_remote_ref_states(&states);+}else+head_name=xstrdup(argv[1]);++if(head_name){+unsignedcharsha1[20];+strbuf_addf(&buf2,"refs/remotes/%s/%s",argv[0],head_name);+/* make sure it's valid */+if(!resolve_ref(buf2.buf,sha1,1,NULL))+result|=error("Not a valid ref: %s",buf2.buf);+elseif(create_symref(buf.buf,buf2.buf,"remote sethead"))+result|=error("Could not setup %s",buf.buf);+free(head_name);+}++strbuf_release(&buf);+strbuf_release(&buf2);+returnresult;+}+staticintprune(intargc,constchar**argv){intdry_run=0,result=0;
From: Jeff King <hidden> Date: 2016-06-15 22:46:09
On Wed, Feb 11, 2009 at 01:01:20AM -0500, Jay Soffian wrote:
1) Teaches git remote a new "sethead" verb:
To set a remote HEAD explicitly:
$ git remote sethead <name> <branch>
To set a remote HEAD to match the upstream repo:
$ git remote sethead <name> -a
To delete a remote HEAD:
$ git remote sethead <name> -d
I like these semantics a lot. I will do worse than bikeshed, though, and
say that I don't like the color you painted it without even proposing a
color of my own. Which is to say, I think "sethead" is not the best
name. But I like it better than the alternatives I've seen, so maybe it
is OK.
Perhaps it is just the run-together word that makes it worse. Something
like "set-head" might be better (I guess I couldn't resist suggesting a
color, after all).
I have a few comments, which I will post in reply to the individual
patches.
-Peff
Hmm. I don't know anything about this code, so maybe it is not trivial.
But anytime you are touching an area that NEEDSWORK, I think it is worth
looking at whether you can fix that problem (since you have already
spent a few brain cycles understanding what is going on in general).
-Peff
From: Jeff King <hidden> Date: 2016-06-15 22:46:09
On Wed, Feb 11, 2009 at 01:01:22AM -0500, Jay Soffian wrote:
+static char *get_head_name(const struct ref *ref)
+{
+ const struct ref *remote_head = NULL;
+ const struct ref *remote_master = NULL;
+ const struct ref *r;
+ for (r = ref; r; r = r->next) {
+ if (!strcmp(r->name, "HEAD"))
+ remote_head = r;
+ if (!strcmp(r->name, "refs/heads/master"))
+ remote_master = r;
+ }
+
+ /* If there's no HEAD value at all, never mind. */
+ if (!remote_head)
+ return NULL;
+
+ /* If refs/heads/master could be right, it is. */
+ if (remote_master && !hashcmp(remote_master->old_sha1,
+ remote_head->old_sha1))
+ return xstrdup(abbrev_branch(remote_master->name));
+
+ /* Look for another ref that points there */
+ for (r = ref; r; r = r->next)
+ if (r != remote_head &&
+ !hashcmp(r->old_sha1, remote_head->old_sha1) &&
+ !prefixcmp(r->name, "refs/heads/"))
+ return xstrdup(abbrev_branch(r->name));
+
+ /* Nothing is the same */
+ return NULL;
+}
Yuck. Cut and paste from builtin-clone.c. It's not so much the number of
lines here (although of course I don't like that, either) but that this
function encompasses a heuristic for matching the HEAD. Which means it
may change in the future, and I really don't want the clone behavior and
the remote behavior to diverge.
So can we refactor it into a library function?
I see that the inputs and outputs aren't exactly the same in both cases,
but I think you could do it like:
struct ref *guess_head_ref(const struct ref *refs_with_head,
const struct ref *refs_that_might_match_head,
struct ref **remote_head_p);
and then just call:
r = guess_head_ref(refs, refs, NULL);
states->head_name = r ? xstrdup(abbrev_branch(r->name)) : NULL;
from git-remote, which at least keeps the changeable parts all
contained.
-Peff
Hmm. I don't know anything about this code, so maybe it is not trivial.
But anytime you are touching an area that NEEDSWORK, I think it is worth
looking at whether you can fix that problem (since you have already
spent a few brain cycles understanding what is going on in general).
I spent about 5 minutes which was enough time for me to realize that
the reason the previous author left it as "NEEDSWORK" is because
fixing it is more than 5 minutes of work. This is the remote object --
maybe you could offer me some clues that allow me to know which of its
fields need to be freed individually:
struct remote {
const char *name;
int origin;
const char **url;
int url_nr;
int url_alloc;
const char **push_refspec;
struct refspec *push;
int push_refspec_nr;
int push_refspec_alloc;
const char **fetch_refspec;
struct refspec *fetch;
int fetch_refspec_nr;
int fetch_refspec_alloc;
/*
* -1 to never fetch tags
* 0 to auto-follow tags on heuristic (default)
* 1 to always auto-follow tags
* 2 to always fetch tags
*/
int fetch_tags;
int skip_default_update;
int mirror;
const char *receivepack;
const char *uploadpack;
/*
* for curl remotes only
*/
char *http_proxy;
};
I *think* const is a clue that the field need not be freed, because
the pointer is to storage that is on the stack. But I wasn't sure, esp
with the double pointers. And I really wasn't sure about the struct
pointers.
Really, I only pretend to know C. :-)
j.
From: Jay Soffian <hidden> Date: 2016-06-15 22:46:09
On Wed, Feb 11, 2009 at 7:26 PM, Jeff King [off-list ref] wrote:
Yuck.
Damn, I knew I wasn't going to slip that one by. :-)
I see that the inputs and outputs aren't exactly the same in both cases,
Which is why I didn't refactor it. The extra code needed to massage
what builtin-remote.c has to the existing function in builtin-clone.c
would've been more LOC than the duplicate code (I think...).
BUT
I'll try. :-)
j.
From: Jeff King <hidden> Date: 2016-06-15 22:46:09
On Wed, Feb 11, 2009 at 08:44:13PM -0500, Jay Soffian wrote:
I spent about 5 minutes which was enough time for me to realize that
the reason the previous author left it as "NEEDSWORK" is because
fixing it is more than 5 minutes of work. This is the remote object --
maybe you could offer me some clues that allow me to know which of its
fields need to be freed individually:
[...]
I *think* const is a clue that the field need not be freed, because
the pointer is to storage that is on the stack. But I wasn't sure, esp
with the double pointers. And I really wasn't sure about the struct
pointers.
OK, I am satisfied that it is not trivial, and doesn't need to be part
of this patch series. :)
Like I said, I don't actually know this corner of the code very well,
but since you hadn't mentioned it in your cover letter, I didn't know if
it was "too lazy to do cleanups" or "code is too scary to be cleaned
up".
-Peff
From: Jeff King <hidden> Date: 2016-06-15 22:46:09
On Wed, Feb 11, 2009 at 08:48:42PM -0500, Jay Soffian wrote:
Damn, I knew I wasn't going to slip that one by. :-)
"Given enough eyeballs, all ugly hacks are shallow." :)
quoted
I see that the inputs and outputs aren't exactly the same in both cases,
Which is why I didn't refactor it. The extra code needed to massage
what builtin-remote.c has to the existing function in builtin-clone.c
would've been more LOC than the duplicate code (I think...).
BUT
I'll try. :-)
See what you can do. But personally, I am not as concerned with reducing
LOC as I am with encapsulating system logic like "this is how you guess
which ref is HEAD". And if you can do both, great. :)
-Peff
Hmm. I don't know anything about this code, so maybe it is not trivial.
But anytime you are touching an area that NEEDSWORK, I think it is worth
looking at whether you can fix that problem (since you have already
spent a few brain cycles understanding what is going on in general).
I spent about 5 minutes which was enough time for me to realize that
the reason the previous author left it as "NEEDSWORK" is because
fixing it is more than 5 minutes of work. This is the remote object --
maybe you could offer me some clues that allow me to know which of its
fields need to be freed individually:
struct remote {
const char *name;
int origin;
const char **url;
int url_nr;
int url_alloc;
const char **push_refspec;
struct refspec *push;
int push_refspec_nr;
int push_refspec_alloc;
const char **fetch_refspec;
struct refspec *fetch;
int fetch_refspec_nr;
int fetch_refspec_alloc;
/*
* -1 to never fetch tags
* 0 to auto-follow tags on heuristic (default)
* 1 to always auto-follow tags
* 2 to always fetch tags
*/
int fetch_tags;
int skip_default_update;
int mirror;
const char *receivepack;
const char *uploadpack;
/*
* for curl remotes only
*/
char *http_proxy;
};
I *think* const is a clue that the field need not be freed, because
the pointer is to storage that is on the stack. But I wasn't sure, esp
with the double pointers. And I really wasn't sure about the struct
pointers.
Actually, the comment is wrong; "remote" comes from remote_get(), which
returns things from a cache in remote.c; there could be a remote_put() to
let the code know that the caller is done with the object, but it wouldn't
presently do anything.
(The code actually reads the config files once, generating info for all of
the configured remotes, and just returns them, except that it will
generate a new object for unconfigured, individually requested URLs)
-Daniel
*This .sig left intentionally blank*
From: Daniel Barkalow <hidden> Date: 2016-06-15 22:46:09
On Wed, 11 Feb 2009, Jay Soffian wrote:
On Wed, Feb 11, 2009 at 7:26 PM, Jeff King [off-list ref] wrote:
quoted
Yuck.
Damn, I knew I wasn't going to slip that one by. :-)
quoted
I see that the inputs and outputs aren't exactly the same in both cases,
Which is why I didn't refactor it. The extra code needed to massage
what builtin-remote.c has to the existing function in builtin-clone.c
would've been more LOC than the duplicate code (I think...).
Isn't it just:
struct ref *head = locate_head(refs, refs, NULL);
return head ? xstrdup(abbrev_branch(head->name)) : NULL;
?
I'd somehow thought I'd moved locate_head() somewhere common, but it
really ought to be done. There were periodic discussions of how you find
out when the remote repo changes its HEAD and you might want to do
something locally about it, and we never came up with a specific thing
for git to do, but the facility is probably useful.
I have the vague memory, as well, that there's some way for a transport to
report that it actually knows that HEAD is a symref to something in
particular, and so git shouldn't guess.
-Daniel
*This .sig left intentionally blank*
No, I don't _think_ so. refs is everything from the remote side (tags,
etc). I want to only match those things under refs/heads.
I think I have to do something like this (this is more or less what
builtin-clone does):
struct ref *remote_refs, *mapped_refs = NULL;
struct refspec branch_refspec;
branch_refspec.force = 0;
branch_refspec.src = branch_refspec.dst = "refs/heads";
remote_refs = transport_get_remote_refs(transport);
get_fetch_map(remote_refs, branch_refspec, &mapped_refs, 0);
head_points_at = locate_head(refs, mapped_refs, NULL);
I'd somehow thought I'd moved locate_head() somewhere common, but it
really ought to be done.
I plan to move it into remote.c.
There were periodic discussions of how you find
out when the remote repo changes its HEAD and you might want to do
something locally about it, and we never came up with a specific thing
for git to do, but the facility is probably useful.
Thus "git remote set-head -a" is the best I could come up with for
setting it to what the remote has.
I have the vague memory, as well, that there's some way for a transport to
report that it actually knows that HEAD is a symref to something in
particular, and so git shouldn't guess.
I think for http://, but not for git://, but I'm *far* from an expert
in this area.
j.
No, I don't _think_ so. refs is everything from the remote side (tags,
etc). I want to only match those things under refs/heads.
I was looking at your code and comparing with locate_head; it looks to me
like you're using "ref" for both of the lists of refs that locate_head()
gets. I think, actually, that checking for refs/heads in locate_head()
would be a reasonable thing in general, and might avoid getting surprising
results in clone with --mirror and odd refs or such.
I think I have to do something like this (this is more or less what
builtin-clone does):
struct ref *remote_refs, *mapped_refs = NULL;
struct refspec branch_refspec;
branch_refspec.force = 0;
branch_refspec.src = branch_refspec.dst = "refs/heads";
remote_refs = transport_get_remote_refs(transport);
get_fetch_map(remote_refs, branch_refspec, &mapped_refs, 0);
head_points_at = locate_head(refs, mapped_refs, NULL);
You should be able to filter the ref list you already have, rather than
refetching, if nothing else.
quoted
I'd somehow thought I'd moved locate_head() somewhere common, but it
really ought to be done.
I plan to move it into remote.c.
Good.
quoted
There were periodic discussions of how you find
out when the remote repo changes its HEAD and you might want to do
something locally about it, and we never came up with a specific thing
for git to do, but the facility is probably useful.
Thus "git remote set-head -a" is the best I could come up with for
setting it to what the remote has.
Yeah, that seems like a good interface.
quoted
I have the vague memory, as well, that there's some way for a transport to
report that it actually knows that HEAD is a symref to something in
particular, and so git shouldn't guess.
I think for http://, but not for git://, but I'm *far* from an expert
in this area.
Yes, it was information only available for http, but there's no reason to
assume that other protocols couldn't provide it, and Junio mentioned a
series from December (maybe never applied) to get the same info for the
native git protocol.
-Daniel
*This .sig left intentionally blank*