Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added check for not intended default workspace location use (#35) #36

Merged
merged 1 commit into from
Apr 22, 2022

Conversation

iloveeclipse
Copy link
Member

See original bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=514333
that was only partly fixed.

The original fix added -Dosgi.dataAreaRequiresExplicitInit=true system
property and changed org.eclipse.core.internal.runtime.DataArea in the
way that it doesn't allow to initialize itself if the workspace location
was not explicitly specified yet - either via -data or via workspace
selection prompt.

But the fix in DataArea does not work if bad client code calls
Platform.getLocation() before calling Plugin.getStateLocation(),
because Platform.getLocation() doesn't run into changed DataArea code at
all.

Exact this happened in commit
eclipse-platform/eclipse.platform.resources@a8a8d82
where workspace init order was slightly changed: original code
initialized LocalMetaArea first (which then failed as supposed in
assertLocationInitialized() if workspace was not set yet), and after
that initialized WorkspaceRoot (which uses Platform.getLocation() after
the check). Changed code initialized WorkspaceRoot first, and because
this uses Platform.getLocation(), the code silently initializes default
workspace location even if -Dosgi.dataAreaRequiresExplicitInit=true is
set.

The current fix makes sure that if the system flag is given, we disallow
silent workspace location initialization by adding similar check in
Platform.getLocation().

Fixes issue
#35

}
if (!location.isSet()) {
boolean explicitInitRequired = Boolean
.valueOf(getBundleContext().getProperty(PROP_REQUIRES_EXPLICIT_INIT));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.valueOf(getBundleContext().getProperty(PROP_REQUIRES_EXPLICIT_INIT));
.parseBoolean(getBundleContext().getProperty(PROP_REQUIRES_EXPLICIT_INIT));

No need to unbox.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks.

…platform#35)

See original bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=514333
that was only partly fixed.

The original fix added -Dosgi.dataAreaRequiresExplicitInit=true system
property and changed org.eclipse.core.internal.runtime.DataArea in the
way that it doesn't allow to initialize itself if the workspace location
was not explicitly specified yet - either via -data or via workspace
selection prompt.

But the fix in DataArea does not work if bad client code calls
Platform.getLocation() *before* calling Plugin.getStateLocation(),
because Platform.getLocation() doesn't run into changed DataArea code at
all.

Exact this happened in commit
eclipse-platform/eclipse.platform.resources@a8a8d82
where workspace init order was slightly changed: original code
initialized LocalMetaArea first (which then failed as supposed in
assertLocationInitialized() if workspace was not set yet), and after
that initialized WorkspaceRoot (which uses Platform.getLocation() after
the check). Changed code initialized WorkspaceRoot first, and because
this uses Platform.getLocation(), the code silently initializes default
workspace location even if -Dosgi.dataAreaRequiresExplicitInit=true is
set.

The current fix makes sure that if the system flag is given, we disallow
silent workspace location initialization by adding similar check in
Platform.getLocation().

Fixes issue
eclipse-platform#35
@iloveeclipse iloveeclipse merged commit c45ef62 into eclipse-platform:master Apr 22, 2022
@iloveeclipse iloveeclipse deleted the issue_35 branch April 22, 2022 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants