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

Why does fast-import need to check the validity of idents? + Other ident adventures

From
=?utf-8?B?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason <avarab@gmail.com>
Date
Feb 3, 2021, 11:57 UTC
Message-ID
<87bld8ov9q.fsf@evledraar.gmail.com>
In-Reply-To
<pull.795.v3.git.git.1590870357549.gitgitgadget@gmail.com>

[Originally sent 5 days ago, but seems to have been a victim of the vger.kernel.org problems at the time, re-sending]

On Sat, May 30 2020, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>

Full snipped E-Mail in the archive: https://lore.kernel.org/git/pull.795.v3.git.git.1590870357549.gitgitgadget@gmail.com/

Show 8 quoted lines
> There are multiple repositories in the wild with random, invalid
> timezones.  Most notably is a commit from rails.git with a timezone of
> "+051800"[1].  A few searches will find other repos with that same
> invalid timezone as well.  Further, Peff reports that GitHub relaxed
> their fsck checks in August 2011 to accept any timezone value[2], and
> there have been multiple reports to filter-repo about fast-import
> crashing while trying to import their existing repositories since they
> had timezone values such as "-7349423" and "-43455309"[3].

I've been looking at some of our duplicate logic here after my mktag series where we now use fsck validation. It had a hardcoded "1400" offset value, which I see fast-import.c still has.

Then in mailmap.c we have parse_name_and_email(), then there's split_ident_line() in ident.c, and of course fsck_ident(). record_person_from_buf() in fmt-merge-msg.c, copy_name() and copy_email() in ref-filter.c. Maybe handle_from() in mailinfo.c also counts. Anyway, aside from the last these are all parsers for "author/committer" lines in commits one way or another.

But I was wondering about fast-import.c in particular. I think Elijah's patch here is obviously good an incremental improvement. But stepping back a bit: who cares about sort-of-fsck validation in fast-import.c anyway?

Shouldn't it just pretty much be importing data as-is, and then we could document "if you don't trust it, run fsck afterwards"?

Or, if it's a use-case people actually care about, then I might see about unifying some of these parser functions as part of a series I'm preparing.

Previous: Jeff KingNext: Junio C Hamano
Message 10 of 12 in “fast-import: accept invalid timezones so we can import existing repos”
  1. fast-import: accept invalid timezones so we can import existing reposElijah Newren via GitGitGadget, May 28, 2020
  2. Jonathan NiederMay 28, 2020
  3. fast-import: add new --date-format=raw-permissive formatElijah Newren via GitGitGadget, May 28, 2020
  4. Junio C HamanoMay 28, 2020
  5. Jonathan NiederMay 29, 2020
  6. Jeff KingMay 29, 2020
  7. Junio C HamanoMay 29, 2020
  8. fast-import: add new --date-format=raw-permissive formatElijah Newren via GitGitGadget, May 30, 2020
  9. Jeff KingMay 30, 2020
  10. Why does fast-import need to check the validity of idents? + Other ident adventures=?utf-8?B?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason, Feb 3, 2021
  11. Junio C HamanoFeb 3, 2021
  12. Ævar Arnfjörð BjarmasonFeb 5, 2021

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.