Manual tools: fail on unexpected errors or warnings within caml_example by Octachron · Pull Request #693 · ocaml/ocaml

Octachron · 2016-07-16T13:56:26Z

This pull request proposes to modify the manual tools caml_tex2 to make it fails on unexpected errors or warnings within a caml_example environment. This change would make easier to notice new warnings or errors triggering on the code examples within the manual (like the few recent cases of deprecated warning or the quite hidden warning
in advexamples.etex corrected within this PR). Combined with the new use of in-tree,
this change would also make safer the potential use of the caml_example in the Langage extensions chapter.

In more detail, the second commit of this pull request makes the following code

\begin{caml_example}
1.0 * 2 ;;
\end{caml_example}

fail with

Error when evaluating a caml_example environment in filename.etex:
unexpected error.
If an error status was expected, add an [@@expect error] annotation.

As explained by the error message above, if the error or warning was intended, it
is necessary to annotate the example either with an attribute:

\begin{caml_example}
1 + 2. [@@expect error] ;;
let f None = None [@@expect warning 8];;
3 + 4 (*optional ok attribute) [@@expect ok];; 
\end{caml_example}

or with a global latex environment option, e.g.

\begin{caml_example}[error]
1 + 2.;;
3. + 4;;
\end{caml_example}

or for an intended warning

\begin{caml_example}[warning=8]
let f None = ();;
\end{caml_example}

Both annotations can be combined if needed

\begin{caml_example}[error]
1 + 2.;;
3. + 4;;
5 +6 [@@expect ok];;
\end{caml_example}

(The existing source files for the manual has been added the needed annotations.)

Moreover, to make this change useful, the first commit in this PR alters the build process of the manual to make the compilation of the manual fails in case of problems when generating the latex files from the ".etex" source files. Without this modifications, the manual build process had a tendency to "silently" generate empty tex files and then go on with the build process. This should no longer be the case.

gasche · 2016-08-03T08:24:22Z

manual/tools/caml_tex2.ml

+
+  let status s = match catch_warning s, catch_error s with
+    | Some w, _ -> w
+    | _, Some e -> e


if you excuse my purely stylistic nitpicking comment, I would prefer the use of None, Some e here to express the ordering using precise patterns instead of just clause ordering. In my opinion this makes the code easier to read, and it also makes it robust to eventual (possibly mistaken) clause reordering.

In the current implementation, the two clauses should be commutative since simultaneous multiple warnings and error are not handled.

Should they be handled? it would increase the complexity of the parsing for options and I am not sure how useful it would be. Maybe detecting the situation and raising an error would be worthwile?

Anyway I am perfectly fine with adding a None to choose an arbitrary ordering of these clauses.

Ah, I read another instance of the same pattern below as implementing a priority order. Don't bother, then.

gasche · 2016-08-03T08:33:03Z

This is excellent work and a much-needed improvement, in particular if we want to get serious about documenting warnings and errors in the manual.

However, I'm worried that the usefulness of this work could be limited by the fact that building the manual is not at all part of the OCaml development workflow, currently. This means that the statu quo of risking the release of inconsistent document is avoided, but what we get instead is Damien fighting for manual sanity around release time, with your occasional much-appreciated contributions catching some of the errors.

I think that making a full manual build part of the common build (or test target) would be too time-consuming, but maybe we could include the production of the .tex files from the .etex files? Or at least have a target for it in the central Makefile, not part of the default developer workflow, but used by our continuous integration builds?

Here would be my proposal:

add a target in the central Makefile that performs the minimal amount of manual work to catch these errors
measure the time it takes on your machine and report it here
include it in .travis-ci.sh before mkdir external-packages

Octachron · 2016-08-05T21:46:21Z

Rather than time, I fear that a full manual build would be more constraining in term of external dependencies: both latex and hevea are not exactly essential tools for building compilers.

However, building both the ".tex" files from the ".etex" source files and the ocamldoc documentation for the standard library seems perfectly doable. I have added a makefile target manual-pregen in the central Makefile to build these files. Some quick tests on my machine yield

make manual-pregen: 5s ± 0.03s
make test: 150s ± 20 s

So the relative increase in time seems limited.

gasche · 2016-08-08T10:53:01Z

I sent Octachron#1 as a proposed change on top of the current PR which builds only the .etex file and not the standard library, and reduces build time from 5s to 1.6s. It's not clear to me whether the standard library's documentation build can fail in useful ways that would justify the three extra seconds.

Octachron · 2016-08-10T20:07:45Z

It's not clear to me whether the standard library's documentation build can fail in useful ways that would justify the three extra seconds.

I can think of at least two examples of manual build failure that would have been caught by building the standard library documentation:

the problem with infix record in Manual: build fix #690
the missing escape characters in brace in Documentation: missing escape characters #419

So it can fail in useful way, but I agree that useful failures are probably very unfrequent and it might make sense to skip these files.

Another point, going further in this direction, we could also only test the generation of the "caml_environment" latex code which would be even lighter. An argument in this direction is that it is the only part of the manual build process that can be broken by unrelated changes in the compiler whereas other part of the build process can only be broken by altering the manual itself or the standard
library documentation.

This commit modifies the build process of the manual to make it fails in case of problems during the generation of the tex files from the etex sources files.

@expect

This commit modifies the manual tool caml_tex2 to catch the status of the output and raises an error in case of unexpected error of warning message. Expected errors or warnings must be now marked explicitly. There are two options to mark these expected errors or warnings: The first option is to use the new optional parameter of the `caml_example` environment, e.g. \begin{caml_example}[warning=3] String.capitalize "a word";; \end{caml_example} or for an error: \begin{caml_example}[error] 1 +. 3.;; \end{caml_example} The second option is to use `[@@expect ..]` attribute before `;;` to override locally the global expected status output: \begin{caml_example} 1 + 2 ;; 1 +. 3. [@@expect error];; String.capitalize [@@expect warning 3];; 3 + 4 ;; \end{caml_example} Note that the two options can be combined together, e.g \begin{caml_example}[error] 1 +. 3.;; (* an error is expected here *) 1. +. 3. [@@expect ok] ;; 1 + 3. ;; (* an error is still expected here *) \end{caml_example}

We introduce a `pregen-etex` Makefile rule that only build the .etex files of the manual (whose OCaml examples may be tested and expected to produce certain outputs), without also building the standard-library documentation. This is faster than the previous `pregen` rule as ocamldoc on the standard library was the bottleneck. On my machine, `pregen-etex` completes in 1.5s-2s, while `pregen` completes in 6s-7s.

Octachron · 2016-09-01T23:17:36Z

@gasche, I have merged your proposed changes, updated the root makefile and travis script.

Moreover, I was quite unhappy with the error reporting; errors were lost in the middle of noise.
So I went ahead and reduced the noisiness of caml_tex2 by adding a verbosity flag and improved the error reporting itself by including the line location, the problematic phrase and output in the error message.

gasche · 2016-09-02T08:04:27Z

Thanks for the additional work! I merged in trunk.

Manual tools: fail on unexpected errors or warnings within caml_example

…ce_412_effects Add ability to discontinue with backtrace

gasche reviewed Aug 3, 2016
View reviewed changes

Octachron and others added 10 commits September 2, 2016 00:59

Manual: fail earlier

03098ea

This commit modifies the build process of the manual to make it fails in case of problems during the generation of the tex files from the etex sources files.

Fix caml_tex2 output pattern matrix

279bcc7

Fix: comment output expectation kind

04b3c03

GPR#693: add a manual-pregen makefile target

b76ee8d

Fix: cmds makefile

547089f

Update travis

a88588b

Update root makefile manual-pregen target

e2fbb91

Manual tools: caml_tex2, improved error reporting

3f349ac

Octachron force-pushed the manual_guarded_caml_example branch from 3635cd1 to 3f349ac Compare September 1, 2016 23:03

gasche merged commit 1b1f5e2 into ocaml:trunk Sep 2, 2016

This was referenced Nov 28, 2016

Manual: caml_example for language extensions #939

Merged

Fixing typo in manual (7.16) #937

Merged

camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017

Merge pull request ocaml#693 from Octachron/manual_guarded_caml_example

7e5333e

Manual tools: fail on unexpected errors or warnings within caml_example

quartz55 pushed a commit to quartz55/ocaml that referenced this pull request Feb 1, 2022

Merge pull request ocaml#693 from ocaml-multicore/discontinue_backtra…

b844564

…ce_412_effects Add ability to discontinue with backtrace

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Manual tools: fail on unexpected errors or warnings within caml_example#693

Manual tools: fail on unexpected errors or warnings within caml_example#693
gasche merged 10 commits intoocaml:trunkfrom
Octachron:manual_guarded_caml_example

Octachron commented Jul 16, 2016

Uh oh!

gasche Aug 3, 2016

Uh oh!

Octachron Aug 5, 2016

Uh oh!

gasche Aug 6, 2016

Uh oh!

gasche commented Aug 3, 2016

Uh oh!

Octachron commented Aug 5, 2016

Uh oh!

gasche commented Aug 8, 2016 •

edited

Loading

Uh oh!

Octachron commented Aug 10, 2016

Uh oh!

Octachron commented Sep 1, 2016

Uh oh!

gasche commented Sep 2, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Octachron commented Jul 16, 2016

Uh oh!

gasche Aug 3, 2016

Choose a reason for hiding this comment

Uh oh!

Octachron Aug 5, 2016

Choose a reason for hiding this comment

Uh oh!

gasche Aug 6, 2016

Choose a reason for hiding this comment

Uh oh!

gasche commented Aug 3, 2016

Uh oh!

Octachron commented Aug 5, 2016

Uh oh!

gasche commented Aug 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Octachron commented Aug 10, 2016

Uh oh!

Octachron commented Sep 1, 2016

Uh oh!

gasche commented Sep 2, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gasche commented Aug 8, 2016 •

edited

Loading