diff --git a/services/clsi/app/js/CompileManager.js b/services/clsi/app/js/CompileManager.js index cf61f16ded..b9800bcc6b 100644 --- a/services/clsi/app/js/CompileManager.js +++ b/services/clsi/app/js/CompileManager.js @@ -198,6 +198,7 @@ function doCompile(request, callback) { flags: request.flags, environment: env, compileGroup: request.compileGroup, + stopOnFirstError: request.stopOnFirstError, }, (error, output, stats, timings) => { // request was for validation only diff --git a/services/clsi/app/js/LatexRunner.js b/services/clsi/app/js/LatexRunner.js index 662e5c9ac1..cfc385e4b9 100644 --- a/services/clsi/app/js/LatexRunner.js +++ b/services/clsi/app/js/LatexRunner.js @@ -12,24 +12,25 @@ const TIME_V_METRICS = Object.entries({ 'sys-time': /System time.*: (\d+.\d+)/m, }) +const COMPILER_FLAGS = { + latex: '-pdfdvi', + lualatex: '-lualatex', + pdflatex: '-pdf', + xelatex: '-xelatex', +} + function runLatex(projectId, options, callback) { - let command - let { + const { directory, mainFile, - compiler, - timeout, image, environment, flags, compileGroup, + stopOnFirstError, } = options - if (!compiler) { - compiler = 'pdflatex' - } - if (!timeout) { - timeout = 60000 - } // milliseconds + const compiler = options.compiler || 'pdflatex' + const timeout = options.timeout || 60000 // milliseconds logger.debug( { @@ -40,28 +41,20 @@ function runLatex(projectId, options, callback) { environment, flags, compileGroup, + stopOnFirstError, }, 'starting compile' ) - // We want to run latexmk on the tex file which we will automatically - // generate from the Rtex/Rmd/md file. - mainFile = mainFile.replace(/\.(Rtex|md|Rmd)$/, '.tex') - - if (compiler === 'pdflatex') { - command = _pdflatexCommand(mainFile, flags) - } else if (compiler === 'latex') { - command = _latexCommand(mainFile, flags) - } else if (compiler === 'xelatex') { - command = _xelatexCommand(mainFile, flags) - } else if (compiler === 'lualatex') { - command = _lualatexCommand(mainFile, flags) - } else { - return callback(new Error(`unknown compiler: ${compiler}`)) - } - - if (Settings.clsi?.strace) { - command = ['strace', '-o', 'strace', '-ff'].concat(command) + let command + try { + command = _buildLatexCommand(mainFile, { + compiler, + stopOnFirstError, + flags, + }) + } catch (err) { + return callback(err) } const id = `${projectId}` // record running project under this id @@ -145,49 +138,55 @@ function killLatex(projectId, callback) { } } -function _latexmkBaseCommand(flags) { - let args = [ +function _buildLatexCommand(mainFile, opts = {}) { + const command = [] + + if (Settings.clsi?.strace) { + command.push('strace', '-o', 'strace', '-ff') + } + + if (Settings.clsi?.latexmkCommandPrefix) { + command.push(...Settings.clsi.latexmkCommandPrefix) + } + + // Basic command and flags + command.push( 'latexmk', '-cd', - '-f', '-jobname=output', '-auxdir=$COMPILE_DIR', '-outdir=$COMPILE_DIR', '-synctex=1', - '-interaction=batchmode', - ] - if (flags) { - args = args.concat(flags) + '-interaction=batchmode' + ) + + // Stop on first error option + if (opts.stopOnFirstError) { + command.push('-halt-on-error') + } else { + // Run all passes despite errors + command.push('-f') } - return (Settings.clsi?.latexmkCommandPrefix || []).concat(args) -} -function _pdflatexCommand(mainFile, flags) { - return _latexmkBaseCommand(flags).concat([ - '-pdf', - Path.join('$COMPILE_DIR', mainFile), - ]) -} + // Extra flags + if (opts.flags) { + command.push(...opts.flags) + } -function _latexCommand(mainFile, flags) { - return _latexmkBaseCommand(flags).concat([ - '-pdfdvi', - Path.join('$COMPILE_DIR', mainFile), - ]) -} + // TeX Engine selection + const compilerFlag = COMPILER_FLAGS[opts.compiler] + if (compilerFlag) { + command.push(compilerFlag) + } else { + throw new Error(`unknown compiler: ${opts.compiler}`) + } -function _xelatexCommand(mainFile, flags) { - return _latexmkBaseCommand(flags).concat([ - '-xelatex', - Path.join('$COMPILE_DIR', mainFile), - ]) -} + // We want to run latexmk on the tex file which we will automatically + // generate from the Rtex/Rmd/md file. + mainFile = mainFile.replace(/\.(Rtex|md|Rmd)$/, '.tex') + command.push(Path.join('$COMPILE_DIR', mainFile)) -function _lualatexCommand(mainFile, flags) { - return _latexmkBaseCommand(flags).concat([ - '-lualatex', - Path.join('$COMPILE_DIR', mainFile), - ]) + return command } module.exports = { diff --git a/services/clsi/app/js/RequestParser.js b/services/clsi/app/js/RequestParser.js index bc35d75060..2b817b0aeb 100644 --- a/services/clsi/app/js/RequestParser.js +++ b/services/clsi/app/js/RequestParser.js @@ -61,6 +61,14 @@ function parse(body, callback) { default: false, type: 'boolean', }) + response.stopOnFirstError = _parseAttribute( + 'stopOnFirstError', + compile.options.stopOnFirstError, + { + default: false, + type: 'boolean', + } + ) response.check = _parseAttribute('check', compile.options.check, { type: 'string', }) diff --git a/services/clsi/test/unit/js/CompileManagerTests.js b/services/clsi/test/unit/js/CompileManagerTests.js index 35c26217b1..237e938a58 100644 --- a/services/clsi/test/unit/js/CompileManagerTests.js +++ b/services/clsi/test/unit/js/CompileManagerTests.js @@ -135,6 +135,7 @@ describe('CompileManager', function () { imageName: (this.image = 'example.com/image'), flags: (this.flags = ['-file-line-error']), compileGroup: (this.compileGroup = 'compile-group'), + stopOnFirstError: false, } this.env = {} }) @@ -187,6 +188,7 @@ describe('CompileManager', function () { flags: this.flags, environment: this.env, compileGroup: this.compileGroup, + stopOnFirstError: this.request.stopOnFirstError, }) .should.equal(true) }) @@ -240,6 +242,7 @@ describe('CompileManager', function () { CHKTEX_ULIMIT_OPTIONS: '-t 5 -v 64000', }, compileGroup: this.compileGroup, + stopOnFirstError: this.request.stopOnFirstError, }) .should.equal(true) }) @@ -263,6 +266,7 @@ describe('CompileManager', function () { flags: this.flags, environment: this.env, compileGroup: this.compileGroup, + stopOnFirstError: this.request.stopOnFirstError, }) .should.equal(true) }) diff --git a/services/clsi/test/unit/js/LatexRunnerTests.js b/services/clsi/test/unit/js/LatexRunnerTests.js index af494b107b..0ac3c299ec 100644 --- a/services/clsi/test/unit/js/LatexRunnerTests.js +++ b/services/clsi/test/unit/js/LatexRunnerTests.js @@ -1,29 +1,34 @@ const SandboxedModule = require('sandboxed-module') const sinon = require('sinon') const { expect } = require('chai') -const modulePath = require('path').join( + +const MODULE_PATH = require('path').join( __dirname, '../../../app/js/LatexRunner' ) describe('LatexRunner', function () { beforeEach(function () { - this.LatexRunner = SandboxedModule.require(modulePath, { + this.Settings = { + docker: { + socketPath: '/var/run/docker.sock', + }, + } + this.commandRunnerOutput = { + stdout: 'this is stdout', + stderr: 'this is stderr', + } + this.CommandRunner = { + run: sinon.stub().yields(null, this.commandRunnerOutput), + } + this.fs = { + writeFile: sinon.stub().yields(), + } + this.LatexRunner = SandboxedModule.require(MODULE_PATH, { requires: { - '@overleaf/settings': (this.Settings = { - docker: { - socketPath: '/var/run/docker.sock', - }, - }), - './Metrics': { - Timer: class Timer { - done() {} - }, - }, - './CommandRunner': (this.CommandRunner = {}), - fs: (this.fs = { - writeFile: sinon.stub().callsArg(2), - }), + '@overleaf/settings': this.Settings, + './CommandRunner': this.CommandRunner, + fs: this.fs, }, }) @@ -35,57 +40,70 @@ describe('LatexRunner', function () { this.callback = sinon.stub() this.project_id = 'project-id-123' this.env = { foo: '123' } + this.timeout = 42000 + this.flags = [] + this.stopOnFirstError = false + + this.call = function (callback) { + this.LatexRunner.runLatex( + this.project_id, + { + directory: this.directory, + mainFile: this.mainFile, + compiler: this.compiler, + timeout: this.timeout, + image: this.image, + environment: this.env, + compileGroup: this.compileGroup, + flags: this.flags, + stopOnFirstError: this.stopOnFirstError, + }, + (error, output, stats, timings) => { + this.timings = timings + callback(error) + } + ) + } }) describe('runLatex', function () { - beforeEach(function () { - this.CommandRunner.run = sinon.stub().callsArgWith(7, null, { - stdout: 'this is stdout', - stderr: 'this is stderr', - }) - }) - describe('normally', function () { beforeEach(function (done) { - this.LatexRunner.runLatex( - this.project_id, - { - directory: this.directory, - mainFile: this.mainFile, - compiler: this.compiler, - timeout: (this.timeout = 42000), - image: this.image, - environment: this.env, - compileGroup: this.compileGroup, - }, - (error, output, stats, timings) => { - this.timings = timings - done(error) - } - ) + this.call(done) }) it('should run the latex command', function () { - this.CommandRunner.run - .calledWith( - this.project_id, - sinon.match.any, - this.directory, - this.image, - this.timeout, - this.env, - this.compileGroup - ) - .should.equal(true) + this.CommandRunner.run.should.have.been.calledWith( + this.project_id, + [ + 'latexmk', + '-cd', + '-jobname=output', + '-auxdir=$COMPILE_DIR', + '-outdir=$COMPILE_DIR', + '-synctex=1', + '-interaction=batchmode', + '-f', + '-pdf', + '$COMPILE_DIR/main-file.tex', + ], + this.directory, + this.image, + this.timeout, + this.env, + this.compileGroup + ) }) it('should record the stdout and stderr', function () { - this.fs.writeFile - .calledWith(this.directory + '/' + 'output.stdout', 'this is stdout') - .should.equal(true) - this.fs.writeFile - .calledWith(this.directory + '/' + 'output.stderr', 'this is stderr') - .should.equal(true) + this.fs.writeFile.should.have.been.calledWith( + this.directory + '/' + 'output.stdout', + 'this is stdout' + ) + this.fs.writeFile.should.have.been.calledWith( + this.directory + '/' + 'output.stderr', + 'this is stderr' + ) }) it('should not record cpu metrics', function () { @@ -95,32 +113,36 @@ describe('LatexRunner', function () { }) }) + describe('with a different compiler', function () { + beforeEach(function (done) { + this.compiler = 'lualatex' + this.call(done) + }) + + it('should set the appropriate latexmk flag', function () { + this.CommandRunner.run.should.have.been.calledWith(this.project_id, [ + 'latexmk', + '-cd', + '-jobname=output', + '-auxdir=$COMPILE_DIR', + '-outdir=$COMPILE_DIR', + '-synctex=1', + '-interaction=batchmode', + '-f', + '-lualatex', + '$COMPILE_DIR/main-file.tex', + ]) + }) + }) + describe('with time -v', function () { beforeEach(function (done) { - this.CommandRunner.run = sinon.stub().callsArgWith(7, null, { - stdout: 'this is stdout', - stderr: - '\tCommand being timed: "sh -c timeout 1 yes > /dev/null"\n' + - '\tUser time (seconds): 0.28\n' + - '\tSystem time (seconds): 0.70\n' + - '\tPercent of CPU this job got: 98%\n', - }) - this.LatexRunner.runLatex( - this.project_id, - { - directory: this.directory, - mainFile: this.mainFile, - compiler: this.compiler, - timeout: (this.timeout = 42000), - image: this.image, - environment: this.env, - compileGroup: this.compileGroup, - }, - (error, output, stats, timings) => { - this.timings = timings - done(error) - } - ) + this.commandRunnerOutput.stderr = + '\tCommand being timed: "sh -c timeout 1 yes > /dev/null"\n' + + '\tUser time (seconds): 0.28\n' + + '\tSystem time (seconds): 0.70\n' + + '\tPercent of CPU this job got: 98%\n' + this.call(done) }) it('should record cpu metrics', function () { @@ -131,18 +153,9 @@ describe('LatexRunner', function () { }) describe('with an .Rtex main file', function () { - beforeEach(function () { - this.LatexRunner.runLatex( - this.project_id, - { - directory: this.directory, - mainFile: 'main-file.Rtex', - compiler: this.compiler, - image: this.image, - timeout: (this.timeout = 42000), - }, - this.callback - ) + beforeEach(function (done) { + this.mainFile = 'main-file.Rtex' + this.call(done) }) it('should run the latex command on the equivalent .tex file', function () { @@ -153,19 +166,9 @@ describe('LatexRunner', function () { }) describe('with a flags option', function () { - beforeEach(function () { - this.LatexRunner.runLatex( - this.project_id, - { - directory: this.directory, - mainFile: this.mainFile, - compiler: this.compiler, - image: this.image, - timeout: (this.timeout = 42000), - flags: ['-shell-restricted', '-halt-on-error'], - }, - this.callback - ) + beforeEach(function (done) { + this.flags = ['-shell-restricted', '-halt-on-error'] + this.call(done) }) it('should include the flags in the command', function () { @@ -178,5 +181,27 @@ describe('LatexRunner', function () { flags[1].should.equal('-halt-on-error') }) }) + + describe('with the stopOnFirstError option', function () { + beforeEach(function (done) { + this.stopOnFirstError = true + this.call(done) + }) + + it('should set the appropriate flags', function () { + this.CommandRunner.run.should.have.been.calledWith(this.project_id, [ + 'latexmk', + '-cd', + '-jobname=output', + '-auxdir=$COMPILE_DIR', + '-outdir=$COMPILE_DIR', + '-synctex=1', + '-interaction=batchmode', + '-halt-on-error', + '-pdf', + '$COMPILE_DIR/main-file.tex', + ]) + }) + }) }) }) diff --git a/services/web/app/src/Features/Compile/ClsiManager.js b/services/web/app/src/Features/Compile/ClsiManager.js index 5facba5c43..8466b32aaf 100644 --- a/services/web/app/src/Features/Compile/ClsiManager.js +++ b/services/web/app/src/Features/Compile/ClsiManager.js @@ -871,7 +871,8 @@ const ClsiManager = { compiler: project.compiler, timeout: options.timeout, imageName: project.imageName, - draft: !!options.draft, + draft: Boolean(options.draft), + stopOnFirstError: Boolean(options.stopOnFirstError), check: options.check, syncType: options.syncType, syncState: options.syncState, diff --git a/services/web/app/src/Features/Compile/CompileController.js b/services/web/app/src/Features/Compile/CompileController.js index 7f237d0b1c..cd13a3510f 100644 --- a/services/web/app/src/Features/Compile/CompileController.js +++ b/services/web/app/src/Features/Compile/CompileController.js @@ -54,6 +54,9 @@ module.exports = CompileController = { if (req.body.draft) { options.draft = req.body.draft } + if (req.body.stopOnFirstError) { + options.stopOnFirstError = req.body.stopOnFirstError + } if (['validate', 'error', 'silent'].includes(req.body.check)) { options.check = req.body.check } diff --git a/services/web/frontend/js/features/pdf-preview/util/compiler.js b/services/web/frontend/js/features/pdf-preview/util/compiler.js index 3bdb084898..3d5fc2acce 100644 --- a/services/web/frontend/js/features/pdf-preview/util/compiler.js +++ b/services/web/frontend/js/features/pdf-preview/util/compiler.js @@ -41,6 +41,7 @@ export default class DocumentCompiler { this.currentDoc = null this.error = undefined this.timer = 0 + this.stopOnFirstError = false this.debouncedAutoCompile = debounce( () => { @@ -82,20 +83,22 @@ export default class DocumentCompiler { const t0 = performance.now() + const body = { + rootDoc_id: this.getRootDocOverrideId(), + draft: this.draft, + check: 'silent', // NOTE: 'error' and 'validate' are possible, but unused + // use incremental compile for all users but revert to a full compile + // if there was previously a server error + incrementalCompilesEnabled: !this.error, + } + if (getMeta('ol-showStopOnFirstError')) { + body.stopOnFirstError = this.stopOnFirstError + } const data = await postJSON( `/project/${this.projectId}/compile?${params}`, - { - body: { - rootDoc_id: this.getRootDocOverrideId(), - draft: this.draft, - check: 'silent', // NOTE: 'error' and 'validate' are possible, but unused - // use incremental compile for all users but revert to a full compile - // if there was previously a server error - incrementalCompilesEnabled: !this.error, - }, - signal: this.signal, - } + { body, signal: this.signal } ) + const compileTimeClientE2E = performance.now() - t0 const { firstRenderDone } = trackPdfDownload(data, compileTimeClientE2E) this.setFirstRenderDone(() => firstRenderDone) diff --git a/services/web/frontend/js/shared/context/local-compile-context.js b/services/web/frontend/js/shared/context/local-compile-context.js index 99b2d8f5aa..a0d5683df1 100644 --- a/services/web/frontend/js/shared/context/local-compile-context.js +++ b/services/web/frontend/js/shared/context/local-compile-context.js @@ -224,6 +224,11 @@ export function LocalCompileProvider({ children }) { compiler.draft = draft }, [compiler, draft]) + // keep stop on first error setting in sync with the compiler + useEffect(() => { + compiler.stopOnFirstError = stopOnFirstError + }, [compiler, stopOnFirstError]) + // pass the "uncompiled" value up into the scope for use outside this context provider useEffect(() => { setUncompiled(changedAt > 0) diff --git a/services/web/test/unit/src/Compile/ClsiManagerTests.js b/services/web/test/unit/src/Compile/ClsiManagerTests.js index 7dbef6d2ed..83921763e5 100644 --- a/services/web/test/unit/src/Compile/ClsiManagerTests.js +++ b/services/web/test/unit/src/Compile/ClsiManagerTests.js @@ -623,6 +623,7 @@ describe('ClsiManager', function () { enablePdfCaching: false, flags: undefined, metricsMethod: 'standard', + stopOnFirstError: false, }, // "01234567890abcdef" rootResourcePath: 'main.tex', resources: [ @@ -718,6 +719,7 @@ describe('ClsiManager', function () { enablePdfCaching: false, flags: undefined, metricsMethod: 'priority', + stopOnFirstError: false, }, rootResourcePath: 'main.tex', resources: [