Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 361675

Summary:

Order mismatch when saving/restoring workspace trees

Product:

[Eclipse Project] Platform

Reporter:

Baltasar Belyavsky <bbelyavsky>

Component:

Resources

Assignee:

James Blackburn <jamesblackburn+eclipse>

Status:

RESOLVED FIXED

QA Contact:

Severity:

major

Priority:

CC:

adrian.ashley, jamesblackburn+eclipse, marcin.swiezawski, pmason, Szymon.Brandys, yevshif

Version:

3.7.1

Flags:

Szymon.Brandys: review? (Szymon.Brandys)

Target Milestone:

3.8 M5

Hardware:

OS:

Windows 7

Whiteboard:

Juno candidate

Bug Depends on:

Bug Blocks:

368085

Attachments:

Description	Flags
Patch generated with EGit - applies to latest 3.8 stream	none
Updated patch - applies to latest 3.8 stream	none
Updated patch - apply to workspace	jamesblackburn+eclipse: iplog+
Testcase to illustrate the bug, and regression-test the fix	jamesblackburn+eclipse: iplog+

Description Baltasar Belyavsky

2011-10-21 11:18:51 EDT

Build Identifier: I20111021-0800 [3.8 M2]

The recently added enhancement to support multiple build-configurations for a single project requires that workspace element-trees are persisted per each build-configuration.  

There currently are a couple of obvious bugs in the logic that persists/restores these additional workspace element-trees - the trees are restored in the order that does not match the order in which they are persisted.  This results in some very unpredictable build behaviour for projects that do support multiple build-configurations.

I'm attaching a patch that fixes these ordering bugs.  Please let me know if I should provide any more information.

Reproducible: Always

Steps to Reproduce:
I cannot come up with a simple scenario to reproduce this problem - it manifests itself only for projects whose builder supports multiple build-configurations.  But the bugs addressed by this patch are obvious enough to hopefully not require anything more than a quick code-review of the attached patch.

Comment 1 Baltasar Belyavsky

2011-10-21 11:22:15 EDT

Created attachment 205739 [details]
Patch generated with EGit - applies to latest 3.8 stream

Comment 2 Baltasar Belyavsky

2011-10-21 14:23:01 EDT

Created attachment 205752 [details]
Updated patch - applies to latest 3.8 stream

Uploading an updated patch - same content, but this one can be applied to workspace using the Apply Patch action.

Comment 3 Baltasar Belyavsky

2011-10-21 14:46:15 EDT

Didn't realize that v3.7.x stream is still active.  It would be great if this patch makes it into v3.7.2.  The same patch applies to the tip of 3.7 branch.

Comment 4 Baltasar Belyavsky

2011-10-28 16:03:35 EDT

Created attachment 206154 [details]
Updated patch - apply to workspace

Found one more problem with the persist/restore ordering.  Updated patch is attached.

Comment 5 Baltasar Belyavsky

2011-11-07 17:17:03 EST

Created attachment 206555 [details]
Testcase to illustrate the bug, and regression-test the fix

Attaching a patch containing the testcase that can be used to regression-test the fix. The same testcase also illustrates the bug described (if run before applying the fix patch).

Comment 6 Szymon Brandys

2011-11-09 06:19:23 EST

James, could you look at the Baltasar work?

Comment 7 James Blackburn

2011-11-15 17:42:47 EST

Looks like a very good catch to me.  AFAICS fix and test looks good.

Szymon, my new employer hasn't signed the committer paperwork yet, can you push this in?

Comment 8 James Blackburn

2011-11-15 17:50:09 EST

BTW as for the origin of this bug, we went through a few iterations of the metadata before arriving at something that was backwards and forwards compatible with the single config workspace layout.  Somehow in that process we clearly messed up the linkage between builders and their trees :s.  When looking at this code I can't help but think it could be made simpler...

Comment 9 Baltasar Belyavsky

2011-11-30 10:42:42 EST

Hi Szymon,

As James has reviewed the patch and the test-case, but isn't able to commit it himself, would you be able to commit it for us? Into 3.7.2 and up, preferably?

Comment 10 James Blackburn

2012-01-07 11:34:40 EST

Thanks for the patch and test Baltasar. Committed to master.

Bug 368085 created for the backport.