Bug 538462 - [Builders] mixing builders with relaxed and null scheduling rule leads to workspaceRoot
Summary: [Builders] mixing builders with relaxed and null scheduling rule leads to wor...
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Resources (show other bugs)
Version: 4.8   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Mickael Istria CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 527212
  Show dependency tree
 
Reported: 2018-08-31 08:29 EDT by Mickael Istria CLA
Modified: 2021-06-24 05:01 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mickael Istria CLA 2018-08-31 08:29:42 EDT
This is continuation of bug 306824.

In bug 306824, it was identified that when mixing builders with a relaxed scheduling rule and a null scheduling rule, a "super" scheduling rule made of only the relaxed one was created to host the build, causingthe changes of the 2nd builder to fail when they weren't under the tree defined by the 1st builder.

We should probably re-audit that and sort out whether and why a super scheduling rule is needed and applied, and evaluate whether we could get rid of it so builders wouldn't run in a super scope that could be too constraining for them.
Comment 1 Mickael Istria CLA 2018-08-31 08:36:26 EDT
(In reply to Mickael Istria from comment #0)
> We should probably re-audit that and sort out whether and why a super
> scheduling rule is needed and applied

bug 306824 was closed in 2010, and introduced the change creating the "super" scheduling rule.
Later, in 2011, with bug 343256, a change was made so that if projects all use relaxed scheduling rule, the "super" scheduling rule is not used.

This means that the creation of the super scheduling rule is not really optimal not bug 343256 is implemented, and this could most likely be safely removed with great gain.
Comment 2 Mickael Istria CLA 2018-08-31 11:40:28 EDT
Actually, from bug 306824, there is one thing I wonder: if on the same project one builder has schedulingRule someResource/ and another has null as schedulingRule, couldn't getRule() return null as it's the most generic rule?
Comment 3 Eclipse Genie CLA 2018-09-03 04:59:06 EDT
New Gerrit change created: https://git.eclipse.org/r/128527
Comment 4 Mickael Istria CLA 2018-09-20 14:42:59 EDT
I plan to merge this soon unless someone has concerns.
Comment 5 Dani Megert CLA 2018-09-26 15:31:56 EDT
(In reply to Mickael Istria from comment #2)
> Actually, from bug 306824, there is one thing I wonder: if on the same
> project one builder has schedulingRule someResource/ and another has null as
> schedulingRule, couldn't getRule() return null as it's the most generic rule?

I think this would be wrong. Other projects might depend on this one, so we must ensure to lock the resource in question.

If you disagree, please provide some test cases involving several projects that depend on each other.
Comment 6 Mickael Istria CLA 2018-10-04 04:29:26 EDT
I'm removing target on this one.
This is more long-running issue, which is not so high priority and as Dani has voiced concerns, it seems like it's not so trivial.
Definitely not something we should rush on and change at the moment.
Comment 7 Mickael Istria CLA 2021-06-24 04:58:48 EDT
(In reply to Mickael Istria from comment #2)
> Actually, from bug 306824, there is one thing I wonder: if on the same
> project one builder has schedulingRule someResource/ and another has null as
> schedulingRule, couldn't getRule() return null as it's the most generic rule?

Thinking more about it as I'm going through reviews and according to comment #5, I think it's a bad idea to assume rule+null should resolve to workspace root.

However, I don't have a better suggestion at the moment.
Comment 8 Mickael Istria CLA 2021-06-24 05:01:01 EDT
(In reply to Mickael Istria from comment #7)
> However, I don't have a better suggestion at the moment.

What's in the patch is actually a better suggestion: it doesn't change the implementation of getRule(), but it change how the workspace will perform builds in that case. It seems much safer as no API is changed.