Attached are a sequence of patches addressing issues related to LyX's
error reporting and log parsing. I give more detailed descriptions of
them in the actual commit messages of the attached patches, but below
I list important points as well as a couple of questions. I have
brought up a couple of these topics in separate emails but since the
patches are dependent, I prefer to start this new thread and I will
reference the other threads when necessary (once the topics are
resolved here I will update the other threads for archive purposes).
0001-Clarify-message-about-an-empty-file.patch
- In brief testing, when I compile a .tex file that does not produce
output, no .pdf is created (not even an empty .pdf). Can anyone
confirm this?
0003-Remove-an-outdated-comment.patch
- A "dummy function" means that the function has no effect, correct?
0004-Allow-cloned-buffers-to-give-alerts-in-runLaTeX.patch
- This is the patch I am least confident about. Conditioning on
!buffer.isClone() caused the condition to always fail so the alerts
were never shown. Is that conditioning still needed? I don't
understand this process well. I imagine that whenever I compile from
the LyX GUI, it clones the buffer (so that I can change the buffer
while compiling is going on). In which case is !buffer.isClone() true
(can you give me steps to reproduce)? Can anyone get the warning to
activate *without* this patch (e.g. try to compile a blank .lyx file)?
Can anyone explain further the necessity of this commit and whether
the Alert system has indeed been rethought in the last few years?
commit bea0925f8ccd617293d9171eef8453d938e3a44f
Author: Abdelrazak Younes <you...@lyx.org>
Date: Fri Dec 18 22:59:59 2009 +0000
Converter: add a safe guard to Alerts because those cannot be
called from another thread. The whole Alert system must be rethought I
am afraid.
0005-Check-exit-code-of-LaTeX-process-in-LaTeX-run.patch
- To see why this patch is useful, open New > New from Template >
ACM-sigplan.lyx and compile. It says that compilation was successful
and shows the PDF. However, there was an error that was missed in
parsing (the terminal shows support/Systemcall.cpp (288): Systemcall:
'pdflatex "newfile1.tex"' finished with exit code 1).
- For a related email thread, see
https://www.mail-archive.com/lyx-devel%40lists.lyx.org/msg185316.html
0006-Improve-log-scanner-to-correctly-report-error.patch
- This fixes the parsing that lead to the particular instance of the
bug fixed in patch 0005.
- For a related email thread, see
https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg185567.html
From 26fe6b3101e530cc4809d947f263affaac9f523c Mon Sep 17 00:00:00 2001
From: Scott Kostyshak <skost...@lyx.org>
Date: Sat, 21 Feb 2015 00:00:51 -0500
Subject: [PATCH 6/6] Improve log scanner to correctly report error
When scanning the LaTeX log, previously we only looked ahead 10 lines
after a "!" line and if we did not find a line number we did not count
an error. This lead to the problem that templates/ACM-sigplan.lyx was
showing a successful export and the PDF was shown (it is still
created despite the error). Now that the exit code of the latex
command is checked (as of the previous commit), an error is correctly
given, but by parsing the log better with this commit, a more
informative error is given.
Increasing the look-ahead to 15 lines leads to correct parsing of
the ACM-sigplan log. The excerpt in the log file where there are more
than 10 lines in-between the "!" line and the line number is below:
! Undefined control sequence.
\@toappear ...ent http://dx.doi.org/10.1145/\@doi
<argument> ...n is removed.]\par \else \@toappear
\fi \if \@reprint
\noinden...
\@begin@tempboxa ...mpboxa #1{\color@begingroup #2
\color@endgroup }\def
\wid...
\@iiiparbox ...tempdima \@parboxrestore #5\@@par }
\ifx \relax #2\else
\setle...
\@copyrightspace ...planconf@finalpage}.\par \fi }
}\end@float
\maketitle ... \@copyrightwanted \@copyrightspace
\fi
l.34 \maketitle
---
src/LaTeX.cpp | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/LaTeX.cpp b/src/LaTeX.cpp
index e0259e6..346fea6 100644
--- a/src/LaTeX.cpp
+++ b/src/LaTeX.cpp
@@ -830,7 +830,10 @@ int LaTeX::scanLogFile(TeXErrors & terr)
if (!getline(ifs, tmp))
break;
tmp = rtrim(tmp, "\r");
- if (++count > 10)
+ // 15 is somewhat arbitrarily chosen, based on practice.
+ // We used 10 for 14 years and increased it to 15 when we
+ // saw one case.
+ if (++count > 15)
break;
} while (!prefixIs(tmp, "l."));
if (prefixIs(tmp, "l.")) {
--
2.1.0
From 05704d0a84067ad7a415891ad9646b09ac897305 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak <skost...@lyx.org>
Date: Fri, 20 Feb 2015 16:17:03 -0500
Subject: [PATCH 5/6] Check exit code of LaTeX process in LaTeX::run
Systemcall::startscript returns the exit code of the LaTeX command
that is run, but the return value was not being checked by
LaTeX::run. Instead, we relied on parsing log files. However, this
parsing is not perfect.
The return value is now checked and if the exit code of the command
is non-zero, an enum value is added to the return and the user is
notified of the error. For LyX 2.2.0 we will notify with a warning.
This will be changed to an error in LyX 2.3.0. We will allow the
user one version to adjust because from the user perspective if we
went straight to reporting the error, this would break compilation
when the user was perhaps content with ignoring the error.
At a higher level, in LyX 2.3.0 if the LaTeX command returns a
non-zero exit code, in the GUI a message such as "Error while
exporting format: PDF (LuaTeX)" will be given instead of "Successful
preview of format: PDF (LuaTeX)" followed by the attempt to open an
incomplete/missing .pdf file.
When run on the commandline, in LyX 2.3.0, lyx -e lualatex
example.lyx will give "Error: LaTeX failed" and a non-zero exit code
where before it gave a zero exit code.
A real example of the bug this commit fixes is LyX's (as of this commit)
ACM-sigplan.lyx template.
Before this commit:
$ lyx -e pdf2 ACM-sigplan.lyx
[...snip...]
support/Systemcall.cpp (288): Systemcall: 'pdflatex "ACM-sigplan.tex"'
finished with exit code 1
$ echo $?
0
Once this patch is fully implemented (LyX 2.3.0):
$ mylyx master -e pdf2 ACM-sigplan.lyx
support/Systemcall.cpp (288): Systemcall: 'pdflatex "ACM-sigplan.tex"'
finished with exit code 1
Error: LaTeX failed
----------------------------------------
LaTeX did not run successfully. The command that was run exited with
error.
$ echo $?
1
---
src/Converter.cpp | 11 +++++++++++
src/LaTeX.cpp | 12 +++++++++++-
src/LaTeX.h | 5 ++++-
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 47a88e7..53c882f 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -658,6 +658,17 @@ bool Converters::runLaTeX(Buffer const & buffer, string const & command,
"Additionally, LyX could not locate "
"the LaTeX log %1$s."), from_utf8(name));
Alert::error(_("LaTeX failed"), str);
+ } else if (result & LaTeX::NONZERO_ERROR) {
+ docstring const str =
+ bformat(_( "The external program\n%1$s\n "
+ "finished with an error. Compilation has been allowed to "
+ "continue, but starting with LyX 2.3.0 this warning "
+ "message will become an error and compilation will stop. "
+ "It is recommended you fix the cause of the external "
+ "program's error (check the logs). "), from_utf8(command));
+ // FIXME: In LyX 2.3.0 the warning will be converted to an error.
+ // Alert::error(_("LaTeX failed"), str);
+ Alert::warning(_("LaTeX failed"), str);
} else if (result & LaTeX::NO_OUTPUT) {
Alert::warning(_("Output is empty"),
_("No output file was generated."));
diff --git a/src/LaTeX.cpp b/src/LaTeX.cpp
index 88be13d..e0259e6 100644
--- a/src/LaTeX.cpp
+++ b/src/LaTeX.cpp
@@ -227,7 +227,8 @@ int LaTeX::run(TeXErrors & terr)
LYXERR(Debug::LATEX, "Run #" << count);
message(runMessage(count));
- startscript();
+ int const exit_code = startscript();
+
scanres = scanLogFile(terr);
if (scanres & ERROR_RERUN) {
LYXERR(Debug::LATEX, "Rerunning LaTeX");
@@ -423,6 +424,7 @@ int LaTeX::run(TeXErrors & terr)
// Write the dependencies to file.
head.write(depfile);
+
if (scanres & NO_OUTPUT) {
// A previous run could have left a PDF and since
// no PDF is created if NO_OUTPUT, we remove any
@@ -432,6 +434,14 @@ int LaTeX::run(TeXErrors & terr)
// be the same so any lingering PDF will be viewed.
deleteFilesOnError();
}
+
+ if (exit_code) {
+ scanres |= NONZERO_ERROR;
+ // FIXME in LyX 2.3.0. NONZERO_ERROR will be converted from a warning
+ // to an error and the below will be uncommented.
+ // deleteFilesOnError();
+ }
+
LYXERR(Debug::LATEX, "Done.");
return scanres;
}
diff --git a/src/LaTeX.h b/src/LaTeX.h
index 615d974..a4637db 100644
--- a/src/LaTeX.h
+++ b/src/LaTeX.h
@@ -138,12 +138,15 @@ public:
///
BIBTEX_ERROR = 16384,
///
+ NONZERO_ERROR = 32768, // the command exited with nonzero status
+ ///
//FIXME: BIBTEX_ERROR has been removed from ERRORS for now, since users were irritated
// about those errors which prevented compilation of previously compiling documents.
// Think about a "gentle" transfer to BibTeX error reporting.
+ //FIXME: In LyX 2.3.0 NONZERO_ERROR will be changed from a warning to an error.
ERRORS = TEX_ERROR + LATEX_ERROR,
///
- WARNINGS = TEX_WARNING + LATEX_WARNING + PACKAGE_WARNING
+ WARNINGS = TEX_WARNING + LATEX_WARNING + PACKAGE_WARNING + NONZERO_ERROR
};
/// This signal emits an informative message
--
2.1.0
From 03131a50920368ee6cfbbe31b64343b5a26e7b6f Mon Sep 17 00:00:00 2001
From: Scott Kostyshak <skost...@lyx.org>
Date: Fri, 20 Feb 2015 17:45:36 -0500
Subject: [PATCH 4/6] Allow cloned buffers to give alerts in runLaTeX()
This was disabled in 2009 in bea0925f but it is now safe (???)
---
src/Converter.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/Converter.cpp b/src/Converter.cpp
index c6ee679..47a88e7 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -652,13 +652,13 @@ bool Converters::runLaTeX(Buffer const & buffer, string const & command,
buffer.bufferErrors(terr, errorList);
// check return value from latex.run().
- if ((result & LaTeX::NO_LOGFILE) && !buffer.isClone()) {
+ if (result & LaTeX::NO_LOGFILE) {
docstring const str =
bformat(_("LaTeX did not run successfully. "
"Additionally, LyX could not locate "
"the LaTeX log %1$s."), from_utf8(name));
Alert::error(_("LaTeX failed"), str);
- } else if ((result & LaTeX::NO_OUTPUT) && !buffer.isClone()) {
+ } else if (result & LaTeX::NO_OUTPUT) {
Alert::warning(_("Output is empty"),
_("No output file was generated."));
}
--
2.1.0
From ba32c2d61a008d9983ddf309095148f93dd084e2 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak <skost...@lyx.org>
Date: Fri, 20 Feb 2015 17:31:53 -0500
Subject: [PATCH 3/6] Remove an outdated comment
LaTeX::deleteFilesOnError is no longer a dummy function.
This comment was interoduced with the first version of this
function, at e6d063c4.
---
src/LaTeX.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/LaTeX.cpp b/src/LaTeX.cpp
index 7619380..88be13d 100644
--- a/src/LaTeX.cpp
+++ b/src/LaTeX.cpp
@@ -110,8 +110,6 @@ LaTeX::LaTeX(string const & latex, OutputParams const & rp,
void LaTeX::deleteFilesOnError() const
{
- // currently just a dummy function.
-
// What files do we have to delete?
// This will at least make latex do all the runs
--
2.1.0
From b56ccb82e806783517f6f4682927804d906a0d3b Mon Sep 17 00:00:00 2001
From: Scott Kostyshak <skost...@lyx.org>
Date: Fri, 20 Feb 2015 16:15:48 -0500
Subject: [PATCH 2/6] Remove PDF in temporary folder on error
This commit solves two issues:
(1) A PDF from a previous run could have been the result of a command
that exited with error (e.g. sometimes pdflatex still produces a PDF if
it exits with error). If the "View" button were clicked a second time
without changing the .lyx file, then the checksum of the .tex file would
not have changed so LyX would show the PDF (which was created from the
first run that exited with error), and this time LyX would not report
the error (because the parsing of the logs only happens when the .tex
file is compiled).
(2) A myfile.tex that results in no output does not yield a myfile.pdf.
Thus, Any myfile.pdf in the temporary directory will not be overriden.
Before this commit, the following scenario was possible: LyX runs
pdflatex which processes myfile.tex and no error is given so LyX opens
myfile.pdf. However, it could have been the scenario that pdflatex did
not exit with error and did not create myfile.pdf, in which case
whichever myfile.pdf is being shown is not correct. To see this bug in
action, start a new document, type "abc", view the PDF, delete "abc",
view the PDF (this correctly gives an error that empty output was
created), view the PDF again (this does not give an error because the
checksum has not changed). The PDF shown will contain "abc".
---
src/LaTeX.cpp | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/LaTeX.cpp b/src/LaTeX.cpp
index e787e1a..7619380 100644
--- a/src/LaTeX.cpp
+++ b/src/LaTeX.cpp
@@ -142,6 +142,10 @@ void LaTeX::deleteFilesOnError() const
// Also remove the aux file
FileName const aux(changeExtension(file.absFileName(), ".aux"));
aux.removeFile();
+
+ // Remove the pdf file, which is sometimes generated even if error
+ FileName const pdf(changeExtension(file.absFileName(), ".pdf"));
+ pdf.removeFile();
}
@@ -421,6 +425,15 @@ int LaTeX::run(TeXErrors & terr)
// Write the dependencies to file.
head.write(depfile);
+ if (scanres & NO_OUTPUT) {
+ // A previous run could have left a PDF and since
+ // no PDF is created if NO_OUTPUT, we remove any
+ // existing PDF and temporary files so that an
+ // incorrect PDF is not displayed, which could otherwise
+ // happen if View is run again because the checksum will
+ // be the same so any lingering PDF will be viewed.
+ deleteFilesOnError();
+ }
LYXERR(Debug::LATEX, "Done.");
return scanres;
}
--
2.1.0
From 3f824e2681f5896f329e6ae3370e4400bf82cd1c Mon Sep 17 00:00:00 2001
From: Scott Kostyshak <skost...@lyx.org>
Date: Sat, 21 Feb 2015 23:46:42 -0500
Subject: [PATCH 1/6] Clarify message about an empty file
When empty output is generated, no PDF is produced. The previous
message made it seem like an empty PDF was produced.
---
src/Converter.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 1e02637..c6ee679 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -660,7 +660,7 @@ bool Converters::runLaTeX(Buffer const & buffer, string const & command,
Alert::error(_("LaTeX failed"), str);
} else if ((result & LaTeX::NO_OUTPUT) && !buffer.isClone()) {
Alert::warning(_("Output is empty"),
- _("An empty output file was generated."));
+ _("No output file was generated."));
}
--
2.1.0
