Re: [PATCH] Remove branch by putting a null sha1 into the ref file.

From: Christian Couder <chriscool@tuxfamily.org>
Date: Sep 20, 2006, 02:56 UTC
Message-ID: <200609200456.24714.chriscool@tuxfamily.org>
In-Reply-To: <Pine.LNX.4.64.0609180926590.4388@g5.osdl.org>

Linus Torvalds wrote:

Show 17 quoted lines

> On Mon, 18 Sep 2006, Christian Couder wrote:
> > With the new packed ref file format from Linus, this should be
> > the new way to remove a branch.
>
> Well, it's not really sufficient.
>
> Somebody should add this test-case
>
> 	git branch test
> 	git branch -d test
> 	git branch test/first
>
> which should work.
>
> It's entirely possible that the proper way to do branch deletion with
> packed branches is to simply re-pack without the old branch, rather than
> the negative branch model.

Yes, and anyway git-branch should probably use some other git command for this, like "git-update-ref <ref> 0" or "git-update-ref --delete <ref>".

Show 8 quoted lines

> I couldn't really decide.
>
> However: this part is definitely correct, considering that we allow the
> null sha1 in other places.
>
> > "refs.c" is fixed so that a null sha1 for a deleted branch does
> > not result in "refs/head/deleted does not point to a valid
> > commit object!" messages.

Well, it was in one of your patch already but I had not seen it. And I wrongly assumed that your patches were all merged into next but this one was not.

Show 15 quoted lines

> And this last part is conceptually ok, but I think the implementaion is
> wrong:
> > "t/t3200-branch.sh" is fixed so that it uses git-show-ref
> > instead of checking that the ref does not exist when a branch
> > is deleted.
>
> I think you should change the
>
> 	! git-show-ref --verify --quiet -- "refs/heads/d/e/f" &&
>
> into
>
> 	git-show-ref --verify --quiet -- "refs/heads/d/e/f" ||
>
> instead, no?

It seems Junio found it ok, and I still think it is ok, but my description of the change is bad.

Maybe git-show-ref should have an option to check that a ref does not exist for example "git-show-ref --inexistant --quiet -- "refs/heads/d/e/f".

Thanks for your comments.

Christian.