Re: update: patch: relative path bugs in cvs (client/server mode only)

[Chris Bohn]

> > regenerated against the head, attached to issue 81, and pasted below
> > (additionally, I fixed a hard-coded path in sanity.sh to be
> > ${CVSROOT_DIRNAME}):
>
> Doesn't look like it.  You may have hardcoded your own path and then
> fixed that.

You are correct: in my earlier patch, I had accidently put a hard-coded path, 
which I fixed in the last patch.

> > Log entry:
> > 2004-03-30        Chris Bohn  <cbohn@rrinc.com>
> >
> >  * client.c, client.h (send_files, send_file_names, send_max_dotdot):
>
> This is a bit more verbose than really necessary for a ChangeLog.
> Please be more abstract in future ChangeLogs.  Describe fixes in more
> detail in the emails containing the patch, in the issue, or in the
> comments in the code when appropriate.

OK; sorry about that.  I thought the information needed to be somewhere, and I 
happened to pick this spot.

> I took out the call in send_file_names.  It is probably unecessary.  The
> test suite tests a lot of cases, and if we couldn't find a broken one
> yet, then I am content to wait for the bug report so we can fix it and
> add a test.

I prefer to error on the side of caution because other times cvs has changed 
with little concern for backward compatibility, and it has bitten me (e.g., 
expansion of the loginfo string to use double quotes instead of single quotes; 
it broke my ability to use my one loginfo script with both cvs servers before 
and after this change).  I do understand one needs to weigh the chance of some 
case breaking vs. keeping the code cleaned up.

> Your patch was extremely mangled, with tabs converted to spaces and who
> knows what else.  I read it and reimplemented it, mostly.

This was only my second patch to cvs, and the first was just a week or so ago. 
Since going through the process, I've wondered why you take patches as diffs in 
text files.  That seems more complicated than needed and seems like more work 
for those who integrate the patch into the main trunk.  Why not accept patches 
as checked-in source on a branch of the file?  Tabs and all formatting will then 
certainly be maintained, and you can diff the files against whatever versions 
are most useful for you.  You don't have to be looking at context and/or line 
numbers to try and piece together where in a particular source file parts of a 
patch belong.  To me, it sounds like an easier way of getting at least the 
source code changes.  You could use tags to identify the patch, etc. for easy 
reference.

> > Index: client.h
> > +/* Calculate and send max-dotdot to the server */
> > +void
> > +send_max_dotdot(int argc, char **argv);
>
> I removed this prototype - this function is only called from within
> client.c and doesn't need to export a prototype.

No problem; I thought the convention was to prototype everything in the header 
file.  This might be a good addition to the HACKING file if convention is to 
only put a prototype in a header if it is used outside of that source.  Looking 
back at the source, I realize there are lots of prototypes just at the top of 
client.c, but I didn't notice that originally, so a line in HACKING mentioning 
this may save you the trouble the next time.

Thanks for integrating it into the trunk; I look forward to a day when my 
company can just download the latest version and use it (for the past 5 years, 
the company has been using a locally modified version because of different 
issues that we had to work around).  The last two issues now appear ready to be 
in the next release.

Chris