search

conda-forge / python-graphviz-feedstock

A conda-smithy repository for python-graphviz.
BSD 3-Clause "New" or "Revised" License
3 stars 12 forks source link

Rendering UNC output-paths break on Windows with v0.13 #32

Closed ankostis closed 2 years ago

ankostis commented 4 years ago

The latest python-graphviz-0.13 & Anaconda's graphviz-2.38.0 on Windows fails when rendering UNC paths, with:

'\\?\C:\Users\vagrant\Documents\plots\static'
CMD.EXE was started with the above path as the current directory.
UNC paths are not supported.  Defaulting to Windows directory.
Error: dot: can't open tmpdx48cqh0

This used to be ok with python-graphviz-8.4.

The reason seems to be that in v0.13 the rendered path gets split into dirname & filename, and the former is used as cwd kwarg in Popen, which chokes with UNC paths in it.

I've tested intermediate releases 0.12 & 0.11.1 and they all had similar behavior (0.10 seemed not to work at all).

Related also to conda-forge/graphviz-feedstock#34 (comment) where it discusses the forthcomin natively built graphviz-2.40, without the dreaded BATs patch causing this problem.

xflr6 commented 4 years ago

Thanks, see graphviz changelog for context.

ankostis commented 4 years ago

Thank you. Is it still splitting the path-to-render into dirname & filename, and feeding the former to Popen(), or not?

xflr6 commented 4 years ago

It is. This is needed to start the rendering process from the directory of the file. IIUC, the more general issue is that one cannot cd into a UNC-path, correct? Maybe mounting the UNC as network drive is a possible workaround.

Feel free to open an issue at https://github.com/xflr6/graphviz/issues replacing this one (with the possibility that it might be closed as WAI in the end).

Maybe we can add an argument to keep the cwd as is or special case UNCs in some way (maybe issueing a warning that relative file references don't work with them).

xflr6 commented 4 years ago

Hm, from a web search on python "UNC paths are not supported" and the error message referring to CMD.EXE I am not sure any more if this might in fact be an issue with the distribution package here (AFAIK, vanilla graphviz (https://pypi.org/project/graphviz/) starts the rendering process with shell=False), can you try with pip install graphviz?

ankostis commented 4 years ago

My apologies, don't have access to my Windows machine in the office (my personal one is a Linux), and due to covid19, it would be quite some time before i get to office, northern Italy :-)

xflr6 commented 4 years ago

No worries :) Long time since I have used SMB shares, maybe I can come up with something for testing..

jakirkham commented 2 years ago

@ankostis, have you had a chance to get access to a Windows machine since and test out @xflr6's suggestions?

jakirkham commented 2 years ago

Going to close this out.

Since this issue was opened, we started building graphviz ourselves on Windows ( https://github.com/conda-forge/graphviz-feedstock/pull/59 ), which allowed us to drop the wrapper batch script mentioned above and some patching we were doing to the python-graphviz package. This would make pretty significant changes on the Windows side of things compared to when the problem previously appeared.

Admittedly there could still be an issue here and there may be room for following up on some things upstream as discussed above. However it is unclear at the moment whether the same issue would still surface.

If the issue does persist on a fresh install, the next step would be to file a new issue (referencing this one) and sharing a simple reproducer (along with the other info requested in the issue template) so folks can debug further.