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

Refactoring of metadata extractors #77

Merged
merged 13 commits into from
Oct 11, 2016
Merged

Conversation

mcyprian
Copy link
Member

@mcyprian mcyprian commented Aug 31, 2016

In this PR:

  • Module extract_distribution replaced by direct subclass of distutils.Command extract_dist
  • Much more simple and readable code, the same performance of metadata extraction
  • Old metadata extractor parsing setup.py script and parsing methods from archive module removed
  • Common interface of SetupPyMetadataExtractor and WheelMetadataExtractor - one data_from_archive method can be used for both
  • Venv metadata extraction rewritten as an decorator
  • Complex tests for all metadata attributes
  • Default empty values in package_data

@mcyprian mcyprian assigned mcyprian and hroncok and unassigned mcyprian Aug 31, 2016
@mcyprian mcyprian force-pushed the extract_dist_refactoring branch 9 times, most recently from a42c760 to 80b5464 Compare September 8, 2016 14:23
@mcyprian mcyprian force-pushed the extract_dist_refactoring branch 2 times, most recently from 829c778 to 075c653 Compare September 12, 2016 15:17
@mcyprian
Copy link
Member Author

If runpy fails to run extract_dist command (Syntax or Runtime error occurs), command is run using external python2 interpreter with --output option. Extracted metadata are sent to standard output in json format, read and deserialized in metadata extractor.

At this moment string "extracted json data:\n" is at the beginning of json metadata, because other messages from setup.py script or command can appear on stdout. Maybe some more sophisticated separator could be used.

@hroncok
Copy link
Member

hroncok commented Sep 12, 2016

Does it also work the other way around (as described here)?

@mcyprian
Copy link
Member Author

No it doesn't. I was focused at the original issue #72. Do you suggest to try running python3 in the subprocess in this case or can we expect that package is python2 incompatible and fail? At the moment when ImportError occurs we don't have any metadata of the package. I am not sure if there is deterministic way to find out if package is python2 compatible or not (classifiers are usually not very confident) even if we had needed metadata.

@hroncok
Copy link
Member

hroncok commented Sep 13, 2016

Simply try the other interpreter. (I.e. 2 on 3 and 3 on 2).

@mcyprian
Copy link
Member Author

Now it works for both cases.

Michal Cyprian added 13 commits October 10, 2016 09:49
- Forbid base version > 2 if epel6 template is used
- Disable with_pythonX macro for version > 2
- Add direct subclass of distutils.Command extract_dist
- Remove old extract_distribution command
- Remove old SetupPyMetadataExtractor parsing setup.py script
- Remove parsing functions from archive module
- Build new SetupPyMetadataExtractor using extract_dist command
- Add complex tests of extracted data for SetupPy and Wheel extractors
- Drop .egg archives support in SetupPyMetadataExtractor
- Rewrite venv metadata extraction as an decorator


- Use subprocess instead of runpy to run extract_dist command
@mcyprian mcyprian force-pushed the extract_dist_refactoring branch 2 times, most recently from ac03cb5 to cb39e29 Compare October 11, 2016 13:06
@mcyprian mcyprian merged commit cb39e29 into master Oct 11, 2016
@mcyprian mcyprian deleted the extract_dist_refactoring branch March 20, 2017 16:01
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 this pull request may close these issues.

2 participants