-
Notifications
You must be signed in to change notification settings - Fork 168
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
Improve workspace lock error dialog. #2347
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,8 +25,10 @@ | |
import java.io.FileOutputStream; | ||
import java.io.IOException; | ||
import java.io.OutputStream; | ||
import java.net.InetAddress; | ||
import java.net.MalformedURLException; | ||
import java.net.URL; | ||
import java.net.UnknownHostException; | ||
import java.util.LinkedHashMap; | ||
import java.util.Map; | ||
import java.util.Properties; | ||
|
@@ -49,7 +51,11 @@ | |
import org.eclipse.osgi.service.datalocation.Location; | ||
import org.eclipse.osgi.util.NLS; | ||
import org.eclipse.swt.SWT; | ||
import org.eclipse.swt.layout.FillLayout; | ||
import org.eclipse.swt.widgets.Composite; | ||
import org.eclipse.swt.widgets.Control; | ||
import org.eclipse.swt.widgets.Display; | ||
import org.eclipse.swt.widgets.Label; | ||
import org.eclipse.swt.widgets.Shell; | ||
import org.eclipse.ui.IWorkbench; | ||
import org.eclipse.ui.PlatformUI; | ||
|
@@ -79,6 +85,20 @@ public class IDEApplication implements IApplication, IExecutableExtension { | |
|
||
private static final String VERSION_FILENAME = "version.ini"; //$NON-NLS-1$ | ||
|
||
private static final String LOCK_FILENAME = ".lock"; //$NON-NLS-1$ | ||
|
||
private static final String DISPLAY_VAR = "DISPLAY"; //$NON-NLS-1$ | ||
|
||
private static final String PROCESS_ID = "process-id"; //$NON-NLS-1$ | ||
|
||
private static final String DISPLAY = "display"; //$NON-NLS-1$ | ||
|
||
private static final String HOST = "host"; //$NON-NLS-1$ | ||
|
||
private static final String USER = "user"; //$NON-NLS-1$ | ||
|
||
private static final String USER_NAME = "user.name"; //$NON-NLS-1$ | ||
|
||
// Use the branding plug-in of the platform feature since this is most likely | ||
// to change on an update of the IDE. | ||
private static final String WORKSPACE_CHECK_REFERENCE_BUNDLE_NAME = "org.eclipse.platform"; //$NON-NLS-1$ | ||
|
@@ -225,6 +245,7 @@ protected Object checkInstanceLocation(Shell shell, Map applicationArguments) { | |
try { | ||
if (instanceLoc.lock()) { | ||
writeWorkspaceVersion(); | ||
writeWsLockInfo(); | ||
return null; | ||
} | ||
|
||
|
@@ -313,6 +334,7 @@ protected Object checkInstanceLocation(Shell shell, Map applicationArguments) { | |
if (instanceLoc.set(workspaceUrl, true)) { | ||
launchData.writePersistedData(); | ||
writeWorkspaceVersion(); | ||
writeWsLockInfo(); | ||
return null; | ||
} | ||
} catch (IllegalStateException e) { | ||
|
@@ -332,17 +354,164 @@ protected Object checkInstanceLocation(Shell shell, Map applicationArguments) { | |
|
||
// by this point it has been determined that the workspace is | ||
// already in use -- force the user to choose again | ||
|
||
String lockInfo = getWorkspaceLockInfo(workspaceUrl); | ||
|
||
MessageDialog dialog = new MessageDialog(null, IDEWorkbenchMessages.IDEApplication_workspaceInUseTitle, | ||
null, NLS.bind(IDEWorkbenchMessages.IDEApplication_workspaceInUseMessage, workspaceUrl.getFile()), | ||
MessageDialog.ERROR, 1, IDEWorkbenchMessages.IDEApplication_workspaceInUse_Retry, | ||
IDEWorkbenchMessages.IDEApplication_workspaceInUse_Choose); | ||
IDEWorkbenchMessages.IDEApplication_workspaceInUse_Choose) { | ||
@Override | ||
protected Control createCustomArea(Composite parent) { | ||
if (lockInfo == null || lockInfo.isBlank()) { | ||
return null; | ||
} | ||
|
||
String displayText = "Workspace lock is held by the following owner:\n".concat(lockInfo); //$NON-NLS-1$ | ||
|
||
Composite container = new Composite(parent, SWT.NONE); | ||
container.setLayout(new FillLayout()); | ||
|
||
Label multiLineText = new Label(container, SWT.NONE); | ||
multiLineText.setText(displayText); | ||
|
||
return container; | ||
} | ||
}; | ||
// the return value influences the next loop's iteration | ||
returnValue = dialog.open(); | ||
// Remember the locked workspace as recent workspace | ||
launchData.writePersistedData(); | ||
} | ||
} | ||
|
||
/** | ||
* Read workspace lock file and parse all the properties present. Based on the | ||
* eclipse version and operating system some or all the properties may not | ||
* present. In such scenario it will return empty string. | ||
* | ||
* @return Previous lock owner details. | ||
*/ | ||
private String getWorkspaceLockInfo(URL workspaceUrl) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please make this method and writeWsLockInfo() protected? This is not API, but we override parts of the class internally and these two methods most likely could need some customization sooner or later. |
||
File lockFile = getLockFile(workspaceUrl); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lockFile can be null. |
||
StringBuilder sb = new StringBuilder(); | ||
Properties props = new Properties(); | ||
try (FileInputStream is = new FileInputStream(lockFile)) { | ||
props.load(is); | ||
String prop = props.getProperty(USER); | ||
if (prop != null) { | ||
sb.append("user: ").append(prop).append(System.lineSeparator()); //$NON-NLS-1$ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please use IDEWorkbenchMessages so the strings could be translated if needed. |
||
} | ||
prop = props.getProperty(HOST); | ||
if (prop != null) { | ||
sb.append("host: ").append(prop).append(System.lineSeparator()); //$NON-NLS-1$ | ||
} | ||
prop = props.getProperty(DISPLAY); | ||
if (prop != null) { | ||
sb.append("display: ").append(prop).append(System.lineSeparator()); //$NON-NLS-1$ | ||
} | ||
prop = props.getProperty(PROCESS_ID); | ||
if (prop != null) { | ||
sb.append("process-id: ").append(prop).append(System.lineSeparator()); //$NON-NLS-1$ | ||
} | ||
return sb.toString(); | ||
} catch (Exception e) { | ||
IDEWorkbenchPlugin.log("Could not read lock file: ", e); //$NON-NLS-1$ | ||
} | ||
return null; | ||
} | ||
|
||
/** | ||
* Write lock owner details onto workspace lock file. Data includes user, host, | ||
* display and current java process id. | ||
*/ | ||
private void writeWsLockInfo() { | ||
Location instanceLoc = Platform.getInstanceLocation(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please first create properties & fill with the data, and only if the properties are not empty, open file for writing. |
||
if (instanceLoc == null || instanceLoc.isReadOnly()) { | ||
return; | ||
} | ||
|
||
File lockFile = getLockFile(instanceLoc.getURL()); | ||
if (lockFile == null) { | ||
return; | ||
} | ||
|
||
Properties props = new Properties(); | ||
|
||
String user = System.getProperty(USER_NAME); | ||
if (user != null) { | ||
props.setProperty(USER, user); | ||
} | ||
String host = getHostName(); | ||
if (host != null) { | ||
props.setProperty(HOST, host); | ||
} | ||
String display = getDisplay(); | ||
if (display != null) { | ||
props.setProperty(DISPLAY, display); | ||
} | ||
String pid = getProcessId(); | ||
if (pid != null) { | ||
props.setProperty(PROCESS_ID, pid); | ||
} | ||
|
||
try (OutputStream output = new FileOutputStream(lockFile)) { | ||
props.store(output, null); | ||
} catch (IOException e) { | ||
IDEWorkbenchPlugin.log("Could not modify lock file", e); //$NON-NLS-1$ | ||
} | ||
} | ||
|
||
private String getDisplay() { | ||
String displayEnv = null; | ||
try { | ||
displayEnv = System.getenv(DISPLAY_VAR); | ||
} catch (Exception e) { | ||
IDEWorkbenchPlugin.log("Failed to read DISPLAY variable.", e); //$NON-NLS-1$ | ||
} | ||
return displayEnv; | ||
} | ||
|
||
private String getProcessId() { | ||
Long pid = null; | ||
try { | ||
pid = ProcessHandle.current().pid(); | ||
} catch (Exception e) { | ||
IDEWorkbenchPlugin.log("Failed to read Java process id.", e); //$NON-NLS-1$ | ||
} | ||
return pid != null ? pid.toString() : null; | ||
} | ||
|
||
private String getHostName() { | ||
String hostName = null; | ||
try { | ||
hostName = InetAddress.getLocalHost().getHostName(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer to check environment variable |
||
} catch (UnknownHostException e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to be consistent, I would also catch here generic Exception, just in case. |
||
IDEWorkbenchPlugin.log("Failed to read host name.", e); //$NON-NLS-1$ | ||
} | ||
return hostName; | ||
} | ||
|
||
private File getLockFile(URL workspaceUrl) { | ||
if (workspaceUrl == null) { | ||
return null; | ||
} | ||
|
||
// make sure the directory exists | ||
File metaDir = new File(workspaceUrl.getPath(), METADATA_FOLDER); | ||
if (!metaDir.exists()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for extra folder check, file check below will do the job too |
||
return null; | ||
} | ||
|
||
// make sure the file exists | ||
File lockFile = new File(metaDir, LOCK_FILENAME); | ||
if (!lockFile.exists()) { | ||
return null; | ||
} | ||
|
||
return lockFile; | ||
} | ||
|
||
@SuppressWarnings("rawtypes") | ||
private static boolean isDevLaunchMode(Map args) { | ||
// see org.eclipse.pde.internal.core.PluginPathFinder.isDevLaunchMode() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use IDEWorkbenchMessages here