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

Resolving symlinks considered harmful #2127

Open
smurfix opened this issue Apr 19, 2023 · 4 comments
Open

Resolving symlinks considered harmful #2127

smurfix opened this issue Apr 19, 2023 · 4 comments

Comments

@smurfix
Copy link

smurfix commented Apr 19, 2023

Steps to reproduce

  1. Get https://github.com/M-o-a-T/moat
  2. git submodule update --init util micro
  3. cd micro
  4. pylint

Current behavior

The problem which this archive exhibits is that moat/micro/link.py is a symlink that refers to moat/micro/_embed/lib/moat/micro/link.py. The directory moat/micro has an _init__.py file while moat/micro/_embed/lib/moat/micro does not.

Now, modpath_from_file_with_callback, in astroid/mdutils.py, calls astroid's _get_relative_base_path function which calls realpath and thus resolves this symlink. As a result, pylint's "is this a proper module" callback looks for the __init__.py file in the wrong place, decides that this is not a module, and refuses to process the import.

Expected behavior

IMHO there's no good reason to call realpath on anything. Why not simply assume that the symlink is there for a good reason?

python -c "from astroid import __pkginfo__; print(__pkginfo__.version)" output

2.14.2, but applies to master

smurfix added a commit to smurfix/astroid that referenced this issue Apr 19, 2023
smurfix added a commit to smurfix/astroid that referenced this issue Apr 19, 2023
smurfix added a commit to smurfix/astroid that referenced this issue Apr 19, 2023
@Pierre-Sassoulas
Copy link
Member

Did you search for the reason in the git history ? It feel like a Chesterton's fence (we should not remove it until we know why it's here in the first place)

@smurfix
Copy link
Author

smurfix commented Apr 24, 2023

Both absolute and real path lookups were added in cd76b49

modutils is using realpath at the moment which will resolve the symlink
and don't allow the import as the file is detected outside of the
available paths.

This change added two tests – which incidentally still work after removing that realpath call.

@DanielNoord
Copy link
Collaborator

Both absolute and real path lookups were added in cd76b49

modutils is using realpath at the moment which will resolve the symlink
and don't allow the import as the file is detected outside of the
available paths.

This change added two tests – which incidentally still work after removing that realpath call.

As you can see from the diff we used realpath before it. I added a comment in your PR with the first introduction of realpath, which sadly doesn't include a test.

@smurfix
Copy link
Author

smurfix commented Apr 24, 2023

Ah, yes, sorry, the real reason realpath was added seems to be pylint-dev/pylint#791

I'll ask there whether somebody can test this.

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 a pull request may close this issue.

3 participants