Community
Participate
Working Groups
PDE Builders, at least ManifestConsistencyChecker and ExtensionPointSchemaBuilder, do use the workspace root as scheduling rule, preventing many other jobs or builders to run in paralllel. We should investigate having them using a more relaxed scheduling rule, such as project+PDEModel.
New Gerrit change created: https://git.eclipse.org/r/120397
Any chance the proposed patch can be reviewed for M7?
(In reply to Mickael Istria from comment #2) > Any chance the proposed patch can be reviewed for M7? Code change look simple. How can I test this patch?
I don't see issues with this patch as all the builders don't seem to go outside of the project boundary. I would like to understand what do you mean by "project+PDEModel" though?
(In reply to Alexander Kurtakov from comment #4) > I don't see issues with this patch as all the builders don't seem to go > outside of the project boundary. I would like to understand what do you mean > by "project+PDEModel" though? I meant PDEState which is a singleton used by all PDE projects. Usage of this singleton is the reason why it seems safer so far to have the rule on all PDE projects so we avoid concurrent modifications of PDEState.
Is someone looking at this? Alex/Lars/Andrey?
I'm not sure how PDE computes extensions and plugins state. If this requires build to run, then the change is not OK, because for example the code in org.eclipse.pde.internal.core.builders.ExtensionsErrorReporter.validateExtension(Element) needs to know about extension points defined in other plugins, to check if the contribution of the current plugin is valid. Same in org.eclipse.pde.internal.core.builders.PluginBaseErrorReporter.validatePluginIDRef(Element, Attr) which needs PluginRegistry to know the model of the referenced bundle.
(In reply to Andrey Loskutov from comment #7) > If this requires build to run, then the change is not OK, because... Why do you think this change would make these methods not functional?
(In reply to Mickael Istria from comment #8) > (In reply to Andrey Loskutov from comment #7) > > If this requires build to run, then the change is not OK, because... > > Why do you think this change would make these methods not functional? Because if the builders would now run in parallel AND PDE registry/model state depends on the build (this is where I need confirmation), the project A build might not see the results of project B build and so validation of "requires B" or "extends B's extension" will not work.
(In reply to Andrey Loskutov from comment #9) > Because if the builders would now run in parallel AND PDE registry/model > state depends on the build (this is where I need confirmation), the project > A build might not see the results of project B build and so validation of > "requires B" or "extends B's extension" will not work. See the scheduling rule that was chosen. It includes all PDE projects, so with this, not 2 PDE projects should build in parallel. That's a defensive approach specifically to avoid such issues, as long as we did not have the guarantee that we can modify safely the PDEState in parallel. Also, the principle of parallel build implementations is that it builds projects according to the dependency graph: no project gets built before its dependencies. So as long as dependencies are correct (that's the topic of bug 532264), then we have the guarantee that the build of project B completes before build of project A start.
I just had another look at the code, and indeed, there is no guard that 2 PDE projects wouldn't run in parallel. I thought I did it, but the code says I didn't. As we're not sure the PDE Model can handle parallel builds, I'll update the patch to a more constrained scheduling rule preventing 2 PDE projects to build in parallel.
(In reply to Mickael Istria from comment #11) > I'll update the > patch to a more constrained scheduling rule preventing 2 PDE projects to > build in parallel. https://git.eclipse.org/r/120397 got updated.
Andrey, can you please look at the updated patch. If ok, it can be merged to M7. If this looks like a risky thing to do in late M7, we can move to 4.9
Gerrit change https://git.eclipse.org/r/120397 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=4f2df0738794a698de1f1e2a55255a1848870bf1
Mickael, please add to N&N for 4.9.
New Gerrit change created: https://git.eclipse.org/r/124544
Gerrit change https://git.eclipse.org/r/124544 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=e02e21dac150ecd21e08dd2026c5a7d9fd866e8a
Any way to verify this bug?
