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

[MRG+1] Ignore explicitly compiled python files. #2386

Merged
merged 1 commit into from Dec 1, 2016

Conversation

@rmax
Copy link
Contributor

@rmax rmax commented Nov 8, 2016

This avoids to include compiled files in the templates directory.

Without this change, pyc files in the templates are included in the build:

$ find build | grep pyc
build/lib/scrapy/templates/project/module/__init__.pyc
build/lib/scrapy/templates/project/module/spiders/__init__.pyc
This avoids to include compiled files in the templates directory.
@codecov-io
Copy link

@codecov-io codecov-io commented Nov 9, 2016

Current coverage is 83.36% (diff: 100%)

Merging #2386 into master will not change coverage

@@             master      #2386   diff @@
==========================================
  Files           161        161          
  Lines          8723       8723          
  Methods           0          0          
  Messages          0          0          
  Branches       1285       1285          
==========================================
  Hits           7272       7272          
  Misses         1201       1201          
  Partials        250        250          

Powered by Codecov. Last update ea83e67...3cd56da

@redapple redapple changed the title Ignore explicitly compiled python files. [MRG+1] Ignore explicitly compiled python files. Nov 9, 2016
@kmike
Copy link
Member

@kmike kmike commented Nov 9, 2016

@rolando could you please explain why are pyc files a problem?

@rmax
Copy link
Contributor Author

@rmax rmax commented Nov 9, 2016

@kmike IMO, it leaves the build dirty as only templates directory have pyc files. But, particularly this was breaking the conda-forge build on windows: conda-forge/staged-recipes#1646 (comment)

The conda build scripts cleaned up pyc files, but not in the templates directory. So when attempting to recreate pyc files it failed because templates already had pyc files around.

@kmike
Copy link
Member

@kmike kmike commented Dec 1, 2016

I am cautious because not having pyc files in install directory means Python will try to create them on first run, and it can be a problem if filesystem permissions don't allow that. Package installation could happen with write access, but user may have only read access.

But apparently setuptools (or is it distutils? or pip?) creates them during the install. Could you please confirm that manifest changes don't prevent pyc file creation during installation?

@rmax
Copy link
Contributor Author

@rmax rmax commented Dec 1, 2016

@kmike The Manifest.in is only for sdist command: https://docs.python.org/2/distutils/sourcedist.html#the-manifest-in-template

The sdist output already does not include any pyc file except those two listed above, which usually don't cause a problem because the installers (or binary packaging tools) compile all pyc files for the given platform.

This PR only aims to have a clean source distribution without any pyc file (those two files listed above).

PD: The source distribution package does not include pyc files but default, but given templates are data directories those two pyc get included.

@kmike
Copy link
Member

@kmike kmike commented Dec 1, 2016

Ok, thanks for clarifying it!

@kmike kmike merged commit 826b7e9 into scrapy:master Dec 1, 2016
3 checks passed
3 checks passed
codecov/patch Coverage not affected when comparing ea83e67...3cd56da
Details
codecov/project 83.36% (+0.00%) compared to ea83e67
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

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