Community
Participate
Working Groups
The BuildManager class does not check whether projects are open before adding them to the list of projects to build. This means that if you have two projects in your workspace, one open and one closed, the progress monitor will slowly reach 50% as it builds the open project but then just jump to 100% as it 'builds' the closed project.
Created attachment 139009 [details] Patch for BuildManager The patch removes inaccessible projects from the list of projects to be built.
*** Bug 45071 has been marked as a duplicate of this bug. ***
Just checking, has there been any progress on this? Thanks.
Created attachment 145928 [details] Patch with a test (In reply to comment #3) > Just checking, has there been any progress on this? > > Thanks. > The patch works fine. In addition I wrote a test for this functionality. Szymon, please review it.
I have looked at the bug some time ago and was not quite sure about the patch. At this point in BuildManager it was quite possible that projects can change their state (be closed or opened), so allocating progress is safe. But if you remove closed projects something may not be built. Someone please check my doubts...
(In reply to comment #5) > I have looked at the bug some time ago and was not quite sure about the patch. > > At this point in BuildManager it was quite possible that projects can change > their state (be closed or opened), so allocating progress is safe. But if you > remove closed projects something may not be built. > > Someone please check my doubts... > It's not possible for other threads to change projects state since build executes within a rule that prevents this from happening. So it could only be done by a builder itself. Since I'm not aware of such case it's probably more accurate to exclude closed projects from total work calculation. The default build rule can be changed by providing custom rule factory but in such case projects could be closed at any stage of the build.
Actually, according to Chris comment it would be better to change the fix proposed by Doug to exclude closed projects only from total work calculation. Right now they are excluded from the list of projects to build. Szymon, what do you think? If you agree I'll prepare a patch.
On second thoughts I think the current behavior is fine. Since closed projects can change their states during a build, we should allocate progress for them too. If we apply any of suggested fixes, we can end up with "too short" progress regarding the real amount of work to perform.
I think the current patch is correct. When a user opens a closed project, a build gets scheduled for it. So it is unnecessary to include closed projects in the current build.
Thanks for explanations :)
(In reply to comment #8) > On second thoughts I think the current behavior is fine. Since closed projects > can change their states during a build, we should allocate progress for them > too. If we apply any of suggested fixes, we can end up with "too short" > progress regarding the real amount of work to perform. > Frankly, I haven't seen opening a project during a build. Os course if such a situation occurs we will actually end up too short with progress. However reporting a progress is more a matter of being accurate in most cases which we fail to do now.
(In reply to comment #9) > I think the current patch is correct. When a user opens a closed project, a > build gets scheduled for it. So it is unnecessary to include closed projects in > the current build. > The problem is that in theory a builder could open other projects in the build process and they would be build given the current state of the code. With the patch applied closed projects would be discarded at earlier stage.
(In reply to comment #11) > Frankly, I haven't seen opening a project during a build. I think I have seen it once (but I am not sure and cannot check). RAD/RSA on first start-up run Migration Builder which adjusts project configuration. And I bet 1$ that it could open some specific projects to retrieve important data (especially when it came to jee). I believe that manipulating total amount of the work is a little bit safer that removing closed projects from build...
(In reply to comment #9) > I think the current patch is correct. When a user opens a closed project, a > build gets scheduled for it. So it is unnecessary to include closed projects in > the current build. Just to support Pawel. If autobuild is enabled, it would be fine. Otherwise you could end up with not build projects. Let's consider a closed project that contains changes to build and a builder that opens it during a workspace build. This project will stay unbuilt, when autobuild is disabled.
Created attachment 145951 [details] Patch_v02 Fix changed according to comment 7.
Created attachment 145952 [details] Patch_v02 tests Patch with tests with minor changes.
Created attachment 146062 [details] Patch_v03 (In reply to comment #15) > Created an attachment (id=145951) [details] > Patch_v02 > > Fix changed according to comment 7. I've added a condition not to report more work than total.
The fact that projects can be opened or closed during a build makes this issue tricky. We talked with Pawel about it and it seems that the best solution would be: - to allocate less work per unordered projects - to update the progress for unordered projects
Created attachment 146074 [details] Patch_v04 The patch allocates 75% on the progress bar for open projects and the remaining 25% for closed.
(In reply to comment #14) > Let's consider a closed project that contains changes to build and a builder > that opens it during a workspace build. > This project will stay unbuilt, when autobuild is disabled. In the case where a builder opens closed project during a build (BTW, it is highly unlikely and I don't know of any such examples) then the opened project is not guaranteed to get built, is it? Because the build manager may already have gone past that project in its list (and found it to be closed at the point at which it tried to build it).
Any thoughts on my last comment, or have I got things wrong? Thx
(In reply to comment #20) > In the case where a builder opens closed project during a build (BTW, it is > highly unlikely and I don't know of any such examples) then the opened project > is not guaranteed to get built, is it? Because the build manager may already > have gone past that project in its list (and found it to be closed at the point > at which it tried to build it). A builder can request for a rebuild (see BuildManger#requestRebuild). This rebuild may be performed in the same basicBuildLoop (BuildManager line 305). Some builders can open closed projects and request for a rebuild on purpose. So closed projects removed from the list would be ignored during subsequent iterations.
(In reply to comment #22) > Some builders can open closed projects and request for a rebuild on purpose. So > closed projects removed from the list would be ignored during subsequent > iterations. OK thanks - I didn't know about the rebuild feature. Complicated isn't it!
(In reply to comment #23) > OK thanks - I didn't know about the rebuild feature. Complicated isn't it! Anyway thanks for looking at it. I think there are other issues to investigate in this area. I would be glad to review any further proposals or fixes. (In reply to comment #19) The simplest fix here is to use ordered.length for projectWork. This can be too little for some cases. However even now the progress can exceed ordered.length + unordered.length, if builders request a rebuild.
Created attachment 146275 [details] Patch_v05
Patch_v05 released to HEAD.
I believe "unordered" may contain open projects in the case where someone has set an explicit build order (IWorkspaceDescription#setBuildOrder). In this case the fix won't be correct because it allocates no progress for unordered open projects. I think you also need to iterate over the unordered projects at this beginning of this method and increment "projectWork" for each accessible unordered project. For the case of projects that get opened during the build cycle I don't think we need to worry about reserving progress space for this (it will be very rare).
