RFC: New Response type to aid rlog reponse parsing

[patrick keshishian]

Greetings,

I am hoping you would be open to adding a new (optional) response
type. Motivation behind this addition is to aid clients distinguish
the free-form log/commit messages in response to an rlog
query/request.

The new response type I am proposing is LOGM. A server recognizing it
in Valid-responses list, will prepend "LOGM " to each log/commit
message line (rather than "M ") while servicing rlog requests. Servers
not recognizing it will work as they do today.

With this change in effect, all guess-work over where a log/commit
message begins and ends is eliminated.


I ran into at least three different log/commit message forms which
confused CVSps and then in turn Git-cvsimport.

I have documented this [1] for interested parties. It is a rather
long-ish document. I have done my best to organize the information for
easy digestion and reference.


Attached is a patch implementing LOGM change against cvs-1.11.23
downloaded from your site (unfortunately I couldn't get sources
through anoncvs).

The changes are fairly simple, and the best I can tell, backward
compatible with unpatched components.

Given this, I hope you are open to accepting these changes.

Cheers,
--patrick

[1] http://sidster.com/code/cvs2git/

[Thorsten Glaser]

Hi,

>I have documented this [1] for interested parties. It is a rather
>long-ish document. I have done my best to organize the information for
>easy digestion and reference.

how interesting. I’ve added a “cvs suck” command to the 1.12 flavour
shipped with MirBSD and Debian (current de-facto “upstream” of GNU
CVS as I seem to be the only one working on it) to download individual
,v files, but that would of course only work if the server supports it.

>Attached is a patch implementing LOGM change against cvs-1.11.23

I’ve looked at it and it’s trivial and probably correct enough,
so I’ll patch my 1.12 branch similarily and add to the (in)“sanity”
testsuite. Then you’ll only have to convince the OpenCVS maintainers
to adapt (if it still exists, that is, haven’t checked) and everyone
is compatible.

Thanks,
//mirabilos
PS: Due to Google’s inability to honour basic Internet standards,
direct messages from Googlemail to my eMail address are likely
to not reach me, or only delayed by up to several weeks.
--
> Wish I had pine to hand :-( I'll give lynx a try, thanks.

Michael Schmitz on nntp://news.gmane.org/gmane.linux.debian.ports.68k
a.k.a. {news.gmane.org/nntp}#news.gmane.linux.debian.ports.68k in pine

[patrick keshishian]

Hi Thorsten,

On 11/4/17, Thorsten Glaser <t...@mirbsd.de> wrote:
> Hi,
>
>>I have documented this [1] for interested parties. It is a rather
>>long-ish document. I have done my best to organize the information for
>>easy digestion and reference.
>
> how interesting. I’ve added a “cvs suck” command to the 1.12 flavour
> shipped with MirBSD and Debian (current de-facto “upstream” of GNU
> CVS as I seem to be the only one working on it) to download individual
> ,v files, but that would of course only work if the server supports it.
>
>>Attached is a patch implementing LOGM change against cvs-1.11.23
>
> I’ve looked at it and it’s trivial and probably correct enough,
> so I’ll patch my 1.12 branch similarily and add to the (in)“sanity”
> testsuite. Then you’ll only have to convince the OpenCVS maintainers
> to adapt (if it still exists, that is, haven’t checked) and everyone
> is compatible.

Thanks for the quick response and vote of confidence.

As far as I can tell OpenCVS has been pretty dormant. I was going to
give them a heads up after getting a feel for the reception of this
change by CVS proper.

Cheers,
--patrick

p.s., sorry about my gmail address :(

[Thorsten Glaser]

Hi patrick,

>As far as I can tell OpenCVS has been pretty dormant. I was going to
>give them a heads up after getting a feel for the reception of this
>change by CVS proper.

as far as I can tell, nobody develops “CVS proper” any more.

I’m pretty much doing my own thing with the 1.12 branch only,
merging patches thrown into my general direction and trying
to take over distro packages, but I’m not a full upstream
maintainer (yet).

bye,
//mirabilos
--
Stéphane, I actually don’t block Googlemail, they’re just too utterly
stupid to successfully deliver to me (or anyone else using Greylisting
and not whitelisting their ranges). Same for a few other providers such
as Hotmail. Some spammers (Yahoo) I do block.

[Thorsten Glaser]

patrick keshishian dixit:

>Attached is a patch implementing LOGM change against cvs-1.11.23

I’ve implemented it differently, especially in order to never
send partial lines to the client (in a network packet). If you
wish, have a look at / review the following patch.

I’m currently sanity.sh-testing it, this will take the night.

--- src/gnu/usr.bin/cvs/src/buffer.c:1.2 Sat Oct 22 14:33:29 2016
+++ src/gnu/usr.bin/cvs/src/buffer.c Sat Nov 18 22:05:10 2017
@@ -1052,7 +1055,14 @@ buf_copy_lines (struct buffer *outbuf, s
}

/* Put in the command. */
- buf_append_char (outbuf, command);
+ switch (command) {
+ case CVS_OUTPUT_EX_LOGM:
+ buf_output0 (outbuf, "LOGM");
+ break;
+ default:
+ buf_append_char (outbuf, command);
+ break;
+ }
buf_append_char (outbuf, ' ');

if (inbuf->data != nldata)
--- src/gnu/usr.bin/cvs/src/client.c:1.8 Sat Aug 12 01:51:22 2017
+++ src/gnu/usr.bin/cvs/src/client.c Sat Nov 18 22:05:11 2017
@@ -3079,6 +3079,7 @@ struct response responses[] =
rs_optional),
RSP_LINE("M", handle_m, response_type_normal, rs_essential),
RSP_LINE("Mbinary", handle_mbinary, response_type_normal, rs_optional),
+ RSP_LINE("LOGM", handle_m, response_type_normal, rs_optional),
RSP_LINE("E", handle_e, response_type_normal, rs_essential),
RSP_LINE("F", handle_f, response_type_normal, rs_optional),
RSP_LINE("MT", handle_mt, response_type_normal, rs_optional),
--- src/gnu/usr.bin/cvs/src/cvs.h:1.7 Fri Oct 21 16:41:16 2016
+++ src/gnu/usr.bin/cvs/src/cvs.h Sat Nov 18 22:05:12 2017
@@ -920,11 +921,15 @@ void tag_check_valid (const char *, int,

/* From server.c and documented there. */
void cvs_output (const char *, size_t);
+void cvs_output_ex (const char *, size_t, int);
void cvs_output_binary (char *, size_t);
void cvs_outerr (const char *, size_t);
void cvs_flusherr (void);
void cvs_flushout (void);
void cvs_output_tagged (const char *, const char *);
+int supported_response (const char *);
+
+#define CVS_OUTPUT_EX_LOGM 0x80000001

extern const char *global_session_id;

--- src/gnu/usr.bin/cvs/src/log.c:1.1.101.2 Tue Apr 19 20:33:18 2005
+++ src/gnu/usr.bin/cvs/src/log.c Sat Nov 18 22:05:12 2017
@@ -145,6 +148,7 @@ static void log_version (struct log_data
RCSNode *, RCSVers *, int);
static int log_branch (Node *, void *);
static int version_compare (const char *, const char *, int);
+static void logm_output (const char *);

static struct log_data log_data;
static int is_rlog;
@@ -1681,11 +1685,10 @@ log_version (struct log_data *log_data,
cvs_output ("*** empty log message ***\n", 0);
else
{
+ /* assert: last thing cvs_output’ed was a newline */
/* FIXME: Technically, the log message could contain a null
byte. */
- cvs_output (p->data, 0);
- if (((char *)p->data)[strlen (p->data) - 1] != '\n')
- cvs_output ("\n", 1);
+ logm_output(p->data);
}
}

@@ -1780,3 +1783,23 @@ version_compare (const char *v1, const c
++v2;
}
}
+
+static void
+logm_output(const char *str)
+{
+ /* assert: str is not empty */
+ size_t len = strlen(str);
+ int buftag = 'M';
+ static char has_logm = 0;
+
+ if (server_active) {
+ if (!has_logm)
+ has_logm = supported_response("LOGM") ? 1 : 2;
+ if (has_logm == 1)
+ buftag = CVS_OUTPUT_EX_LOGM;
+ }
+
+ cvs_output_ex(str, len, buftag);
+ if (/*len > 0 &&*/ str[len - 1] != '\n')
+ cvs_output_ex("\n", 1, buftag);
+}
--- src/gnu/usr.bin/cvs/src/server.c:1.14 Sat Aug 12 01:08:25 2017
+++ src/gnu/usr.bin/cvs/src/server.c Sat Nov 18 22:05:12 2017
@@ -548,8 +553,8 @@ alloc_pending_warning (size_t size)



-static int
-supported_response (char *name)
+int
+supported_response (const char *name)
{
struct response *rs;

@@ -7703,6 +7708,12 @@ krb_encrypt_buffer_initialize( struct bu
void
cvs_output (const char *str, size_t len)
{
+ cvs_output_ex (str, len, 'M');
+}
+
+void
+cvs_output_ex (const char *str, size_t len, int buftag)
+{
if (len == 0)
len = strlen (str);
#ifdef SERVER_SUPPORT
@@ -7711,7 +7722,7 @@ cvs_output (const char *str, size_t len)
if (buf_to_net)
{
buf_output (saved_output, str, len);
- buf_copy_lines (buf_to_net, saved_output, 'M');
+ buf_copy_lines (buf_to_net, saved_output, buftag);
}
# if HAVE_SYSLOG_H
else
@@ -7726,7 +7737,7 @@ cvs_output (const char *str, size_t len)
if (protocol)
{
buf_output (saved_output, str, len);
- buf_copy_lines (protocol, saved_output, 'M');
+ buf_copy_lines (protocol, saved_output, buftag);
buf_send_counted (protocol);
}
# if HAVE_SYSLOG_H


bye,
//mirabilos
--
13:22⎜«neurodamage» mira, what's up man? I have a CVS question for you in #cvs
13:22⎜«neurodamage» since you're so good w. it │ «neurodamage:#cvs» i love you
13:28⎜«neurodamage:#cvs» you're a handy guy to have around for systems stuff ☺
16:06⎜<Draget:#cvs> Thank god I found you =) 20:03│«bioe007:#cvs» mira2k: ty
17:14⎜<ldiain:#cvs> Thanks big help you are :-) <bioe007> mira|nwt: ty again
18:35⎜«alturiak:#cvs» mirabilos: aw, nice. thanks :o
18:36⎜«ThunderChicken:#cvs» mirabilos FTW! 23:03⎜«mithraic:#cvs» aaah. thanks
18:41⎜«alturiak:#cvs» phew. thanks a bunch, guys. you just made my weekend :-)
18:10⎜«sumit:#cvs» mirabilos: oh ok.. thanks for that
21:57⎜<bhuey:#cvs> yeah, I really appreciate help
18:50⎜«grndlvl:#cvs» thankyou 18:50⎜«grndlvl:#cvs» worked perfectly
20:50⎜<paolo:#cvs> i see. mirabilos, thnks for your support
00:36⎜«halirutan:#cvs» ok, the obvious way:-) thx
18:44⎜«arcfide:#cvs» mirabilos, I am running OpenBSD. 18:59⎜«arcfide:#cvs»
Hrm, yes, I see what you mean. 19:01⎜«arcfide:#cvs» Yeah, thanks for the help.
21:33⎜«CardinalFang:#cvs» Ugh. Okay. Sorry for the dumb question. Thank you
21:34⎜<centosian:#cvs> mirabilos: whoa that's sweet
21:52⎜«garrett__:#cvs» much appreciated «garrett__:#cvs» thanks for your time
23:39⎜<symons:#cvs> this worked, thank you very much 16:26⎜<schweizer:#cvs> ok
thx, i'll try that 20:00⎜«stableable:#cvs» Thank you. 20:50⎜«s833:#cvs»
mirabilos: thanks a lot. 19:34⎜<bobbytek:#cvs> Thanks for confirming :)
20:08⎜<tsolox:#cvs> ...works like a charm.. thanks mirabilos

[patrick keshishian]

Hi Thorsten,

Sorry for the delayed reply (project I'm working on is taking
far too many cycles).

On Sun, Nov 19, 2017 at 07:58:47PM -0800, patrick keshishian wrote:

> ---------- Forwarded message ----------
> From: Thorsten Glaser <t...@mirbsd.de>
> Date: Sat, 18 Nov 2017 23:56:07 +0000 (UTC)
> Subject: Re: RFC: New Response type to aid rlog reponse parsing
> To: patrick keshishian <pkes...@gmail.com>
> Cc: info...@nongnu.org
>
> patrick keshishian dixit:
>
> >Attached is a patch implementing LOGM change against cvs-1.11.23
>
> I've implemented it differently, especially in order to never
> send partial lines to the client (in a network packet). If you
> wish, have a look at / review the following patch.

Just from reading the diff it looks OK. However, I didn't comprehend
the sending "partial lines to the client" bit.

> I'm currently sanity.sh-testing it, this will take the night.

Hopefully your tests don't show any regression.

I can patch my CVS server with your diff when I break away from
my work.

Thanks for the update!

Happy Thanksgiving!

--patrick

p.s., sending from a different account (not subscribed to CVS list)
hopefully it'll make it.

> + /* assert: last thing cvs_output???ed was a newline */

[Thorsten Glaser]

Hi patrick,

>Sorry for the delayed reply (project I'm working on is taking
>far too many cycles).

don’t worry, I know all about that.

>Just from reading the diff it looks OK. However, I didn't comprehend
>the sending "partial lines to the client" bit.

There was some comment in cvs_output() about being careful to
only write to the output stream when a full line has been
accumulated.

>> I'm currently sanity.sh-testing it, this will take the night.
>
>Hopefully your tests don't show any regression.

It didn’t. I also manually tested the server by interacting
with 'cvs server' from the commandline, and it looked fine.

>I can patch my CVS server with your diff when I break away from
>my work.

Mind that this diff is based on continued development from
the 1.12.13 release, not the ancient 1.11.x branch, though.

>Thanks for the update!

You’re welcome!

>Happy Thanksgiving!

Erm, not here (Europe), but thanks anyway, and same to you.

>p.s., sending from a different account (not subscribed to CVS list)
>hopefully it'll make it.

It made it, both over the list and separately.

bye,
//mirabilos
--
Support mksh as /bin/sh and RoQA dash NOW!
‣ src:bash (358 (380) bugs: 1 RC, 246 (262) I&N, 111 (117) M&W, 0 F&P)
‣ src:dash (110 (129) bugs: 0 RC, 61 (67) I&N, 49 (62) M&W, 0 F&P)
‣ src:mksh (0 bugs: 0 RC, 0 I&N, 0 M&W, 0 F&P)
dash has two RC bugs they just closed because they don’t care about quality…