Please see attached patch.
>From 9577e22ad00c2ad13f48224b8fc5c87f9286db80 Mon Sep 17 00:00:00 2001
From: Richard Heck <rgh...@lyx.org>
Date: Mon, 19 Feb 2018 22:43:44 -0500
Subject: [PATCH] Fix a problem with background cancellation.
The problem was that, if we killed export when some graphic was
being converted, it would only cancel that process, and then it
would just continue with the next one. To deal with that, we need
to do a few things:
1. Modify the return values for some of the Converters routines,
so we can tell when something has been canceled.
2. Throw an exception from InsetGraphics when this has happened
during LaTeX or HTML export, but ONLY when the Buffer is a
clone. We shouldn't be able to 'cancel' export when we're, say,
generating code for the preview pane, but this keeps us on the
safe side, I think.
That exception then has to be caught, obviously, back in the
export routines in Buffer.
Probably Coverity will be unhappy about something here, but I'll
deal with that problem as it arises.
---
src/Buffer.cpp | 13 +++++++--
src/Converter.cpp | 62 ++++++++++++++++++++++--------------------
src/Converter.h | 22 ++++++++++++---
src/frontends/qt4/GuiView.cpp | 4 +--
src/insets/ExternalSupport.cpp | 6 ++--
src/insets/InsetGraphics.cpp | 19 ++++++++++---
src/insets/InsetInclude.cpp | 8 ++++--
7 files changed, 88 insertions(+), 46 deletions(-)
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 7cabe6fce1..8051135bd1 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -1102,7 +1102,7 @@ bool Buffer::importFile(string const & format, FileName const & name, ErrorList
return false;
FileName const lyx = tempFileName("Buffer_importFileXXXXXX.lyx");
- if (theConverters().convert(0, name, lyx, name, format, "lyx", errorList)) {
+ if (theConverters().convert(0, name, lyx, name, format, "lyx", errorList) == Converters::SUCCESS) {
bool const success = readFile(lyx) == ReadSuccess;
removeTempFile(lyx);
return success;
@@ -1702,6 +1702,7 @@ bool Buffer::makeLaTeXFile(FileName const & fname,
try {
writeLaTeXSource(os, original_path, runparams, output);
}
+ catch (ConversionException const &) { return false; }
catch (EncodingException const & e) {
docstring const failed(1, e.failed_char);
ostringstream oss;
@@ -2113,7 +2114,10 @@ void Buffer::makeLyXHTMLFile(FileName const & fname,
updateBuffer(UpdateMaster, OutputUpdate);
updateMacroInstances(OutputUpdate);
- writeLyXHTMLSource(ofs, runparams, FullSource);
+ try {
+ writeLyXHTMLSource(ofs, runparams, FullSource);
+ }
+ catch (ConversionException const &) { return; }
ofs.close();
if (ofs.fail())
@@ -4298,9 +4302,12 @@ Buffer::ExportStatus Buffer::doExport(string const & target, bool put_in_tempdir
ErrorList & error_list = d->errorLists[error_type];
string const ext = theFormats().extension(format);
FileName const tmp_result_file(changeExtension(filename, ext));
- bool const success = converters.convert(this, FileName(filename),
+ Converters::RetVal const retval = converters.convert(this, FileName(filename),
tmp_result_file, FileName(absFileName()), backend_format, format,
error_list);
+ if (retval == Converters::KILLED)
+ return ExportCancel;
+ bool success = (retval == Converters::SUCCESS);
// Emit the signal to show the error list or copy it back to the
// cloned Buffer so that it can be emitted afterwards.
diff --git a/src/Converter.cpp b/src/Converter.cpp
index d433f83c98..3d15374ff9 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -384,18 +384,20 @@ bool Converters::checkAuth(Converter const & conv, string const & doc_fname,
}
-bool Converters::convert(Buffer const * buffer,
+Converters::RetVal Converters::convert(Buffer const * buffer,
FileName const & from_file, FileName const & to_file,
FileName const & orig_from,
string const & from_format, string const & to_format,
ErrorList & errorList, int conversionflags)
{
if (from_format == to_format)
- return move(from_format, from_file, to_file, false);
+ return move(from_format, from_file, to_file, false) ?
+ SUCCESS : FAILURE;
if ((conversionflags & try_cache) &&
ConverterCache::get().inCache(orig_from, to_format))
- return ConverterCache::get().copy(orig_from, to_format, to_file);
+ return ConverterCache::get().copy(orig_from, to_format, to_file) ?
+ SUCCESS : FAILURE;
Graph::EdgePath edgepath = getPath(from_format, to_format);
if (edgepath.empty()) {
@@ -427,20 +429,20 @@ bool Converters::convert(Buffer const * buffer,
_("Converter killed"),
bformat(_("The running converter\n %1$s\nwas killed by the user."),
from_utf8(command)));
- return false;
+ return KILLED;
}
if (to_file.isReadableFile()) {
if (conversionflags & try_cache)
ConverterCache::get().add(orig_from,
to_format, to_file);
- return true;
+ return SUCCESS;
}
}
// only warn once per session and per file type
static std::map<string, string> warned;
if (warned.find(from_format) != warned.end() && warned.find(from_format)->second == to_format) {
- return false;
+ return FAILURE;
}
warned.insert(make_pair(from_format, to_format));
@@ -449,7 +451,7 @@ bool Converters::convert(Buffer const * buffer,
"format files to %2$s.\n"
"Define a converter in the preferences."),
from_ascii(from_format), from_ascii(to_format)));
- return false;
+ return FAILURE;
}
// buffer is only invalid for importing, and then runparams is not
@@ -560,7 +562,7 @@ bool Converters::convert(Buffer const * buffer,
if (!checkAuth(conv, buffer ? buffer->absFileName() : string(),
buffer && buffer->params().shell_escape))
- return false;
+ return FAILURE;
if (conv.latex()) {
// We are not importing, we have a buffer
@@ -576,8 +578,9 @@ bool Converters::convert(Buffer const * buffer,
LYXERR(Debug::FILES, "Running " << command);
// FIXME KILLED
// Check changed return value here.
- if (!runLaTeX(*buffer, command, runparams, errorList))
- return false;
+ RetVal const retval = runLaTeX(*buffer, command, runparams, errorList);
+ if (retval != SUCCESS)
+ return retval;
} else {
if (conv.need_aux() && !run_latex) {
// We are not importing, we have a buffer
@@ -606,9 +609,9 @@ bool Converters::convert(Buffer const * buffer,
<< " to update aux file");
// FIXME KILLED
// Check changed return value here.
- if (!runLaTeX(*buffer, command,
- runparams, errorList))
- return false;
+ RetVal const retval = runLaTeX(*buffer, command, runparams, errorList);
+ if (retval != SUCCESS)
+ return retval;
}
}
@@ -668,7 +671,7 @@ bool Converters::convert(Buffer const * buffer,
_("Converter killed"),
bformat(_("The running converter\n %1$s\nwas killed by the user."),
from_utf8(command)));
- return false;
+ return KILLED;
}
if (!real_outfile.empty()) {
@@ -697,10 +700,10 @@ bool Converters::convert(Buffer const * buffer,
_("Converter killed"),
bformat(_("The running converter\n %1$s\nwas killed by the user."),
from_utf8(command)));
- return false;
+ return KILLED;
}
if (!scanLog(*buffer, command, makeAbsPath(logfile, path), errorList))
- return false;
+ return FAILURE;
}
}
@@ -709,11 +712,15 @@ bool Converters::convert(Buffer const * buffer,
Alert::information(_("Process Killed"),
bformat(_("The conversion process was killed while running:\n%1$s"),
wrapParas(from_utf8(command))));
- } else if (res == Systemcall::TIMEOUT) {
+ return KILLED;
+ }
+ if (res == Systemcall::TIMEOUT) {
Alert::information(_("Process Timed Out"),
bformat(_("The conversion process:\n%1$s\ntimed out before completing."),
wrapParas(from_utf8(command))));
- } else if (conv.to() == "program") {
+ return KILLED;
+ }
+ if (conv.to() == "program") {
Alert::error(_("Build errors"),
_("There were errors during the build process."));
} else {
@@ -723,14 +730,14 @@ bool Converters::convert(Buffer const * buffer,
bformat(_("An error occurred while running:\n%1$s"),
wrapParas(from_utf8(command))));
}
- return false;
+ return FAILURE;
}
}
}
Converter const & conv = converterlist_[edgepath.back()];
if (conv.To()->dummy())
- return true;
+ return SUCCESS;
if (!conv.result_dir().empty()) {
// The converter has put the file(s) in a directory.
@@ -745,14 +752,14 @@ bool Converters::convert(Buffer const * buffer,
Alert::error(_("Cannot convert file"),
bformat(_("Could not move a temporary directory from %1$s to %2$s."),
from_utf8(from), from_utf8(to)));
- return false;
+ return FAILURE;
}
}
- return true;
+ return SUCCESS;
} else {
if (conversionflags & try_cache)
ConverterCache::get().add(orig_from, to_format, outfile);
- return move(conv.to(), outfile, to_file, conv.latex());
+ return move(conv.to(), outfile, to_file, conv.latex()) ? SUCCESS : FAILURE;
}
}
@@ -826,9 +833,7 @@ bool Converters::scanLog(Buffer const & buffer, string const & /*command*/,
}
-// FIXME KILL
-// Probably need to return an INT here
-bool Converters::runLaTeX(Buffer const & buffer, string const & command,
+Converters::RetVal Converters::runLaTeX(Buffer const & buffer, string const & command,
OutputParams const & runparams, ErrorList & errorList)
{
buffer.setBusy(true);
@@ -851,7 +856,7 @@ bool Converters::runLaTeX(Buffer const & buffer, string const & command,
if (result == Systemcall::KILLED) {
Alert::error(_("Export canceled"),
_("The export process was terminated by the user."));
- return result;
+ return KILLED;
}
if (result & LaTeX::ERRORS)
@@ -880,7 +885,6 @@ bool Converters::runLaTeX(Buffer const & buffer, string const & command,
_("No output file was generated."));
}
-
buffer.setBusy(false);
int const ERROR_MASK =
@@ -888,7 +892,7 @@ bool Converters::runLaTeX(Buffer const & buffer, string const & command,
LaTeX::ERRORS |
LaTeX::NO_OUTPUT;
- return (result & ERROR_MASK) == 0;
+ return (result & ERROR_MASK) == 0 ? SUCCESS : FAILURE;
}
diff --git a/src/Converter.h b/src/Converter.h
index 8f63aba512..a9521c5b19 100644
--- a/src/Converter.h
+++ b/src/Converter.h
@@ -30,9 +30,17 @@ class ErrorList;
class Format;
class Formats;
-typedef std::vector<Format const *> FormatList;
+class ConversionException : public std::exception {
+public:
+ ConversionException() {}
+ virtual ~ConversionException() throw() {}
+ virtual const char * what() const throw()
+ { return "Exception caught in conversion routine!"; }
+};
+typedef std::vector<Format const *> FormatList;
+
///
class Converter {
public:
@@ -124,7 +132,13 @@ public:
typedef std::vector<Converter> ConverterList;
///
typedef ConverterList::const_iterator const_iterator;
-
+ /// Return values for converter runs
+ enum RetVal {
+ SUCCESS = 0,
+ FAILURE = 1,
+ KILLED = 1000
+ };
+
///
Converter const & get(int i) const { return converterlist_[i]; }
///
@@ -169,7 +183,7 @@ public:
try_cache = 1 << 1
};
///
- bool convert(Buffer const * buffer,
+ RetVal convert(Buffer const * buffer,
support::FileName const & from_file, support::FileName const & to_file,
support::FileName const & orig_from,
std::string const & from_format, std::string const & to_format,
@@ -210,7 +224,7 @@ private:
bool scanLog(Buffer const & buffer, std::string const & command,
support::FileName const & filename, ErrorList & errorList);
///
- bool runLaTeX(Buffer const & buffer, std::string const & command,
+ RetVal runLaTeX(Buffer const & buffer, std::string const & command,
OutputParams const &, ErrorList & errorList);
///
ConverterList converterlist_;
diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp
index cc5a3df44b..1048fe42b4 100644
--- a/src/frontends/qt4/GuiView.cpp
+++ b/src/frontends/qt4/GuiView.cpp
@@ -2364,8 +2364,8 @@ static bool import(GuiView * lv, FileName const & filename,
string const tofile =
support::changeExtension(filename.absFileName(),
theFormats().extension(*it));
- if (!theConverters().convert(0, filename, FileName(tofile),
- filename, format, *it, errorList))
+ if (theConverters().convert(0, filename, FileName(tofile),
+ filename, format, *it, errorList) != Converters::SUCCESS)
return false;
loader_format = *it;
break;
diff --git a/src/insets/ExternalSupport.cpp b/src/insets/ExternalSupport.cpp
index 3a214ca8d0..146caa9032 100644
--- a/src/insets/ExternalSupport.cpp
+++ b/src/insets/ExternalSupport.cpp
@@ -328,12 +328,14 @@ void updateExternal(InsetExternalParams const & params,
// FIXME (Abdel 12/08/06): Is there a need to show these errors?
ErrorList el;
- bool const success =
+ // FIXME Cancel Export
+ // Can this be canceled? Do we need to take some other action then?
+ Converters::RetVal const success =
theConverters().convert(&buffer, temp_file, abs_to_file,
params.filename, from_format, to_format, el,
Converters::try_default | Converters::try_cache);
- if (!success) {
+ if (success != Converters::SUCCESS) {
LYXERR(Debug::EXTERNAL, "external::updateExternal. "
<< "Unable to convert from " << from_format << " to " << to_format);
}
diff --git a/src/insets/InsetGraphics.cpp b/src/insets/InsetGraphics.cpp
index 02a5630673..a07296c6ee 100644
--- a/src/insets/InsetGraphics.cpp
+++ b/src/insets/InsetGraphics.cpp
@@ -714,9 +714,15 @@ string InsetGraphics::prepareFile(OutputParams const & runparams) const
// FIXME (Abdel 12/08/06): Is there a need to show these errors?
ErrorList el;
- if (theConverters().convert(&buffer(), temp_file, to_file, params().filename,
+ Converters::RetVal const rv =
+ theConverters().convert(&buffer(), temp_file, to_file, params().filename,
from, to, el,
- Converters::try_default | Converters::try_cache)) {
+ Converters::try_default | Converters::try_cache);
+ if (rv == Converters::KILLED) {
+ LYXERR0("Graphics preparation killed.");
+ if (buffer().isClone() && buffer().isExporting())
+ throw ConversionException();
+ } else if (rv == Converters::SUCCESS) {
runparams.exportdata->addExternalFile(tex_format,
to_file, output_to_file);
runparams.exportdata->addExternalFile("dvi",
@@ -939,10 +945,15 @@ string InsetGraphics::prepareHTMLFile(OutputParams const & runparams) const
// FIXME (Abdel 12/08/06): Is there a need to show these errors?
ErrorList el;
- bool const success =
+ Converters::RetVal const rv =
theConverters().convert(&buffer(), temp_file, to_file, params().filename,
from, to, el, Converters::try_default | Converters::try_cache);
- if (!success)
+ if (rv == Converters::KILLED) {
+ if (buffer().isClone() && buffer().isExporting())
+ throw ConversionException();
+ return string();
+ }
+ if (rv != Converters::SUCCESS)
return string();
runparams.exportdata->addExternalFile("xhtml", to_file, output_to_file);
return output_to_file;
diff --git a/src/insets/InsetInclude.cpp b/src/insets/InsetInclude.cpp
index 89f2533111..e0c4c588e2 100644
--- a/src/insets/InsetInclude.cpp
+++ b/src/insets/InsetInclude.cpp
@@ -811,12 +811,16 @@ void InsetInclude::latex(otexstream & os, OutputParams const & runparams) const
// If needed, use converters to produce a latex file from the child
if (tmpwritefile != writefile) {
ErrorList el;
- bool const success =
+ Converters::RetVal const retval =
theConverters().convert(tmp, tmpwritefile, writefile,
included_file,
inc_format, tex_format, el);
+ if (retval == Converters::KILLED) {
+ LYXERR0("Need to throw an exception here.");
+ return;
+ }
- if (!success && !runparams.silent) {
+ if (retval != Converters::SUCCESS && !runparams.silent) {
docstring msg = bformat(_("Included file `%1$s' "
"was not exported correctly.\n "
"LaTeX export is probably incomplete."),
--
2.13.6
