git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Robin RosenbergNext: Shawn O. Pearce
Message 4 of 5 in “Circular references shouldn't be created”
  1. Circular references shouldn't be createdSohn, Matthias, Sep 17, 2009
  2. Avery PennarunSep 17, 2009
  3. Robin RosenbergSep 17, 2009
  4. Sohn, MatthiasSep 18, 2009
  5. Shawn O. PearceSep 18, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.