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

Pack LICENSE in a different way #74

Merged
merged 2 commits into from
Jul 10, 2019
Merged

Pack LICENSE in a different way #74

merged 2 commits into from
Jul 10, 2019

Conversation

rayluo
Copy link
Collaborator

@rayluo rayluo commented Jul 10, 2019

It turns out the approach we used in PR #69 would cause the LICENSE being copied into /usr/local, which would typically end up with insufficient permission error while some downstream build pipeline (such as, OSX) attempts to install msal without explicitly specifying --user option.

This PR uses a different to pack that LICENSE file and ship it inside this library's directory.

rayluo added 2 commits July 10, 2019 12:17
Previously the LICENSE file would end up in `/usr/local` which would require root permission
@rayluo rayluo merged commit 6fbbb21 into dev Jul 10, 2019
@rayluo rayluo deleted the package_data branch July 10, 2019 20:57
@rayluo rayluo restored the package_data branch July 10, 2019 20:58
@rayluo rayluo mentioned this pull request Jul 10, 2019
@rayluo rayluo deleted the package_data branch July 10, 2019 22:10
@ghost
Copy link

ghost commented Jul 11, 2019

I can't find the LICENSE file in the pypi package anymore after this change. I think you also need a MANIFEST.in file.

@rayluo
Copy link
Collaborator Author

rayluo commented Jul 11, 2019

Hmm, @roederja2 , thanks for bringing this to our attention. We don't immediately have a solution, but we can be transparent on what we know so far.

Admittedly, when working on this very PR (#74) we assumed that the canonical package_data would do its job, therefore did not really check the actual outcome. When I tried it just now with pip install msal==0.4.1 i.e. the version without either the recent data_files or package_data adjustment, and comparing it to pip install msal==0.5.0 i.e. the version with data_files, and then the pip install msal==0.5.1 i.e. the version with the latest package_data, they ALL install a "my_virtual_env/Lib/site-packages/msal-0.x.x.dist-info/LICENSE" into my environment. In fact, if we download that raw msal-0.5.1-py2-none-any.whl file and open it manually, there IS a msal-0.5.1.dist-info/LICENSE file in it. In that sense, this package_data param seems like an no-op.

On the other hand, the pip install msal==0.5.0 i.e. the data_files version indeed generates one extra file at "my_virtual_env/LICENSE". That behavior unnecessarily caused downstream pipeline trouble of permission issue, because, if not using a virtual environment, that LICENSE would be attempted to be written into /usr/local on OSX. Even if some sort of virtual environment is being used, that behavior is not correct to begin with, because that "my_virtual_env/LICENSE" location does not belong to msal library, and it would possibly be overwritten by other libraries which happen to use same way to distribute their LICENSE, hence defeat the purpose.

We are open to hear a better solution on this. At least, next time we will know how to test it.

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.

1 participant