fix typo in log message

add comment as suggested by vlorentz

rebased patch

new patch with some suggestions from ardumont applied

remove self.log.info() call that was used for debugging

new version which uses a mock class for the test

move path-safety check into a helper function

This PDF contains my SVN Loader review report.

• add missing check for "/../" to the rcsparse code path

I found one additional problem. See D6823.

Unless I have overlooked something, all currently known issues have now been addressed.

integrate tweaks suggested by vlorentz and squash commits

I have started test conversions of the OpenBSD CVS repository.

remove unnecessary tweaking of final \n in log buffer

fix typo in comment

addressed review comments; see my responses to various comments for details

In D6708#174520, @ardumont wrote:

I guess it's currently limited to deal with "only" a 2 keywords in the same line case.
And we'll try and fix futher if any other cases is detected later.

In D6678#174111, @stsp wrote:

• add a test for conversion of a file which contains a Header keyword

• add a test for conversion of a file which contains a Header keyword

In D6678#174093, @stsp wrote:

And since our tests control the absolute paths to the test repositories they also control the expanded value of keywords like $Header$.
This means the tests would pass even without this fix in place. So I am not sure such a test would really be meaningful.

In D6678#174093, @stsp wrote:

This means the tests would pass even without this fix in place. So I am not sure such a test would really be meaningful.

In D6678#174084, @vlorentz wrote:

wow...

Good catch, thanks. Could you add a test for this (so we don't accidentally change it if we ever refactor the loader in the future)?

This patch is wrong. Keywords stored in already-expanded form in the repository are no longer being expanded. We need a different fix for this issue.

D6684 addresses another keyword expansion issue found while testing conversion of CVS's own history.

The above problem with the Header keyword can be worked around (at least for the GNU savannah site) with the patch in D6678.

Another problem with keyword expansion found during testing:

In T3691#73518, @stsp wrote:

There is another problem related to keywords: Some CVS-based projects use custom keywords, instead of the standard $Id$ keyword. This prevents wrong expansion of $Id$ when code is imported from one project to another. Usually the project's name will be used as the custom keyword name, such as $OpenBSD$ or $NetBSD$, instead of $Id$. At present, to expand keywords correctly in this case, we need to use the pserver access method to benefit from server-side keyword expansion. But we will end up with different hashes if rsync is used to import the same origin again. We might be able to auto-detect use of custom keywords if the rsync server allows access to the CVSROOT folder, but this is not always the case. If CVSROOT is hidden from rsync, the only reliable way to detect custom keywords would be a parameter that gets passed into the loader. We could, for example, allow passing the name of a custom keyword as a parameter embedded in the origin URL.

changes suggested by vlorentz