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

Add LICENSE, README and CHANGES to source tarball #867

Merged
merged 4 commits into from Jun 14, 2020

Conversation

akrabat
Copy link
Member

@akrabat akrabat commented May 22, 2020

These files should be in the PyPI download. I've also updated the copyright year to 2020 in the license.

Fixes #866.

@akrabat akrabat requested a review from lornajane May 22, 2020 08:33
akrabat added a commit to akrabat/rst2pdf that referenced this pull request May 22, 2020
@sergiopasra
Copy link

package_data is not the place for these files, they usually are included in the package with MANIFEST.in

@akrabat
Copy link
Member Author

akrabat commented May 22, 2020

package_data is not the place for these files, they usually are included in the package with MANIFEST.in

Useful to know, thanks.

I looked up the docs for package_data and where it said:

These files are often data that’s closely related to the package’s implementation, or text files containing documentation that might be of interest to programmers using the package.

So it seemed to be the right place for these text files.

@sergiopasra
Copy link

This is the documentation for MANIFEST.in

https://packaging.python.org/guides/using-manifest-in/

In fact, depending on the tooling, some files such as LICENSE.txt are automatically included or not.

The problem with package_data is that the files there are installed. With CHANGES, AUTHORS, LICENSE and similar files, you need them in the tarball (LICENSE in particular) but not installed in your system.

This is the MANIFEST.in file of astopy, for example https://github.com/astropy/astropy/blob/master/MANIFEST.in

akrabat added a commit to akrabat/rst2pdf that referenced this pull request May 22, 2020
@akrabat akrabat force-pushed the include-additional-files-in-tarball branch from fef0e7f to d6c4f9b Compare May 22, 2020 12:35
@akrabat
Copy link
Member Author

akrabat commented May 22, 2020

Thanks! There's so much information on packaging.python.org that I struggle with it as a new-to-python person.

PR updated.

@akrabat
Copy link
Member Author

akrabat commented May 22, 2020

@sergiopasra Is there any documentation on whether the license file should be referenced in MANIFEST.in or in setup.cfg's license_files property?

@sergiopasra
Copy link

@sergiopasra Is there any documentation on whether the license file should be referenced in MANIFEST.in or in setup.cfg's license_files property?

No, as far as I know. But you can use both

@akrabat
Copy link
Member Author

akrabat commented May 22, 2020

Thanks. I'll worry about setup.cfg as a separate thing.

@stephenfin
Copy link
Contributor

stephenfin commented Jun 9, 2020

The merge conflict needs resolving, but this is correct. Validation steps:

  1. Generate sdist without this change:

    $ git checkout master
    $ python setup.py sdist
    $ mv dist/rst2pdf-0.98.dev0.tar.gz /tmp/old.tar.gz
    
  2. Generate sdist with this change:

    $ hub pr checkout 867  # using the hub wrapper, btw
    $ python setup.py sdist
    $ mv dist/rst2pdf-0.98.dev0.tar.gz /tmp/new.tar.gz
    
  3. Extract sdists:

    $ cd /tmp
    $ tar -xvzf old.tar.gz
    $ mv rst2pdf-0.98.dev0 old
    $ tar -xvzf new.tar.gz
    $ mv rst2pdf-0.98.dev0 new
    
  4. Compare SOURCES.txt to ensure these files are added:

    $ diff -u old/rst2pdf.egg-info/SOURCES.txt new/rst2pdf.egg-info/SOURCES.txt
    --- old/rst2pdf.egg-info/SOURCES.txt    2020-06-09 12:21:18.000000000 +0100
    +++ new/rst2pdf.egg-info/SOURCES.txt    2020-06-09 12:22:20.000000000 +0100
    @@ -1,3 +1,6 @@
    +CHANGES.rst
    +LICENSE.txt
    +MANIFEST.in
     README.rst
     setup.cfg
     setup.py
    $ diff -burq old new
    Only in new: CHANGES.rst
    Only in new: LICENSE.txt
    Only in new: MANIFEST.in
    Files old/rst2pdf/extensions/inkscape_r2p.py and new/rst2pdf/extensions/inkscape_r2p.py differ
    Files old/rst2pdf.egg-info/requires.txt and new/rst2pdf.egg-info/requires.txt differ
    Files old/rst2pdf.egg-info/SOURCES.txt and new/rst2pdf.egg-info/SOURCES.txt differ
    Files old/setup.py and new/setup.py differ
    

(ignore the changes in unrelated files - the patches are on different bases)

Adds README, LICENSE and CHANGES to the source distribution tarball.
These files shouldn't be left hanging around after a release.
@akrabat akrabat force-pushed the include-additional-files-in-tarball branch from 6bee632 to 56ad8d1 Compare June 13, 2020 09:25
@akrabat
Copy link
Member Author

akrabat commented Jun 13, 2020

Rebased to lastest master

@lornajane
Copy link
Contributor

Thanks @stephenfin for the amazing instructions, I learned how to test this PR and also a little more of how the packaging works for Python :)

Copy link
Contributor

@lornajane lornajane left a comment

Choose a reason for hiding this comment

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

Seems sound!

@lornajane lornajane merged commit 9947b5e into rst2pdf:master Jun 14, 2020
@akrabat akrabat deleted the include-additional-files-in-tarball branch June 14, 2020 13:21
@akrabat akrabat added this to the 1.0 milestone Jun 20, 2020
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.

rst2pdf 0.97 in PyPI doesn't include LICENSE.txt
4 participants