search

rcls / crap

Cvs Remote Access Program
GNU General Public License v3.0
46 stars 12 forks source link

Crap should honor CVS repository syntax #4

Closed lewellyn closed 10 years ago

lewellyn commented 10 years ago

Currently, one can't just take a CVS command and intuit the syntax for crap. It would be nice for users if this was possible.

To use a fairly good (well, good for illustration...) example, MirBSD's jupp documents their command for a CVS checkout as follows (on https://www.mirbsd.org/jupp.htm#docs ):

$ env CVS_RSH=ssh cvs -d _anoncvs@anoncvs.mirbsd.org:/cvs co -PA jupp

It would be sensical to lots of people for this to work:

$ crap-clone :ext:_anoncvs@anoncvs.mirbsd.org:/cvs jupp

However, this reports:

ssh: Could not resolve hostname anoncvs.mirbsd.org:: Name or service not known
Reading from CVS server failed: Connection reset by peer

It's not exactly documented well that one ought to replace the : with a / ending up with :ext:_anoncvs@anoncvs.mirbsd.org//cvs.

However, this particular repository is a bit "special" in that even that specification won't work:

Valid-requests Root Valid-responses valid-requests Command-prep Referrer Repository Directory Relative-directory Max-dotdot Static-directory Sticky Entry Kopt Checkin-time Modified Is-modified UseUnchanged Unchanged Notify Hostname LocalDir Questionable Argument Argumentx Global_option Gzip-stream wrapper-sendme-rcsOptions Set expand-modules ci co update diff log rlog list rlist global-list-quiet ls add remove update-patches gzip-file-contents status rdiff tag rtag import admin export history release watch-on watch-off watch-add watch-remove watchers editors edit init annotate rannotate noop version suck
RCS file name '/cvs/contrib/code/jupp/Attic/.jmacsrc,v' does not start with prefix '/cvs/jupp/'

Instead, it requires:

$ crap-clone :ext:anoncvs@anoncvs.mirbsd.org//cvs contrib/code/jupp

This only vaguely resembles the original command, though it seems reasonably sensical once you've thought about what's going on. But, it seems that it should be possible to make it work as someone new to the tool might expect.

rcls commented 10 years ago

Ok it looks to me like there are two things here, could you correct me if I have misunderstood:

First, crap-clone is getting the separator between hostname and path wrong; it should be a ':' instead of looking for a '/'. Should be easy to fix... I suspect the precise logic should be ':' is a separator if present, if not present then '/' starts the path - I'll attempt to confirm the details.

Second, "jupp" vs. "contrib/code/jupp" - this looks like a use of cvs modules, where cvs allows a config file to graft parts of the repo to different paths... Yup crap-clone does not support that, you have to do the translation by hand.

There is no reason why modules should not be implemented in crap-clone it is just a pain to shoe-horn it into all the path handling.... I doubt that I personally will implement this.

lewellyn commented 10 years ago

Indeed, this felt like two separate issues, but I couldn't find a good way to validate that (and reporting them) beyond doing the work myself to try to fix it. :) And if it turned out to be "by design", that'd be wasted work.

But yes, : is the common separator used for CVSROOT definitions, even if it's marked optional by the Cederqvist: http://ximbiot.com/cvs/manual/cvs-1.12.13/cvs_2.html#SEC26

However, the // is probably considered buggy behavior, since crap-clone treats a single slash as a separator and does not seem to pass it to the server (n.b. I haven't looked to verify this in the code yet).

And the second issue is indeed related to the use of modules. I didn't know if it was something which was actually supposed to work or not. I'm getting tempted to write some docs, and that might be worthwhile to mention in a "Caveats" section. :) It's one of those things that once you've seen it once, you know how to fix it again later... But the first time can cause some head scratching.

rcls commented 10 years ago

I have fixed the parsing of :ext: - If you use : then it separates the host and path, a / now starts the path (so // should no longer be needed).

lewellyn commented 10 years ago

I've tested that : works properly, at least for the default port, and // is no longer needed for me.

I've noted the modules bit in the BUGS & CAVEATS section of my man page pull request. As I don't feel like wading down the rabbit hole that is labeled "CVS modules" myself right now, I am fine with just writing a very short blurb and leaving it at that. :)

Thanks!