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.

Bug 58313 - Extend FileOwnerQuery.markExternalOwner to handle individual files and non-existing files/roots
Summary: Extend FileOwnerQuery.markExternalOwner to handle individual files and non-ex...
Status: RESOLVED FIXED
Alias: None
Product: projects
Classification: Unclassified
Component: Generic Infrastructure (show other bugs)
Version: 5.x
Hardware: All All
: P3 blocker (vote)
Assignee: Jan Lahoda
URL:
Keywords: API, API_REVIEW_FAST
Depends on: 60297
Blocks: 57656
  Show dependency tree
 
Reported: 2005-04-26 08:47 UTC by Jan Lahoda
Modified: 2005-09-05 10:10 UTC (History)
1 user (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
Patch for registration of files and non-existing files and folders. (22.67 KB, patch)
2005-05-13 16:01 UTC, Jan Lahoda
Details | Diff
Improved patch for individual files and non-existent files/folders registration. (22.73 KB, patch)
2005-05-16 14:56 UTC, Jan Lahoda
Details | Diff
Patch for issue #57656. (5.08 KB, patch)
2005-05-16 14:57 UTC, Jan Lahoda
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Lahoda 2005-04-26 08:47:45 UTC
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.
Comment 1 Jesse Glick 2005-04-27 17:48:29 UTC
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
this entails.
Comment 2 Jan Lahoda 2005-05-02 15:59:29 UTC
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
(project directory)
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.
Comment 3 Jesse Glick 2005-05-02 20:01:31 UTC
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.
Comment 4 Jan Lahoda 2005-05-12 12:53:34 UTC
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.
Comment 5 Jesse Glick 2005-05-12 19:50:47 UTC
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 =
    Collections.synchronizedMap(new HashMap());

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.
Comment 6 Jan Lahoda 2005-05-12 19:55:04 UTC
Ooops, copy&paste mistake, sorry. The last line should read:
project2URI.put(owner.getProjectDirectory(), root);
Comment 7 Jesse Glick 2005-05-13 00:27:56 UTC
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.
Comment 8 Jan Lahoda 2005-05-13 16:00:14 UTC
Attaching patch that allows registration of individual files and non-existing
folders/files.
Comment 9 Jan Lahoda 2005-05-13 16:01:33 UTC
Created attachment 22127 [details]
Patch for registration of files and non-existing files and folders.
Comment 10 Jesse Glick 2005-05-14 22:31:34 UTC
Patch to projectapi/nbproject/project.properties is bogus.

Don't call Thread.sleep(30000) in the test; set

static {
    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?
Comment 11 Jan Lahoda 2005-05-15 09:37:31 UTC
Yes, I will correct the patch and then I will send it to apireviews.
Comment 12 Jan Lahoda 2005-05-16 14:56:44 UTC
Created attachment 22142 [details]
Improved patch for individual files and non-existent files/folders registration.
Comment 13 Jan Lahoda 2005-05-16 14:57:55 UTC
Created attachment 22143 [details]
Patch for issue #57656.
Comment 14 Jan Lahoda 2005-05-16 15:10:16 UTC
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.
Comment 15 Jan Lahoda 2005-05-23 16:50:44 UTC
If there are no objections, I will commit the change tomorrow.
Comment 16 Jesse Glick 2005-05-24 19:38:21 UTC
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).
Comment 17 Jan Lahoda 2005-05-27 10:54:50 UTC
Implemented:
Checking in projectapi/apichanges.xml;
/cvs/projects/projectapi/apichanges.xml,v  <--  apichanges.xml
new revision: 1.3; previous revision: 1.2
done
Checking in projectapi/manifest.mf;
/cvs/projects/projectapi/manifest.mf,v  <--  manifest.mf
new revision: 1.8; previous revision: 1.7
done
Checking in projectapi/src/org/netbeans/api/project/FileOwnerQuery.java;
/cvs/projects/projectapi/src/org/netbeans/api/project/FileOwnerQuery.java,v  <--
 FileOwnerQuery.java
new revision: 1.7; previous revision: 1.6
done
Checking in
projectapi/src/org/netbeans/modules/projectapi/SimpleFileOwnerQueryImplementation.java;
/cvs/projects/projectapi/src/org/netbeans/modules/projectapi/SimpleFileOwnerQueryImplementation.java,v
 <--  SimpleFileOwnerQueryImplementation.java
new revision: 1.9; previous revision: 1.8
done
Checking in
projectapi/test/unit/src/org/netbeans/api/project/FileOwnerQueryTest.java;
/cvs/projects/projectapi/test/unit/src/org/netbeans/api/project/FileOwnerQueryTest.java,v
 <--  FileOwnerQueryTest.java
new revision: 1.8; previous revision: 1.7
done