Re: [topgit] tg update error

From: martin f krafft <madduck@debian.org>
Date: Feb 13, 2009, 06:28 UTC
Message-ID: <20090213062818.GB16434@piper.oerlikon.madduck.net>
In-Reply-To: <7vocx7i6xh.fsf@gitster.siamese.dyndns.org>

also sprach Junio C Hamano <gitster@pobox.com> [2009.02.13.0014 +0100]:

Show 7 quoted lines

> > TopGit would need to make a proper branch, merge the bases into
> > it, merge that branch into the topic branch, and the probably
> > delete the branch pointer, as it's no longer needed and would
> > only pollute the refs/heads/* namespace.
> 
> So it happens purely inside TopGit and the end user never sees
> a state that HEAD points outside refs/heads/, right?

Yes.

> Why can't the base flipping operation you descibed be done on
> detached HEAD?  Perhaps with a shell variable or two that hold
> commit object names you need to keep track of while it is doing is
> work?

I am not sure I understand. Isn't that what's currently happening?

Have a look at line 110 of tg-update.sh:

  http://git.debian.org/?p=collab-maint/topgit.git;a=blob;f=tg-update.sh;hb=HEAD#l110

Show 5 quoted lines

> But the end users don't.  If TopGit dies (or killed) during the base
> flipping operation, doesn't the end user left in a funny state (granted, a
> detached HEAD is also a funny state, but it is already a known funny state
> they are familiar with.  HEAD that is a symref but points outside
> refs/heads/ is a lot funnier).

If topgit is killed, yes, then the repo could be left in a funny state. I suppose this could be addressed by putting proper traps in place.

If the merge fails, however, then the user is advised what to do; see lines 114ff.

> You did not actually answer a larger question.

It wasn't asked to me before... ;)

> What other undocumented features/restrictions does the code depend
> on, that tightening them to help normal git users inadvertently
> may cause breakages similar to this one in TopGit?

I think Petr would need to help out answering this.

I agree that it would be good to address each such occurrence in turn and replace it with a method that only makes use of the public API. Up until now, however,

  git checkout -q "refs/top-bases/$name"

was not really something undocmented or restricted. I find it rather difficult to separate public-as-in-every-user-can-and-should-use-this features from restricted-better-be-left-alone-unless-you-really-know-what-you-are-doing features with Git. This has gotten *a lot* better, but the fact that I can still call e.g. git update-ref (as opposed to e.g. git _update-ref) and potentially turn my repository upside down exemplifies this.

Maybe Petr remembers all the instances when he sneakily used tricks to make things work, and then we can look at each of them in turn.

Maybe some of you could go through the code (which isn't /that/ much), looking for instances of not-so-public API abuse and help us identify them too.

Cheers,

-- 
 .''`.   martin f. krafft <madduck@d.o>      Related projects:
: :'  :  proud Debian developer               http://debiansystem.info
`. `'`   http://people.debian.org/~madduck    http://vcs-pkg.org
  `-  Debian - when you have better things to do than fixing systems
 
"without music, life would be a mistake."
                                                 - friedrich nietzsche