Use fallback root project when folder project group has no owning project

[jtulach]

• this is a PR extracted from PoC: More prominent Open Folder (as a group) action #9631 proof of concept work
• the PR provided a FallbackFactory to create FallbackProject for any folder
• the PR also contains two "obvious" usages
  • when opening after start
  • when creating Folder of Projects group
• fix MultiSourceFileProvider - done in c22ea60
• Remove the API per request

[jtulach]

One obvious usage of this new "fallback project" functionality is opening of projects on startup.

The files on disk may change since the NetBeans shutdown. Sometimes it happens that original projects are no longer recognized even the original directory path exists.

In such case it is still better to open the "fallback project" rather than opening nothing.

[jtulach]

When using File/Project Groups... menu:

Selecting the menu item opens a dialog

and allows one to click the New group... button to open another dialog. After choosing Folder of Projects and browsing for it:

... one can click "Create Group" button. Which scans for all the projects in the given folder and opens them in Projects view (after closing all previous ones).

No Projects Found

It may happen that there are no projects found during the scan. Leaving the Projects View empty. That's very weird.

With this one line change, the root of the folder is always opened: either as a real project or as a "fallback project".

Works great with #9602 and its nesting capabilities. The root folder is opened on the top and all other projects are nested beneath it.

[neilcsmith-net]

This exception message seems off? Doesn't this throw even if the fallback isn't needed?

Personally I'd prefer a default method over the additional type here, but I know your feelings there.

[jtulach]

Done in c01de98 - together with introduction of a new argument type, it is binary compatible change as analyzed here.

[neilcsmith-net]

Thanks! Almost what I was going to suggest with the opening flags. I don't agree with all of your post on default methods (JSONList), but I do understand the need for considering compatibility.

In light of which, I am still concerned about the exception in ProjectManager, and the potential for certain places to start throwing exceptions that didn't before. I'd still allow findProjectOrFallback to return null if we're going to use it as a replacement.

[jtulach]

concerned about the exception in ProjectManager

Which exception do you have in mind? The NPE that could be throw here? Well, that's never going to be thrown as with FindOptions.WITH_FALLBACK argument the implementation never returns null. Just (as a result of introducing a default method for all the FindOptions variants) the type system (nor @NonNull & co. annotations) really can express that easily.

[neilcsmith-net]

Which exception do you have in mind? The NPE that could be throw here? Well, that's never going to be thrown as with FindOptions.WITH_FALLBACK argument the implementation never returns null.

Exactly that NPE. Sure, your implementation never returns null, but the public SPI interface default method does. Now, if we're treating that as an SPI that can only have one implementation, that's fine, but then the arguments about maintaining compatibility don't really hold.

So, we could have ProjectManager::findProjectOrFallback throw an exception like this in cases where the implementation doesn't override the default method. But it is not then a simple drop-in replacement for ProjectManager::findProject. Your changes in DirectoryGroup and OpenProjectList retain the null check that is no longer needed, but will hit a different code path if an SPI implementation causes an exception - the latter will catch it, but the former will blow up I think?

So, if we want to treat this as a simple drop-in replacement across our internal code and treat it as a public SPI, I think it's easier to allow a null return as with ProjectManager::findProject when fallback creation fails. If not, then the usages in this PR might need a rethink.

[jtulach]

Exactly that NPE.

OK. I'll think about our alternatives.

if we're treating that as an SPI that can only have one implementation

• I believe we do
  • the other implementation was part of Oracle's Project TailWind which is long time dead as far as I can imagine, CCing @tzezula and @sdedic
• e.g. for all practical purposes there is just a single implementation NbProjectManager
• the assumption there was just a single implementation was the major reason why usage of default methods seemed justifiable in this case to me

[lahodaj]

(Whatever we do in this area, @eppleton is right we need a more proper check in the source file launcher check; that can be done separately.)

I assume the end goal here is to achieve the "Open Folder"/"Add to Workspace" actions, as mentioned in the recent e-mail. I do wholeheartedly agree with adding (some) support for "workspaces" (i.e. ability to work without projects, or without specifying projects one-by-one).

What I am not quite sure if whether starting with an API change is the best move. Whether mapping this to project groups matches the requirements also remains to be seen.

I think I would first try to implement the actions, and from that it would lead what API changes are needed (I suspect some will be needed). #9631 implemented the Open Folder as Workspace action, but not the Add to Workspace action; and also the resulting UI is I think not ideal: when I do "Open Folder as Workspace", would I really expect to see all the nested projects in the Projects tab? (I realize this is a consequence of reusing the Project Groups directly, but a novice user is likely to be confused by this, I think.) OTOH, I do agree it would be good to open the projects under the workspace folder.

I guess if I would be doing this, I would:

• try to make the feature end-to-end first. It does not have to be merged as a single PR, but ensuring the end result is as good as possible before adjusting APIs would be nice
• if there were explicit actions to open a folder as a workspace/add folder to a workspace, we wouldn't probably need the fallback API - the "workspace folder project" could be created by an ordinary project factory, if needed; (the action would mark the directory as "needs workspace project, if there's no better" in the userdir, or something alike)
• some part of the system would, when a project matching the workspace folder directory opens, scan the given folder and open all nested projects
• there could be a UI tweak to not show these projects, unless explicitly open by the user; this is probably the trickiest part to implement, and would probably need an API

[MartinBalin]

Fallbacks break existing project recognition consumers
Once cached, a fallback is returned by ordinary findProject(). Consequently, FileOwnerQuery assigns previously unowned Java files to it, disabling single-file Run/Debug and classpath support, which require a null owner. It also makes ProjectGenerator.createProject() reject the folder as already containing a project. Preserve these existing consumers’ behavior before exposing fallbacks through the ordinary cache.

Cache clearing returns before replacing fallbacks under write access — NbProjectManager.java (line 517)
postReadRequest() defers execution until the current write lock is released. Creating project metadata, calling clearNonProjectCache(), then calling findProject() within that lock therefore still returns the fallback. Replacement needs to take effect before the subsequent lookup.

Replaced fallbacks remain valid and can retain file ownership — NbProjectManager.java (line 525)
Removal updates only dir2Proj. The fallback remains in proj2Factory, so isValid(fallback) stays true. When replacement is triggered by a factory lookup change, the file-owner cache also remains stale: findProject() returns the real project while FileOwnerQuery returns the fallback. Retire the old instance and invalidate ownership caches together.

[jtulach]

Thanks Martine. Some of these are similar to Toni's findings. Some are new. All fixed in e348182

[jtulach]

Thank you Jan.

... the end goal here is to ...

• The end goal of this PR is to implement ProjectManager.findProjectOrFallback API change and use it at two places, where it makes obvious sense:
  • when opening after start
  • when creating Folder of Projects group
• please find my reply to your suggestion to provide new ProjectFactory in the newly started discussion thread associated with NbProjectManager class

What I am not quite sure if whether starting with an API change is the best move.

• the goal of this PR is to avoid changing the UI (and that's a good move from my perspective)
  • if you have concerns about deeper changes to UX, I suggest you to express them in PR-9631
  • please find my reply there
• leave this PR fully focused on the ProjectManager.findProjectOrFallback API change, if possible, thank you!

[jtulach]

• Thanks for the reviews this PR has obtained so far.
• As far as I can tell, all the issues were addressed.
• Reviewers, please take another look. Thank you.

[eppleton]

This enables single-file support under a fallback, but MultiSourceRootProvider (around line 510, RootPathResourceImplementation.includes) still filters the source path with FileOwnerQuery.getOwner(fo) == null. So a loose file under a fallback now gets a source path, but that path excludes the file itself and its siblings.

I checked it in MultiSourceRootProviderTest on this branch: a folder with pack/A.java and pack/B.java, opened via findProjectOrFallback. findClassPath(A, SOURCE) returns a path rooted at the folder, but contains(A) and contains(B) are both false. Without the fallback, both are true.

Run/Debug is enabled now, but A can't use B and the files are probably not indexed. Maybe includes() should apply the same rule as here (no owner, or an owner without Java source groups), ideally through a shared helper so the two can't drift apart again.

[jtulach]

• Shared utility introduced in c22ea60
• but I don't understand the test case
• I tried pack.A referencing pack.B

• however that doesn't work in any previous version of NetBeans either
• maybe the whole java/java.file.launcher module is a bit fragile?

[dukehoff]

You need to activate Source File Launcher Indexing for it to work:

[JaroslavTulach]

• These options aren't even visible in non-Java projects
• need 86be29e to make them appear
• then the support work on JDK 25 in empty directories as well as non-Java projects

• these fixes in Java File Launcher deserve their own PR: Allow usage of Java Single File Launcher in non-Java projects #9666

[neilcsmith-net]

What I am not quite sure if whether starting with an API change is the best move. Whether mapping this to project groups matches the requirements also remains to be seen.
...
I guess if I would be doing this, I would:

* try to make the feature end-to-end first. It does not have to be merged as a single PR, but ensuring the end result is as good as possible before adjusting APIs would be nice

* if there were explicit actions to open a folder as a workspace/add folder to a workspace, we wouldn't probably need the fallback API - the "workspace folder project" could be created by an ordinary project factory,

Sorry, having given this some more thought, I agree with @lahodaj here. There is more to discuss. I think the requirement for this new API is not currently proven, and handling this probably can, and should, be via an ordinary project factory. As we're effectively in our ramp down phase to the next release, the end user benefits for this are currently minimal, there may still be some fallout from the implementation, and the API may lock us in to something that is the wrong approach, I don't think we should fast track this in. Let's hold fire for the next cycle and consider along with the wider impact usage that goes with it.

So, that's a -1 from me to merging this prior to branching NB 32. Unless one of our active maintainers wants to make a strong argument in the other direction.

I would really like to see the ability to opt in to opening any folder as a project. Don't get me wrong, I'm not negative on solving the problem. It should be possible for the user to open any folder as a project. There is potential there to address a whole bunch of user requests, including but not just making the folder group function better. eg. I'd like to see the inverse of our current folder action Open Project of Folder that is Open Folder as Project. We could even see fallback project more as a basic project. Having it registered as a standard project factory allows for it to be configured or hidden as with anything else in the layer system. That might be important for platform users before we bake this facility into the API.

if needed; (the action would mark the directory as "needs workspace project, if there's no better" in the userdir, or something alike)

In terms of marker, I would suggest the presence of the default AuxiliaryConfiguration on a folder, by either file attributes or the shareable .netbeans.xml. The file attributes for these are imported on upgrade as of NB31, and either of those things are likely to be used for storing project information if we extend to that.

[eppleton]

@neilcsmith-net @lahodaj I understand not wanting to rush a new API in during ramp-down. But I'd hate for the work done here over the last few days to just sit until the next cycle, so we should at least split out the single-file launcher fix for NB 32. It's useful independently of the fallback API: Run/Debug of a single Java file and its Run Single File properties currently don't work in C/C++, PHP and other non-Java projects, as @jtulach's screenshots show. The change is small and has tests (the shared findJavaOwner check in java.file.launcher plus the JavaNode change). You both said it needs fixing anyway. As its own PR it could still make NB 32.

[jtulach]

• one issue that @lahodaj warned about is:
  • creating a fallback project inside of another project
  • disconnects the FileOwnerQuery.getOwner from the right project with the right lookup
  • then the .java sources may not get the right classpath, etc.
• simplest way is to prevent that altogether
  • the adbcfc4 change does that

• even if one selects src directory in a Maven project
• the owning maven getProjectDirectory() is selected as the "Folder of Projects"

[jtulach]

• one set of comments related the new Project.Manager.findProjectOrFallback API
  • some suggested to implement this functionality without this API method
• I can do it, if that's the preferred option. Let's vote:
  • +1 to create (keep the current) API
  • -1 to remove the API
• @neilcsmith-net points out this cannot be Apache vote. That's correct.

• GitHub's thumbs up/down emotions are suitable for counting
  • I used that in the past to quantify opinions
• what I am looking for
  • some people expressed their wish to go without the API
  • some comments suggested we can do API, but only for NetBeans 33
  • opinions might have changed meanwhile
• it is additional work to remove the API as already proposed, so press one of the emoticons and let me know if I should invest the work.

[neilcsmith-net]

A pull request is not the place to hold a vote. Nor is it a good way to review what is technically better. And it is secondary to first trying to find consensus.

What requirements are there that cannot be met by a standard project factory, and what SPI would be required to meet those? eg. ProjectFromFolderProvider or marker annotation or similar? What else? How does that compare to the proposed API?

How could a fallback solution be disabled, replaced, reordered, etc. if it is not a factory registered in the layer?

Edit :

Sorry, having given this some more thought, I agree with @lahodaj here. There is more to discuss. I think the requirement for this new API is not currently proven, and handling this probably can, and should, be via an ordinary project factory ... the API may lock us in to something that is the wrong approach... Let's hold fire for the next cycle and consider along with the wider impact usage that goes with it.

So, that's a -1 from me ... Unless one of our active maintainers wants to make a strong argument in the other direction.

I do intend to keep my -1 on this until the project factory solution is considered, and the pros and cons to both approaches are weighed correctly considering what we're trying to achieve here. The project factory solution has cons, and will require some SPI to work, but in my opinion has far fewer cons than this API.

[jtulach]

project factory solution is considered,

• API removed in 59db5c2
• the FallbackFactory and its usages are all in ide/projectui ...
• ... e.g. no API is needed

The project factory solution has cons,

• it certainly does - cleaning up internals of ProjectManager when new project factories get enabled is now complicated and limited - some tests may still be failing - pending a fix...

and will require some SPI to work, but in my opinion has far fewer cons than this API.

• I believe Neil talks about some marker on disk when mentioning SPI here
• but the goal of this PR was never to make permanent changes on disk!
• the whole purpose of ProjectManager.findProjectOrFallback was to create the fallback project only temporarily while the IDE is running

[eppleton]

Looked at 83c7fd7. Moving everything into ide/projectui as a plain ProjectFactory addresses the API concerns nicely.

One blocker: the last commit stops normal projects from being reopened on startup (details inline). The rest are leftovers from removing the API.

I ran the projectui tests that restore projects from OpenProjectListSettings. Five of them pass on master and on 6d3969b, but fail on 83c7fd7:

• CloseProjectsTest: "Just fifteen left nodes expected:<15> but was:<0>"
• ProjectsRootNodeFindTest, ProjectsRootNodeInitializedSoonerTest, OpenProjectListSetMain2Test and OpenProjectHookThrowsExceptionTest hang until the 300s timeout.

(ProjectsRootNodePhysicalViewTest times out on master too, so I left it out.)

In the new tests, FallbackFactoryTest.testFallbackProjectLookup and ProjectManagerFallbackTest.testFactoryChangeUpdatesFileOwner fail for me as well. I assume those are the ones you mentioned are still pending.

[eppleton]

This skips every regular project too: a project directory is owned by its own project, so FileOwnerQuery.getOwner(dir) is non-null for every real project in the saved list, and URLs2Projects returns nothing for them. With this commit, no project is reopened on startup.

I think the owner check only belongs on the fallback path: use findProject(dir) as before, and fall back only when that returns null and no other project owns dir.

[eppleton]

Leftover from the API version? projectapi.nb has no source changes any more, but its javac.release (17 → 21) and its projectapi dependency (1.78 → 1.107, in project.xml) are still raised.

[eppleton]

This looks unrelated to the PR. MavenConfiguration isn't visible from projectapi, so @see MavenConfiguration can't resolve; the previous cross-module link worked.

[eppleton]

Since the tests moved to projectui, this file only has whitespace changes plus this now-unused justADir field. The org.openide.util.ui.svg test dependency added to ide/projectapi/nbproject/project.xml also seems unneeded now. Maybe revert both?