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

Usage of pathlib.Path.resolve() breaks symlink trees / forests #711

Closed
EricCousineau-TRI opened this issue Mar 8, 2022 · 4 comments · Fixed by #712
Closed

Usage of pathlib.Path.resolve() breaks symlink trees / forests #711

EricCousineau-TRI opened this issue Mar 8, 2022 · 4 comments · Fixed by #712
Labels

Comments

@EricCousineau-TRI
Copy link
Contributor

EricCousineau-TRI commented Mar 8, 2022

Description

Unfortunate case of spacebar heating 😅

We have some bazel code which runs notebooks.
Bazel works by creating a "runfiles" directory, which contains a series of symlinks from files that may not come from the same directories.

#415 introduced the following logic:

rootdir_abspath = pathlib.Path(self.root_dir).resolve()
file_rawpath = pathlib.Path(self.file_to_run)
combined_path = (rootdir_abspath / file_rawpath).resolve()
is_child = str(combined_path).startswith(str(rootdir_abspath))

However (as a surprise to me), pathlib.Path.resolve() will not only make a path absolute, but it will resolve against symlinks.

This is an internal project, but similar code was hoisted to one of our public projects:
https://github.com/RobotLocomotion/drake/tree/v1.0.0/tools/jupyter

Reproduce

No direct repro script yet; however, I confirmed this will break Bazel out of sandboxes.
If I inject some print statements in this area, I get the following:

$ bazel run //:my_jupyter_notebook
...
file_rawpath: /path/to/project/bazel-bin/my_notebook.runfiles/project/my_notebook.ipynb
rootdir_abspath: /bazel/cache/execroot/anzu/bazel-out/k8-opt/bin/my_notebook.runfiles/project
combined_path: /path/to/project/my_notebook.ipynb
...
`root_dir` and `file_to_run` are incompatible. They don't share the same subtrees. Make sure `file_to_run` is on the same path as `root_dir`.

Expected behavior

It should only get absolute path, not the real path, and it should work fine.

Possible workarounds:

  • In our own setup, use os.path.realpath on the notebook
    • Feasible, may do this, but would still like to keep the option of using runfiles
  • File PR and wait - better
  • Copy all files to the physically same directory - eww
  • Monkey patch serverapp.py to use os.path.abspath() - meh

Context

  • Operating System and version: Ubuntu 20.04
  • Browser and version: Firefox 97.0.2
  • Jupyter Server version: 1.13.5
Troubleshoot Output
$ bazel run //tools/jupyter:jupyter troubleshoot
...
$PATH:
        {effective venv}
        /usr/local/sbin
        /usr/local/bin
        /usr/sbin
        /usr/bin
        /sbin
        /bin
        /usr/games
        /usr/local/games
        /snap/bin

sys.path:
{effective venv}
/usr/lib/python38.zip
/usr/lib/python3.8
/usr/lib/python3.8/lib-dynload
/usr/local/lib/python3.8/dist-packages
/usr/lib/python3/dist-packages

sys.executable:
/usr/bin/python3.8

sys.version:
3.8.10 (default, Nov 26 2021, 20:14:08)
[GCC 9.3.0]

platform.platform():
Linux-5.13.0-30-generic-x86_64-with-glibc2.29

which -a jupyter:
/bazel/cache/execroot/project/bazel-out/k8-opt/bin/external/pip_deps/data/python3.8/bin/jupyter
/path/to/project/wip/project/bazel-bin/tools/jupyter/jupyter.runfiles/pip_deps/data/python3.8/bin/jupyter
/usr/bin/jupyter
/bin/jupyter

pip list:
Package Version
------------------------ --------------------
...
jupyter 1.0.0
jupyter-client 7.1.2
jupyter-console 6.4.3
jupyter-core 4.9.2
jupyter-packaging 0.11.1
jupyter-server 1.13.5
jupyterlab 3.3.0
jupyterlab-pygments 0.1.2
jupyterlab-server 2.10.3
jupyterlab-widgets 1.0.2
...

Command Line Output N/A (I believe)
Browser Output N/A
@welcome
Copy link

welcome bot commented Mar 8, 2022

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@davidbrochart
Copy link
Contributor

Thanks for reporting @EricCousineau-TRI.
It looks like using .absolute() instead of .resolve() would work.

@EricCousineau-TRI
Copy link
Contributor Author

Nice! Didn't see .absolute() in the Python docs for some reason, so I guess that's another bug 😿
Tested in our local setup, and it worked perfectly!

See #712

@EricCousineau-TRI
Copy link
Contributor Author

Ah, was about to file bug report for Python, but looks like it's already fixed in newer (dev) docs:
https://docs.python.org/3.11/library/pathlib.html#id5

image

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

Successfully merging a pull request may close this issue.

2 participants