Currently, the FileOwnerQuery.markExternalOwner can be used to mark only
existing directories as an external root. I propose to allow registration of
files and non-existing files and folders as external roots.
The reason for registartiong of individual files is that several (freeform)
projects may produce jars into the same directory (so it is not enough to
specify the owner of the directory, as the jars are owned by different
projects). The same applies to NetBeans projects.
The reason for allowing registration of non-existing files and folders as
external roots is that it is necessary to specify an output jar as belonging to
a project before the output jar (or even directory where it resides) is created.
See also issue #57656.
Registration of individual files is I think a no-brainer.
Registration of nonexistent files/dirs is trickier. Currently the registry is a
weak map from FileObject locations to FileObject owners. This works well because
there is no real need to unregister owners explicitly - if the project is
collected, and there are no other references to the external location, the map
entry disappears automatically. If we permit nonexistent files/dirs, we need to
keep a URI (or similar) as the key, and it is not obvious to me how the
lifecycle of this should be managed. I guess if the value is also made a URI, we
could just make it a non-weak map and just accept the very small memory leak
Re registration of external roots: yes, this is not trivial.
But, I think that this could be solved by a pair of WeakHashMaps:
map1: URI->FileObject maps a (weakly referenced) URI to a weakly held FileObject
map2: FileObject->URI maps a (weakly referenced) FileObject (project directory)
to the URI (not weak).
Now, as long as the project directory's FileObject is strongly referenced, the
URI is strongly referenced. Once the project directory's FO is collected, the
URI is only weakly referenced and the mapping is removed.
The assumption is that once noone holds the project directory, the project is
removed/closed and the mapping is useless.
The proposed trick would not work in case the initial registration is done using
mEO(URI), unless that also tried to look up a FileObject for the URI. Don't
know, might be enough, would have to analyze a bit more.
Sorry, but I did not understand your last comment. I would propose to add method:
markExternalOwner(URI root, Project owner, int algorithm)
to the FileOwnerQuery. I do not see why the approach with two weak maps would
not work in this case. I would do it this way:
Map uri2Project = new WeakHashMap();
Map project2URI = new WeakHashMap();
uri2Project.put(root, new WeakReference(owner.getProjectDirectory()));
project2URI.put(owner.getProjectDirectory(), new WeakReference(root));
Am I missing something?
I will try to implement this, if you do not mind.
In such a case the registration will simply be garbage collected in the next GC
cycle, even if the owner project is held strongly elsewhere.
FWIW, I don't currently believe it is possible to implement this correctly
without *some* kind of memory leak. I can't prove it, but I think it is
logically impossible. The best I can think of is to keep just one registry:
private static final Map/*<URI,URI>*/ externalOwners =
Here the key is the the URI to the external root and the value is the URI to the
project directory. The project itself can be collected and the entry will
remain, but it will consume little space.
Ooops, copy&paste mistake, sorry. The last line should read:
In that case I think it works. If the project is closed and collected, the
mapping will be removed, unless and until it is reopened, which is acceptable.
Attaching patch that allows registration of individual files and non-existing
Created attachment 22127 [details]
Patch for registration of files and non-existing files and folders.
Patch to projectapi/nbproject/project.properties is bogus.
Don't call Thread.sleep(30000) in the test; set
TimedWeakReference.TIMEOUT = 0;
fileObject2URI in the test may more simply just throw Exception. In the main
class, a FileStateInvalidException probably should be rethrown as an
IllegalArgumentException (since no one should pass an invalid FileObject in);
and you can use URI.create rather than new URI(...) (no checked exc.) since
URL.toString() ought to yield a valid URI in all cases.
Otherwise looks good to me. Are you going to submit to apireviews?
Yes, I will correct the patch and then I will send it to apireviews.
Created attachment 22142 [details]
Improved patch for individual files and non-existent files/folders registration.
Created attachment 22143 [details]
Patch for issue #57656.
I would like to ask for API review for this change. The proposed patch is
attached as 58313c.diff. The change is used by fix of issue #57656, attached is
also diff that solves this issue (57656.diff) for reference.
If there are no objections, I will commit the change tomorrow.
For the issue #57656 part, I disagree with the approach taken. (Please attach
the patch to that issue and evaluate it separately.) Prefer to keep the existing
style for defining external files as is already used for folders, so that the
project.xml is explicit about what it does, and <export> is used to control only
AntArtifactQuery; so need to add <build-file> to the general schema, analogous
to <build-folder>. This means creating a new rev of the general freeform schema
and making sure both old and new schemas are interpreted. The GUI customizer
should already be defining <build-folder> in project.xml when you set an output
location for some source root which is not inside the project dir. This should
be extended to define either <build-folder> and/or <build-file> (depending on
whether e.g. FileUtil.isArchiveFile on each location); the customizer should
write a new-schema project.xml iff <build-file> would be needed (must warn user
of this as for the j2seproject upgrade); and the SourcesHelper used in the
freeform project should be configured from both <build-file> and <build-folder>.
Also need unit test patch for new SourcesHelper behavior.
Patch for this issue looks OK to me. BTW spelling: "collectible" not
"collectable" (please don't ask me why).
Checking in projectapi/apichanges.xml;
/cvs/projects/projectapi/apichanges.xml,v <-- apichanges.xml
new revision: 1.3; previous revision: 1.2
Checking in projectapi/manifest.mf;
/cvs/projects/projectapi/manifest.mf,v <-- manifest.mf
new revision: 1.8; previous revision: 1.7
Checking in projectapi/src/org/netbeans/api/project/FileOwnerQuery.java;
new revision: 1.7; previous revision: 1.6
new revision: 1.9; previous revision: 1.8
new revision: 1.8; previous revision: 1.7