Commit 1e362b0f authored by Oswald Buddenhagen's avatar Oswald Buddenhagen

overhaul process argument handling

get away from argument stringlists. instead, use native shell command
lines which support quoting/splitting, environment variable expansion
and redirections with well-understood semantics.

Task-number: QTCREATORBUG-542
Task-number: QTCREATORBUG-1564
parent 531c70f0
......@@ -32,6 +32,8 @@
#include "utils_global.h"
#include "environment.h"
#include <QtCore/QStringList>
namespace Utils {
......@@ -45,10 +47,10 @@ public:
QString workingDirectory() const { return m_workingDir; }
void setWorkingDirectory(const QString &dir) { m_workingDir = dir; }
QStringList environment() const { return m_environment; }
void setEnvironment(const QStringList &env) { m_environment = env; }
void setEnvironment(const Environment &env) { m_environment = env; }
Environment environment() const { return m_environment; }
virtual bool start(const QString &program, const QStringList &args) = 0;
virtual bool start(const QString &program, const QString &args) = 0;
virtual void stop() = 0;
virtual bool isRunning() const = 0;
......@@ -63,14 +65,15 @@ public:
static QStringList fixWinEnvironment(const QStringList &env);
// Quote a Windows command line correctly for the "CreateProcess" API
static QString createWinCommandline(const QString &program, const QStringList &args);
static QString createWinCommandline(const QString &program, const QString &args);
// Create a bytearray suitable to be passed on as environment
// to the "CreateProcess" API (0-terminated UTF 16 strings).
static QByteArray createWinEnvironment(const QStringList &env);
#endif
private:
protected:
QString m_workingDir;
QStringList m_environment;
Environment m_environment;
};
} //namespace Utils
......
......@@ -51,49 +51,57 @@ QStringList AbstractProcess::fixWinEnvironment(const QStringList &env)
return envStrings;
}
QString AbstractProcess::createWinCommandline(const QString &program, const QStringList &args)
static QString quoteWinCommand(const QString &program)
{
const QChar doubleQuote = QLatin1Char('"');
const QChar blank = QLatin1Char(' ');
const QChar backSlash = QLatin1Char('\\');
// add the programm as the first arg ... it works better
QString programName = program;
if (!programName.startsWith(doubleQuote) && !programName.endsWith(doubleQuote) && programName.contains(blank)) {
programName.insert(0, doubleQuote);
programName.replace(QLatin1Char('/'), QLatin1Char('\\'));
if (!programName.startsWith(doubleQuote) && !programName.endsWith(doubleQuote)
&& programName.contains(QLatin1Char(' '))) {
programName.prepend(doubleQuote);
programName.append(doubleQuote);
}
// add the prgram as the first arrg ... it works better
programName.replace(QLatin1Char('/'), backSlash);
QString cmdLine = programName;
if (args.empty())
return cmdLine;
return programName;
}
static QString quoteWinArgument(const QString &arg)
{
if (!arg.length())
return QString::fromLatin1("\"\"");
QString ret(arg);
// Quotes are escaped and their preceding backslashes are doubled.
ret.replace(QRegExp(QLatin1String("(\\\\*)\"")), QLatin1String("\\1\\1\\\""));
if (ret.contains(QRegExp(QLatin1String("\\s")))) {
// The argument must not end with a \ since this would be interpreted
// as escaping the quote -- rather put the \ behind the quote: e.g.
// rather use "foo"\ than "foo\"
ret.replace(QRegExp(QLatin1String("(\\\\*)$")), QLatin1String("\"\\1"));
ret.prepend(QLatin1Char('"'));
}
return ret;
}
cmdLine += blank;
for (int i = 0; i < args.size(); ++i) {
QString tmp = args.at(i);
// in the case of \" already being in the string the \ must also be escaped
tmp.replace(QLatin1String("\\\""), QLatin1String("\\\\\""));
// escape a single " because the arguments will be parsed
tmp.replace(QString(doubleQuote), QLatin1String("\\\""));
if (tmp.isEmpty() || tmp.contains(blank) || tmp.contains('\t')) {
// The argument must not end with a \ since this would be interpreted
// as escaping the quote -- rather put the \ behind the quote: e.g.
// rather use "foo"\ than "foo\"
QString endQuote(doubleQuote);
int i = tmp.length();
while (i > 0 && tmp.at(i - 1) == backSlash) {
--i;
endQuote += backSlash;
}
cmdLine += QLatin1String(" \"");
cmdLine += tmp.left(i);
cmdLine += endQuote;
} else {
cmdLine += blank;
cmdLine += tmp;
}
QString AbstractProcess::createWinCommandline(const QString &program, const QStringList &args)
{
QString programName = quoteWinCommand(program);
foreach (const QString &arg, args) {
programName += QLatin1Char(' ');
programName += quoteWinArgument(arg);
}
return programName;
}
QString AbstractProcess::createWinCommandline(const QString &program, const QString &args)
{
QString programName = quoteWinCommand(program);
if (!args.isEmpty()) {
programName += QLatin1Char(' ');
programName += args;
}
return cmdLine;
return programName;
}
QByteArray AbstractProcess::createWinEnvironment(const QStringList &env)
......
......@@ -54,7 +54,7 @@ public:
ConsoleProcess(QObject *parent = 0);
~ConsoleProcess();
bool start(const QString &program, const QStringList &args);
bool start(const QString &program, const QString &args);
void stop();
void setMode(Mode m);
......
......@@ -29,6 +29,9 @@
#include "consoleprocess.h"
#include "environment.h"
#include "qtcprocess.h"
#include <QtCore/QCoreApplication>
#include <QtCore/QDir>
#include <QtCore/QSettings>
......@@ -66,6 +69,7 @@ ConsoleProcessPrivate::ConsoleProcessPrivate() :
m_mode(ConsoleProcess::Run),
m_appPid(0),
m_stubSocket(0),
m_tempFile(0),
m_settings(0)
{
}
......@@ -114,18 +118,49 @@ void ConsoleProcess::setSettings(QSettings *settings)
d->m_settings = settings;
}
bool ConsoleProcess::start(const QString &program, const QStringList &args)
bool ConsoleProcess::start(const QString &program, const QString &args)
{
if (isRunning())
return false;
QtcProcess::SplitError perr;
QStringList pargs = QtcProcess::prepareArgs(args, &perr, &m_environment, &m_workingDir);
QString pcmd;
if (perr == QtcProcess::SplitOk) {
pcmd = program;
} else {
if (perr != QtcProcess::FoundMeta) {
emit processMessage(tr("Quoting error in command."), true);
return false;
}
if (d->m_mode == Debug) {
// FIXME: QTCREATORBUG-2809
emit processMessage(tr("Debugging complex shell commands in a terminal"
" is currently not supported."), true);
return false;
}
pcmd = QLatin1String("/bin/sh");
pargs << QLatin1String("-c") << (QtcProcess::quoteArg(program) + QLatin1Char(' ') + args);
}
QtcProcess::SplitError qerr;
QStringList xtermArgs = QtcProcess::prepareArgs(terminalEmulator(d->m_settings), &qerr,
&m_environment, &m_workingDir);
if (qerr != QtcProcess::SplitOk) {
emit processMessage(qerr == QtcProcess::BadQuoting
? tr("Quoting error in terminal command.")
: tr("Terminal command may not be a shell command."), true);
return false;
}
const QString err = stubServerListen();
if (!err.isEmpty()) {
emit processMessage(msgCommChannelFailed(err), true);
return false;
}
if (!environment().isEmpty()) {
QStringList env = m_environment.toStringList();
if (!env.isEmpty()) {
d->m_tempFile = new QTemporaryFile();
if (!d->m_tempFile->open()) {
stubServerShutdown();
......@@ -134,14 +169,13 @@ bool ConsoleProcess::start(const QString &program, const QStringList &args)
d->m_tempFile = 0;
return false;
}
foreach (const QString &var, environment()) {
foreach (const QString &var, env) {
d->m_tempFile->write(var.toLocal8Bit());
d->m_tempFile->write("", 1);
}
d->m_tempFile->flush();
}
QStringList xtermArgs = terminalEmulator(d->m_settings).split(QLatin1Char(' ')); // FIXME: quoting
xtermArgs
#ifdef Q_OS_MAC
<< (QCoreApplication::applicationDirPath() + QLatin1String("/../Resources/qtcreator_process_stub"))
......@@ -153,7 +187,7 @@ bool ConsoleProcess::start(const QString &program, const QStringList &args)
<< msgPromptToClose()
<< workingDirectory()
<< (d->m_tempFile ? d->m_tempFile->fileName() : QString())
<< program << args;
<< pcmd << pargs;
QString xterm = xtermArgs.takeFirst();
d->m_process.start(xterm, xtermArgs);
......
......@@ -28,6 +28,8 @@
**************************************************************************/
#include "consoleprocess.h"
#include "environment.h"
#include "qtcprocess.h"
#include "winutils.h"
#include <windows.h>
......@@ -115,18 +117,28 @@ QProcess::ExitStatus ConsoleProcess::exitStatus() const
return d->m_appStatus;
}
bool ConsoleProcess::start(const QString &program, const QStringList &args)
bool ConsoleProcess::start(const QString &program, const QString &args)
{
if (isRunning())
return false;
QString pcmd;
QString pargs;
if (d->m_mode != Run) { // The debugger engines already pre-process the arguments.
pcmd = program;
pargs = args;
} else {
QtcProcess::prepareCommand(program, args, &pcmd, &pargs, &m_environment, &m_workingDir);
}
const QString err = stubServerListen();
if (!err.isEmpty()) {
emit processMessage(msgCommChannelFailed(err), true);
return false;
}
if (!environment().isEmpty()) {
QStringList env = m_environment.toStringList();
if (!env.isEmpty()) {
d->m_tempFile = new QTemporaryFile();
if (!d->m_tempFile->open()) {
stubServerShutdown();
......@@ -138,7 +150,7 @@ bool ConsoleProcess::start(const QString &program, const QStringList &args)
QTextStream out(d->m_tempFile);
out.setCodec("UTF-16LE");
out.setGenerateByteOrderMark(false);
foreach (const QString &var, fixWinEnvironment(environment()))
foreach (const QString &var, fixWinEnvironment(env))
out << var << QChar(0);
out << QChar(0);
}
......@@ -159,7 +171,7 @@ bool ConsoleProcess::start(const QString &program, const QStringList &args)
<< d->m_stubServer.fullServerName()
<< workDir
<< (d->m_tempFile ? d->m_tempFile->fileName() : 0)
<< createWinCommandline(program, args)
<< createWinCommandline(pcmd, pargs)
<< msgPromptToClose();
const QString cmdLine = createWinCommandline(
......
......@@ -332,62 +332,6 @@ bool Environment::operator==(const Environment &other) const
return m_values == other.m_values;
}
QStringList Environment::parseCombinedArgString(const QString &program)
{
QStringList args;
QString tmp;
int quoteCount = 0;
bool inQuote = false;
// handle quoting. tokens can be surrounded by double quotes
// "hello world". three consecutive double quotes represent
// the quote character itself.
for (int i = 0; i < program.size(); ++i) {
if (program.at(i) == QLatin1Char('"')) {
++quoteCount;
if (quoteCount == 3) {
// third consecutive quote
quoteCount = 0;
tmp += program.at(i);
}
continue;
}
if (quoteCount) {
if (quoteCount == 1)
inQuote = !inQuote;
quoteCount = 0;
}
if (!inQuote && program.at(i).isSpace()) {
if (!tmp.isEmpty()) {
args += tmp;
tmp.clear();
}
} else {
tmp += program.at(i);
}
}
if (!tmp.isEmpty())
args += tmp;
return args;
}
QString Environment::joinArgumentList(const QStringList &arguments)
{
QString result;
const QChar doubleQuote = QLatin1Char('"');
foreach (QString arg, arguments) {
if (!result.isEmpty())
result += QLatin1Char(' ');
arg.replace(QString(doubleQuote), QLatin1String("\"\"\""));
if (arg.contains(QLatin1Char(' '))) {
arg.insert(0, doubleQuote);
arg += doubleQuote;
}
result += arg;
}
return result;
}
/** Expand environment variables in a string.
*
* Environment variables are accepted in the following forms:
......
......@@ -94,9 +94,6 @@ public:
const QStringList & additionalDirs = QStringList()) const;
QStringList path() const;
static QStringList parseCombinedArgString(const QString &program);
static QString joinArgumentList(const QStringList &arguments);
QString expandVariables(const QString &) const;
QStringList expandVariables(const QStringList &) const;
......
......@@ -243,7 +243,7 @@ CMakeBuildConfiguration *CMakeBuildConfigurationFactory::create(ProjectExplorer:
MakeStep *cleanMakeStep = new MakeStep(cleanSteps);
cleanSteps->insertStep(0, cleanMakeStep);
cleanMakeStep->setAdditionalArguments(QStringList() << "clean");
cleanMakeStep->setAdditionalArguments("clean");
cleanMakeStep->setClean(true);
CMakeOpenProjectWizard copw(cmtarget->cmakeProject()->projectManager(),
......
......@@ -202,12 +202,12 @@ void CMakeOpenProjectWizard::setMsvcVersion(const QString &version)
m_msvcVersion = version;
}
QStringList CMakeOpenProjectWizard::arguments() const
QString CMakeOpenProjectWizard::arguments() const
{
return m_arguments;
}
void CMakeOpenProjectWizard::setArguments(const QStringList &args)
void CMakeOpenProjectWizard::setArguments(const QString &args)
{
m_arguments = args;
}
......@@ -426,7 +426,6 @@ void CMakeRunPage::runCMake()
{
m_runCMake->setEnabled(false);
m_argumentsLineEdit->setEnabled(false);
QStringList arguments = Utils::Environment::parseCombinedArgString(m_argumentsLineEdit->text());
CMakeManager *cmakeManager = m_cmakeWizard->cmakeManager();
#ifdef Q_OS_WIN
......@@ -464,11 +463,11 @@ void CMakeRunPage::runCMake()
m_output->clear();
if (m_cmakeWizard->cmakeManager()->isCMakeExecutableValid()) {
m_cmakeProcess = new QProcess();
m_cmakeProcess = new Utils::QtcProcess();
connect(m_cmakeProcess, SIGNAL(readyReadStandardOutput()), this, SLOT(cmakeReadyReadStandardOutput()));
connect(m_cmakeProcess, SIGNAL(readyReadStandardError()), this, SLOT(cmakeReadyReadStandardError()));
connect(m_cmakeProcess, SIGNAL(finished(int)), this, SLOT(cmakeFinished()));
cmakeManager->createXmlFile(m_cmakeProcess, arguments, m_cmakeWizard->sourceDirectory(), m_buildDirectory, env, generator);
cmakeManager->createXmlFile(m_cmakeProcess, m_argumentsLineEdit->text(), m_cmakeWizard->sourceDirectory(), m_buildDirectory, env, generator);
} else {
m_runCMake->setEnabled(true);
m_argumentsLineEdit->setEnabled(true);
......@@ -522,7 +521,7 @@ void CMakeRunPage::cmakeFinished()
}
m_cmakeProcess->deleteLater();
m_cmakeProcess = 0;
m_cmakeWizard->setArguments(Utils::Environment::parseCombinedArgString(m_argumentsLineEdit->text()));
m_cmakeWizard->setArguments(m_argumentsLineEdit->text());
//TODO Actually test that running cmake was finished, for setting this bool
emit completeChanged();
}
......
......@@ -32,8 +32,8 @@
#include <utils/environment.h>
#include <utils/wizard.h>
#include <utils/qtcprocess.h>
#include <QtCore/QProcess>
#include <QtGui/QPushButton>
#include <QtGui/QComboBox>
#include <QtGui/QLineEdit>
......@@ -81,8 +81,8 @@ public:
QString sourceDirectory() const;
void setBuildDirectory(const QString &directory);
CMakeManager *cmakeManager() const;
QStringList arguments() const;
void setArguments(const QStringList &args);
QString arguments() const;
void setArguments(const QString &args);
Utils::Environment environment() const;
QString msvcVersion() const;
void setMsvcVersion(const QString &version);
......@@ -93,7 +93,7 @@ private:
CMakeManager *m_cmakeManager;
QString m_buildDirectory;
QString m_sourceDirectory;
QStringList m_arguments;
QString m_arguments;
QString m_msvcVersion;
bool m_creatingCbpFiles;
Utils::Environment m_environment;
......@@ -140,7 +140,7 @@ private:
CMakeOpenProjectWizard *m_cmakeWizard;
QPlainTextEdit *m_output;
QPushButton *m_runCMake;
QProcess *m_cmakeProcess;
Utils::QtcProcess *m_cmakeProcess;
QLineEdit *m_argumentsLineEdit;
Utils::PathChooser *m_cmakeExecutable;
QComboBox *m_generatorComboBox;
......
......@@ -32,6 +32,7 @@
#include "cmakeproject.h"
#include <utils/synchronousprocess.h>
#include <utils/qtcprocess.h>
#include <coreplugin/icore.h>
#include <coreplugin/uniqueidmanager.h>
......@@ -102,7 +103,7 @@ bool CMakeManager::hasCodeBlocksMsvcGenerator() const
// we probably want the process instead of this function
// cmakeproject then could even run the cmake process in the background, adding the files afterwards
// sounds like a plan
void CMakeManager::createXmlFile(QProcess *proc, const QStringList &arguments,
void CMakeManager::createXmlFile(Utils::QtcProcess *proc, const QString &arguments,
const QString &sourceDirectory, const QDir &buildDirectory,
const Utils::Environment &env, const QString &generator)
{
......@@ -117,11 +118,16 @@ void CMakeManager::createXmlFile(QProcess *proc, const QStringList &arguments,
QString buildDirectoryPath = buildDirectory.absolutePath();
buildDirectory.mkpath(buildDirectoryPath);
proc->setWorkingDirectory(buildDirectoryPath);
proc->setEnvironment(env.toStringList());
proc->setEnvironment(env);
const QString srcdir = buildDirectory.exists(QLatin1String("CMakeCache.txt")) ?
QString(QLatin1Char('.')) : sourceDirectory;
proc->start(cmakeExecutable(), QStringList() << srcdir << arguments << generator);
QString args;
Utils::QtcProcess::addArg(&args, srcdir);
Utils::QtcProcess::addArgs(&args, arguments);
Utils::QtcProcess::addArg(&args, generator);
proc->setCommand(cmakeExecutable(), args);
proc->start();
}
QString CMakeManager::findCbpFile(const QDir &directory)
......
......@@ -44,6 +44,10 @@
QT_FORWARD_DECLARE_CLASS(QProcess)
QT_FORWARD_DECLARE_CLASS(QLabel)
namespace Utils {
class QtcProcess;
}
namespace CMakeProjectManager {
namespace Internal {
......@@ -66,8 +70,8 @@ public:
void setCMakeExecutable(const QString &executable);
void createXmlFile(QProcess *process,
const QStringList &arguments,
void createXmlFile(Utils::QtcProcess *process,
const QString &arguments,
const QString &sourceDirectory,
const QDir &buildDirectory,
const Utils::Environment &env,
......
......@@ -141,14 +141,9 @@ QString CMakeRunConfiguration::baseWorkingDirectory() const
return m_workingDirectory;
}
QStringList CMakeRunConfiguration::commandLineArguments() const
QString CMakeRunConfiguration::commandLineArguments() const
{
return environment().expandVariables(baseCommandLineArguments());
}
QStringList CMakeRunConfiguration::baseCommandLineArguments() const
{
return Utils::Environment::parseCombinedArgString(m_arguments);
return m_arguments;
}
QString CMakeRunConfiguration::title() const
......@@ -329,7 +324,7 @@ CMakeRunConfigurationWidget::CMakeRunConfigurationWidget(CMakeRunConfiguration *
fl->setMargin(0);
fl->setFieldGrowthPolicy(QFormLayout::ExpandingFieldsGrow);
QLineEdit *argumentsLineEdit = new QLineEdit();
argumentsLineEdit->setText(Utils::Environment::joinArgumentList(cmakeRunConfiguration->baseCommandLineArguments()));
argumentsLineEdit->setText(cmakeRunConfiguration->commandLineArguments());
connect(argumentsLineEdit, SIGNAL(textChanged(QString)),
this, SLOT(setArguments(QString)));
fl->addRow(tr("Arguments:"), argumentsLineEdit);
......
......@@ -68,7 +68,7 @@ public:
QString executable() const;
RunMode runMode() const;
QString workingDirectory() const;
QStringList commandLineArguments() const;
QString commandLineArguments() const;
Utils::Environment environment() const;
QWidget *createConfigurationWidget();
......@@ -104,7 +104,6 @@ protected:
private:
void setUserWorkingDirectory(const QString &workingDirectory);
QString baseWorkingDirectory() const;
QStringList baseCommandLineArguments() const;
void ctor();
enum BaseEnvironmentBase { CleanEnvironmentBase = 0,
......
......@@ -209,7 +209,7 @@ CMakeTarget *CMakeTargetFactory::create(ProjectExplorer::Project *parent, const
MakeStep *cleanMakeStep = new MakeStep(cleanSteps);
cleanSteps->insertStep(0, cleanMakeStep);
cleanMakeStep->setAdditionalArguments(QStringList() << "clean");
cleanMakeStep->setAdditionalArguments("clean");
cleanMakeStep->setClean(true);
t->addBuildConfiguration(bc);
......
......@@ -39,6 +39,8 @@
#include <projectexplorer/projectexplorer.h>
#include <projectexplorer/gnumakeparser.h>
#include <utils/qtcprocess.h>
#include <QtGui/QFormLayout>
#include <QtGui/QGroupBox>
#include <QtGui/QCheckBox>
......@@ -78,7 +80,7 @@ MakeStep::MakeStep(BuildStepList *bsl, MakeStep *bs) :
m_clean(bs->m_clean),
m_futureInterface(0),
m_buildTargets(bs->m_buildTargets),
m_additionalArguments(bs->m_buildTargets)
m_additionalArguments(Utils::QtcProcess::joinArgs(bs->m_buildTargets))
{
ctor();
}
......@@ -117,7 +119,7 @@ bool MakeStep::fromMap(const QVariantMap &map)
{
m_clean = map.value(QLatin1String(CLEAN_KEY)).toBool();
m_buildTargets = map.value(QLatin1String(BUILD_TARGETS_KEY)).toStringList();
m_additionalArguments = map.value(QLatin1String(ADDITIONAL_ARGUMENTS_KEY)).toStringList();
m_additionalArguments = map.value(QLatin1String(ADDITIONAL_ARGUMENTS_KEY)).toString();
return BuildStep::fromMap(map);
}
......@@ -132,8 +134,8 @@ bool MakeStep::init()
setCommand(bc->toolChain()->makeCommand());
QStringList arguments = m_buildTargets;
arguments << additionalArguments();
QString arguments = Utils::QtcProcess::joinArgs(m_buildTargets);
Utils::QtcProcess::addArgs(&arguments, additionalArguments());
setArguments(arguments);
setEnvironment(bc->environment());
setIgnoreReturnValue(m_clean);
......@@ -191,12 +193,12 @@ void MakeStep::setBuildTarget(const QString &buildTarget, bool on)
m_buildTargets = old;
}
QStringList MakeStep::additionalArguments() const
QString MakeStep::additionalArguments() const
{
return m_additionalArguments;
}
void MakeStep::setAdditionalArguments(const QStringList &list)
void MakeStep::setAdditionalArguments(const QString &list)
{
m_additionalArguments = list;
}
......@@ -239,7 +241,7 @@ MakeStepConfigWidget::MakeStepConfigWidget(MakeStep *makeStep)