Re: [Fwd: Re: [cp-patches] RFC: file resources can be directory too]
Hi Olivier, On Sun, 2006-03-05 at 18:24 +0100, Olivier Jolly wrote: since no one objected on the fact that we should consider directories as valid resources, I committed the attached trivial patch with this changelog 2006-03-05 Olivier Jolly [EMAIL PROTECTED] Fixes PR 22813 * java/net/URLClassLoader.java (FileURLLoader.getResource): Allows directories as valid resources. This does introduce the following mauve regressions according to builder/classpath-testresults: Baseline from: Sun Mar 5 19:36:19 UTC 2006 Regressions: FAIL: gnu.testlet.java.net.URLClassLoader.getResource: no . (number 1) FAIL: gnu.testlet.java.net.URLClassLoader.getResource: no .. (number 1) Could you investigate and possibly update this Mauve test if you think it is wrong (I believe I wrote this test so obviously I think it is right, but it was a long, long time ago so I might have misunderstood things back then.) Thanks, Mark signature.asc Description: This is a digitally signed message part
[Fwd: Re: [cp-patches] RFC: file resources can be directory too]
Hi,
since no one objected on the fact that we should consider directories
as valid resources, I committed the attached trivial patch with this
changelog
2006-03-05 Olivier Jolly [EMAIL PROTECTED]
Fixes PR 22813
* java/net/URLClassLoader.java (FileURLLoader.getResource):
Allows directories as valid resources.
Cheers
Olivier
Message original
Sujet: Re: [cp-patches] RFC: file resources can be directory too
Date: Wed, 01 Mar 2006 12:27:42 +0100
De: Olivier Jolly [EMAIL PROTECTED]
Pour: Mark Wielaard [EMAIL PROTECTED]
Copie: Robin Green [EMAIL PROTECTED], classpath-patches
classpath-patches@gnu.org
Références: [EMAIL PROTECTED]
[EMAIL PROTECTED]
[EMAIL PROTECTED]
Mark Wielaard wrote:
On Tue, 2006-02-28 at 16:53 -0600, Archie Cobbs wrote:
Olivier Jolly wrote:
following the mauve testlet commit, here is the fix proposition in
classpath.
Basically, we were explicitly preventing directories to be retrieved
as file resources, while I don't see any reason to do so (nor sun
implementors afaik).
So, I propose this very trivial patch to deal with it.
Looks like I was the one who disabled this behavior in rev 1.32.
Right now I can't remember why. But I think there was some bug in
the previous code. Note rev 1.32 removed code in getInputStream()
too, which is where ISTR the bug was.
I can't access the archives right now (web site too slow) but it
might be worth checking them for the original reasoning for the change.
Had something to do with something working with file:// URLs but not
http:// URLs, or the other way around, or...
Archived discussion is here:
http://lists.gnu.org/archive/html/classpath/2005-07/msg00071.html
Which also points to the following (open) bug:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22813
And I found this mauve suggestion from Robin Green (CCed):
http://sourceware.org/ml/mauve-patches/2004/msg00118.html
(Which embarrassingly enough never got checked in. Sorry Robin!)
Thanks Mark for finding those references.
As far as I understand, the directories were treated in a special way in
URLClassLoader for getLength and getInputStream but were valid Resource
from the FileUrlClassLoader point of view. With the removing of the
special treatment in getLength and getInputStream for directories, it
had been choosen to prevent directories from being considered as
Resource to avoid the blurry case of trying to get the length or the
inputStream off a directory.
The problem was moved from the getLength and getInputStream toward
FileUrlClassLoader.getResource.
However, on a Resource, we can do other stuff than getLength and
getInputStream which are also valid for directories, and I think we
agree that getResource on a directory which actually exists in the list
of urls the FileUrlClassLoader is looking for should return a valid File
instance refering to this directory.
So, I think we're moving the problem in another direction, and it's up
to the FileInputStream and File to know how to behave with directories,
which is, imho, the right place to wonder how to handle this.
FileUrlClassLoader should blindly rely on the File cie implementations.
Yesterday Tom pointed at this bug and I didn't think it was exactly the
same since the initial bug report and the reply were not talking exactly
the same thing. The initial bug report is the problem we're dealing this
right now while the answer seems to deal with the multiplicity of result
of getResources. I'm been tricked by the answer.
Finally, I think that Robin was right about his assumption and that his
patch proposition which should be commited (I did a similar modification
locally to have both sun and patched classpath implementation happy with
all those tests).
Cheers,
Thanks in advance for feedback
Mark
Olivier
Index: URLClassLoader.java
===
RCS file: /sources/classpath/classpath/java/net/URLClassLoader.java,v
retrieving revision 1.44
diff -u -r1.44 URLClassLoader.java
--- URLClassLoader.java 18 Jan 2006 00:19:13 - 1.44
+++ URLClassLoader.java 28 Feb 2006 21:26:35 -
@@ -539,7 +539,7 @@
try
{
File file = new File(dir, name).getCanonicalFile();
- if (file.exists() !file.isDirectory())
+ if (file.exists())
return new FileResource(this, file);
}
catch (IOException e)
Re: [cp-patches] RFC: file resources can be directory too
On Tue, 2006-02-28 at 16:53 -0600, Archie Cobbs wrote: Olivier Jolly wrote: following the mauve testlet commit, here is the fix proposition in classpath. Basically, we were explicitly preventing directories to be retrieved as file resources, while I don't see any reason to do so (nor sun implementors afaik). So, I propose this very trivial patch to deal with it. Looks like I was the one who disabled this behavior in rev 1.32. Right now I can't remember why. But I think there was some bug in the previous code. Note rev 1.32 removed code in getInputStream() too, which is where ISTR the bug was. I can't access the archives right now (web site too slow) but it might be worth checking them for the original reasoning for the change. Had something to do with something working with file:// URLs but not http:// URLs, or the other way around, or... Archived discussion is here: http://lists.gnu.org/archive/html/classpath/2005-07/msg00071.html Which also points to the following (open) bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22813 And I found this mauve suggestion from Robin Green (CCed): http://sourceware.org/ml/mauve-patches/2004/msg00118.html (Which embarrassingly enough never got checked in. Sorry Robin!) Cheers, Mark signature.asc Description: This is a digitally signed message part
[cp-patches] RFC: file resources can be directory too
Hi,
following the mauve testlet commit, here is the fix proposition in
classpath.
Basically, we were explicitly preventing directories to be retrieved
as file resources, while I don't see any reason to do so (nor sun
implementors afaik).
So, I propose this very trivial patch to deal with it.
Since it seems that there already were a discussion (or rather fuss)
about the nature of resource and how to deal with non plain files, if
this patch seems too daring for you, don't hesitate to shout and discuss
about it.
Olivier
2006-02-28 Olivier Jolly [EMAIL PROTECTED]
* java/net/URLClassLoader.java (FileURLLoader.getResource):
allows directories as valid resources.
Index: URLClassLoader.java
===
RCS file: /sources/classpath/classpath/java/net/URLClassLoader.java,v
retrieving revision 1.44
diff -u -r1.44 URLClassLoader.java
--- URLClassLoader.java 18 Jan 2006 00:19:13 - 1.44
+++ URLClassLoader.java 28 Feb 2006 21:26:35 -
@@ -539,7 +539,7 @@
try
{
File file = new File(dir, name).getCanonicalFile();
- if (file.exists() !file.isDirectory())
+ if (file.exists())
return new FileResource(this, file);
}
catch (IOException e)
Re: [cp-patches] RFC: file resources can be directory too
Olivier Jolly wrote: following the mauve testlet commit, here is the fix proposition in classpath. Basically, we were explicitly preventing directories to be retrieved as file resources, while I don't see any reason to do so (nor sun implementors afaik). So, I propose this very trivial patch to deal with it. Looks like I was the one who disabled this behavior in rev 1.32. Right now I can't remember why. But I think there was some bug in the previous code. Note rev 1.32 removed code in getInputStream() too, which is where ISTR the bug was. I can't access the archives right now (web site too slow) but it might be worth checking them for the original reasoning for the change. Had something to do with something working with file:// URLs but not http:// URLs, or the other way around, or... Otherwise it looks OK to me. -Archie __ Archie Cobbs *CTO, Awarix* http://www.awarix.com
