Re: [PATCH] fast-import: catch garbage after marks in from/merge

[Jonathan Nieder <jrnieder@gmail.com>]

Pete Wyckoff wrote:
> jrnieder@gmail.com wrote on Sun, 01 Apr 2012 18:12 -0500:

>> Why allow whitespace after the mark number?
>
> Fear of breaking existing fast-import users that might happen
> to have stray whitespace, or \r\n terminators.

Oh.  I think worrying about that sort of thing is a very good thing.
In particular, the possibility of \r\n terminators from an importer
that writes its output in text mode on Windows is somewhat scary.

Luckily such a problem would be caught as soon as the importer uses
a filemodify (M) command:

 - if it quotes the path:

	fatal: Garbage after path in: M ...

 - if it doesn't quote the path, the resulting import would use
   filenames with trailing CRs.

So we narrowly escape that problem.

Other types of stray whitespace at eol seem less likely but it's
probably worth letting a patch like this incubate in the "next" branch
for a little while to see if anyone complains.

The reason to care instead of just being permissive is that git
fast-import is not the only fast-import backend.  The tighter we can
make the parsing without breaking existing frontends, the better the
chance that a frontend that was only tested against git fast-import
will be usable against all other backends, too.

In other words, there are two requirements in tension here
(compatibility with sloppy frontends, compatibility with fussy
backends).  Some day fast-import might want to learn a --permissive
option to reconcile them.  In this particular case being strict
unconditionally seems safe enough.

My only other hint is that as Dmitry mentioned it would be nice to
nice to use the same logic for other places where fast-import parses a
number at the end of a line, so people don't keep on making the same
print vs println mistake and sending new patches to detect it in each
place one at a time.

Sane?

Thanks,
Jonathan