Re: [PATCH] cvsserver: change generation of CVS author names

From: Lars Noschinski <lars@public.noschinski.de>
Date: Jan 4, 2009, 11:13 UTC
Message-ID: <20090104111245.GA7732@lars.home.noschinski.de>
In-Reply-To: <7vwsdc3ulg.fsf@gitster.siamese.dyndns.org>

* Junio C Hamano <gitster@pobox.com> [09-01-03 23:36]:

Show 14 quoted lines

>Fabian Emmes <fabian.emmes@rwth-aachen.de> writes:
>
>> CVS username is generated from local part email address.
>> We take the whole local part but restrict the character set to the
>> Portable Filename Character Set, which is used for Unix login names
>> according to Single Unix Specification v3.
>>
>> Signed-off-by: Fabian Emmes <fabian.emmes@rwth-aachen.de>
>> Signed-off-by: Lars Noschinski <lars@public.noschinski.de>
>
>Stating "we should have done this from day one" is one thing (even though
>"because some standard says so" is not particularly a good justification
>without "and matches the way people use CVS in the real world in practice"
>appended to it).

Documentation about valid cvs/rcs usernames is a bit scarce. When we wrote the patch, we did not find much more information than "the cvs username is supposed to be the login name". In my limited CVS experience, I never saw CVS user names which were not (unix) login names.

After this mail, I looked to the RCS source code (see checkidentifier() in rcslex.c) which tells us that anything (encoded in ISO-8859-1) consisting of IDCHAR, LETTER, Letter, DIGIT and PERIOD, containing at least one IDCHAR, LETTER or Letter is a valid username (for the character classes, see http://avalon.hoffentlich.net/~cebewee/rcs-charmap.txt) The most important character _not_ allowed in an user name is the @ sign, so we cannot use the full mail address.

So our patch generates a valid username for any "sane" local part. In a few corner cases like "!#$%&'*+-/=?^_`.{|}~@example.com" our patch generates a result worse than the original - an empty username. This is probably something we should fix.

Obviously, the short names generated are not necessarily unique, which can be irritating, but is not a problem from a technical point of view. Improving this would probably require to store a map of mail addresses to cvs user names.

Show 11 quoted lines

>"We should suddenly change the behaviour" is quite a different thing and
>it depends on what follows that sentence if the change is justifiable.  We
>do not want to hear "...; screw the existing repositories if they have
>nonconforming names.".  It is Ok if it is "...; existing repositories will
>be affected, but the damage is limited to very minor set of operations,
>namely X, Y and Z".
>
>In other words, is there any backward compatibility issue when a
>repository that has served existing CVS users and checkouts with older
>version switches to the patched one?  If there is one, is that grave
>enough that we should care?

Obviously the reported user names change. To the best of my knowledge (but I'm just a barely experienced CVS user) those names are not stored anywhere on the client and are regenerated by git-cvsserver for every request, so even old repositories get the new names for all commits.

   - Lars.