sdedic edited a comment on pull request #3480: URL: https://github.com/apache/netbeans/pull/3480#issuecomment-1017229259
> Well, in this case the isGradleProjectLoaded() and the getGradlePorject() in NbGradleProjectImpl does not really agree, if the project is only initialized with a FALLBACK grade project. That would result infinite nesting at AbstractProjectLoader:74 Is there a test or a sample code I can try ? > Strangely it did not cause problems elsewhere so far. This is more or less by accident, but the behaviour is stable: `getGradleProject()` is first called from within `NbGradleProjectFactory.loadProject`, before the new project is passed to the ProjectManager. So the first request before anyone can see `NbGradleProjectImpl` (under normal circumstances) is for `EVALUATED`. Should that fail, e.g. the actually "loaded" project is just `FALLBACK`, further requests for `EVALUATED` are rejected (unless `force` flag is in effect), since the result of that is already known. As a result, if the Loader sees non-null as a project (2nd+ attempts), it should not recurse. The recusion could happen, if one would force loading of a `FALLBACK` project. It's can be better to ensure that if a `getGradleProject` is called **from within project loading process**, it gets always some stable value. IMHO making this call from within the loader can indicate a bug either: since the project is being loaded, it cannot return "the project". > I'm going to give it a try and factor these out somehow. Will file another PR on that. I would say "please don't" as there are many things that can break with such a change. When I reworker the loading, `isGradleProjectLoaded()` users were using its `false` as an indication that none of the project information can be used. This is the case of a `dumped` project (forcibly closed): even FALLBACK project has some information (derived from directory structure) in it already. The main reason for the rewrite was that Gradle project loading was breaking API semantics: project queries, such as ClassPathProvider or SourceForBinaryQuery must work even though the project is not UI-opened (through OpenProjects) - that's the requirement of project APIs in general. In the UI, the breakage manifests when you open a file from a closed Gradle project, for example by going to symbol declaration from some other file. In absence of correct information (i.e. compile ClassPath), all library symbols in the opened file are underlined, find usages, implementations etc do not work. This is different from J2SE project behaviour (which is the de-facto standard to follow) or Maven. Now the loading process works in a way that ensures that if someone asks for project information, and the information is available in a serialized form, it will be loaded. In this (implicit) scenario, it is desirable not to execute Gradle for project configuration, as it may take considerable time not anticipated by the query / calling code semantics. The target state (now aim = EVALUATED) has to be identified differently from FULL state, so that the loading process can stop before actually executing Gradle - and the API clients can say what level of quality should be reached. I am not saying that exactly EVALUATED must be used, but it is a logical candidate: it is "a state that was once correct, but need not to be any more" - which is OK for queries that need at least some data, and can fire events when the data change. With this setup, an unintentional caller of `getGradleProject` gets the best-effort data state, if available. A caller that cares more can use `NbGradleProject.toQuality()` with EVALUATED (best-effort fast), FULL (allow gradle execution) or FULL_ONLINE (allow updates) as it is suitable for the client. Having `getGradleProject` ask for EVALUATED is a shorthand - otherwise all callers would likely need to call `toQuality` in order to access the project information. --- To prevent the exact recursion, I can offer different techniques, depending on how the recursion happens exactly: - call a **different** accessor in NbGradleProjectImpl, that will really return the `current` (= the previous for the loader) project - ensure that the loading process will **always** get the same (current/previous) reference to the project from `getGradleProject`: during the load, as it is defined in the internal APIs, `getGradleProject` can't really return the 'current' (being loaded) project, as that instance is the product of the Loader and does not exist before it completes. But as there are not IMHO any additional calls from Loaders except ReloadContext, I'd just use that alternative (direct) accessor. Need a scenario when the recursion actually happens to write a proper test. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
