Cyclic dependency between ProjectPreferences and Workspace init

[laeubi]

Fix #86

Its not that nice because we use deprecated Preferences API here, but this prevents the implicit early Workspace access and is consistent to how ResourcesPlugin.getEncoding() and WorkspaceRoot.getDefaultCharset(boolean) behaves here.

Actually it would be good if none would ever call ResourcesPlugin.getEncoding() but instead Workspace.getRoot().getDefaultCharset() but unless this do not change we should simply keep this behavior here.

[eclipse-releng-bot]

The Jenkins build of this PR has now completed. See details at https://ci.eclipse.org/platform/job/eclipse.platform.resources/job/PR-87/1/

[vogella]

So any activator which uses regular preferences API may cause the same issue?

[laeubi]

So any activator which uses regular preferences API may cause the same issue?

No that is a bit different as it will trigger activation of the Resource plugin and then "wait" for Workspace init (hopefully this does not take to long ...) while in this case we are in the (internal) init phase so should avoid relying to much on other parts that indirectly "recall".

Anyways I opened eclipse-equinox/equinox.bundles#24 to make this more declarative and do not require plugins to use activator at all or call static methods for this common use-case.

[iloveeclipse]

I think we don't need legacy code if we change calls order as proposed.

[eclipse-releng-bot]

The Jenkins build of this PR has now completed. See details at https://ci.eclipse.org/platform/job/eclipse.platform.resources/job/PR-87/2/

[iloveeclipse]

I would prefer to keep the initializePreferenceLookupOrder() inside open() call. The changes in the workspace init order are really hard to understand (like eclipse-platform/eclipse.platform.runtime#35 that was caused by a "trivial" 1:1 change in a8a8d82).

[laeubi]

Old order should be preserved now (just moved the save a bit above), looking at the code it feels like both statements should be moved down throw new ResourceException .. strange enough there is already a WorkspacePreferences object created maybe we better should use that anyways??

[eclipse-releng-bot]

The Jenkins build of this PR has now completed. See details at https://ci.eclipse.org/platform/job/eclipse.platform.resources/job/PR-87/3/

[iloveeclipse]

Old order should be preserved now (just moved the save a bit above),

thanks.

looking at the code it feels like both statements should be moved down throw new ResourceException

That was my original proposal (to move preferences order init after setting encoding)

.. strange enough there is already a WorkspacePreferences object created maybe we better should use that anyways??

The WorkspacePreferences is a "legacy wrapper" to few preference values that were previously in an xml file, see #93

[laeubi]

So probably it would make sense to first getting #93 done and then rebase on that change?

[iloveeclipse]

So probably it would make sense to first getting #93 done and then rebase on that change?

Sure, that's the reason you are set as reviewer on PR :-)

[laeubi]

Rebased now.

[eclipse-releng-bot]

The Jenkins build of this PR has now completed. See details at https://ci.eclipse.org/platform/job/eclipse.platform.resources/job/PR-87/4/

[iloveeclipse]

Thanks, looks good, I'm looking into it now once again, just to be sure we don't change any logic here.

[iloveeclipse]

LGTM.