Re: CVSNT text-vs-binary

Michael Haggerty <[email protected]> Mon, 06 Apr 2015 13:08:07 +0200
Newsgroups gmane.comp.version-control.subversion.cvs2svn.devel
Message-ID <[email protected]>
On 04/06/2015 12:46 AM, sags wrote:
> I attach here a patch I suggest for handling the CVSNT-specific way
> of dealing with text-versus-binary attributes. It is split into small
> *.patch files to ease the review.

Awesome! It looks like you have really integrated your changes well into
the existing structure of cvs2svn, and you commits are in wonderfully
easy-to-digest steps.

I've applied your patches and pushed them to my GitHub account to make
them easier to view:

    https://github.com/mhagger/cvs2svn

branches sags/rcsparse-newphrases and sags/cvsnt-modes.

> The patches in the A-series modify rcparse to parse newphrases and
> pass then to the client.

These changes look very sane. I like the way you have implemented
backwards-compatibility.

The rcsparse code originally comes from the ViewVC project,
http://viewvc.tigris.org/. Here the situation is not as bad as it is for
svntest (see my other email with respect to your "alignment" patches),
because the upstream project is not evolving very quickly. I just
updated cvs2svn to the current version of svntest; there were only a few
lines of code that were different.

Nevertheless, it would be best for your changes to be applied to svntest
first, and then synchronized from there to cvs2svn. I've CCed cmpilato
at that project; Michael, is this something you'd be willing to consider?

> The patches in the B-series modify cvs2svn-proper to deal with
> per-revision 'file modes', as used by CVSNT. A 'mode' property is
> added to revisions and set according to 'kopt' revision headers, if
> any. A number of PropertyHandlers (those that deal with EOLs and
> keyword expansion) are then changed to work both on files and
> revisions.

This will be really helpful for people trying to migrate from CVSNT,
which until now has been a big gap in the features of cvs2svn. I'm
especially happy to help people get away from CVSNT, which has gotten
quite nastily proprietary. Its sponsor seems to honor the GPL only in
the most reluctant and inconvenient way that they can.


I thought I remembered that CVSNT also added a new kind of binary "diff"
format for storing the difference between revisions of binary files. I
also thought that such revisions would be marked with some other "kopt"
value. Is that right, or is "b" the only kopt value that can appear in
CVSNT?

If there is a new binary "diff" format, I suppose that means that CVSNT
users will have to continue using --use-cvs to extract their revisions.
Is that correct?

> All tests from run_tests.py pass or fail in an identical manner both
> before and after applying these patches. The tests that fail (on my
> system, Windows + CVSNT) are:
>=20
> (i) The 3 marked as XFAIL:
>     XFAIL: run-tests.py 86: reveal a second bug that created a branch twi=
ce
>     XFAIL: run-tests.py 128: optimal tag after deleting files
>     XFAIL: run-tests.py 162: check adding .cvsignore to an existing branc=
h
>=20
> (ii) The following two, which fail with a RunProgramException:
>     FAIL:  run-tests.py 132: verify that --use-rcs and --use-internal-co =
agree
>     FAIL:  run-tests.py 133: verify that --use-internal-co --exclude=3D..=
. works
>=20
>     Looking at the code I see these two make use of "--use-rcs" and I don=
't have a
>     RCS executable, so I guess this is normal.
>=20
> As to handling CVSNT repositories, I used this version and it worked
> fine. That repo has files with "expand @b@;" at file level (being
> created a long time ago, I guess with standard CVS), files without
> file-level "expand" but with per-revision "kopt b;" (CVSNT-style),
> and files that have both types of headers. What I don't have is files
> with keywords to be expanded so that is not really tested, only EOL
> handling.

It would be great if you could put a couple of smallish CVSNT files in
our test suite and write some tests that verify that their revisions are
being extracted correctly. Probably these files should go into a new
test repository under test-data. Having tests will make it clear that
the feature works and will also prevent the feature from being broken
inadvertently by future changes.

Given that CVSNT is apparently no longer freely downloadable, it will be
problematic for some people (me, for instance) to run any tests that
rely on a CVSNT executable. So if it is possible for part of the CVSNT
support to be tested without CVSNT, that would be very helpful. Ideally,
tests that depend on CVSNT would also somehow disable themselves when
CVSNT isn't installed.

> Hope this will be of some help, sags
>=20
> PS: I hope this is the correct list for suggesting patches. If not, I
> apologize.

This is the right list.

Yours,
Michael

--=20
Michael Haggerty
[email protected]

------------------------------------------------------
http://cvs2svn.tigris.org/ds/viewMessage.do?dsForumId=3D1667&dsMessageId=3D=
3110929

To unsubscribe from this discussion, e-mail: [[email protected]=
s.org].