Skip to content

add -shell-escape flag to xelatex to fix test errors#353

Closed
amueller wants to merge 1 commit intojupyter:masterfrom
amueller:minted_need_shellescape
Closed

add -shell-escape flag to xelatex to fix test errors#353
amueller wants to merge 1 commit intojupyter:masterfrom
amueller:minted_need_shellescape

Conversation

@amueller
Copy link
Copy Markdown
Contributor

Fixes #350.
I'm not entirely sure if this is the right fix. I'm also not sure why travis didn't error on this.

@takluyver
Copy link
Copy Markdown
Member

I suspect Latex isn't installed on Travis, so those tests are skipped. I don't know enough about latex to know if this is the right fix. @michaelpacer, your input would be welcome :-)

@jankatins
Copy link
Copy Markdown
Contributor

I cherry-picked this commit into #356

@mpacer
Copy link
Copy Markdown
Member

mpacer commented Aug 17, 2016

So everything I can find on this would suggest this as fine as anything else (in that what we're doing is allowing LaTeX to have access to the shell directly through the use of the \write18{} command). If we don't trust one of the third party libraries that we're using by default, it's probably better we run into that problem in a Travis build than on a user build.

@takluyver
Copy link
Copy Markdown
Member

I don't know much about the way Latex works, but the concern is that the flag would allow shell commands not just by libraries we're using, but by the notebook being converted, which may be untrusted. Like our security model in the browser, it's a matter of expectations: if you explicitly ask for code to be executed, we trust that you understand the implications, but if you are opening/converting a document, you probably don't expect that action to execute arbitrary code, so you may not have checked it beforehand.

As @flying-sheep suggested on #356, if someone wants to do something extra with latex, they can always nbconvert --to latex and then run Latex on the result themselves. We also offer a config option for the Latex command to run. But -shell-escape does not sound like something that we should enable by default.

@takluyver
Copy link
Copy Markdown
Member

Merged #367 instead, so this should no longer be needed.

@takluyver takluyver closed this Aug 20, 2016
@amueller
Copy link
Copy Markdown
Contributor Author

Thanks for looking into it!

@mpacer mpacer modified the milestone: 5.0 Oct 20, 2016
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

Successfully merging this pull request may close these issues.

Test failure in jack-of-none builds

4 participants