Re: Is there really any interest in a patch to allow cvs 1.11.6 to run

[Paul Edwards]

"Kelly F. Hickel" <kfh@mqsoftware.com> wrote in message 
news:mailman.1068.1065197114.21628.bug-cvs@gnu.org...
> |FAIL: basica-6.2
> |make[2]: *** [localcheck] Error 1
> |make[2]: Leaving directory `/usr/local/cvs/cvs-1.11.6_nsk/src'
> |make[1]: *** [check-am] Error 2
> |make[1]: Leaving directory `/usr/local/cvs/cvs-1.11.6_nsk/src'
> |make: *** [check-recursive] Error 1
> |/usr/local/cvs/cvs-1.11.6_nsk:
> |
> |How much does this matter?

That is a context diff failing.

ie cvs diff -c <filename>

not producing expected results.

I think you want to make sure you can do a context diff!  If the
explanation is that there is something wrong with the your
shell etc, then you probably don't care.

>> If there are any sanity.sh tests failing, your fix is wrong because it
>> breaks another part of CVS and the fix will not be accepted.

> [Kelly Hickel] Well, The fix isn't wrong, because the unchanged code has
> the same problem.

IMO, because of this, I don't see that it is your responsibility
to get that working.

> All the fix does is get around the platform
> strangeness about size limited read/writes.

It would be helpful if you could document why read/write don't
work on Tandem as per other systems.

> So, I haven't broken
> anything new, but cvs is actually usable, instead of un-usable.

Sounds good to me.

> |I'm about to upgrade the OS to the latest version, so I want to wait
> |any make sure that the unmodified source still has problems on the
> |latest version, then I'll be ready to send out the (relatively small)
> |patch.
>
> Per the HACKING in the top level of the CVS source distribution:
>
> |The general rule for portability is that it is only worth including
> |portability cruft for systems on which people are actually testing
and
> |using new CVS releases.  Without testing, CVS will fail to be
portable
> |for any number of unanticipated reasons.
>
>
> This means that unless we see that Tandem gets regular testing, the
CVS
> developers are unlikely to feel that your patch is useful even if it
> begins to work since it will likely not stay useful for long.

I think it should be considered on a case by case basis.  I doubt
anything will change in cvs 1.11.x that will stuff up his
safe_read/safe_write additions, in part because they are outside
of the normal CVS code, ie they are not invasive.

CVS probably supports a number of old systems which don't
get regularly tested until they break again.  With CVS 1.11.x,
I hope we can eventually get a version that works on all those
systems, ie doesn't keep getting broken with new development.

I see that as the actual purpose of cvs 1.11.x, ie to end up with
a product that works, on as many systems as possible (unless
a complete rewrite, or highly invasive change is required to
make it work).  This is exactly the version where all the
ancient systems that rarely get tested, ie Amiga etc, can submit
their changes and they will find a stable home.

I don't see a problem if 1.11.9 works on Amiga but not Tandem,
1.11.10 works on Tandem but not Amiga, and 1.11.11 works
on both.  That's great!  That's progress.  It's a hell of a lot better
than 1.11.8 not working on either!!!

A disclaimer saying that the only systems that CVS is actually
tested on are a, b and c, and that you advise anyone not using
such a system to run sanity.sh to see if their "non-standard"
platform still works, should be fine.

The HACKING was written when there was no cvs 1.11.x,
after all.

>> i.e. If somebody is not running nightly testing on the system, then it
>> is much easier for another developer to check in a fix for something
>> else that breaks your portability cruft but which is not spotted until
>> you tell it's broken after the next CVS release.

Better than being permanently broken IMO.

> [Kelly Hickel] Well, it seems a little unlikely that anyone (except
> maybe me) is going to loan you a $250,000 machine (smallest
> configuration available) to run nightly tests on.  If you have a scheme
> that lets remote people do tests for you and mails back the results we
> can maybe work something out.

I think this is way unnecessary.

>> The exception to this rule can be when you can find a fix already
>> incorporated in some GNULIB module we use (or get your fix
incorporated
>> into GNULIB).  We hope to pick up portability cruft from GNULIB that
we
>> might not actually be testing but which gets testing elsewhere.

> [Kelly Hickel] Well, I guess I may try to find time to take a look at
> GNULIB, I doubt it's fixed, but maybe it is.

I'm not familiar with this, but perhaps putting safe_read and
safe_write into gnulib rather than subr.c would be more
appropriate, since they really have nothing to do with CVS?

>  This is the reason that
> I'm still using cvs 1.10.7 (which doesn't pass it's tests either), it's
> just too difficult to upgrade, because I have to re-port the world every
> time.

I don't think this will be necessary with the 1.11.x stream.  There
is no new functionality being added, the world shouldn't change.

I think at the very least you should post a patch here (which will
get stored in google) that says "this is how to get Tandem to
work on cvs 1.11.(6/8)".  It is likely your fix will remain valid
on all remaining 1.11.x releases.  A patch to the INSTALL file
and to subr.c.

> Given the above comments, I'll probably just send the latest to
> the ITUG folks, so that they can update their ported GNU tools
> repository.

I hope you can post it here too.

> By the way, I didn't end up changing any source files other than subr.c,
> if that helps (I added two methods).  Again, (if you didn't see the
> earlier posts) I basically just added a safe_read and safe_write, which
> some of the comments in client.c seem to be asking for.

> Still, I suppose it's better to have a "no" (which is how I interpret
> your response), to having no answer at all.

I think it would be really helpful if you could ask the ITUG folks
why read() doesn't work as expected.  Perhaps a more appropriate
solution would be to have a README.TANDEM that says "in
order to use CVS, you must either set your operating system
MAXREAD value to 65535 or else add the following fragment of
code to the end of subr.c."

I wouldn't say the answer is "no", I would say the answer is "why
is this problem occurring in the first place, and would it be more
appropriate to fix it in gnulib"?  Which is a reasonable question to
ask, because if read() doesn't work properly on Tandem, it
probably doesn't just break CVS, it probably breaks millions of
other programs.  It doesn't make sense to update a million
different software products when you can fix it by just setting an
environment variable properly, or at worse, add it to a gnulib
that is used by some of these other millions of products.

But definitely a documentation update to INSTALL, explaining
why Tandem doesn't work, and what can be done to work around
the problem, surely that would be very useful and accepted?

BTW, it would be helpful to know what size Tandem is actually
capable of reading by default.  Is it 8k or 16k or what?  Did
Tandem document that?

BFN.  Paul.