DeviceSupport: Add more error output

Previously most errors when opening shells were completely
opaque to the user. This patch adds error output either via
QMessageBox if there is another modal dialog, or as flashing
disrupting messages.

Change-Id: I54be7a90295b61c23c739294c2d1d37c288ad273
Reviewed-by: hjk <hjk@qt.io>
This commit is contained in:
Marcus Tillmanns
2023-10-06 08:51:17 +02:00
parent 113efbfc85
commit 1fabd72514
17 changed files with 185 additions and 104 deletions

View File

@@ -243,7 +243,8 @@ expected_str<QByteArray> DeviceShell::checkCommand(const QByteArray &command)
if (out.contains("<missing>")) {
m_shellScriptState = State::Failed;
m_missingFeatures.append(QString::fromUtf8(command));
return make_unexpected(Tr::tr("Command %1 was not found.").arg(QString::fromUtf8(command)));
return make_unexpected(
Tr::tr("Command \"%1\" was not found.").arg(QString::fromUtf8(command)));
}
return out;

View File

@@ -508,13 +508,16 @@ std::optional<FilePath> FilePath::refersToExecutableFile(MatchScope matchScope)
expected_str<FilePath> FilePath::tmpDir() const
{
if (needsDevice()) {
const Environment env = deviceEnvironment();
if (env.hasKey("TMPDIR"))
return withNewPath(env.value("TMPDIR")).cleanPath();
if (env.hasKey("TEMP"))
return withNewPath(env.value("TEMP")).cleanPath();
if (env.hasKey("TMP"))
return withNewPath(env.value("TMP")).cleanPath();
const expected_str<Environment> env = deviceEnvironmentWithError();
if (!env)
return make_unexpected(env.error());
if (env->hasKey("TMPDIR"))
return withNewPath(env->value("TMPDIR")).cleanPath();
if (env->hasKey("TEMP"))
return withNewPath(env->value("TEMP")).cleanPath();
if (env->hasKey("TMP"))
return withNewPath(env->value("TMP")).cleanPath();
if (osType() != OsTypeWindows)
return withNewPath("/tmp");
@@ -1707,6 +1710,13 @@ FilePaths FilePath::searchAllInPath(const FilePaths &additionalDirs,
}
Environment FilePath::deviceEnvironment() const
{
expected_str<Environment> env = deviceEnvironmentWithError();
QTC_ASSERT_EXPECTED(env, return {});
return *env;
}
expected_str<Environment> FilePath::deviceEnvironmentWithError() const
{
if (needsDevice()) {
QTC_ASSERT(s_deviceHooks.environment, return {});

View File

@@ -164,6 +164,7 @@ public:
[[nodiscard]] FilePath relativeChildPath(const FilePath &parent) const;
[[nodiscard]] FilePath relativePathFrom(const FilePath &anchor) const;
[[nodiscard]] Environment deviceEnvironment() const;
[[nodiscard]] expected_str<Environment> deviceEnvironmentWithError() const;
[[nodiscard]] FilePaths devicePathEnvironmentVariable() const;
[[nodiscard]] FilePath withNewPath(const QString &newPath) const;
[[nodiscard]] FilePath withNewMappedPath(const FilePath &newPath) const;
@@ -301,7 +302,7 @@ public:
std::function<expected_str<DeviceFileAccess *>(const FilePath &)> fileAccess;
std::function<QString(const FilePath &)> deviceDisplayName;
std::function<bool(const FilePath &, const FilePath &)> ensureReachable;
std::function<Environment(const FilePath &)> environment;
std::function<expected_str<Environment>(const FilePath &)> environment;
std::function<bool(const FilePath &left, const FilePath &right)> isSameDevice;
std::function<expected_str<FilePath>(const FilePath &)> localSource;
std::function<void(const FilePath &, const Environment &)> openTerminal;

View File

@@ -6,27 +6,28 @@
#include "externalterminalprocessimpl.h"
#include "filepath.h"
#include "process.h"
#include "utilstr.h"
#include <QMutex>
namespace Utils::Terminal {
FilePath defaultShellForDevice(const FilePath &deviceRoot)
expected_str<FilePath> defaultShellForDevice(const FilePath &deviceRoot)
{
if (deviceRoot.osType() == OsTypeWindows)
return deviceRoot.withNewPath("cmd.exe").searchInPath();
const Environment env = deviceRoot.deviceEnvironment();
if (!env.hasChanges())
return {};
const expected_str<Environment> env = deviceRoot.deviceEnvironmentWithError();
if (!env)
return make_unexpected(env.error());
FilePath shell = FilePath::fromUserInput(env.value_or("SHELL", "/bin/sh"));
FilePath shell = FilePath::fromUserInput(env->value_or("SHELL", "/bin/sh"));
if (!shell.isAbsolutePath())
shell = env.searchInPath(shell.nativePath());
shell = env->searchInPath(shell.nativePath());
if (shell.isEmpty())
return shell;
return make_unexpected(Tr::tr("Could not find any shell"));
return deviceRoot.withNewMappedPath(shell);
}

View File

@@ -71,7 +71,7 @@ struct NameAndCommandLine
CommandLine commandLine;
};
QTCREATOR_UTILS_EXPORT FilePath defaultShellForDevice(const FilePath &deviceRoot);
QTCREATOR_UTILS_EXPORT expected_str<FilePath> defaultShellForDevice(const FilePath &deviceRoot);
class QTCREATOR_UTILS_EXPORT Hooks
{

View File

@@ -65,6 +65,7 @@
#include <QHeaderView>
#include <QHostAddress>
#include <QLoggingCategory>
#include <QMessageBox>
#include <QNetworkInterface>
#include <QPushButton>
#include <QRandomGenerator>
@@ -302,7 +303,7 @@ public:
RunResult runInShell(const CommandLine &cmd, const QByteArray &stdInData = {});
bool updateContainerAccess();
expected_str<void> updateContainerAccess();
void changeMounts(QStringList newMounts);
bool ensureReachable(const FilePath &other);
void shutdown();
@@ -314,7 +315,7 @@ public:
QString repoAndTagEncoded() const { return deviceSettings->repoAndTagEncoded(); }
QString dockerImageId() const { return deviceSettings->imageId(); }
Environment environment();
expected_str<Environment> environment();
CommandLine withDockerExecCmd(const CommandLine &cmd,
const std::optional<Environment> &env = std::nullopt,
@@ -329,7 +330,7 @@ public:
expected_str<QString> createContainer();
expected_str<void> startContainer();
void stopCurrentContainer();
void fetchSystemEnviroment();
expected_str<void> fetchSystemEnviroment();
std::optional<FilePath> clangdExecutable() const
{
@@ -587,31 +588,42 @@ DockerDevice::DockerDevice(std::unique_ptr<DockerDeviceSettings> deviceSettings)
setMachineType(IDevice::Hardware);
setAllowEmptyCommand(true);
setOpenTerminal([this](const Environment &env, const FilePath &workingDir) {
setOpenTerminal([this](const Environment &env,
const FilePath &workingDir) -> expected_str<void> {
Q_UNUSED(env); // TODO: That's the runnable's environment in general. Use it via -e below.
if (!updateContainerAccess())
return;
if (d->containerId().isEmpty()) {
MessageManager::writeDisrupting(Tr::tr("Error starting remote shell. No container."));
return;
}
expected_str<void> result = d->updateContainerAccess();
if (!result)
return result;
if (d->containerId().isEmpty())
return make_unexpected(Tr::tr("Error starting remote shell. No container."));
expected_str<FilePath> shell = Terminal::defaultShellForDevice(rootPath());
if (!shell)
return make_unexpected(shell.error());
Process proc;
proc.setTerminalMode(TerminalMode::Detached);
proc.setEnvironment(env);
proc.setWorkingDirectory(workingDir);
proc.setCommand({Terminal::defaultShellForDevice(rootPath()), {}});
proc.setCommand({*shell, {}});
proc.start();
if (proc.error() != QProcess::UnknownError && MessageManager::instance()) {
MessageManager::writeDisrupting(
Tr::tr("Error starting remote shell: %1").arg(proc.errorString()));
}
return {};
});
addDeviceAction({Tr::tr("Open Shell in Container"), [](const IDevice::Ptr &device, QWidget *) {
device->openTerminal(device->systemEnvironment(), FilePath());
addDeviceAction(
{Tr::tr("Open Shell in Container"), [](const IDevice::Ptr &device, QWidget *) {
expected_str<Environment> env = device->systemEnvironmentWithError();
if (!env) {
QMessageBox::warning(ICore::dialogParent(), Tr::tr("Error"), env.error());
return;
}
expected_str<void> result = device->openTerminal(*env, FilePath());
if (!result)
QMessageBox::warning(ICore::dialogParent(), Tr::tr("Error"), result.error());
}});
}
@@ -625,7 +637,7 @@ void DockerDevice::shutdown()
d->shutdown();
}
bool DockerDevice::updateContainerAccess() const
expected_str<void> DockerDevice::updateContainerAccess() const
{
return d->updateContainerAccess();
}
@@ -909,10 +921,10 @@ expected_str<void> DockerDevicePrivate::startContainer()
return m_shell->start();
}
bool DockerDevicePrivate::updateContainerAccess()
expected_str<void> DockerDevicePrivate::updateContainerAccess()
{
if (QThread::currentThread() != thread()) {
bool result = false;
expected_str<void> result;
QMetaObject::invokeMethod(this,
&DockerDevicePrivate::updateContainerAccess,
Qt::BlockingQueuedConnection,
@@ -921,23 +933,23 @@ bool DockerDevicePrivate::updateContainerAccess()
}
if (m_isShutdown)
return false;
return make_unexpected(Tr::tr("Device is shutdown"));
if (DockerApi::isDockerDaemonAvailable(false).value_or(false) == false)
return false;
return make_unexpected(Tr::tr("Docker system is not reachable"));
if (m_shell)
return true;
if (m_shell && m_shell->state() == DeviceShell::State::Succeeded)
return {};
auto result = startContainer();
expected_str<void> result = startContainer();
if (result) {
deviceSettings->containerStatus.setText(Tr::tr("Running"));
return true;
return result;
}
qCWarning(dockerDeviceLog) << "Failed to start container:" << result.error();
deviceSettings->containerStatus.setText(result.error());
return false;
const QString error = QString("Failed to start container: %1").arg(result.error());
deviceSettings->containerStatus.setText(result.error().trimmed());
return make_unexpected(error);
}
void DockerDevice::setMounts(const QStringList &mounts) const
@@ -1025,7 +1037,7 @@ expected_str<FilePath> DockerDevice::localSource(const FilePath &other) const
return d->localSource(other);
}
Environment DockerDevice::systemEnvironment() const
expected_str<Environment> DockerDevice::systemEnvironmentWithError() const
{
return d->environment();
}
@@ -1036,18 +1048,20 @@ void DockerDevice::aboutToBeRemoved() const
detector.undoAutoDetect(id().toString());
}
void DockerDevicePrivate::fetchSystemEnviroment()
expected_str<void> DockerDevicePrivate::fetchSystemEnviroment()
{
if (!updateContainerAccess())
return;
expected_str<void> result = updateContainerAccess();
if (!result)
return result;
QString stdErr;
if (m_shell && m_shell->state() == DeviceShell::State::Succeeded) {
const RunResult result = runInShell({"env", {}});
const QString out = QString::fromUtf8(result.stdOut);
m_cachedEnviroment = Environment(out.split('\n', Qt::SkipEmptyParts), q->osType());
return;
}
stdErr = QString::fromUtf8(result.stdErr);
} else {
Process proc;
proc.setCommand(withDockerExecCmd({"env", {}}));
proc.start();
@@ -1055,10 +1069,13 @@ void DockerDevicePrivate::fetchSystemEnviroment()
const QString remoteOutput = proc.cleanedStdOut();
m_cachedEnviroment = Environment(remoteOutput.split('\n', Qt::SkipEmptyParts), q->osType());
stdErr = proc.cleanedStdErr();
}
const QString remoteError = proc.cleanedStdErr();
if (!remoteError.isEmpty())
qCWarning(dockerDeviceLog) << "Cannot read container environment:", qPrintable(remoteError);
if (stdErr.isEmpty())
return {};
return make_unexpected("Could not read container environment: " + stdErr);
}
RunResult DockerDevicePrivate::runInShell(const CommandLine &cmd, const QByteArray &stdInData)
@@ -1313,10 +1330,13 @@ bool DockerDevicePrivate::addTemporaryMount(const FilePath &path, const FilePath
return true;
}
Environment DockerDevicePrivate::environment()
expected_str<Environment> DockerDevicePrivate::environment()
{
if (!m_cachedEnviroment)
fetchSystemEnviroment();
if (!m_cachedEnviroment) {
expected_str<void> result = fetchSystemEnviroment();
if (!result)
return make_unexpected(result.error());
}
QTC_ASSERT(m_cachedEnviroment, return {});
return m_cachedEnviroment.value();

View File

@@ -74,9 +74,9 @@ public:
bool ensureReachable(const Utils::FilePath &other) const override;
Utils::expected_str<Utils::FilePath> localSource(const Utils::FilePath &other) const override;
Utils::Environment systemEnvironment() const override;
Utils::expected_str<Utils::Environment> systemEnvironmentWithError() const override;
bool updateContainerAccess() const;
Utils::expected_str<void> updateContainerAccess() const;
void setMounts(const QStringList &mounts) const;
bool prepareForBuild(const ProjectExplorer::Target *target) override;

View File

@@ -106,7 +106,13 @@ DockerDeviceWidget::DockerDeviceWidget(const IDevice::Ptr &device)
this,
[this, logView, dockerDevice, searchPaths, deviceSettings] {
logView->clear();
dockerDevice->updateContainerAccess();
expected_str<void> startResult = dockerDevice->updateContainerAccess();
if (!startResult) {
logView->append(Tr::tr("Failed to start container."));
logView->append(startResult.error());
return;
}
const FilePath clangdPath
= dockerDevice->filePath("clangd")

View File

@@ -55,17 +55,21 @@ DesktopDevice::DesktopDevice()
= QString::fromLatin1("%1-%2").arg(DESKTOP_PORT_START).arg(DESKTOP_PORT_END);
setFreePorts(Utils::PortList::fromString(portRange));
setOpenTerminal([](const Environment &env, const FilePath &path) {
setOpenTerminal([](const Environment &env, const FilePath &path) -> expected_str<void> {
const Environment realEnv = env.hasChanges() ? env : Environment::systemEnvironment();
const FilePath shell = Terminal::defaultShellForDevice(path);
const expected_str<FilePath> shell = Terminal::defaultShellForDevice(path);
if (!shell)
return make_unexpected(shell.error());
Process process;
process.setTerminalMode(TerminalMode::Detached);
process.setEnvironment(realEnv);
process.setCommand({shell, {}});
process.setCommand({*shell, {}});
process.setWorkingDirectory(path);
process.start();
return {};
});
}
@@ -117,7 +121,7 @@ FilePath DesktopDevice::filePath(const QString &pathOnDevice) const
return FilePath::fromParts({}, {}, pathOnDevice);
}
Environment DesktopDevice::systemEnvironment() const
expected_str<Environment> DesktopDevice::systemEnvironmentWithError() const
{
return Environment::systemEnvironment();
}

View File

@@ -31,7 +31,7 @@ public:
bool usableAsBuildDevice() const override;
bool handlesFile(const Utils::FilePath &filePath) const override;
Utils::Environment systemEnvironment() const override;
Utils::expected_str<Utils::Environment> systemEnvironmentWithError() const override;
Utils::FilePath rootPath() const override;
Utils::FilePath filePath(const QString &pathOnDevice) const override;

View File

@@ -425,10 +425,13 @@ DeviceManager::DeviceManager(bool isInstance) : d(std::make_unique<DeviceManager
return device->fileAccess();
};
deviceHooks.environment = [](const FilePath &filePath) {
deviceHooks.environment = [](const FilePath &filePath) -> expected_str<Environment> {
auto device = DeviceManager::deviceForPath(filePath);
QTC_ASSERT(device, qDebug() << filePath.toString(); return Environment{});
return device->systemEnvironment();
if (!device) {
return make_unexpected(
Tr::tr("No device found for path \"%1\"").arg(filePath.toUserOutput()));
}
return device->systemEnvironmentWithError();
};
deviceHooks.deviceDisplayName = [](const FilePath &filePath) {

View File

@@ -226,10 +226,11 @@ bool IDevice::canOpenTerminal() const
return bool(d->openTerminal);
}
void IDevice::openTerminal(const Environment &env, const FilePath &workingDir) const
expected_str<void> IDevice::openTerminal(const Environment &env, const FilePath &workingDir) const
{
QTC_ASSERT(canOpenTerminal(), return);
d->openTerminal(env, workingDir);
QTC_ASSERT(canOpenTerminal(),
return make_unexpected(Tr::tr("Opening a terminal is not supported.")));
return d->openTerminal(env, workingDir);
}
bool IDevice::isEmptyCommandAllowed() const
@@ -302,6 +303,13 @@ FileTransferInterface *IDevice::createFileTransferInterface(
}
Environment IDevice::systemEnvironment() const
{
expected_str<Environment> env = systemEnvironmentWithError();
QTC_ASSERT_EXPECTED(env, return {});
return *env;
}
expected_str<Environment> IDevice::systemEnvironmentWithError() const
{
DeviceFileAccess *access = fileAccess();
QTC_ASSERT(access, return Environment::systemEnvironment());

View File

@@ -192,7 +192,8 @@ public:
void setupId(Origin origin, Utils::Id id = Utils::Id());
bool canOpenTerminal() const;
void openTerminal(const Utils::Environment &env, const Utils::FilePath &workingDir) const;
Utils::expected_str<void> openTerminal(const Utils::Environment &env,
const Utils::FilePath &workingDir) const;
bool isEmptyCommandAllowed() const;
void setAllowEmptyCommand(bool allow);
@@ -212,7 +213,9 @@ public:
virtual Utils::ProcessInterface *createProcessInterface() const;
virtual FileTransferInterface *createFileTransferInterface(
const FileTransferSetupData &setup) const;
virtual Utils::Environment systemEnvironment() const;
Utils::Environment systemEnvironment() const;
virtual Utils::expected_str<Utils::Environment> systemEnvironmentWithError() const;
virtual void aboutToBeRemoved() const {}
@@ -230,7 +233,8 @@ protected:
virtual void fromMap(const Utils::Store &map);
virtual Utils::Store toMap() const;
using OpenTerminal = std::function<void(const Utils::Environment &, const Utils::FilePath &)>;
using OpenTerminal = std::function<Utils::expected_str<void>(const Utils::Environment &,
const Utils::FilePath &)>;
void setOpenTerminal(const OpenTerminal &openTerminal);
void setDisplayType(const QString &type);
void setOsType(Utils::OsType osType);

View File

@@ -104,6 +104,7 @@
#include <coreplugin/imode.h>
#include <coreplugin/iversioncontrol.h>
#include <coreplugin/locator/directoryfilter.h>
#include <coreplugin/messagemanager.h>
#include <coreplugin/minisplitter.h>
#include <coreplugin/modemanager.h>
#include <coreplugin/navigationwidget.h>
@@ -3641,13 +3642,18 @@ void ProjectExplorerPluginPrivate::openTerminalHere(const EnvironmentGetter &env
&& !buildDevice->ensureReachable(workingDir))
workingDir.clear();
const FilePath shell = Terminal::defaultShellForDevice(buildDevice->rootPath());
const expected_str<FilePath> shell = Terminal::defaultShellForDevice(buildDevice->rootPath());
if (!shell.isEmpty() && buildDevice->rootPath().needsDevice()) {
Terminal::Hooks::instance().openTerminal({CommandLine{shell, {}}, workingDir, environment});
} else {
Terminal::Hooks::instance().openTerminal({workingDir, environment});
if (!shell) {
Core::MessageManager::writeDisrupting(
Tr::tr("Failed opening terminal.\n%1").arg(shell.error()));
return;
}
if (buildDevice->rootPath().needsDevice())
Terminal::Hooks::instance().openTerminal({CommandLine{*shell, {}}, workingDir, environment});
else
Terminal::Hooks::instance().openTerminal({workingDir, environment});
}
void ProjectExplorerPluginPrivate::openTerminalHereWithRunEnv()
@@ -3676,10 +3682,17 @@ void ProjectExplorerPluginPrivate::openTerminalHereWithRunEnv()
if (!device->filePath(workingDir.path()).exists() && !device->ensureReachable(workingDir))
workingDir.clear();
const FilePath shell = Terminal::defaultShellForDevice(device->rootPath());
if (!shell.isEmpty() && device->rootPath().needsDevice()) {
const expected_str<FilePath> shell = Terminal::defaultShellForDevice(device->rootPath());
if (!shell) {
Core::MessageManager::writeDisrupting(
Tr::tr("Failed opening terminal.\n%1").arg(shell.error()));
return;
}
if (device->rootPath().needsDevice()) {
Terminal::Hooks::instance().openTerminal(
{CommandLine{shell, {}}, workingDir, runnable.environment});
{CommandLine{*shell, {}}, workingDir, runnable.environment});
} else {
Terminal::Hooks::instance().openTerminal({workingDir, runnable.environment});
}

View File

@@ -37,6 +37,7 @@
#include <QDateTime>
#include <QLoggingCategory>
#include <QMessageBox>
#include <QMutex>
#include <QReadWriteLock>
#include <QRegularExpression>
@@ -971,7 +972,8 @@ LinuxDevice::LinuxDevice()
}
}});
setOpenTerminal([this](const Environment &env, const FilePath &workingDir) {
setOpenTerminal([this](const Environment &env,
const FilePath &workingDir) -> expected_str<void> {
Process proc;
// If we will not set any environment variables, we can leave out the shell executable
@@ -985,10 +987,15 @@ LinuxDevice::LinuxDevice()
proc.setEnvironment(env);
proc.setWorkingDirectory(workingDir);
proc.start();
return {};
});
addDeviceAction({Tr::tr("Open Remote Shell"), [](const IDevice::Ptr &device, QWidget *) {
device->openTerminal(Environment(), FilePath());
expected_str<void> result = device->openTerminal(Environment(), FilePath());
if (!result)
QMessageBox::warning(nullptr, Tr::tr("Error"), result.error());
}});
}

View File

@@ -87,25 +87,28 @@ void TerminalWidget::setupPty()
if (shellCommand.executable().isRootPath()) {
writeToTerminal(Tr::tr("Connecting ...\r\n").toUtf8(), true);
// We still have to find the shell to start ...
m_findShellWatcher.reset(new QFutureWatcher<FilePath>());
m_findShellWatcher.reset(new QFutureWatcher<expected_str<FilePath>>());
connect(m_findShellWatcher.get(), &QFutureWatcher<FilePath>::finished, this, [this] {
const FilePath result = m_findShellWatcher->result();
if (!result.isEmpty()) {
m_openParameters.shellCommand->setExecutable(m_findShellWatcher->result());
const expected_str<FilePath> result = m_findShellWatcher->result();
if (result) {
m_openParameters.shellCommand->setExecutable(*result);
restart(m_openParameters);
return;
}
writeToTerminal(
("\r\n\033[31m" + Tr::tr("Could not find shell to start.") + "\r\n").toUtf8(), true);
writeToTerminal(("\r\n\033[31m"
+ Tr::tr("Failed to start shell: %1").arg(result.error()) + "\r\n")
.toUtf8(),
true);
});
m_findShellWatcher->setFuture(Utils::asyncRun([shellCommand] {
const FilePath result = Utils::Terminal::defaultShellForDevice(
m_findShellWatcher->setFuture(Utils::asyncRun([shellCommand]() -> expected_str<FilePath> {
const expected_str<FilePath> result = Utils::Terminal::defaultShellForDevice(
shellCommand.executable());
if (result.isExecutableFile())
if (result && !result->isExecutableFile())
return make_unexpected(
Tr::tr("'%1' is not executable.").arg(result->toUserOutput()));
return result;
return FilePath{};
}));
return;

View File

@@ -110,7 +110,7 @@ private:
Internal::ShortcutMap m_shortcutMap;
std::unique_ptr<QFutureWatcher<Utils::FilePath>> m_findShellWatcher;
std::unique_ptr<QFutureWatcher<Utils::expected_str<Utils::FilePath>>> m_findShellWatcher;
};
} // namespace Terminal