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

Fix permissions issue on install, and update revision version number. #123

Merged
merged 1 commit into from
Nov 22, 2017

Conversation

lizroth
Copy link
Contributor

@lizroth lizroth commented Nov 22, 2017

See also #122

Testing:

  • Reproduced permissions error using sdist install (setup.py sdist + pip install aws-encryption-sdk-cli*.tar.gz)
  • Resolved permissions error with this patch applied using sdist install
  • Double checked MANIFEST.in and verified output .tar.gz still includes metadata files (README.rst, CHANGELOG.rst, LICENSE, requirements.txt)
  • Builds / unit tests pass

Assigning @mattsb42-aws as reviewer -- it's possible there's something I've missed here such that this isn't the right fix, but I think this might do it and I wanted to get a PR out ASAP to help prevent people from being blocked on install.

@lizroth lizroth requested a review from mattsb42-aws November 22, 2017 00:31
Copy link
Member

@mattsb42-aws mattsb42-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, this should do it. To be honest I'm not entirely sure why I had those in as data_files in setup.py to begin with.

For some context, MANIFEST.in determines what is included in the sdist build, which is where those files are needed. setup.py data_files determines extra files that are actually installed from the dist file, and these files do not need to be installed.

@lizroth lizroth merged commit 46e9f34 into aws:master Nov 22, 2017
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