Complete http-pull; where should it go?

Complete http-pull; where should it go?

[Daniel Barkalow]

I've been working on http-pull, and I've made it able to download the
target commit from ...git/refs/<dir>/<name> (instead of making you figure
it out yourself), and also write the target it looks up to your local
.git/refs/<w-d>/<w-n> (which doesn't have to be at all related to the
source one. In fact, I just got the latest Linus tree with:

git-http-pull -t -w heads/linus heads/master \
  http://www.kernel.org/pub/scm/git/git.git/
git-read-tree $(cat .git/refs/heads/linus)
git-checkout-cache -a
git-update-cache --refresh

(and I didn't get any of the history, although I could have if I wanted
to; and I could get it now if I decided I needed it).

My question is, where does this belong? It's based on adding to the core
as it has been the knowledge that .git/refs/*/* consists of hex-format
hash files, both locally and on remote servers. I think this level of
information belongs in the plumbing; at least, if people are to be able to
use different git-based systems to access the same repositories, they have
to agree. And there seems to be that much agreement, and so it makes sense
to make it part of the core.

(For that matter, people seem to agree that refs/heads/ has heads, and
refs/heads/master is the one you want to pull if you don't know
otherwise; I didn't include this information at all)

	-Daniel
*This .sig left intentionally blank*

Re: Complete http-pull; where should it go?

[Linus Torvalds]

On Sun, 1 May 2005, Daniel Barkalow wrote:
>
> My question is, where does this belong? It's based on adding to the core
> as it has been the knowledge that .git/refs/*/* consists of hex-format
> hash files, both locally and on remote servers.

So the main reason I _don't_ like programs that automatically look up the 
refs etc is that its' often simply WRONG.

For example, if I want to know what objects I have in my object directory 
that are needed for a release, I want to be able to tell fsck to list the 
objects that are extraneous for that release _regardless_ of the fact that 
I may have .git/refs/*/* files that point to other things.

So if fsck-cache automatically looks up references in .git/refs/ like in
one of your earlier patches, then instead of adding value to the program,
you actually _remove_ value from it by making it less flexible, and
enforcing a world-view that is not necessarily the only view.

This is why I want the true _plumbing_ to not care about these things, and 
if you include references to trees, you _list_ them explicitly. 

And if you want to have a command that takes implied references, then just 
make a script that does that for you, rather than making the core plumbing 
understand it.

This is a classic "ease of use" vs "power-user" issue. I'm very
fundamentally of the opinion that power-users are good, and that ease of
use is done by having scripts that turn normal ops into "power user"  
operations.

That's the unix way, really.

		Linus

Re: Complete http-pull; where should it go?

[Daniel Barkalow]

On Sun, 1 May 2005, Linus Torvalds wrote:

> For example, if I want to know what objects I have in my object directory 
> that are needed for a release, I want to be able to tell fsck to list the 
> objects that are extraneous for that release _regardless_ of the fact that 
> I may have .git/refs/*/* files that point to other things.
>
> So if fsck-cache automatically looks up references in .git/refs/ like in
> one of your earlier patches, then instead of adding value to the program,
> you actually _remove_ value from it by making it less flexible, and
> enforcing a world-view that is not necessarily the only view.

It's true that you might not want to include all of the refs; but doesn't
it make more sense to support the standard arrangement of refs (i.e.,
they're in .git/refs/kind/name) for the ones you want to include, rather
than having to pull out the hex to pass in yourself?

> This is why I want the true _plumbing_ to not care about these things, and 
> if you include references to trees, you _list_ them explicitly. 

Right; what I want to make programs able to do is take explicit
references, instead of only taking the objects they reference. So you say
heads/master or heads/linus instead of
"198b0fb635ed8a007bac0c16eab112c5e2c7995c".

The part that makes this important is that the user may be trying to look
up a reference on a remote machine using the same connection that the
objects will come over, and this is impractical without having the program
know how to handle reference files.

> And if you want to have a command that takes implied references, then just 
> make a script that does that for you, rather than making the core plumbing 
> understand it.

Agreed; which references to use are up to either the power user or the
script, not the core. I'm just interested in having a core implementation
for using them when specified.

	-Daniel
*This .sig left intentionally blank*

Re: Complete http-pull; where should it go?

[Linus Torvalds]

On Sun, 1 May 2005, Daniel Barkalow wrote:
> 
> Right; what I want to make programs able to do is take explicit
> references, instead of only taking the objects they reference. So you
> say heads/master or heads/linus instead of
> "198b0fb635ed8a007bac0c16eab112c5e2c7995c".

That's fine. 

This is really just an issue of havign a function that does "get_sha1()", 
and then making the things that take command line arguments use that 
one instead of "get_sha1_hex()".

Then you can have rules like:
 - if it's a hex number, take it
 - if it's a filename,  look it up
 - if ".git/refs + str is a filename, look it up.

Something like

	int get_sha1(char *str, unsigned char *result)
	{
		static char pathname[PATH_MAX];

		if (get_sha1_hex(str, result) == 0)
			return 0;
		if (get_sha1_file(str, result) == 0)
			return 0;
		snprintf(pathname, sizeof(pathname), ".git/refs/%s", str);
		if (get_sha1_file(pathname, result) == 0)
			return 0;
		...
	}

where you have

	int get_sha1_file(char *path, unsigned char *result)
	{
		char buffer[60];
		int fd = open(path, O_RDONLY);
		int len;

		if (fd < 0)
			return -1;
		len = read(fd, buffer, sizeof(buffer));
		close(fd);
		if (len < 40)
			return -1;
		return get_sha1_hex(buffer, result);
	}
			
or whatever.

The _only_ thing I want to be careful about is that all the _internal_
stuff still have to use the strict "get_sha1_hex()" function, ie we should
never _ever_ accept a tree object where the "sha1" ends up being anything
but the hex thing. So this "generalized get_sha1()" would have to be used 
only on real user input (ie argv[] array and the like).

		Linus

Re: Complete http-pull; where should it go?

[Daniel Barkalow]

On Sun, 1 May 2005, Linus Torvalds wrote:

> On Sun, 1 May 2005, Daniel Barkalow wrote:
> > 
> > Right; what I want to make programs able to do is take explicit
> > references, instead of only taking the objects they reference. So you
> > say heads/master or heads/linus instead of
> > "198b0fb635ed8a007bac0c16eab112c5e2c7995c".
> 
> That's fine. 
> 
> This is really just an issue of havign a function that does "get_sha1()", 
> and then making the things that take command line arguments use that 
> one instead of "get_sha1_hex()".

I added a get_ref_sha1() that just does the .git/refs/ case; I think that
most things that want to accept various forms will also want to know what
form they got and do something with it, so get_sha1() isn't a big win.

The biggest thing so far is actually writing the references when the user
has specified where to write them, and I wanted to restrict that to a
target of the right format.

> The _only_ thing I want to be careful about is that all the _internal_
> stuff still have to use the strict "get_sha1_hex()" function, ie we should
> never _ever_ accept a tree object where the "sha1" ends up being anything
> but the hex thing. So this "generalized get_sha1()" would have to be used 
> only on real user input (ie argv[] array and the like).

Agreed.

	-Daniel
*This .sig left intentionally blank*

Re: Complete http-pull; where should it go?

[Linus Torvalds]

On Sun, 1 May 2005, Daniel Barkalow wrote:
> 
> I added a get_ref_sha1() that just does the .git/refs/ case; I think that
> most things that want to accept various forms will also want to know what
> form they got and do something with it, so get_sha1() isn't a big win.

I really really disagree.

For example, take something like "git-diff-cache". I think it's a big
bonus if you can just write

	git-diff-cache HEAD

and it will pick up the sha1 ID from .git/HEAD. 

In general, I think a "get_sha1()" that tries as many user-friendly things 
as possible is good for the command line. Within reason. I really don't 
like the "try to autocomplete a partial sha1" thing, but I wouldn't at all 
mind having various helpers like

	git-diff-tree HEAD.p1 HEAD

where the ".p1" would just make it look up the first parent of the thing 
in HEAD (which in turn would mean "look up the sha1 from the .git/HEAD 
file".

Many of these are really easy to do, and I think most people would tend to 
prefer having the choice.

		Linus

Re: Complete http-pull; where should it go?

[Daniel Barkalow]

On Sun, 1 May 2005, Linus Torvalds wrote:

> On Sun, 1 May 2005, Daniel Barkalow wrote:
> > 
> > I added a get_ref_sha1() that just does the .git/refs/ case; I think that
> > most things that want to accept various forms will also want to know what
> > form they got and do something with it, so get_sha1() isn't a big win.
> 
> I really really disagree.
> 
> For example, take something like "git-diff-cache". I think it's a big
> bonus if you can just write
> 
> 	git-diff-cache HEAD
> 
> and it will pick up the sha1 ID from .git/HEAD. 

True, but with -p, you'd probably want to give lines like

--- HEAD/Makefile
+++ (cache)/Makefile

when it's not just hex (and "linus/Makefile" for heads/linus, not
"heads/linus/Makefile", which would break things). I think it's generally
best to call each of the things in the list to try and not only use the
first one that understands, but also do program-specific special things
depending on which one that was.

> In general, I think a "get_sha1()" that tries as many user-friendly things 
> as possible is good for the command line. Within reason. I really don't 
> like the "try to autocomplete a partial sha1" thing, but I wouldn't at all 
> mind having various helpers like
> 
> 	git-diff-tree HEAD.p1 HEAD
> 
> where the ".p1" would just make it look up the first parent of the thing 
> in HEAD (which in turn would mean "look up the sha1 from the .git/HEAD 
> file".

Maybe :p1, to use characters that aren't frequently in filenames?

> Many of these are really easy to do, and I think most people would tend to 
> prefer having the choice.

I agree with that, but I'm not sure how useful having one function that
just looks up the sha1 without further reporting is. In any case, it's an
easy addition, and it's plausible that similar functions would work for
any similar information needed.

	-Daniel
*This .sig left intentionally blank*

Re: Complete http-pull; where should it go?

[Linus Torvalds]

On Sun, 1 May 2005, Linus Torvalds wrote:
> 
> For example, take something like "git-diff-cache". I think it's a big
> bonus if you can just write
> 
> 	git-diff-cache HEAD
> 
> and it will pick up the sha1 ID from .git/HEAD. 

Ok, I just committed this. Almost all commands that take SHA1 strings from 
the command line can now use the shortened format. 

Right now it only does the "filename" version, and it tries to append 
".git/" to it if it doesn't work, so you can do

	git-diff-cache HEAD

or something like that if you want to. Where ".git/HEAD" would also have
worked, or ".git/refs/heads/master" (or just "refs/heads/master").

For example, if you have a refs/tags directory which has been populated 
with files that point to the tags, you can do

	git-diff-tree -p refs/tags/v2.6.12-rc3 HEAD

and it will do exactly what you'd expect it to do.

I didn't do the ".p1" for parent thing. It would be easy to add, I just 
haven't decided on a nice syntax for it (I'd like something that is nice 
and doesn't need quoting, but the ".p1" seems a bit _too_ ugly).

			Linus

Re: Complete http-pull; where should it go?

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> I didn't do the ".p1" for parent thing. It would be easy to add, I just 
LT> haven't decided on a nice syntax for it (I'd like something that is nice 
LT> and doesn't need quoting, but the ".p1" seems a bit _too_ ugly).

Shameless plug but could you take a look at how my JIT does it,
at <http://members.cox.net/junkio/JITNotes.txt>, especially
scroll down to near the bottom "Syntax to naming a snapshot or a
commit" section?

I still think this kind of thing belongs to Porcelain not
Plumbing, though.

Re: Complete http-pull; where should it go?

[Linus Torvalds]

On Sun, 1 May 2005, Junio C Hamano wrote:
>
> Shameless plug but could you take a look at how my JIT does it,
> at <http://members.cox.net/junkio/JITNotes.txt>, especially
> scroll down to near the bottom "Syntax to naming a snapshot or a
> commit" section?

Hmm.. I actually implemented it, because it's simple to implement, but now 
that I've tried using it, I just find it to be too much like line noise to 
do

	git-diff-tree -p 0-HEAD HEAD

for "parent to current". The notation "-HEAD" also works, but since that
looks like a command line flag, it's usually not useful, except when
combined with other stuff, ie "0---HEAD" is "three parents up".

> I still think this kind of thing belongs to Porcelain not
> Plumbing, though.

I don't entirely disagree, and I suspect that Porcelain ends up using the 
pure hex numbers which are always unambiguous.

That said, I end up still using the raw plumbing, so it's actually nice to 
have some simple shorthand. I just don't know if the parent thing ends up 
being all that useful.

Anyway, it's there. I don't love the parent syntax, but on the other hand,
I don't know how often I'd even end up using it (while the tag and HEAD
names are _very_ useful), so maybe it just doesn't matter.

		Linus

Re: Complete http-pull; where should it go?

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> Hmm.. I actually implemented it, because it's simple to
LT> implement, but now that I've tried using it, I just find it
LT> to be too much like line noise to do

LT> 	git-diff-tree -p 0-HEAD HEAD

LT> for "parent to current". The notation "-HEAD" also works,
LT> but since that looks like a command line flag, it's usually
LT> not useful, except when combined with other stuff, ie
LT> "0---HEAD" is "three parents up".

That's why I said I do not think this belongs to the Plumbing.

JIT command-line would do just "jit-diff" to get the diff
between the work tree and the commit (or snapshot) it is based
upon, without parameter.  Fully spelled out, it would be
"jit-diff -1:1".  This kind of syntax, although is quite useful
in JIT context (e.g. saying "jit-patch --2" to mean "apply
changes between snnapshot 2 and two parents up of it to my
current work tree"), I tend to agree that it is a bit too weird
for the core Plumbing layer, for which Unixy syntax is more
suitable.

Re: Complete http-pull; where should it go?

[Linus Torvalds]

On Sun, 1 May 2005, Junio C Hamano wrote:
> 
> That's why I said I do not think this belongs to the Plumbing.

I've removed the ugly parent format, but to make it easier to script the 
thing I removed, I just pushed out an improvement to "cat-file" which 
makes it use the "read_object_with_reference()" helper to cat the object.

That means that if you use "git-cat-file tree" on a tag object, then it
will first look up the tag object, then the commit object and finally the
tree obejct, and it will cat that tree object. Of course, normally you'd
use it with a "commit" object and get the pleasure of having it "just
work" with tags too.

So for example, to get the parent of a commit (whether a tag that points
to a commit, or a real commit), you'd script something like

	git-cat-file commit v2.6.12-rc3 | sed -n '/^parent /{s/parent //;p;q}'

which gets the first parent of the commit that was pointed to by the 
"v2.6.12-rc3" tag.

If you want to see the tag itself, you'd just do

	git-cat-file tag v2.6.12-rc3

like before, of course.

And "git-cat-file -t" always gives the top-level tag, so if you ask the 
type of "v2.6.12-rc3" it will say "tag".

			Linus

Re: Complete http-pull; where should it go?

[Daniel Barkalow]

On Sun, 1 May 2005, Linus Torvalds wrote:

> That said, I end up still using the raw plumbing, so it's actually nice to 
> have some simple shorthand. I just don't know if the parent thing ends up 
> being all that useful.

I still prefer having get_sha1() handle just hex, refs, and files in
.git. Actually, just having it handle hex would probably be sufficient,
since the Porcelain can patch it cleanly with whatever is locally useful.

Any thoughts on my patches, which actually barely overlap with this
functionality?

	-Daniel
*This .sig left intentionally blank*

Re: Complete http-pull; where should it go?

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> On Sun, 1 May 2005, Daniel Barkalow wrote:
>> 
>> Right; what I want to make programs able to do is take explicit
>> references, instead of only taking the objects they reference. So you
>> say heads/master or heads/linus instead of
>> "198b0fb635ed8a007bac0c16eab112c5e2c7995c".

LT> That's fine. 

LT> This is really just an issue of havign a function that does "get_sha1()", 
LT> and then making the things that take command line arguments use that 
LT> one instead of "get_sha1_hex()".

LT> Then you can have rules like:
LT>  - if it's a hex number, take it
LT>  - if it's a filename,  look it up
LT>  - if ".git/refs + str is a filename, look it up.

LT> The _only_ thing I want to be careful about is that all the _internal_
LT> stuff still have to use the strict "get_sha1_hex()" function, ie we should
LT> never _ever_ accept a tree object where the "sha1" ends up being anything
LT> but the hex thing. So this "generalized get_sha1()" would have to be used 
LT> only on real user input (ie argv[] array and the like).

I agree with Linus.  Dan, you are welcome to lift the code from
the find-in-vicinity() function in jit-util.c in my JIT toolset,
found at http://members.cox.net/junkio/.