jupyter / nbconvert

Jupyter Notebook Conversion
https://nbconvert.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
1.73k stars 566 forks source link

commit bae0ece breaks PDFExporter.run_latex #181

Open rlabbe opened 8 years ago

rlabbe commented 8 years ago

I have two machines, both pointing to a dropbox directory containing Jupyter notebooks (evidence that the source data is identical for both machines).

I create a .tex file with

ipython nbconvert --to latex --template book book.ipynb

Then I have a script to convert the tex file to pdf. The lines relevant to this bug are:

import nbconvert.exporters.pdf as pdf

p = pdf.PDFExporter()
p.run_latex(name)

where name in this context would be book.tex

I run the script via command line from the same directory where book.tex resides.

On one machine the script works perfectly. On a second machine the script fails on the call to run_latex with the error "I can't find file `book.tex'.

I've set a breakpoint and stepped through the code. The two computers have subtly different versions of nbconvert/exporters/pdf.py.

The bug manifests itself in PDFExporter.run_command() in the lines

    shell = (sys.platform == 'win32')
    if shell:
        command = subprocess.list2cmdline(command)

This code was introduced in commit bae0ece .

Prior to this code executing command contains the list ['pdflatex', 'book.tex']. Afterwards command contains the string 'pdflatex book.tex'. This string subsequently causes the subprocess.Popen() call to fail with the aforementioned error.

Maybe my environment/installation is bad, you say. Maybe. But if I use the debugger to force shell=False then the command executes and book.pdf is generated, same as on the computer where this works flawlessly (with the older(prior to bae0ece commit) pdf.py file).

Both computers have an Anaconda Python 3.5.1 install. The machine with the problem is windows 7, 64-bit, the one with no problem is Windows 10.

If you need more information or more debugging please ask. In the meantime I'm going to delete those three lines from pdf.py so I can continue working.

minrk commented 8 years ago

Thanks for the report, this is getting confusing. We now know for certain that there are some systems (always Windows) where this fails if shell=True, and others where it fails unless shell=True. Frustratingly, both work when I test myself, so it seems to be something non-default that causes both failures, or perhaps my tests are too trivial.

The fact that it's not finding book.tex suggests to me that the CWD is being changed for you when shell=True, but I don't know how that could be.

I'm generally not inclined to use shell=True if we can fix the original issue without setting this, but I think there were reasons that was not feasible.

rlabbe commented 8 years ago

I'll put some code in to print the directory at the point of the call, and report back. If there is anything else I can do let me know.

rlabbe commented 8 years ago

Actually, that is exactly what is happening. I have windows set up to always open a command prompt in my \dev subdirectory. Convenient, I guess, until code programatically launches a shell.

When I disabled that, the code runs fine.

For completeness, I have this turned on in a registry key: Computer\HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Command Processor\AutoRun = c:\ && cd \dev

Finally, I changed the assignment under the if shell: to:

command = 'cd {} && {}'.format(os.getcwd(), subprocess.list2cmdline(command))

That works regardless of whether my registry key exists or not. It is not a full solution. If the shell is on drive c: (say), and you want it to be somewhere on e: then the cd will fail. So you need to first extract the drive letter, go to that drive, then execute the cd. This is quite a hack (how many places do you call subprocess.Popen) so I'm going to guess you are going to pass on implementing it?

edit: I tried using cwd=os.getcwd() in the Popen call, but that did not work.

takluyver commented 8 years ago

OK, so it looks like we have a clear understanding of what's causing the problem with shell=True - a non default setting is changing the cwd when cmd.exe starts. But looking at the commit and PRs #140 and #132, I can't understand the problem with leaving shell=False. @minrk, can you recall any more details?

Conceptually, I think shell=False is preferable; Latex shouldn't depend on being run inside cmd.exe.

minrk commented 8 years ago

this was the key reason for shell=True. If something about that isn't true, or there's a better fix where we can keep shell=False, that seems preferable.

takluyver commented 8 years ago

Ah, it sounds like you need shell=True if the command you want to invoke is defined by a batch file rather than a .exe. That's annoying.

mpacer commented 7 years ago

Was this ever resolved?