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

Re: [PATCH 2/2] sequencer: fix "rebase -i --root" corrupting author header timezone

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Jul 30, 2018, 18:45 UTC
Message-ID
<CAPig+cQyWHSmww59qP8DTyCFpbB8HdL+cnANX8uSXVsuOviBCw@mail.gmail.com>
In-Reply-To
<f079bf7b-d627-20dc-cd93-dfa3496f6877@talktalk.net>
On Mon, Jul 30, 2018 at 8:20 AM Phillip Wood <phillip.wood@talktalk.net> wrote:
Show 25 quoted lines
> On 30/07/18 10:29, Eric Sunshine wrote:
> > When "git rebase -i --root" creates a new root commit, it corrupts the
> > "author" header's timezone by repeating the last digit:
> > [...]
> > Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> > ---
> > diff --git a/sequencer.c b/sequencer.c
> > @@ -654,6 +654,7 @@ static int write_author_script(const char *message)
> > +     strbuf_addch(&buf, '\'');
> > @@ -724,7 +725,11 @@ static const char *read_author_ident(struct strbuf *buf)
> > -             sq_dequote(in);
> > +             if (!sq_dequote(in)) {
> > +                     warning(_("bad quoting on %s value in '%s'"),
> > +                             keys[i], rebase_path_author_script());
> > +                     return NULL;
>
> I think we want to handle the broken author script properly rather than
> returning NULL. If we had a single function
> int read_author_script(const char **name, const char **author, const
> char **date)
> to read the author script that tried sq_dequote() and then fell back to
> code based on read_env_script() that handled the missing "'" at the end
> and also the bad quoting of "'" if sq_dequote() failed it would make it
> easier to fix the existing bugs, rather than having to fix
> read_author_ident() and read_env_script() separately. What do you think?

That makes sense as a long-term plan, however, I'm concerned with the immediate problem that a released version of Git can (and did, in my case) corrupt commit objects. So, in the short term, I think it makes sense to get this minimal fix landed, and build the more "correctly engineered" solution on top of it, without the pressure of worrying about corruption spreading.

Previous: Phillip WoodNext: Eric Sunshine
Message 6 of 13 in “fix "rebase -i --root" corrupting root commit”
  1. 0/2 fix "rebase -i --root" corrupting root commitEric Sunshine, Jul 30, 2018
  2. 1/2 sequencer: fix "rebase -i --root" corrupting author headerEric Sunshine, Jul 30, 2018
  3. Johannes SchindelinJul 30, 2018
  4. 2/2 sequencer: fix "rebase -i --root" corrupting author header timezoneEric Sunshine, Jul 30, 2018
  5. Phillip WoodJul 30, 2018
  6. Eric SunshineJul 30, 2018
  7. Eric SunshineJul 30, 2018
  8. Johannes SchindelinJul 30, 2018
  9. Eric SunshineJul 30, 2018
  10. Phillip WoodJul 30, 2018
  11. Junio C HamanoJul 30, 2018
  12. Johannes SchindelinJul 30, 2018
  13. Eric SunshineJul 30, 2018

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.