Pavel,
Thanks for your reasoned explanation.
My main objection to this thread is that the code is fairly recent code
(2016) that was in 8164130: Simplify doclet IOEException handling and
reviewed by both Kumar and Bhavesh.
In other words, the code works, is not broken, and it's not worth the
effort to rewrite it again just to try and shrink the code by a few lines.
In terms of general philosophy, when any IO problem occurs, I think it
is important to specify the file involved, to give the end-user the best
information for that person to diagnose the external condition. To me,
that is better and more specific than saying "error copying from A to B,
something went wrong with one of them".
I'm also old-school enough to want to copy files of unknown size by
copying the contents of a stream, rather than internalizing the
contents, just to simplify the copying code.
I also do not like solely relying on detail messages in exceptions, and
IO exceptions in particular. The messages are not localized, and there
is no specification of the content. It's not even specified whether you
get a meaningful subtype of IOException, like FileNotFoundException.
Yes, I believe in showing the message to the user in case it might
provide information, but no, I don't think we should expect anything
helpful to be provided there.
In terms of Tech Debt, there are bigger fish to fry, lower-hanging fruit
on the tree that deserve our attention.
-- Jon
On 6/25/20 11:27 AM, Pavel Rappo wrote:
On 24 Jun 2020, at 15:28, Jonathan Gibbons <jonathan.gibb...@oracle.com> wrote:
Yes, it's an oversight that the ProblemList entry is commented out and not
deleted. I'll fix that.
Thanks.
It is intended to be helpful to users to provide details in the exception about the file
causing the problem. The different exceptions identify the file in question, which is
probably more important/useful than the READ/WRITE, which is there to help provide
helpful diagnostic messages. This is all working around the appalling
under-specification of java.io.IOException, which provides NO mandated information other
than the single bit of info that "an IO error occurred".
I understand the motivation you are describing and mostly agree with it.
However, I think we could be helpful in a more balanced way: javadoc is likely
doing more work than is required to produce quality diagnostic messages.
I don't consider it would be progress if end users have to analyze stack traces
to determine the context of a message that has been reported.
I do not suggest that end users should analyze stack traces; I suggest to
investigate how much bigger blocks (think: Lego) of I/O functionality we can
use until quality of messages that the end user sees suffers significantly. Can
it be that if we accurately capture the context of an I/O operation performed
by a bigger block, that context will still be enough to diagnose the problem?
(That is, even if that block does not provide any diagnostic information by
itself, which I don't think I have ever seen: there's always something.)
For example. To copy file A to file B, javadoc does the following. It reads a portion of file A and
then writes that portion to file B. This repeats until file A is exhausted and everything that has
been read from file A has been also written to file B, or an error has occurred. This way, if an
I/O error occurs, javadoc always knows whether this was a "read from A" or "write to
B" kind of error.
I think that knowing that is superfluous in most cases. If we choose a bigger
block (e.g. java.nio.file.Files#copy(java.nio.file.Path, java.nio.file.Path,
java.nio.file.CopyOption...)) and capture the implicated files in a
DocFileIOException, that could be more practical.
Could a message like "Error copying: A to B" be just as good as "Error writing file:
B"? In practise, however, that difference will be more like between
Error writing file: B
(java.nio.file.AccessDeniedException: B)
and
Error copying: A to B
(java.nio.file.NoSuchFileException: A)
Error copying: A to B
(java.nio.file.FileAlreadyExistsException: B)
Error copying: A to B
(java.nio.file.FileSystemException: B: Operation not permitted)
-Pavel
-- Jon
On 6/24/20 5:19 AM, Pavel Rappo wrote:
Jon,
Consider deleting that entry in ProblemList.txt instead of commenting it out.
Otherwise, the change looks good.
***
That change reminded me of a question I had: do we really need to split I/O errors into
"read" and "write" errors and associate filenames with I/O errors on the Java
level?
I'm asking this because it's very tempting to refactor that code. For example,
thanks to NIO, we could fold code like this into a one-liner:
public void copyFile(DocFile fromFile) throws DocFileIOException {
try (OutputStream output = openOutputStream()) {
try (InputStream input = fromFile.openInputStream()) {
byte[] bytearr = new byte[1024];
int len;
while ((len = read(fromFile, input, bytearr)) != -1) {
write(this, output, bytearr, len);
}
} catch (IOException e) {
throw new DocFileIOException(fromFile,
DocFileIOException.Mode.READ, e);
}
} catch (IOException e) {
throw new DocFileIOException(this, DocFileIOException.Mode.WRITE,
e);
}
}
I'm pretty sure that if any of the java.nio.file.Files's methods throws
IOException, the user will be able to quickly and accurately diagnose the error
using the accompanying message.
Should I create an RFE to investigate that?
-Pavel
On 15 Jun 2020, at 19:41, Jonathan Gibbons <jonathan.gibb...@oracle.com> wrote:
Please review a change to enable a javadoc test to be removed from the problem
list.
The test was excluded because an IOException was occurring when running the test
on Windows, Most of the change here is support code to diagnose the problem, and
to investigate whether the issue can be fixed.
Eventually, a reference was found on a Microsoft site, indicating that normal
directories
cannot be made read-only on Windows. Thus the primary fix is to skip each
test-case
if the setup for the test case fails to create a readonly directory on Windows.
In addition doc comments and -Xdoclint:-missing are added to reduce the number
of
irrelevant diagnostics in the output.
In conjunction with the fix for JDK-8152313, this will fix all javadoc tests on
the
LangTools exclude list.
-- Jon
JBS: https://bugs.openjdk.java.net/browse/JDK-8164597
Webrev: http://cr.openjdk.java.net/~jjg/8164597/webrev.00/
