From 3db513cfc9e254483a0abbc375d8bf1cd7b97e7c Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 15 May 2020 16:08:10 +0100 Subject: [PATCH 1/4] record latexmk output --- services/clsi/app/js/CompileManager.js | 5 +++++ services/clsi/app/js/ResourceWriter.js | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/services/clsi/app/js/CompileManager.js b/services/clsi/app/js/CompileManager.js index 3bf54bc75b..500adfcea0 100644 --- a/services/clsi/app/js/CompileManager.js +++ b/services/clsi/app/js/CompileManager.js @@ -217,6 +217,11 @@ module.exports = CompileManager = { error = new Error('compilation') error.validate = 'fail' } + // make top-level output accesible to user, write in background for simplicity + if (output != null) { + fs.writeFile(Path.join(compileDir, "output.stdout"), output.stdout, () => { }) + fs.writeFile(Path.join(compileDir, "output.stderr"), output.stderr, () => { }) + } // compile was killed by user, was a validation, or a compile which failed validation if ( (error != null ? error.terminated : undefined) || diff --git a/services/clsi/app/js/ResourceWriter.js b/services/clsi/app/js/ResourceWriter.js index ed3a1e8760..750be323ba 100644 --- a/services/clsi/app/js/ResourceWriter.js +++ b/services/clsi/app/js/ResourceWriter.js @@ -201,6 +201,10 @@ module.exports = ResourceWriter = { // knitr cache should_delete = false } + if (path.match(/^output.(stdout|stderr)$/)) { + // latexmk output + should_delete = true + } if (path.match(/^output-.*/)) { // Tikz cached figures (default case) should_delete = false From 63770bf3905d7ed3e0009f501acd78c035df53e1 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 20 May 2020 11:45:29 +0100 Subject: [PATCH 2/4] clean up the stdout/stderr recording --- services/clsi/app/js/CompileManager.js | 5 ---- services/clsi/app/js/LatexRunner.js | 32 ++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/services/clsi/app/js/CompileManager.js b/services/clsi/app/js/CompileManager.js index 500adfcea0..3bf54bc75b 100644 --- a/services/clsi/app/js/CompileManager.js +++ b/services/clsi/app/js/CompileManager.js @@ -217,11 +217,6 @@ module.exports = CompileManager = { error = new Error('compilation') error.validate = 'fail' } - // make top-level output accesible to user, write in background for simplicity - if (output != null) { - fs.writeFile(Path.join(compileDir, "output.stdout"), output.stdout, () => { }) - fs.writeFile(Path.join(compileDir, "output.stderr"), output.stderr, () => { }) - } // compile was killed by user, was a validation, or a compile which failed validation if ( (error != null ? error.terminated : undefined) || diff --git a/services/clsi/app/js/LatexRunner.js b/services/clsi/app/js/LatexRunner.js index 972f1fe7c3..fe737c76c6 100644 --- a/services/clsi/app/js/LatexRunner.js +++ b/services/clsi/app/js/LatexRunner.js @@ -19,6 +19,7 @@ const Settings = require('settings-sharelatex') const logger = require('logger-sharelatex') const Metrics = require('./Metrics') const CommandRunner = require('./CommandRunner') +const fs = require('fs') const ProcessTable = {} // table of currently running jobs (pids or docker container names) @@ -127,9 +128,36 @@ module.exports = LatexRunner = { : undefined, x5 => x5[1] ) || 0 - return callback(error, output, stats, timings) + // record output files + LatexRunner.writeLogOutput(project_id, directory, output, () => { + return callback(error, output, stats, timings) + }) + })) + }, + + writeLogOutput(project_id, directory, output, callback) { + if (!output) { + return callback() + } + // internal method for writing non-empty log files + function _writeFile(file, content, cb) { + if (content && content.length > 0) { + fs.writeFile(file, content, (err) => { + if (err) { + logger.error({ project_id, file }, "error writing log file") // don't fail on error + } + cb() + }) + } else { + cb() } - )) + } + // write stdout and stderr, ignoring errors + _writeFile(Path.join(directory, "output.stdout"), output.stdout, () => { + _writeFile(Path.join(directory, "output.stderr"), output.stderr, () => { + callback() + }) + }) }, killLatex(project_id, callback) { From a684619bceada1a806e169f117b6d895de124948 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 20 May 2020 11:52:53 +0100 Subject: [PATCH 3/4] add unit tests --- .../clsi/test/unit/js/LatexRunnerTests.js | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/services/clsi/test/unit/js/LatexRunnerTests.js b/services/clsi/test/unit/js/LatexRunnerTests.js index b468b83183..15902e9b84 100644 --- a/services/clsi/test/unit/js/LatexRunnerTests.js +++ b/services/clsi/test/unit/js/LatexRunnerTests.js @@ -37,7 +37,10 @@ describe('LatexRunner', function() { done() {} }) }, - './CommandRunner': (this.CommandRunner = {}) + './CommandRunner': (this.CommandRunner = {}), + 'fs': (this.fs = { + writeFile: sinon.stub().callsArg(2) + }) } }) @@ -83,6 +86,21 @@ describe('LatexRunner', function() { ) .should.equal(true) }) + + 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) + }) }) describe('with an .Rtex main file', function() { From 6d5dfb7758a2fc682c3f9578455c7f2fc71fac7f Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 20 May 2020 14:12:08 +0100 Subject: [PATCH 4/4] clean up log file deletion and add unit test --- services/clsi/app/js/ResourceWriter.js | 8 +++----- .../clsi/test/unit/js/ResourceWriterTests.js | 20 ++++++++++++++++++- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/services/clsi/app/js/ResourceWriter.js b/services/clsi/app/js/ResourceWriter.js index 750be323ba..97e971e1d5 100644 --- a/services/clsi/app/js/ResourceWriter.js +++ b/services/clsi/app/js/ResourceWriter.js @@ -201,10 +201,6 @@ module.exports = ResourceWriter = { // knitr cache should_delete = false } - if (path.match(/^output.(stdout|stderr)$/)) { - // latexmk output - should_delete = true - } if (path.match(/^output-.*/)) { // Tikz cached figures (default case) should_delete = false @@ -235,7 +231,9 @@ module.exports = ResourceWriter = { path === 'output.pdf' || path === 'output.dvi' || path === 'output.log' || - path === 'output.xdv' + path === 'output.xdv' || + path === 'output.stdout' || + path === 'output.stderr' ) { should_delete = true } diff --git a/services/clsi/test/unit/js/ResourceWriterTests.js b/services/clsi/test/unit/js/ResourceWriterTests.js index a632c1bdff..1080765b7b 100644 --- a/services/clsi/test/unit/js/ResourceWriterTests.js +++ b/services/clsi/test/unit/js/ResourceWriterTests.js @@ -230,6 +230,12 @@ describe('ResourceWriter', function() { { path: '_markdown_main/30893013dec5d869a415610079774c2f.md.tex', type: 'tex' + }, + { + path: 'output.stdout' + }, + { + path: 'output.stderr' } ] this.resources = 'mock-resources' @@ -256,7 +262,19 @@ describe('ResourceWriter', function() { .should.equal(true) }) - it('should delete the extra files', function() { + it('should delete the stdout log file', function () { + return this.ResourceWriter._deleteFileIfNotDirectory + .calledWith(path.join(this.basePath, 'output.stdout')) + .should.equal(true) + }) + + it('should delete the stderr log file', function () { + return this.ResourceWriter._deleteFileIfNotDirectory + .calledWith(path.join(this.basePath, 'output.stderr')) + .should.equal(true) + }) + + it('should delete the extra files', function () { return this.ResourceWriter._deleteFileIfNotDirectory .calledWith(path.join(this.basePath, 'extra/file.tex')) .should.equal(true)