From 1ddf9283f2ab8316cd105b10c0e6f77f83ab6f32 Mon Sep 17 00:00:00 2001 From: Michael Mazour Date: Fri, 10 May 2019 16:51:40 +0100 Subject: [PATCH] Add flags option to request JSON Adds a `flags` parameter to the request JSON, appearing under the `compile.options` key (alongside such stalwarts as `compiler`, `timeout`, etc.). This is primarily to support `-file-line-error` as an option, but could have other uses as well. `flags` should be an array of strings, or absent. If supplied, the listed arguments are added to the base latexmk command. --- .../clsi/app/coffee/CompileManager.coffee | 3 +- services/clsi/app/coffee/LatexRunner.coffee | 41 ++++++++++--------- services/clsi/app/coffee/RequestParser.coffee | 8 +++- .../unit/coffee/CompileManagerTests.coffee | 18 ++++---- .../test/unit/coffee/LatexRunnerTests.coffee | 18 ++++++++ .../unit/coffee/RequestParserTests.coffee | 32 +++++++++++---- 6 files changed, 82 insertions(+), 38 deletions(-) diff --git a/services/clsi/app/coffee/CompileManager.coffee b/services/clsi/app/coffee/CompileManager.coffee index 0a98e87db4..ad924e2b1e 100644 --- a/services/clsi/app/coffee/CompileManager.coffee +++ b/services/clsi/app/coffee/CompileManager.coffee @@ -93,6 +93,7 @@ module.exports = CompileManager = compiler: request.compiler timeout: request.timeout image: request.imageName + flags: request.flags environment: env }, (error, output, stats, timings) -> # request was for validation only @@ -130,7 +131,7 @@ module.exports = CompileManager = return callback(error) if error? OutputCacheManager.saveOutputFiles outputFiles, compileDir, (error, newOutputFiles) -> callback null, newOutputFiles - + stopCompile: (project_id, user_id, callback = (error) ->) -> compileName = getCompileName(project_id, user_id) LatexRunner.killLatex compileName, callback diff --git a/services/clsi/app/coffee/LatexRunner.coffee b/services/clsi/app/coffee/LatexRunner.coffee index 3571af20bf..29433f83e3 100644 --- a/services/clsi/app/coffee/LatexRunner.coffee +++ b/services/clsi/app/coffee/LatexRunner.coffee @@ -8,27 +8,27 @@ ProcessTable = {} # table of currently running jobs (pids or docker container n module.exports = LatexRunner = runLatex: (project_id, options, callback = (error) ->) -> - {directory, mainFile, compiler, timeout, image, environment} = options + {directory, mainFile, compiler, timeout, image, environment, flags} = options compiler ||= "pdflatex" timeout ||= 60000 # milliseconds - logger.log directory: directory, compiler: compiler, timeout: timeout, mainFile: mainFile, environment: environment, "starting compile" + logger.log directory: directory, compiler: compiler, timeout: timeout, mainFile: mainFile, environment: environment, flags:flags, "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 = LatexRunner._pdflatexCommand mainFile + command = LatexRunner._pdflatexCommand mainFile, flags else if compiler == "latex" - command = LatexRunner._latexCommand mainFile + command = LatexRunner._latexCommand mainFile, flags else if compiler == "xelatex" - command = LatexRunner._xelatexCommand mainFile + command = LatexRunner._xelatexCommand mainFile, flags else if compiler == "lualatex" - command = LatexRunner._lualatexCommand mainFile + command = LatexRunner._lualatexCommand mainFile, flags else return callback new Error("unknown compiler: #{compiler}") - + if Settings.clsi?.strace command = ["strace", "-o", "strace", "-ff"].concat(command) @@ -63,31 +63,32 @@ module.exports = LatexRunner = else CommandRunner.kill ProcessTable[id], callback - _latexmkBaseCommand: (Settings?.clsi?.latexmkCommandPrefix || []).concat([ - "latexmk", "-cd", "-f", "-jobname=output", "-auxdir=$COMPILE_DIR", "-outdir=$COMPILE_DIR", - "-synctex=1","-interaction=batchmode" - ]) + _latexmkBaseCommand: (flags) -> + args = ["latexmk", "-cd", "-f", "-jobname=output", "-auxdir=$COMPILE_DIR", "-outdir=$COMPILE_DIR", "-synctex=1","-interaction=batchmode"] + if flags + args = args.concat(flags) + (Settings?.clsi?.latexmkCommandPrefix || []).concat(args) - _pdflatexCommand: (mainFile) -> - LatexRunner._latexmkBaseCommand.concat [ + _pdflatexCommand: (mainFile, flags) -> + LatexRunner._latexmkBaseCommand(flags).concat [ "-pdf", Path.join("$COMPILE_DIR", mainFile) ] - - _latexCommand: (mainFile) -> - LatexRunner._latexmkBaseCommand.concat [ + + _latexCommand: (mainFile, flags) -> + LatexRunner._latexmkBaseCommand(flags).concat [ "-pdfdvi", Path.join("$COMPILE_DIR", mainFile) ] - _xelatexCommand: (mainFile) -> - LatexRunner._latexmkBaseCommand.concat [ + _xelatexCommand: (mainFile, flags) -> + LatexRunner._latexmkBaseCommand(flags).concat [ "-xelatex", Path.join("$COMPILE_DIR", mainFile) ] - _lualatexCommand: (mainFile) -> - LatexRunner._latexmkBaseCommand.concat [ + _lualatexCommand: (mainFile, flags) -> + LatexRunner._latexmkBaseCommand(flags).concat [ "-lualatex", Path.join("$COMPILE_DIR", mainFile) ] diff --git a/services/clsi/app/coffee/RequestParser.coffee b/services/clsi/app/coffee/RequestParser.coffee index b887ed30b1..c1912f62d9 100644 --- a/services/clsi/app/coffee/RequestParser.coffee +++ b/services/clsi/app/coffee/RequestParser.coffee @@ -12,7 +12,7 @@ module.exports = RequestParser = compile = body.compile compile.options ||= {} - + try response.compiler = @_parseAttribute "compiler", compile.options.compiler, @@ -33,6 +33,10 @@ module.exports = RequestParser = response.check = @_parseAttribute "check", compile.options.check, type: "string" + response.flags = @_parseAttribute "flags", + compile.options.flags, + default: [], + type: "object" # The syncType specifies whether the request contains all # resources (full) or only those resources to be updated @@ -68,7 +72,7 @@ module.exports = RequestParser = originalRootResourcePath = rootResourcePath sanitizedRootResourcePath = RequestParser._sanitizePath(rootResourcePath) response.rootResourcePath = RequestParser._checkPath(sanitizedRootResourcePath) - + for resource in response.resources if resource.path == originalRootResourcePath resource.path = sanitizedRootResourcePath diff --git a/services/clsi/test/unit/coffee/CompileManagerTests.coffee b/services/clsi/test/unit/coffee/CompileManagerTests.coffee index 608a3e55a4..c4b0f85941 100644 --- a/services/clsi/test/unit/coffee/CompileManagerTests.coffee +++ b/services/clsi/test/unit/coffee/CompileManagerTests.coffee @@ -13,7 +13,7 @@ describe "CompileManager", -> "./ResourceWriter": @ResourceWriter = {} "./OutputFileFinder": @OutputFileFinder = {} "./OutputCacheManager": @OutputCacheManager = {} - "settings-sharelatex": @Settings = + "settings-sharelatex": @Settings = path: compilesDir: "/compiles/dir" synctexBaseDir: -> "/compile" @@ -108,6 +108,7 @@ describe "CompileManager", -> compiler: @compiler = "pdflatex" timeout: @timeout = 42000 imageName: @image = "example.com/image" + flags: @flags = ["-file-line-error"] @env = {} @Settings.compileDir = "compiles" @compileDir = "#{@Settings.path.compilesDir}/#{@project_id}-#{@user_id}" @@ -117,7 +118,7 @@ describe "CompileManager", -> @OutputCacheManager.saveOutputFiles = sinon.stub().callsArgWith(2, null, @build_files) @DraftModeManager.injectDraftMode = sinon.stub().callsArg(1) @TikzManager.checkMainFile = sinon.stub().callsArg(3, false) - + describe "normally", -> beforeEach -> @CompileManager.doCompile @request, @callback @@ -135,6 +136,7 @@ describe "CompileManager", -> compiler: @compiler timeout: @timeout image: @image + flags: @flags environment: @env }) .should.equal true @@ -146,15 +148,15 @@ describe "CompileManager", -> it "should return the output files", -> @callback.calledWith(null, @build_files).should.equal true - + it "should not inject draft mode by default", -> @DraftModeManager.injectDraftMode.called.should.equal false - + describe "with draft mode", -> beforeEach -> @request.draft = true @CompileManager.doCompile @request, @callback - + it "should inject the draft mode header", -> @DraftModeManager.injectDraftMode .calledWith(@compileDir + "/" + @rootResourcePath) @@ -173,6 +175,7 @@ describe "CompileManager", -> compiler: @compiler timeout: @timeout image: @image + flags: @flags environment: {'CHKTEX_OPTIONS': '-nall -e9 -e10 -w15 -w16', 'CHKTEX_EXIT_ON_ERROR':1, 'CHKTEX_ULIMIT_OPTIONS': '-t 5 -v 64000'} }) .should.equal true @@ -191,6 +194,7 @@ describe "CompileManager", -> compiler: @compiler timeout: @timeout image: @image + flags: @flags environment: @env }) .should.equal true @@ -297,7 +301,7 @@ describe "CompileManager", -> @CommandRunner.run .calledWith( "#{@project_id}-#{@user_id}", - ['/opt/synctex', "pdf", synctex_path, @page, @h, @v], + ['/opt/synctex', "pdf", synctex_path, @page, @h, @v], "#{@Settings.path.compilesDir}/#{@project_id}-#{@user_id}", @Settings.clsi.docker.image, 60000, @@ -330,7 +334,7 @@ describe "CompileManager", -> @directory = "#{@Settings.path.compilesDir}/#{@project_id}-#{@user_id}" @file_path = "$COMPILE_DIR/#{@file_name}" @command =[ "texcount", "-nocol", "-inc", @file_path, "-out=" + @file_path + ".wc"] - + @CommandRunner.run .calledWith("#{@project_id}-#{@user_id}", @command, @directory, @image, @timeout, {}) .should.equal true diff --git a/services/clsi/test/unit/coffee/LatexRunnerTests.coffee b/services/clsi/test/unit/coffee/LatexRunnerTests.coffee index c26fa642b2..77c6edbcc0 100644 --- a/services/clsi/test/unit/coffee/LatexRunnerTests.coffee +++ b/services/clsi/test/unit/coffee/LatexRunnerTests.coffee @@ -59,3 +59,21 @@ describe "LatexRunner", -> mainFile = command.slice(-1)[0] mainFile.should.equal "$COMPILE_DIR/main-file.tex" + describe "with a flags option", -> + beforeEach -> + @LatexRunner.runLatex @project_id, + directory: @directory + mainFile: @mainFile + compiler: @compiler + image: @image + timeout: @timeout = 42000 + flags: ["-file-line-error", "-halt-on-error"] + @callback + + it "should include the flags in the command", -> + command = @CommandRunner.run.args[0][1] + flags = command.filter (arg) -> + (arg == "-file-line-error") || (arg == "-halt-on-error") + flags.length.should.equal 2 + flags[0].should.equal "-file-line-error" + flags[1].should.equal "-halt-on-error" diff --git a/services/clsi/test/unit/coffee/RequestParserTests.coffee b/services/clsi/test/unit/coffee/RequestParserTests.coffee index f63bc55e3d..e263e49207 100644 --- a/services/clsi/test/unit/coffee/RequestParserTests.coffee +++ b/services/clsi/test/unit/coffee/RequestParserTests.coffee @@ -1,6 +1,7 @@ SandboxedModule = require('sandboxed-module') sinon = require('sinon') require('chai').should() +expect = require('chai').expect modulePath = require('path').join __dirname, '../../../app/js/RequestParser' tk = require("timekeeper") @@ -22,7 +23,7 @@ describe "RequestParser", -> resources: [] @RequestParser = SandboxedModule.require modulePath, requires: "settings-sharelatex": @settings = {} - + afterEach -> tk.reset() @@ -55,7 +56,7 @@ describe "RequestParser", -> beforeEach -> delete @validRequest.compile.options.compiler @RequestParser.parse @validRequest, (error, @data) => - + it "should set the compiler to pdflatex by default", -> @data.compiler.should.equal "pdflatex" @@ -66,6 +67,21 @@ describe "RequestParser", -> it "should set the imageName", -> @data.imageName.should.equal "basicImageName/here:2017-1" + describe "with flags set", -> + beforeEach -> + @validRequest.compile.options.flags = ["-file-line-error"] + @RequestParser.parse @validRequest, (error, @data) => + + it "should set the flags attribute", -> + expect(@data.flags).to.deep.equal ["-file-line-error"] + + describe "with flags not specified", -> + beforeEach -> + @RequestParser.parse @validRequest, (error, @data) => + + it "it should have an empty flags list", -> + expect(@data.flags).to.deep.equal [] + describe "without a timeout specified", -> beforeEach -> delete @validRequest.compile.options.timeout @@ -88,7 +104,7 @@ describe "RequestParser", -> it "should set the timeout (in milliseconds)", -> @data.timeout.should.equal @validRequest.compile.options.timeout * 1000 - + describe "with a resource without a path", -> beforeEach -> delete @validResource.path @@ -175,7 +191,7 @@ describe "RequestParser", -> it "should return the url in the parsed response", -> @data.resources[0].url.should.equal @url - + describe "with a resource with a content attribute", -> beforeEach -> @validResource.content = @content = "Hello world" @@ -185,7 +201,7 @@ describe "RequestParser", -> it "should return the content in the parsed response", -> @data.resources[0].content.should.equal @content - + describe "without a root resource path", -> beforeEach -> delete @validRequest.compile.rootResourcePath @@ -225,13 +241,13 @@ describe "RequestParser", -> } @RequestParser.parse @validRequest, @callback @data = @callback.args[0][1] - + it "should return the escaped resource", -> @data.rootResourcePath.should.equal @goodPath - + it "should also escape the resource path", -> @data.resources[0].path.should.equal @goodPath - + describe "with a root resource path that has a relative path", -> beforeEach -> @validRequest.compile.rootResourcePath = "foo/../../bar.tex"