Re: git-svn and huge data and modifying the git-svn-HEAD branch directly

Linus Torvalds [off-list ref] writes:

But if somebody does the get_sha1() magic, and Junio agrees, then I think 
it would be a great thing to do.

I am inclined to agree here.

Some caveats upfront, though.

Since I was bitten at least once by attempting get_sha1() to
deal with ambiguous names (the issue was between heads and tags
but I think there are similar issues here) I am really reluctant
to have the function look at anywhere other than heads/ and
tags/ without explicit prefix.

Currently get_sha1_basic() says:

	* look in $GIT_DIR with these prefixes in turn and take
          the first match: "", "refs", "refs/tags", "refs/heads".

The extended one _would_ in addition say one of these things:

	* if none of the above prefixes work, try other
          directories under refs/ as prefixes and take the first
          match.

	or

	* if none of the above prefixes work, try other
          directories under refs/ as prefixes and if there is a
          unique match take it.  If there are more than one
          match, do not take either.

In the context of get_sha1(), get_sha1_basic() is used like
this:

	* if get_sha1_basic() finds an answer, use it.
          Otherwise see if it is an abbreviated object name.

The behaviour of a naive implementation of the former would
depend on readdir() and traversal order, which makes (from the
end user's point of view) a hard to understand confusion that is
not reproducible.  Another repository cloned from such would
even give you different answers.

The latter at first sounds sane, but it has a subtle issue,
which was what bitten me previously between heads/ and tags/.
In that broken version, if you have a head called "dead" and a
tag with the same name, neither was taken ("they are not unique,
so do not take either!") and we ended up finding an object whose
SHA1 name began with those two bytes 0xDE 0xAD.  I do not think
this has happened in the field, fortunately, but it would have
been quite hard to diagnose.

So if we were to do it, I would say do the latter, but be very
careful to make sure you fail the whole get_sha1() when you bail
out of the "try possible prefixes" codepath because of
ambiguity.  There may be other issues involved, but I wouldn't
know -- I reverted the "do not take either if they are
ambiguous between heads/ and tags/" patch primarily because of
the reason from the above paragraph, but also did not want to
deal with any other potential issues to keep my sanity ;-).

On Wednesday 01 March 2006 20:11, Junio C Hamano wrote:

The latter at first sounds sane, but it has a subtle issue,
which was what bitten me previously between heads/ and tags/.
In that broken version, if you have a head called "dead" and a
tag with the same name, neither was taken ("they are not unique,
so do not take either!") and we ended up finding an object whose
SHA1 name began with those two bytes 0xDE 0xAD.  I do not think
this has happened in the field, fortunately, but it would have
been quite hard to diagnose.

So if we were to do it, I would say do the latter, but be very
careful to make sure you fail the whole get_sha1() when you bail
out of the "try possible prefixes" codepath because of
ambiguity.

Yes.
Any ambiguity is a source of confusion and user error. Better
bail out. If it is not a performance problem, it would be better
to integrate the check for abbreviated object name into the
ambiguity analysis, and not have 2 stages of searching.
It probably would be a good idea to print out the ambigous names
with the error message, so that you can copy&paste the correct
full name afterwards.

If we go for the .git/refs/remotes/... and have an ambiguity becaues
of remote shortcut names, a error message pointing at a "git-rename-remote"
command would be handy, allowing the user to cleanup the namespace.

There may be other issues involved, but I wouldn't 
know -- I reverted the "do not take either if they are
ambiguous between heads/ and tags/" patch primarily because of
the reason from the above paragraph, but also did not want to
deal with any other potential issues to keep my sanity ;-).

I think the real problem here is that names like "dead" can be interpreted
as abbreviated object name. When you introduce such a name as head or tag,
you have a potential ambiguity which can get real at any time.
Perhaps it would be good to print out a warning when the user is about to
create a head or tag name which can be interpreted as abbreviated object name?

Josef

On 3/2/06, Junio C Hamano [off-list ref] wrote:

Linus Torvalds [off-list ref] writes:

quoted

But if somebody does the get_sha1() magic, and Junio agrees, then I think
it would be a great thing to do.

I am inclined to agree here.

Aren't we doing a lot of work (changes in core git, and corresponding
changes in the porcelain) when simple changes in porcelain would
suffice? Let's imagine that

 - git-commit refuses to commit to a head that has a corresponding
remote (cg-commit does this already with heads that match something in
'branches')
 - git-$SCMimport scripts generate a semi-bogus remotes/headname entry
 - git-pull/push can spot and ignore the semi-bogus remotes/headname entry
 - this means that `touch remotes/foo` is now a cheap way of making
the head readonly
 - depending on the git-$SCMimport script, the remotes/headname file
can perhaps contain useful configuration data for the import, so
git-$SCMimport headname does the right thing.

cheers,


martin

On Thu, 2 Mar 2006 10:40:41 +1300, "Martin Langhoff" wrote:

Aren't we doing a lot of work (changes in core git, and corresponding
changes in the porcelain) when simple changes in porcelain would
suffice? Let's imagine that

There might be a simpler change to solve the git-svn-HEAD issue. But I
was about to independently bring up the issue that I wanted to hide
away the "remote-tracking" branches.

I currently get a lot of noise in "git branch" output that are
remote-tracking branches that I will never commit to. (I use a -origin
suffix to help me filter them out, but I'd prefer not to see them at
all here.)

Meanwhile, as I've been teaching new git users, I've had to carefully
teach:

1) Never commit to a branch name that appears on the right side of ':'
   in a Pull: refspec.

2) BTW, that ':' might be only implicit. A refspec of "branch" is
   equivalent to "branch:branch" so don't commit to those either.

That's pretty painful, so I really think these remote-tracking refs
belong outside of refs/heads.

-Carl

On Wed, 1 Mar 2006, Carl Worth wrote:

Meanwhile, as I've been teaching new git users, I've had to carefully
teach:

1) Never commit to a branch name that appears on the right side of ':'
   in a Pull: refspec.

2) BTW, that ':' might be only implicit. A refspec of "branch" is
   equivalent to "branch:branch" so don't commit to those either.

That's pretty painful, so I really think these remote-tracking refs
belong outside of refs/heads.

In the same vein, I think the refs/remotes/<remotename>/<branchname> 
naming will make it possible for people who track multiple remotes to 
sanely work with the fact that they might track 10 separate branches from 
Jeff, one branch from me, and a couple of branches from Greg in the same 
tree, without just going crazy.

I agree that we could solve the "don't touch that branch" issue another 
way, by just making them read-only. But the reason I like the separate 
namespace is that it just seems to organize the branches really well, and 
in an unambiguous - and logical - manner.

I too find myself looking at "git branch" output, and a lot of it is stuff 
I don't really care about - much of it is just the branches I got for just 
tracking Junio's git repo.

		Linus