[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] #7 OS400/EBCDIC Port: Make svn_io_copy_file() CCSID insensitive.

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2006-03-10 14:11:35 CET

Paul Burba wrote:
> Julian Foad <julianfoad@btopenworld.com> wrote on 03/07/2006 09:56:00 AM:
>
>>(1) [...] The destination should be opened in the same
>>mode, and have the same CCSID as the source. [...]
>
> FWIW, IBM APR does not provide any way to obtain, specify, or otherwise
> manipulate a file's CCSID. It can be done with lower-level system calls;
> IBM provides a rather "cumbersome" (as in requiring a new 30+ line
> function) API (Qp0lSetAttr) to change the CCSID of an IFS file.

OK. Perhaps it's safe to pretty much ignore the CCSIDs, always setting it to
UTF-8. I still think the CCSID should be copied by any "file copy" function,
in theory, but, in practical terms, I was thinking that either

* the unwanted change of CCSID when copying a non-UTF8 file could be a problem
in some cases, or

* all files being copied by Subversion will already have a UTF-8 CCSID in which
case no translation will happen when copied in text mode so the patch is not needed

Now I can see that the second isn't true (e.g. "svn add non-UTF8-file"), and
maybe the first isn't either; I'll leave that to your judgement, as I haven't
thought through the possible scenarios and don't know the system well enough
anyway. If it's not a problem in practice, then fair enough.

>>(2) [...] That's the bug that Max Bowsher found for the
>>CygWin port of APR: it needs to read and write in binary mode. The same fix
>>should make the reads and writes insensitive to CCSIDs on OS400.
>>
>>This should all be done within APR, of course. I can accept this patch on the
>>basis that it won't get into APR soon enough, but I would like some assurance
>>that one of you will make an effort to get it into APR.
>
> We'll push IBM to make this change, as we have with others, in their port
> of APR as soon as it's in open source APR. We'll also suggest to them
> that they may simply want to fix the problem in their port of APR,
> regardless of what happens with open source APR, but this is less likely.
> Regardless of what IBM elects to do, please understand that this is an
> informal process; there's no mailing list for IBM APR or anything like
> that. We simply don't have much power to make them do anything they don't
> want to do.

OK.

We - that is, Subversion developers, some of whom are also APR developers - can
and should independently push for APR to be fixed in this way. I've just sent
a message about this ("[APR] apr_file_copy() improvements").

>>To complete this patch, svn_io_copy_file() needs to say in its doc-string that
>>it does a "byte-for-byte" copy, as it is no longer just a wrapper around
>>apr_file_copy(). (Eventually, I hope apr_file_copy() will be fixed and
>>documented to do the same.)
>
> Do we really want to do that? It becomes true for OS400 yes, and it's
> been implicitly true for *nix, but it still isn't true for CygWin, no? Am
> I being too much of a stickler here...or did you mean for the doc-string
> to indicate that there are platform dependent differences?

Sorry, I thought svn_io_copy_file() said it was a wrapper around
apr_file_copy(), but it doesn't.

We obviously want svn_io_copy_file() to perform a byte-for-byte copy.
Therefore we want to document it as such, because, as we have seen in APR, if
we don't then there is confusion about what it should do to text files. The
fact that it does not actually do so on all platforms then becomes a definite
bug, rather than just unspecified behaviour as it is at present, and I think
that's a good thing.

It seems logical to document the intended behaviour at the same time as fixing
the implementation (on OS400) to conform to that behaviour.

Apart from that, this patch is fine.

Sorry for the delay in responding this time.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Mar 10 14:12:50 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.