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

installation failed on Jython #14

Closed
bb-migration opened this Issue Jun 5, 2013 · 12 comments

Comments

Projects
None yet
1 participant
@bb-migration

bb-migration commented Jun 5, 2013

Originally reported by: yyuu (Bitbucket: yyuu, GitHub: yyuu)


The installation script failed on the latest Jython beta release of 2.7-beta1.

I tested with beta release of 2.7 because current release version (2.5.3) lacks ssl module.

% python --version
Jython 2.7b1
% python setup.py install
Traceback (most recent call last):
  File "setup.py", line 145, in <module>
    for cmd in SETUP_COMMANDS
  File "/home/yyuu/.pyenv/versions/jython-2.7-beta1/Lib/distutils/core.py", line 112, in setup
    _setup_distribution = dist = klass(attrs)
  File "/home/yyuu/.pyenv/versions/jython-2.7-beta1/Lib/distutils/core.py", line 112, in setup
    _setup_distribution = dist = klass(attrs)
  File "/home/yyuu/work/repos/hg/setuptools/setuptools/dist.py", line 266, in __init__
    _Distribution.__init__(self,attrs)
  File "/home/yyuu/.pyenv/versions/jython-2.7-beta1/Lib/distutils/dist.py", line 287, in __init__
    self.finalize_options()
  File "/home/yyuu/work/repos/hg/setuptools/setuptools/dist.py", line 298, in finalize_options
    ep.require(installer=self.fetch_build_egg)
  File "/home/yyuu/work/repos/hg/setuptools/pkg_resources.py", line 2185, in require
    map(working_set.add,
  File "/home/yyuu/work/repos/hg/setuptools/pkg_resources.py", line 2403, in requires
    dm = self._dep_map
  File "/home/yyuu/work/repos/hg/setuptools/pkg_resources.py", line 2392, in _dep_map
    if invalid_marker(marker):
  File "/home/yyuu/work/repos/hg/setuptools/pkg_resources.py", line 1209, in invalid_marker
    evaluate_marker(text)
  File "/home/yyuu/work/repos/hg/setuptools/pkg_resources.py", line 1209, in invalid_marker
    evaluate_marker(text)
  File "/home/yyuu/work/repos/hg/setuptools/pkg_resources.py", line 1292, in evaluate_marker
    import parser
ImportError: No module named parser

// BTW, new setuptools should support Jython?


@bb-migration

This comment has been minimized.

bb-migration commented Jun 7, 2013

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Thanks for testing!

Yes, it should support Jython.

@bb-migration

This comment has been minimized.

bb-migration commented Jun 8, 2013

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


It appears on first blush that Jython 2.7 doesn't provide the 'parser' module. I'll have to research further.

@bb-migration

This comment has been minimized.

bb-migration commented Jun 8, 2013

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


It seems Jython does not support the parser module and never will. Therefore, setuptools will need to do something different here.

I'll first consider using ast for Python 2.5 and later (including Jython).

@bb-migration

This comment has been minimized.

bb-migration commented Jun 8, 2013

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Merge legacy evaluation as a fallback with markerlib-based evaluation of environment markers. Fixes #14

@bb-migration

This comment has been minimized.

bb-migration commented Jun 8, 2013

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


After further investigation, realized there was a marker implementation in Distribute. It wasn't an exact match, though, as it only implements PEP 345 markers. As a result, I've marked the inline implementation as legacy and defer to the markerlib implementation, which uses ast in favor of parser. I expect markerlib will be updated later to support metadata 2.0.

I'd appreciate if @pje and @dholth would review these changes and comment on the fix. It's a delicate balance supporting multiple metadata versions and Python versions together, so I welcome a more unified approach if possible. In the short term, however, I'm going to release 0.7.2 on bitbucket downloads for our Jython users.

@bb-migration

This comment has been minimized.

bb-migration commented Jun 8, 2013

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I note that this latest changeset causes test failures so I won't be releasing until I resolve those.

@bb-migration

This comment has been minimized.

bb-migration commented Jun 8, 2013

Original comment by pje (Bitbucket: pje, GitHub: pje):


Note that the inline marker support is based on the newer PEP 426, so a fair number of those test failures indicate holes in markerlib that allow you to specify things that aren't valid under 426. (Actually, some of the tests look like failures to comply even with PEP 345, let alone 426.)

I took a quick whack at trying to use ast instead of parser in the PEP 426-supporting bits, but actually making a PEP 426-compatible parser using the "ast" module turns out to be rather nontrivial, because the ast objects deliberately drop certain data that's available in the concrete parser. It would have to use the (much slower) tokenize module to first verify the string.

Anyway, based on the above, I think you have it backwards what is the "legacy" -- in this case, it's _markerlib, which does not currently conform to either of the two current environment marker PEPs. I'd suggest that it would make more sense to fall back to (something like) _markerlib for Jython, than doing it the other way around.

_markerlib should be fixed to actually support PEP 345, at least if there's anything actually using it (which I kind of doubt, but maybe Daniel can clarify). And going forward, for PEP 426, it should also support being as strict as the pkg_resources version is about what it disallows, even if it's used in a common codebase.

All that being said, replacing the PEP-compliant markers implementation with a non-compliant one doesn't make any sense in the main line codebase; Jython should be treated as the special case here, falling back to a spruced up (but probably still not 100% PEP-compliant) version of _markerlib.

@bb-migration

This comment has been minimized.

bb-migration commented Jun 9, 2013

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Yes. You're right. My initial expectation was marker lib would be more complete and correct because it was its own library and the reference implementation,, and because the setuptools implementation was put together experimentally to support an limited need. I see now the opposite is true, and I agree the setuptools implementation is superior.

I'd like to see a complete, tested, reference implementation in markerlib at some point, but I'll defer to the best available for now.

@bb-migration

This comment has been minimized.

bb-migration commented Jun 9, 2013

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I've updated the code to fix the tests and continuing to use the internal implementation for evaluating markers when available, but fallback to markerlib when 'parser' is not available. Note that this means that markers will always evaluate to False on Jython 2.5 (as ast module isn't available and markerlib will evaluate them to False).

However, Jython 2.7 should now work well with the latest release (0.7.2).

@bb-migration

This comment has been minimized.

bb-migration commented Jun 9, 2013

Original comment by arfrever (Bitbucket: arfrever, GitHub: arfrever):


Jython 2.5 actually contains ast module.

@bb-migration

This comment has been minimized.

bb-migration commented Jun 9, 2013

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Thanks for the detail. I thought that might be the case. The CPython docs indicate ast was added in Python 2.6, but perhaps Jython back-ported it to 2.5 for implementation support. In any case, if it's present, then Jython 2.5 should fall back to markerlib for environment marker evaluation as well as 2.7 does.

@bb-migration

This comment has been minimized.

bb-migration commented Jul 22, 2013

Original comment by dholth (Bitbucket: dholth, GitHub: dholth):


You have my permission to deprecate markerlib. It's necessarily more permissive than the spec. I did just update it to support _-separated names throughout. How's the speed?

Evaluating all markers as False is a pretty good strategy when you can't evaluate them properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment