latex: recover from a missing SageTeX input, and reject single quotes in filenames - #300
Draft
haraldschilly wants to merge 2 commits into
Draft
latex: recover from a missing SageTeX input, and reject single quotes in filenames#300haraldschilly wants to merge 2 commits into
haraldschilly wants to merge 2 commits into
Conversation
…ut is missing Port of the fix in cocalc.com PR sagemathinc/cocalc#8681 (issue sagemathinc/cocalc#8680): `sagetex -> clean -> build` aborted the build with a confusing error, and the only workaround was "force build". The cocalc.com bug was `sha1sum` exiting nonzero under `err_on_exit: true`. That code no longer exists here -- the LaTeX pipeline moved into `@cocalc/app-document-build` and hashes via `runtime.hash()` -- but the same failure survived the move in a new form: `runtime.hash()` goes through `openRegularFile()`, which throws `ENOENT` for a missing file, and the pipeline turned that into a `transport` diagnostic and returned early, killing the build. Root cause of the missing file is unchanged: the LaTeX stage uses `aggregate_key: generation` when not forced, so a non-forced build after a clean can be served from the aggregate cache. Its cached stdout still mentions `sagetex.sty`, so the pipeline enters the SageTeX branch, but `.sagetex.sage` was never regenerated. The existing forced re-run only covers the `outputDirectory != null` path, leaving the `output_directory: null` case broken. Now the pipeline probes `runtime.exists()` first and, when the generated input is absent, re-runs LaTeX forced to regenerate it rather than giving up. A file that is still missing afterwards leaves the hash empty, which drops `aggregate_key` so the SageTeX stage cannot be deduped against a run whose input we could not identify; real I/O errors still surface as `transport` diagnostics. `FakeRuntime.hash()` previously returned a fixed value for any path, so no test could observe this. It now mirrors the real runtime and throws `ENOENT` for files absent from `existing`, which is what makes the new regression test bite.
…name Port of the filename-hardening half of cocalc.com PR sagemathinc/cocalc#8681. Adds `is_bad_latex_filename()` to `@cocalc/util/misc` and uses it for both copies of the previous bare `/\s\s+/` check: `Actions.init_bad_filename()` in the frontend and the guard at the top of `runLatexPipeline()`. Both failure modes were verified locally against TeX Live 2026 rather than assumed from the upstream PR, because the cocalc.com fix guarded a `sha1sum` call that no longer exists here: - Consecutive spaces are a TeX limitation, unchanged since cocalc#3230 was filed against TeX Live 2017. `pdflatex "3 spaces.tex"` still reports `I can't find file "3 spaces.tex"` -- TeX collapses the run of spaces while resolving the name. Not something CoCalc can escape its way around, so the check stays. - Single quotes are *not* a TeX limitation: `latexmk "author's-notes.tex"` builds fine when the argument reaches the process via execve. They break CoCalc's own command assembly. `ensureTargetPathIsCorrect()` wraps the filename as `'${filename}'` and `pythontexStage()` builds `... '${base}'`, both run with `bash: true`, so a quote in the name terminates the string early: bash reports `unexpected EOF while looking for matching '`. Rejecting quotes matches upstream and keeps the two checks in one place. Properly escaping the filename for the `bash: true` command strings would be the better long-term fix and would let these names build, but that touches command construction well beyond this port.
Contributor
|
Found 10 test failures on Blacksmith runners: Failures
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light.png)
Port of cocalc.com PR sagemathinc/cocalc#8681
(issue sagemathinc/cocalc#8680) to cocalc-ai.
Neither half applied verbatim: the LaTeX build moved out of the frontend into
@cocalc/app-document-build, and thesha1sumcall the upstream PR patched no longerexists. Both underlying problems are live here anyway, in new forms.
1. Missing SageTeX input no longer kills the build
Upstream,
sagetex -> clean -> buildfailed becausesha1sumexited nonzero undererr_on_exit: true; the only workaround was "force build".Here the same failure survived the architecture move:
runtime.hash()goes throughopenRegularFile(), which throwsENOENT, and the pipeline turned that into atransportdiagnostic and returned early, killing the build.The trigger is unchanged. The LaTeX stage uses
aggregate_key: generationwhen notforced, so a non-forced build after a clean can be served from the aggregate cache. Its
cached stdout still mentions
sagetex.sty, so the pipeline enters the SageTeX branch,but
.sagetex.sagewas never regenerated. The existing forced re-run only covers theoutputDirectory != nullpath, leavingoutput_directory: nullbroken.Now the pipeline probes
runtime.exists()first and, when the generated input is absent,re-runs LaTeX forced to regenerate it. A file still missing afterwards leaves the hash
empty, which drops
aggregate_keyso the SageTeX stage cannot be deduped against a runwhose input could not be identified. Real I/O errors still surface as
transportdiagnostics.
Worth a look during review:
FakeRuntime.hash()previously returned a fixed value forany path, so no test could observe this class of bug. It now mirrors the real runtime
and throws
ENOENTfor files absent fromexisting— that is what makes the newregression test bite. Verified by reverting the fix: the test fails with the build
stopping dead after a single LaTeX pass.
2. Shared
is_bad_latex_filenameAdds
is_bad_latex_filename()to@cocalc/util/miscand uses it for both copies of theprevious bare
/\s\s+/check —Actions.init_bad_filename()in the frontend and theguard at the top of
runLatexPipeline().Both failure modes were verified locally against TeX Live 2026 rather than assumed from
upstream, and they turned out to have different causes:
against TeX Live 2017.
pdflatex "3 spaces.tex"still reportsI can't find file "3 spaces.tex"— TeX collapses the run of spaces while resolvingthe name. Not something CoCalc can escape around, so the check stays.
latexmk "author's-notes.tex"buildsfine when the argument reaches the process via execve. They break CoCalc's own command
assembly.
ensureTargetPathIsCorrect()wraps the filename as'${filename}'andpythontexStage()builds... '${base}', both run withbash: true, so a quoteterminates the string early and bash reports
unexpected EOF while looking for matching '.Rejecting quotes matches upstream and keeps the two checks in one place. Properly
escaping the filename for the
bash: truecommand strings would be the better long-termfix and would let these names build, but that touches command construction well beyond
this port — flagging it rather than doing it here.
Validation
document-build(21 tests),util/misc(65),latex-editor(214),tscon all threepackages,
lint:frontend— all pass.Manually verified in the local workspace runtime: a
.texfile whose name contains twoconsecutive spaces is rejected immediately on Build. The SageTeX half could not be
exercised end-to-end (no local sage install), so it rests on the regression test.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.