Skip to content

Commit

Permalink
Fix RunManager copyRequiredFiles Regression
Browse files Browse the repository at this point in the history
Variable name was transposed when extracting `copyRequiredFiles`
function during the last bug fix.

@macumber
  • Loading branch information
lefticus committed Jun 24, 2014
1 parent 60c2e9d commit fb79c6e
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 18 deletions.
34 changes: 17 additions & 17 deletions openstudiocore/src/runmanager/lib/LocalProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ namespace detail {
m_parameters(t_parameters), m_outdir(t_outdir),
m_expectedOutputFiles(t_expectedOutputFiles),
m_stdin(t_stdin),
m_copiedRequiredFiles(copyRequiredFiles(t_requiredFiles, t_outdir))
m_copiedRequiredFiles(copyRequiredFiles(t_tool, t_requiredFiles, t_basePath))

This comment has been minimized.

Copy link
@lefticus

lefticus Jun 24, 2014

Author Contributor

@macumber @kbenne I cleaned up a few things because I saw them while I was in here, but this is the actual bug fix. The change from t_outdir to t_basePath.

{
LOG(Info, "Creating LocalProcess");

Expand Down Expand Up @@ -130,7 +130,7 @@ namespace detail {
directoryChanged(openstudio::toQString(m_outdir));
}

std::set<openstudio::path> LocalProcess::copyRequiredFiles(const std::vector<std::pair<openstudio::path, openstudio::path> > &t_requiredFiles,
std::set<openstudio::path> LocalProcess::copyRequiredFiles(const ToolInfo &t_tool, const std::vector<std::pair<openstudio::path, openstudio::path> > &t_requiredFiles,
const openstudio::path &t_basePath)
{
using namespace boost::filesystem;
Expand All @@ -152,7 +152,7 @@ namespace detail {
{
frompath = baserelative;
} else {
frompath = m_tool.localBinPath.parent_path() / frompath;
frompath = t_tool.localBinPath.parent_path() / frompath;
}
}

Expand Down Expand Up @@ -487,18 +487,20 @@ namespace detail {

void LocalProcess::processError(QProcess::ProcessError t_e)
{
m_fileCheckTimer.stop();
QFileInfo qfi(toQString(m_tool.localBinPath));
QFileInfo outdirfi(toQString(m_outdir));
LOG(Error, "LocalProcess processError: " << t_e
<< " exe: " << toString(m_tool.localBinPath)
<< " workingdir: " << toString(m_outdir)
<< " fileexists: " << qfi.isFile()
<< " fileexecutable: " << qfi.isExecutable()
<< " ErrorValue: " << t_e
<< " outdirexists: " << outdirfi.isFile()
<< " outdirisdirectory: " << outdirfi.isDir());

m_fileCheckTimer.stop();
QFileInfo qfi(toQString(m_tool.localBinPath));
QFileInfo outdirfi(toQString(m_outdir));
LOG(Error, "LocalProcess processError: " << t_e
<< " exe: " << toString(m_tool.localBinPath)
<< " workingdir: " << toString(m_outdir)
<< " fileexists: " << qfi.isFile()
<< " fileexecutable: " << qfi.isExecutable()
<< " ErrorValue: " << t_e
<< " outdirexists: " << outdirfi.isFile()
<< " outdirisdirectory: " << outdirfi.isDir());
QCoreApplication::processEvents();
directoryChanged(openstudio::toQString(m_outdir));

QProcess::ProcessState state = m_process.state();

Expand All @@ -511,9 +513,7 @@ namespace detail {
emit error(t_e);
}

QCoreApplication::processEvents();
directoryChanged(openstudio::toQString(m_outdir));
}
}

void LocalProcess::handleOutput(const QByteArray &qba, bool stderror)
{
Expand Down
2 changes: 1 addition & 1 deletion openstudiocore/src/runmanager/lib/LocalProcess.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ namespace detail {

static void kill(QProcess &t_process, bool t_force); //< Does an appropriate process tree kill on Windows

std::set<openstudio::path> copyRequiredFiles(const std::vector<std::pair<openstudio::path, openstudio::path> > &t_requiredFiles,
static std::set<openstudio::path> copyRequiredFiles(const ToolInfo &t_tool, const std::vector<std::pair<openstudio::path, openstudio::path> > &t_requiredFiles,
const openstudio::path &t_basePath);

/// Immutable members, do not need thread mutex protection
Expand Down

2 comments on commit fb79c6e

@macumber
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @lefticus ! This fixes the issue I saw today, I'll run the rest of the unit tests tonight and try to run a PAT project. Then I think we will want to put this in the release @axelstudios

@macumber
Copy link
Contributor

Choose a reason for hiding this comment

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

@lefticus @axelstudios I was able to run Larry's PAT project and all the unit tests check out so I think we should put this in the release and remake packages

Please sign in to comment.