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

give write access to template files after copying with startproject #4604

Merged
merged 3 commits into from Jun 10, 2020

Conversation

MMesch
Copy link
Contributor

@MMesch MMesch commented Jun 1, 2020

scrapy startproject myproject fails with permission denied if the original template permissions are read-only. This is the case on nix-based systems that ensure that installed libraries are immutable. I am not sure what the best place is to put these permissions but here is a quick fix.

@MMesch MMesch force-pushed the fix-startproject-permissions branch from 3b46c02 to 33f911f Compare Jun 1, 2020
@codecov
Copy link

codecov bot commented Jun 1, 2020

Codecov Report

Merging #4604 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4604      +/-   ##
==========================================
+ Coverage   84.74%   84.76%   +0.01%     
==========================================
  Files         163      163              
  Lines        9967     9978      +11     
  Branches     1486     1489       +3     
==========================================
+ Hits         8447     8458      +11     
  Misses       1254     1254              
  Partials      266      266              
Impacted Files Coverage Δ
scrapy/commands/startproject.py 100.00% <100.00%> (ø)

@wRAR
Copy link
Contributor

wRAR commented Jun 2, 2020

Doing it in _copytree() after copystat is better IMO.

@MMesch
Copy link
Contributor Author

MMesch commented Jun 4, 2020

Shouldn't the _copytree and set_rw_permissions functions go into utils or something? I think they are fairly general.

@wRAR
Copy link
Contributor

wRAR commented Jun 4, 2020

If they are used only in one place in Scrapy and don't look useful for writing spiders, then I'd say no.

@MMesch MMesch force-pushed the fix-startproject-permissions branch from 2161597 to 0cabf40 Compare Jun 4, 2020
@MMesch
Copy link
Contributor Author

MMesch commented Jun 4, 2020

I think this is fine now (from my side). Handing over to @wRAR if that's OK for you.

@wRAR
Copy link
Contributor

wRAR commented Jun 4, 2020

My first comment still applies.

@MMesch
Copy link
Contributor Author

MMesch commented Jun 5, 2020

There you go. Coding after someone else's taste, for someone else's code is a bit frustrating if it is done without "thank you" and voluntarily in one's free time. So please just adapt, merge or don't as you please. I think that's more efficient and little effort for you.

Thank you for your feedback.

Copy link
Member

@Gallaecio Gallaecio left a comment

Thank you!

I’ve made minor style changes for consistency with the code base, I hope that’s OK.

wRAR
wRAR approved these changes Jun 10, 2020
@wRAR wRAR merged commit 092f6fd into scrapy:master Jun 10, 2020
2 checks passed
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.

None yet

3 participants