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

Error parsing Os #34

Closed
nuno-andre opened this issue Dec 13, 2019 · 4 comments
Closed

Error parsing Os #34

nuno-andre opened this issue Dec 13, 2019 · 4 comments
Labels
bug Something isn't working

Comments

@nuno-andre
Copy link
Contributor

nuno-andre commented Dec 13, 2019

[tool.pyflow.dependencies]
pywebview = { git = "https://github.com/r0x0r/pywebview" }

causes

❯ pyflow install
thread 'main' panicked at 'Problem parsing Os in extras: openbsd6', src\dep_types.rs:706:49

because of

install_requires = [
    ...,
    'PyQt5 ; sys_platform == "openbsd6"',
]

The point is that I'm running pyflow in Windows.

Given that sys.platform is built dynamically in most POSIX systems, as are other PEP508's environment markers, I think that a best-effort parsing of install_requires would be a more resilient approach.

Thank you for your work :) Python is greatly in need of tools like this.

@David-OConnor David-OConnor added the bug Something isn't working label Dec 13, 2019
@David-OConnor
Copy link
Owner

David-OConnor commented Dec 13, 2019

Fixed in latest commit, for the case of BSD. Like you suggest, I'm going to parse pep 508 and handle all cases, so this doesn't come up for a different OS - that PEP's exactly what we need to prevent this from coming up again.

@David-OConnor
Copy link
Owner

It looks like neither PEP 508 nor the sys module docs have a comprehensive list. this SO post is the best I can find. Also using some tricks like if the name contains bsd, linux etc. Let me know if you have a more comprehensive/official list.

@nuno-andre
Copy link
Contributor Author

The point is that these requirements are abstract, and were devised to be interpreted by the application according its context. So there will be nothing like an exhaustive list of values for those requirements.

And I think this is a great design because, to my mind, there's no reliable way to model such an ever changing domain.

I usually find them easier to handle by reversing the evaluation in a lazy and best-effort approach, rather than attempting to model every single dependency; since you know how to evaluate the markers of the systems you support, but not many others which actually do not matter. In this way, this issue is not a bug, but an enhancement ;)

The flow in naïve-Python would be roughly like:

requirements_that_matter = list()

for requirement in install_requires:
    for marker, value in requirement.markers.items():
        if value not in this_specific_system.get_values_for(marker):
            break
    requirements_that_matter.append(requirement)

I think this approach makes the dependency management easier, especially the transitive ones.

It's great to hear about your commit. I'm going to try the build again and I'll give you feedback. Thanks!

@David-OConnor
Copy link
Owner

Should be fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants