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

config.py._consider_importhook assumes SOURCES.txt uses platform-specific separator while it is not #2591

Closed
throwable-one opened this issue Jul 19, 2017 · 4 comments

Comments

@throwable-one
Copy link

@throwable-one throwable-one commented Jul 19, 2017

From here: https://svn.python.org/projects/sandbox/trunk/setuptools/doc/formats.txt

The filenames always use / as a path separator, which must be
converted back to a platform-specific path whenever they are read.

My SOURCES.txt has / even on Windows.

Now loo0k at _pytest/config.py:1034

 metadata_files = 'RECORD', 'SOURCES.txt'

        package_files = (
            entry.split(',')[0]
            for entrypoint in pkg_resources.iter_entry_points('pytest11')
            for metadata in metadata_files
            for entry in entrypoint.dist._get_metadata(metadata)
        )

        for fn in package_files:
            is_simple_module = os.sep not in fn and fn.endswith('.py') # LOOK HERE !!!!
            is_package = fn.count(os.sep) == 1 and fn.endswith('__init__.py')
            if is_simple_module:
                module_name, ext = os.path.splitext(fn)
                hook.mark_rewrite(module_name)
            elif is_package:
                package_name = os.path.dirname(fn)
                hook.mark_rewrite(package_name)

os.sep should not be used here. It leads to errors like one module is considered to be is_simple_module on **nix, but not on Windows.

@throwable-one
Copy link
Author

@throwable-one throwable-one commented Jul 19, 2017

bad
Take a look

@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Jul 19, 2017

Indeed, thanks for pointing that out. Would you like to open a PR with this fix?

@srinivasreddy
Copy link
Contributor

@srinivasreddy srinivasreddy commented Jul 28, 2017

I have raised a PR here - #2630

@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Aug 6, 2017

Fixed by #2630

@nicoddemus nicoddemus closed this Aug 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.