diff options
author | Andrew Ferrazzutti | 2015-01-13 19:04:38 +0000 |
---|---|---|
committer | Roland Grunberg | 2015-01-20 17:39:56 +0000 |
commit | 65f45c4e16870192967d685e206a0147e36784f3 (patch) | |
tree | b18599e49d90034a61ab760e6f5dbcfe6326657e /systemtap | |
parent | bbd6f0cd39c60992f4d3e097c1e3b002ef2e246b (diff) | |
download | org.eclipse.linuxtools-65f45c4e16870192967d685e206a0147e36784f3.tar.gz org.eclipse.linuxtools-65f45c4e16870192967d685e206a0147e36784f3.tar.xz org.eclipse.linuxtools-65f45c4e16870192967d685e206a0147e36784f3.zip |
Systemtap: Improve no-stap errors; allow projectless processes.
Using Systemtap IDE without stap installed causes too many error dialogs to appear.
Simplify/streamline the kinds of error dialogs that are shown when stap is missing.
Also allow RuntimeProcessFactory to launch processes that don't have a host project,
which is required for the above error dialog improvements.
Change-Id: I39daf04c28d2dab53fa46c7c6b8de64a6c3f4ae1
Signed-off-by: Andrew Ferrazzutti <aferrazz@redhat.com>
Reviewed-on: https://git.eclipse.org/r/39540
Tested-by: Hudson CI
Reviewed-by: Roland Grunberg <rgrunber@redhat.com>
Tested-by: Roland Grunberg <rgrunber@redhat.com>
Diffstat (limited to 'systemtap')
7 files changed, 92 insertions, 68 deletions
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/FunctionParser.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/FunctionParser.java index 3e325220dc..22bfadc33f 100644 --- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/FunctionParser.java +++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/FunctionParser.java @@ -190,10 +190,8 @@ public final class FunctionParser extends TreeTapsetParser { } @Override - protected int addTapsets(String[] additions, IProgressMonitor monitor) { - String tapsetContents = SharedParser.getInstance().getTapsetContents(); + protected int addTapsets(String tapsetContents, String[] additions, IProgressMonitor monitor) { boolean canceled = false; - // Search tapset contents for all files provided by each added directory. for (int i = 0; i < additions.length; i++) { int firstTagIndex = 0; diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/Messages.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/Messages.java index f1647a9700..3ec958ee0f 100644 --- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/Messages.java +++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/Messages.java @@ -25,7 +25,7 @@ public class Messages extends NLS { public static String ProbeParser_aliasProbes; public static String ProbeParser_illegalArgMessage; public static String TapsetParser_CannotRunStapMessage; - public static String TapsetParser_CannotRunStapTitle; + public static String TapsetParser_CannotRunRemoteStapMessage; public static String TapsetParser_ErrorRunningSystemtap; public static String TapsetParser_RemoteCredentialErrorTitle; public static String TapsetParser_RemoteCredentialErrorMessage; diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/ProbeParser.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/ProbeParser.java index 3bf8c17681..6e48682de0 100644 --- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/ProbeParser.java +++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/ProbeParser.java @@ -39,7 +39,8 @@ public final class ProbeParser extends TreeTapsetParser { public static final String PROBE_REGEX = "(?s)(?<!\\w)probe\\s+{0}\\s*\\+?="; //$NON-NLS-1$ private static final String TAPSET_PROBE_REGEX = "probe {0} \\+?="; //$NON-NLS-1$ - private static final Pattern PROBE_GROUP_PATTERN = Pattern.compile("[^\\.\\( ]+"); //$NON-NLS-1$ + private static final String PROBE_FORM_CHECK_REGEX = "\\w+((\\(\\w+\\))?(\\.\\w+)?)*( \\$?\\w+:\\w+)*"; //$NON-NLS-1$ + private static final Pattern PROBE_GROUP_PATTERN = Pattern.compile("[^\\.\\(]+"); //$NON-NLS-1$ private static ProbeParser parser = null; public static ProbeParser getInstance(){ @@ -95,6 +96,9 @@ public final class ProbeParser extends TreeTapsetParser { if (result != IStatus.OK) { return result; } + if (!doQuickErrorCheck(probeDump)) { + return IStatus.ERROR; + } boolean canceled = false; try (Scanner st = new Scanner(probeDump)) { @@ -105,9 +109,8 @@ public final class ProbeParser extends TreeTapsetParser { break; } String tokenString = st.nextLine(); - String probeName = (new StringTokenizer(tokenString)).nextToken(); - groupNode = addOrFindProbeGroup(extractProbeGroupName(probeName), groupNode, statics); - groupNode.add(makeStaticProbeNode(probeName)); + groupNode = addOrFindProbeGroup(extractProbeGroupName(tokenString), groupNode, statics); + groupNode.add(makeStaticProbeNode(tokenString)); } } statics.sortTree(); @@ -135,6 +138,9 @@ public final class ProbeParser extends TreeTapsetParser { if (result != IStatus.OK) { return result; } + if (!doQuickErrorCheck(probeDump)) { + return IStatus.ERROR; + } boolean canceled = false; try (Scanner st = new Scanner(probeDump)) { @@ -163,6 +169,23 @@ public final class ProbeParser extends TreeTapsetParser { } /** + * Performs a quick check of validity in a probe dump. + * @param probeDump The output of a call to stap that prints a probe list. + * @return <code>false</code> if the output of the dump is invalid. + */ + private boolean doQuickErrorCheck(String probeDump) { + if (probeDump == null) { + return false; + } + // Check just the first probe printed + int end = probeDump.indexOf('\n'); + if (end != -1) { + probeDump = probeDump.substring(0, end); + } + return Pattern.matches(PROBE_FORM_CHECK_REGEX, probeDump); + } + + /** * Adds a single probe alias to the collection. * @param probeLine A line of probe information printed by a call to "stap -L". * @param aliases The tree of probe aliases. The probe will be added to this tree. @@ -318,8 +341,7 @@ public final class ProbeParser extends TreeTapsetParser { } @Override - protected int addTapsets(String[] additions, IProgressMonitor monitor) { - String tapsetContents = SharedParser.getInstance().getTapsetContents(); + protected int addTapsets(String tapsetContents, String[] additions, IProgressMonitor monitor) { boolean canceled = false; TreeNode aliases = tree.getChildByName(Messages.ProbeParser_aliasProbes); Map<String, ArrayList<String>> fileToItemMap = new HashMap<>(); diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/SharedParser.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/SharedParser.java index c845f28fff..b59cb3ac68 100644 --- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/SharedParser.java +++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/SharedParser.java @@ -92,32 +92,32 @@ public final class SharedParser extends TapsetParser { * run a dummy stap script to obtain the tapset contents, which will be cached into * memory. Subsequent calls will simply read the saved contents. * @return The string contents of tapsets, or <code>null</code> if there was an - * error in obtaining this information. + * error in obtaining this information, or an empty string if the operation was cancelled. */ synchronized String getTapsetContents() { - if (tapsetContents == null) { - run(null); - } - return tapsetContents; + return tapsetContents != null ? tapsetContents : runAction(); } @Override protected IStatus run(IProgressMonitor monitor) { + return createStatus(verifyRunResult(runAction())); + } + + private String runAction() { String contents = runStap(STAP_OPTIONS, STAP_DUMMYPROBE, false); - int result = verifyRunResult(contents); - if (result != IStatus.OK) { - return createStatus(result); - } - // Exclude the dump of the test script by excluding everything before the second pathname - // (which is the first actual tapset file, not the input script). - int firstTagIndex = contents.indexOf(TAG_FILE); - if (firstTagIndex != -1) { - int beginIndex = contents.indexOf(TAG_FILE, firstTagIndex + 1); - if (beginIndex != -1) { - tapsetContents = contents.substring(beginIndex); + if (verifyRunResult(contents) == IStatus.OK) { + // Exclude the dump of the test script by excluding everything before the second pathname + // (which is the first actual tapset file, not the input script). + int firstTagIndex = contents.indexOf(TAG_FILE); + if (firstTagIndex != -1) { + int beginIndex = contents.indexOf(TAG_FILE, firstTagIndex + 1); + if (beginIndex != -1) { + tapsetContents = contents.substring(beginIndex); + } } + return tapsetContents; } - return createStatus(IStatus.OK); + return contents; } } diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/TapsetParser.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/TapsetParser.java index 29574fab3c..e2307a1598 100644 --- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/TapsetParser.java +++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/TapsetParser.java @@ -14,12 +14,12 @@ package org.eclipse.linuxtools.internal.systemtap.ui.ide.structures; import java.io.File; import java.io.IOException; import java.net.ConnectException; +import java.text.MessageFormat; import java.util.concurrent.atomic.AtomicBoolean; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.jobs.Job; -import org.eclipse.jface.dialogs.MessageDialog; import org.eclipse.jface.preference.IPreferenceStore; import org.eclipse.linuxtools.internal.systemtap.ui.ide.IDEPlugin; import org.eclipse.linuxtools.internal.systemtap.ui.ide.StringOutputStream; @@ -31,10 +31,8 @@ import org.eclipse.linuxtools.systemtap.ui.consolelog.preferences.ConsoleLogPref import org.eclipse.linuxtools.tools.launch.core.factory.LinuxtoolsProcessFactory; import org.eclipse.linuxtools.tools.launch.core.factory.RuntimeProcessFactory; import org.eclipse.swt.SWT; -import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.MessageBox; import org.eclipse.swt.widgets.Shell; -import org.eclipse.ui.IWorkbenchWindow; import org.eclipse.ui.PlatformUI; import org.eclipse.ui.dialogs.PreferencesUtil; @@ -54,7 +52,6 @@ import com.jcraft.jsch.JSchException; */ public abstract class TapsetParser extends Job { - private static AtomicBoolean displayingError = new AtomicBoolean(false); private static AtomicBoolean displayingCredentialDialog = new AtomicBoolean(false); protected TapsetParser(String jobTitle) { @@ -73,7 +70,23 @@ public abstract class TapsetParser extends Job { * Generates a {@link Status} with the provided severity. */ protected IStatus createStatus(int severity) { - return new Status(severity, IDEPlugin.PLUGIN_ID, ""); //$NON-NLS-1$ + String message; + switch (severity) { + case IStatus.ERROR: + if (IDEPlugin.getDefault().getPreferenceStore().getBoolean(IDEPreferenceConstants.P_REMOTE_PROBES)) { + IPreferenceStore p = ConsoleLogPlugin.getDefault().getPreferenceStore(); + message = MessageFormat.format( + Messages.TapsetParser_CannotRunRemoteStapMessage, + p.getString(ConsoleLogPreferenceConstants.SCP_USER), + p.getString(ConsoleLogPreferenceConstants.HOST_NAME)); + } else { + message = Messages.TapsetParser_CannotRunStapMessage; + } + break; + default: + message = ""; //$NON-NLS-1$ + } + return new Status(severity, IDEPlugin.PLUGIN_ID, message); } /** @@ -123,19 +136,17 @@ public abstract class TapsetParser extends Job { } } - try { - if (!remote) { + if (!remote) { + try { return runLocalStap(args, getErrors); - } else { - return runRemoteStapAttempt(args, getErrors); + } catch (InterruptedException e) { + return ""; //$NON-NLS-1$ + } catch (IOException e) { + return null; } - } catch (IOException e) { - displayError(Messages.TapsetParser_ErrorRunningSystemtap, e.getMessage()); - } catch (InterruptedException e) { - return ""; //$NON-NLS-1$ + } else { + return runRemoteStap(args, getErrors); } - - return null; } /** @@ -155,8 +166,9 @@ public abstract class TapsetParser extends Job { private String runLocalStap(String[] args, boolean getErrors) throws IOException, InterruptedException { Process process = RuntimeProcessFactory.getFactory().exec( args, EnvironmentVariablesPreferencePage.getEnvironmentVariables(), null); + // An IOException should be thrown if there's a problem with exec, but to cover possible error + // cases where an exception is not thrown (process is null), return null to signify an error. if (process == null) { - displayError(Messages.TapsetParser_CannotRunStapTitle, Messages.TapsetParser_CannotRunStapMessage); return null; } @@ -177,21 +189,26 @@ public abstract class TapsetParser extends Job { } } - private String runRemoteStapAttempt(String[] args, boolean getErrors) { + private String runRemoteStap(String[] args, boolean getErrors) { int attemptsLeft = 3; while (true) { try { - return runRemoteStap(args, getErrors); + if (Thread.currentThread().isInterrupted()) { + return ""; //$NON-NLS-1$ + } + return runRemoteStapAttempt(args, getErrors); } catch (JSchException e) { if (!(e.getCause() instanceof ConnectException) || --attemptsLeft == 0) { askIfEditCredentials(); - return null; + // Return empty string instead of null to act as "cancel" signal, to + // avoid showing another error dialog on top of the credential edit dialog. + return ""; //$NON-NLS-1$ } } } } - private String runRemoteStap(String[] args, boolean getErrors) throws JSchException { + private String runRemoteStapAttempt(String[] args, boolean getErrors) throws JSchException { StringOutputStream str = new StringOutputStream(); StringOutputStream strErr = new StringOutputStream(); @@ -204,11 +221,9 @@ public abstract class TapsetParser extends Job { Channel channel = LinuxtoolsProcessFactory.execRemoteAndWait(args, str, strErr, user, host, password, port, EnvironmentVariablesPreferencePage.getEnvironmentVariables()); if (channel == null) { - displayError(Messages.TapsetParser_CannotRunStapTitle, Messages.TapsetParser_CannotRunStapMessage); - } else { - channel.getSession().disconnect(); + return null; } - + channel.getSession().disconnect(); return (!getErrors ? str : strErr).toString(); } @@ -231,17 +246,4 @@ public abstract class TapsetParser extends Job { } } - private void displayError(final String title, final String error) { - if (displayingError.compareAndSet(false, true)) { - Display.getDefault().asyncExec(new Runnable() { - @Override - public void run() { - IWorkbenchWindow window = PlatformUI.getWorkbench().getActiveWorkbenchWindow(); - MessageDialog.openWarning(window.getShell(), title, error); - displayingError.set(false); - } - }); - } - } - } diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/TreeTapsetParser.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/TreeTapsetParser.java index 095c867f84..c339b964dd 100644 --- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/TreeTapsetParser.java +++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/TreeTapsetParser.java @@ -95,7 +95,7 @@ public abstract class TreeTapsetParser extends TapsetParser { String tapsetContents = SharedParser.getInstance().getTapsetContents(); result = verifyRunResult(tapsetContents); if (result == IStatus.OK) { - result = addTapsets(tapsetChanges.additions, monitor); + result = addTapsets(tapsetContents, tapsetChanges.additions, monitor); } } } @@ -114,11 +114,13 @@ public abstract class TreeTapsetParser extends TapsetParser { /** * After changing the list of imported tapsets, loads in the tapsets that were added. * Tapset contents are guaranteed to be loaded at the time this method is called. + * @param tapsetContents the set of tapset contents as generated by {@link SharedParser#getTapsetContents()}. + * Included as a parameter to have the caller manage the tapset instead of the callee. * @param additions A non-empty list of added tapset directories. * @param monitor The progress monitor for the operation. * @return An {@link IStatus} severity level for the result of the operation. */ - protected abstract int addTapsets(String[] additions, IProgressMonitor monitor); + protected abstract int addTapsets(String tapsetContents, String[] additions, IProgressMonitor monitor); /** * @return The tree that this parser constructs. diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/messages.properties b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/messages.properties index e5be6a7a6c..171e087383 100644 --- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/messages.properties +++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/messages.properties @@ -16,8 +16,8 @@ ProbeParser_errorInitializingStaticProbes=Could not initialize static probe list ProbeParser_staticProbes=Static Probes ProbeParser_aliasProbes=Probe Aliases ProbeParser_illegalArgMessage=The first-level children of this tree must be two nodes named "Static Probes" and "Probe Alias" respectively. -TapsetParser_CannotRunStapMessage=Make sure SystemTap is installed. -TapsetParser_CannotRunStapTitle=Cannot Run SystemTap +TapsetParser_CannotRunStapMessage=Cannot run SystemTap. Make sure SystemTap is installed. +TapsetParser_CannotRunRemoteStapMessage=Cannot run SystemTap on the remote host {0}@{1}. Make sure SystemTap is installed on the remote machine. TapsetParser_ErrorRunningSystemtap=Error Running SystemTap TapsetParser_RemoteCredentialErrorTitle=Remote Login Error TapsetParser_RemoteCredentialErrorMessage=Unable to login to the remote host for loading tapset contents. Edit the login credentials now? |