tectonic-typesetting / tectonic

A modernized, complete, self-contained TeX/LaTeX engine, powered by XeTeX and TeXLive.
https://tectonic-typesetting.github.io/
Other
3.99k stars 162 forks source link

shell escape: fixes creation of temporary subdirectories #1151

Closed derchr closed 2 months ago

derchr commented 9 months ago

Fixes #1149. The MR applies the fix as described in https://github.com/tectonic-typesetting/tectonic/issues/1149#issuecomment-1943822679. I added also added some error reporting around the call of std::fs::create_dir_all.

codecov[bot] commented 9 months ago

Codecov Report

Attention: Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 46.34%. Comparing base (fed29a8) to head (7d2839e). Report is 73 commits behind head on master.

Files with missing lines Patch % Lines
src/driver.rs 40.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1151 +/- ## ========================================== - Coverage 46.34% 46.34% -0.01% ========================================== Files 170 170 Lines 65120 65125 +5 ========================================== + Hits 30180 30182 +2 - Misses 34940 34943 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rm-dr commented 9 months ago

I think this looks good. The only tests that fail are codecov and clippy, both of which are caused by code outside this PR.

This does remind me of #1145, though. We probably don't want to create the user's home directory if it doesn't already exist. The directory here will usually be a temporary directory, but this would create a home directory with unexpected permissions if ShellEscapeMode::ExternallyManagedDir points to a subdir of a non-existing home.

pkgw commented 9 months ago

Maybe the cap_std crate, mentioned in #1106, would also be helpful for limiting the depth of directory creation? It seems like it might be overkill, but if we're using it anyway, maybe that's OK.

rm-dr commented 9 months ago

cap_std isn't in Tectonic yet, and is indeed a bit overkill. This could be handled with some fairly simple logic, once we decide on what Tectonic should do when there is no home directory.

Zelzahn commented 8 months ago

Hey, what's the status of this PR? And what's still needed until it can be merged?