Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix how we build and find our standard noun images (email, telephone, etc). #15

Open
kfogel opened this issue Apr 8, 2020 · 2 comments

Comments

@kfogel
Copy link
Member

kfogel commented Apr 8, 2020

This is a refiling of issue #11. We understand the problem much better now than when we filed that issue, so it's good to have a clean re-explanation.

In latex/otsletterhead2.ltx there are these two commands:

  \includesvg[height=0.85em]{noun_Email_3027864_ltgreen.svg}
  ...
  \includesvg[height=1.5em]{noun_Telephone_2591756_ltgreen.svg}

This usage of \includesvg has two problems:

  1. Those commands rely on finding the .svg files in the current working directory. But the files don't live in the directory where you're building your PDF; they live here in the ots-doctools project (in the latex/ subdirectory, currently, although probably we should move them to a top-level images/ directory at some point).

  2. The includesvg command relies on the LaTeX write18 command to invoke inkscape as a subprocess (and possibly some other things, such as ImageMagick convert and even pdflatex itself -- the full conversion process is quite complex). However, write18 only works if at the top level we pass -shell-escape to latexmk, and passing -shell-escape is a security hole: if someone were to build a maliciously-crafted LaTeX document, then the document could cause arbitrary commands to be run.

So, those are the problems. Now let's stroll -- just part-way -- down solution lane:

You'd think it would be easy to solve (1) by adjusting the graphics search path via \graphicspath{some_path}, but the parameter to that command is either absolute, relative to the current working directory, or relative to the top TeX file (with "./" prefix; see docs). None of these do quite what we want, which would be "relative to ${OTS_DOCTOOLS_DIR}" So, jury is still out on how to properly solve (1).

Meanhile, (2) is also tricky. The obvious answer would be to move the image-conversion pipeline out of the includesvg LaTeX package and to somewhere safe, like our Makefile. But determining exactly what external commands includesvg runs, so we can duplicate them, is not trivial. I've looked at the source code of the svg package, and it's a maze of write18 calls. If we implement all the workarounds (copy the source .svg files to cwd and add the --shell-escape options to the ots-doctools Makefile), we see that a successful run produces two outputs based on noun_Email_3027864_ltgreen_svg: noun_Email_3027864_ltgreen_svg-tex.pdf and noun_Email_3027864_ltgreen_svg-tex.pdf_tex. However, just running Inkscape manually (e.g., inkscape -D -z -f noun_Email_3027864_ltgreen.svg --export-pdf=noun_Email_3027864_ltgreen.pdf --export-latex) doesn't produce those files; instead, it produces noun_Email_3027864_ltgreen.pdf and noun_Email_3027864_ltgreen.pdf_tex. Clearly the svg package is doing some Stuff, and if we want to duplicate that Stuff we'll need to learn exactly what it is.

For now, I've "solved" (1) via a Makefile kluge that copies the .svg source files to cwd before latexmk is run, and "solved" (2) by adding the -shell-escape option back. See commit 8d25e42.

kfogel added a commit that referenced this issue Apr 10, 2020
This fixes the problems described in issue #11, but is not a solution
that anyone should invite home to dinner.  See issue #15 for details.
@mrpiggi
Copy link

mrpiggi commented May 8, 2020

Just wanted to mention, that package svg now supports Inkscape 1.0 CLI and is even capable to distinguish between the deprecated interface and the current one (mrpiggi/svg#20).

@kfogel
Copy link
Member Author

kfogel commented May 11, 2020

Thank you, @mrpiggi! This is great -- we'll upgrade. It'll be nice to be able to undo the compatibility shim @jvasile put in place temporarily.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants