This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Summary: | Support jimage file in FileUtil.getArchiveFile, FileUtil.getArchiveRoot | ||
---|---|---|---|
Product: | platform | Reporter: | Tomas Zezula <tzezula> |
Component: | Filesystems | Assignee: | Tomas Zezula <tzezula> |
Status: | NEW --- | ||
Severity: | normal | CC: | anebuzelsky, apireviews, jhavlin, jlahoda, jtulach, mentlicher, sdedic |
Priority: | P2 | Keywords: | API, API_REVIEW |
Version: | 8.1 | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | TASK | Exception Reporter: | |
Attachments: |
Diff file
Diff with renamed isArchiveRoot method Diff with renamed isArchiveRoot method |
Description
Tomas Zezula
2015-05-28 11:36:43 UTC
Created attachment 153920 [details]
Diff file
Usage of isArchiveRoot in the debugger.jpda: http://hg.netbeans.org/jet-main/rev/3393535cf2a2 The code converts classpath URLs to the paths preserving the relocation in the archive and back. This may be a requirement for methods returning the path in archive as I described above. FileUtil.archiveOrDirForURL(URL) uses FU.isArchiveRoot() and FU.getArchiveFile() but keeping the semantic compatibility. For URLs with path inside an archive return null. Even if such a behaviour is strange it probably cannot be changed as it's documented in Javadoc and covered by unit tests. http://hg.netbeans.org/jet-main/rev/4c53ebc6155f Thank you very much, Tomas, for describing the problem and writing the patch.
The only thing that seems a little confusing to me is name of methods "isArchiveRoot(FileObject)". I would expect that such method returns true only if the passed FileObject is the root directory (/) contained in the archive, but the method returns true if the FileObject is just "inside an archive".
So, name like "isInsideArchive" would sound more correct to me :-)
> Open questions:
> Should the FileUtil be extended by methods preserving the path in archive like:
> getArchiveArtefact(archiveFile, relativePathInArchive) ...
> getArchiveFileAndPath(archiveArtefact) ...
As this is would make code in debugger (and possibly in other modules) simpler, we can add these methods to FileUtil, I think.
What about having getArchiveFileAndPath for both URL and FileObject?
(Maybe URL version will be sufficient, I don't know much about the use-cases.)
Signatures could be be like this:
FileObject getArchiveArtefact(FileObject archiveFile, String relativePathInArchive)
Pair<FileObject, String> getArchiveFileAndPath(FileObject archiveArtefact) // FileObject version
Pair<URL, String> getArchiveFileAndPath(URL archiveArtefact) // URL version
Is is what you meant?
Thanks Jardo for review. The isArchiveRoot comes from getArchiveFile <-> getArchiveRoot naming which we already have in the FileUtil. There is already isArchiveFile, so I've added the isArchiveRoot. The isInsideArchive is fine for me. Regarding the getArchiveArtifact and getArchiveArtifactAndPath I agree that thare should be versions for both URL and FileObject. I believe they do not need changes in SPI just adding the methods into FileUtil. I have already such methods in java.j2seplatform and probably debugger.jpda. I will check and attach a link. (In reply to Tomas Zezula from comment #5) > Thanks Jardo for review. > The isArchiveRoot comes from getArchiveFile <-> getArchiveRoot naming which > we already have in the FileUtil. There is already isArchiveFile, so I've > added the isArchiveRoot. > The isInsideArchive is fine for me. I see. What about boolean isArchiveArtefact(FileObject) or boolean canGetArchiveFile(FileObject)? (Although it would be more consistent with rest of archive-related API, isInsideArchive sounds more natural to me.) > Regarding the getArchiveArtifact and getArchiveArtifactAndPath I agree that > thare should be versions for both URL and FileObject. I believe they do not > need changes in SPI just adding the methods into FileUtil. I have already > such methods in java.j2seplatform and probably debugger.jpda. I will check > and attach a link. Thank you very much. Thanks Jardo! The isArchiveArtefact sounds best to me, so I've used it. Regarding the getArchiveArtifact and getArchiveArtifactAndPath I will create a new API review for these utility methods as they are independent to this SPI and base API. Created attachment 157191 [details]
Diff with renamed isArchiveRoot method
Created attachment 157193 [details]
Diff with renamed isArchiveRoot method
I've changed the "isArchiveArtefact" to "isArchiveArtifact" as the "Artifact" is more common in NetBeans APIs (eg. AntArtifact).
Integrated into 'main-silver', will be available in build *201511301649* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-silver/rev/9bab0452ef4d User: Tomas Zezula <tzezula@netbeans.org> Log: #252644:Support jimage file in FileUtil.getArchiveFile, FileUtil.getArchiveRoot |