frej / fast-export

A mercurial to git converter using git-fast-import
http://repo.or.cz/w/fast-export.git
808 stars 255 forks source link

hg2git: Fix up user ids with '<' in the wrong place #243

Closed bwh-ct closed 4 years ago

bwh-ct commented 4 years ago

user_re currently matches ids that git fast-import will reject, like 'name <email@example.com' or 'name> email@example.com'. Change it to disallow both '<' and '>' anywhere except where they are required.

templatefilters.email() also doesn't filter these out, so replace any remaining '<' and '>' characters with '?'.

Closes #242.

frej commented 4 years ago

As you have noticed, Mercurial allows almost anything as an author string, but this is not the right way to deal with the problem. If for example you are trying to do a high-quality repository conversion you want the conversion to stop when errors are found and not just silently mangle an author name.

For historical reasons, fast-export contains some arbitrary name mangling, but that is something I'd prefer to remove, I definitely do not want to add new ways to magically try to fix up malformed author names. Fast-export tries to not inflict arbitrary naming policy on the user, instead it aims to provide mechanisms allowing the user to apply their own policy. In this case, the proper way to deal with malformed author strings is to use a mapping file or a plugin.

I don't reject patches lightly, considering the effort that has gone into creating them, but from the explanation above I hope you understand my position.

bwh-ct commented 4 years ago

OK, I originally tried to do this with a plugin but found that the committer id is not exposed to plugins. So shall I send a PR to fix that?

frej commented 4 years ago

the committer id is not exposed to plugins

That is an oversight, I would be much obliged if you provided a PR exposing the committer to the plugins.

frej commented 4 years ago

I whipped up a patch, check that you can do the mangling you want in a plugin with the fix in #244 and report back. If it works I'll merge #244 to master.