RE: [PATCH JGIT] Circular references shouldn't be created
- From
Sohn, Matthias <matthias.sohn@sap.com>
- Date
- Sep 18, 2009, 06:37 UTC
- Message-ID
- <C89280B882467443A695734861B942B28759DEAA@DEWDFECCR09.wdf.sap.corp>
- In-Reply-To
- <200909180051.47794.robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> wrote on Freitag, 18. September 2009 00:52
Show 45 quoted lines
>torsdag 17 september 2009 23:40:12 skrev Avery Pennarun
> <apenwarr@gmail.com>:
> > On Thu, Sep 17, 2009 at 3:23 PM, Sohn, Matthias
> <matthias.sohn@sap.com> wrote:
> > > void link(final String name, final String target) throws
> IOException {
> > > + if (name.equals(target))
> > > + throw new IllegalArgumentException(
> > > + "illegal circular reference
> : symref " + name
> > > + + " cannot
> refer to " + target);
> >
> > This isn't a very thorough fix. It doesn't catch longer loops, like
> >
> > HEAD -> chicken -> HEAD
> >
> > or
> >
> > a -> b -> c -> d -> a
> >
> > Experimenting with original git.git's implementation, I see that this
> > is allowed:
> >
> > git symbolic-ref refs/heads/boink refs/heads/boink
> >
> > It succeeds and creates a file that looks like this:
> >
> > ref: refs/heads/boink
> >
> > And "git show-ref refs/heads/boink" says: nothing (but returns an
> error code).
> >
> > And "git log refs/heads/boink" says:
> >
> > warning: ignoring dangling symref refs/heads/boink.
> > fatal: ambiguous argument 'refs/heads/boink': unknown revision or
> > path not in the working tree.
> > Use '--' to separate paths from revisions
> >
> > Clearly, in git.git, symref loops are caught at ref read time, not
> > write time. This makes sense, since someone might foolishly twiddle
> > the repository by hand and you don't want to get into an infinite loop
> > in that case. Also, it's potentially useful to allow people to set
> > invalid symrefs *temporarily*, as part of a multi step process.Looks like I was a bit short-sighted yesterday, I will try to cook a better solution.
Show 14 quoted lines
> > I had already written a patch much like this when I decided we need to > do much better. > > I think we should do this in the UI by not allowing the user to make a > choice that would result in a loop and fixing the way the UI resolves > choices. When creating a new branch we should analyze the selected > ref and dereference it if it is a symbolic name like HEAD or if it is a > tag, > and perhaps show it like "HEAD (refs/heads/master)" in the the dialog. > > Using unresolvable refs as the base for a new branch should be > disallowed. >
If we would do it in the EGit UI how about catching such cases in other applications using JGit ?
-- Matthias