First shot at removing usage of _markerlib and switching to the PEP 508 implementation in packaging.

#164 Merged at c3ca485
  1. Steve Kowalik

This updates the vendored packaging copy to the work-in-progress PEP 508 implementation, drops _markerlib, and switches pkg_resources to making use of the new packaging features.

  • Issues #122: Unify environment marker support resolved

Comments (8)

  1. Jason R. Coombs

    I've merged these changes into the pr-164 bookmark, and immediately, I see three issues;

    First, it seems that the tests fail on Python 2.6. Apparently, the mock that previously prevented that failure is no longer suitable for this implementation.

    Second, when running the tests on OS X (and probably any other non-Linux system), the tests fail with

    =========================================== FAILURES ============================================
    _____________________________ [doctest] pkg_resources/api_tests.txt ______________________________
    376     >>> print(im('"""x"""=="x"'))
    377     Invalid marker: '"""x"""=="x"'
    379     >>> print(im(r"x\n=='x'"))
    380     Invalid marker: "x\\n=='x'"
    382     >>> print(im("'y'"))
    383     Invalid marker: "'y'"
    385     >>> em("'linux' in sys_platform")
    /Users/jaraco/Dropbox/code/main/setuptools/pkg_resources/api_tests.txt:385: DocTestFailure
    ================== 1 failed, 129 passed, 7 skipped, 2 xfailed in 54.80 seconds ===================

    The test probably shouldn't be hard-coding the platform. I observe that a test that already covered this case (but referenced win32) was removed.

    Third, and probably most importantly, the commits alter the API tests to eliminate a lot of tests that were previously supported, and drastically reduces the amount of information reported about invalid markers, removing any detail about the reason for a marker being invalid. Does PEP 508 explicitly designate these changes, or are these limitations only an artifact of using a different implementation for evaluating markers?

  2. Jason R. Coombs

    I observe another issue.

    Fourth, it seems platform_python_implementation is missing, and the tests for it that were added in setuptools 18.5 were removed. In 035ae59fa7cb, I've added tests to nominally capture these requirements.

  3. Jason R. Coombs

    @embray has brought to my attention another issue - this version of packaging also has a dependency on six, so that dependency will need to be vendored as well.

    1. E. Madison Bray

      Though I think the issue should be fixed in packaging. It only uses six for one minor import that can be handled without it.

  4. E. Madison Bray

    I was also about to report the platform_python_implementation thing--it is reported as an invalid marker.

  5. Steve Kowalik author

    I'll look at the test failures.

    PEP 508 does not explicitly designate those changes, it's a limitation of using pyparsing to parse markers and requirements. I could add "Parse error at or near <closest 8 characters to string>", but I don't think I can pull out the last component that didn't match the parse string.

  6. Jason R. Coombs

    Thanks Steve. It's not strictly necessary to have the same error messages as before, but losing meaningful messages would be an unfortunate degradation, and acceptable only if it's our only good option.

    You might also want to follow #229, where I'm working on a technique for more readily vendoring dependencies, such as six.

  7. Robert Collins

    python_implementation is what was platform_python_implementation. I believe that that was an editing error in PEP 508 which noone caught - sorry!.

    So, we should: - update the PEP - update the packaging PR for this to be platform_python_implementation safe too, and then this PR won't be conflicting.