Add packaging scripts for uploading to PyPI.

[Julian]

Closes: #11.

After including these, you'd upload to PyPI by running e.g. python3 setup.py sdist bdist_wheel && twine upload dist/*.

(You'd need twine installed, so if you don't already have it, first e.g. python3 -m venv venv && venv/bin/python -m pip install twine)

[newren]

Thanks for sending this in! Sorry for the delay in responding.

In addition to the individual comments on various pieces:

• I would expect 'make release' would run the necessary code rather than needing to run some separate step. That could probably be added separate to this PR, but thought I'd mention it for context in case it affects how you respond to any of my other comments.
• I'm kind of sad to see three new files added to the toplevel directory. I hesitated adding INSTALL and Makefile because I didn't want another file there. I even hesitated on README.md. And I was annoyed at having three COPYING* files, though I wanted it to be clear that which text was verbatim copied from elsewhere. Is there a reasonable way to avoid this, or should I just give up already on my simple "You only need one file to make use of this new tool" pitch?
• As per git.git guidelines, remove the full stop at the end of the commit summary (https://github.com/git/git/blob/master/Documentation/SubmittingPatches#L106-L108)
• As per git.git guidelines, please provide a Signed-off-by on your commit message (https://github.com/git/git/blob/master/Documentation/SubmittingPatches#L293)

[Julian]

I would expect 'make release' would run the necessary code rather than needing to run some separate step. That could probably be added separate to this PR, but thought I'd mention it for context in case it affects how you respond to any of my other comments.

I didn't notice your Makefile, but yeah sure, if that's how you cut releases today I can put the requisite commands in there. Sounds good.

I'm kind of sad to see three new files added to the toplevel directory. I hesitated adding INSTALL and Makefile because I didn't want another file there. I even hesitated on README.md. And I was annoyed at having three COPYING* files, though I wanted it to be clear that which text was verbatim copied from elsewhere. Is there a reasonable way to avoid this, or should I just give up already on my simple "You only need one file to make use of this new tool" pitch?

Technically speaking... not all 3 are "required"... a minimum of 2 are required for "correctness", though if you leave one out (pyproject.toml) stuff will still work today (but maybe not in X years from now), and if you leave another out (setup.cfg) you sometimes get tempted into antipatterns (setup.py is just normal Python code, and so you can do arbitrarily terrible programming things in it instead of just specifying your build process as plain static data, which is why setup.cfg exists -- but if 2 is really better than 3, in theory I can move all the stuff from setup.cfg into setup.py again)

It doesn't need to live top-level though too -- if it seems clearer to you I could move them into a python subdirectory or something?

[Julian]

Uh on the makefile though, are you OK with me assuming that you install twine once globally and not install it as part of every make release?

[newren]

Technically speaking... not all 3 are "required"... a minimum of 2 are required for "correctness", though if you leave one out (pyproject.toml) stuff will still work today (but maybe not in X years from now), and if you leave another out (setup.cfg) you sometimes get tempted into antipatterns (setup.py is just normal Python code, and so you can do arbitrarily terrible programming things in it instead of just specifying your build process as plain static data, which is why setup.cfg exists -- but if 2 is really better than 3, in theory I can move all the stuff from setup.cfg into setup.py again)

It doesn't need to live top-level though too -- if it seems clearer to you I could move them into a python subdirectory or something?

Maybe a directory called release/ ? (And perhaps I can split some code out of the Makefile and stick it into that directory as well.)

Also, in response to your other question, yeah go ahead and assume that I install twine once globally; no need to do that as part of every make release.

[Julian]

OK, think this is ready for another review.

[newren]

Thanks for all the fixes! This is looking pretty good, but I flagged a few more things I missed on the first round. Also, your Signed-off-by is missing your email address; could you fix that up?

[newren]

While I've written virtually everything right now, part of the point of being open source is to allow others to contribute so I'm worried this will go stale. Is an author field necessary? What do larger multi-author projects do?

[Julian]

You get a warning if you specify nothing -- I think the real PyPI may reject the package, but not sure -- the test PyPI doesn't.

In theory there's also maintainer instead of author.

In practice there are varying approaches, but the main common one is to list something to the effect of "main author and others" and then if need be link to a contributor's file.

E.g. here's what coverage.py does (in the meta link in the left sidebar).

[glensc]

@newren:

Don't link to branches!

• see https://news.ycombinator.com/item?id=8046710
  and https://help.github.com/articles/getting-permanent-links-to-files/
• hit y
• and perhaps https://github.com/josephfrazier/octopermalinker

[newren]

@newren:

Don't link to branches!

@glensc: I'm confused; where in this pull request was that done?

[newren]

Due to #20, I updated the installation instructions to mention package managers. I also included a link for pip, which currently says it is coming soon and references this PR.

[Julian]

Due to #20, I updated the installation instructions to mention package managers. I also included a link for pip, which currently says it is coming soon and references this PR.

Cool!

[Julian]

Also FWIW as of.. yesterday, in the next release of setuptools-scm you will only need 2 and not 3 files now :)

[glensc]

@newren here:

• Add packaging scripts for uploading to PyPI. #16 (review)

As per git.git guidelines, remove the full stop at the end of the commit summary (git/git:Documentation/SubmittingPatches@master#L106-L108)
As per git.git guidelines, please provide a Signed-off-by on your commit message (git/git:Documentation/SubmittingPatches@master#L293)

[Julian]

Not sure how useful that is to mention :) -- it's for directly showing someone (me) a particular line at a particular point in time, not something that's permanently going somewhere and can drift out of sync.

[newren]

Also FWIW as of.. yesterday, in the next release of setuptools-scm you will only need 2 and not 3 files now :)

Cool!

[newren]

Looks good.

Before I merge, a few questions on usage (sorry for all the newbie questions; I have only used pip a few times and usually get python packages from yum/dnf install):

• If I checkout v2.23.0, and add in these files and run cd release && python3 setup.py sdist bdist_wheel && twine upload dist/*, will it notice that I have version v2.23.0 checked out and upload a 2.23.0 version, or will it notice that the most recent tag is v2.24.0 and try to mark it with that version?

• If my upload is bad, is the only remedy to upload a newer version? Or can the existing version be re-uploaded if we find there was a problem?

• Should I upload both 2.23.0 and 2.24.0 to PyPI, or does PyPI only make the latest version available?

[Julian]

Before I merge, a few questions on usage (sorry for all the newbie questions; I have only used pip a few times and usually get python packages from yum/dnf install):

No worries!

If I checkout v2.23.0, and add in these files and run cd release && python3 setup.py sdist bdist_wheel && twine upload dist/*, will it notice that I have version v2.23.0 checked out and upload a 2.23.0 version, or will it notice that the most recent tag is v2.24.0 and try to mark it with that version?

The files didn't exist back then, but assuming you copy them in, yeah this will work fine IIRC. If you do the two steps separately you'll see what version gets picked up -- and even more specifically, you can run python setup.py --version and it will tell you what version it thinks this is without building a distribution.

You also may want to test this out first on the test PyPI so you get a feel for things before sending it out, because...

If my upload is bad, is the only remedy to upload a newer version? Or can the existing version be re-uploaded if we find there was a problem?

Correct -- you can never mutate an existing release on PyPI -- you can only push a new one after bumping the version.

Should I upload both 2.23.0 and 2.24.0 to PyPI, or does PyPI only make the latest version available?

It exposes all uploaded versions -- as for whether you should or not -- my recommendation as a maintainer would be "yes if you know those two versions are widely used [e.g. because debian packages your 2.23.0], but no need otherwise".

Happy to answer any other questions too.

[newren]

Sorry for the delay; your update came in on US Thanksgiving. I was planning to get this all out yesterday, but between being sick and running into a few issues, I'm still not there.

Here's what I ran:

python3 -m venv venv
venv/bin/pip3 install --upgrade setuptools pip
venv/bin/pip3 install twine wheel
venv/bin/python3 setup.py sdist bdist_wheel

and then I took a look at the stuff under dist/. In particular, unzip -c dist/git_filter_repo-2.23.0-py2.py3-none-any.whl | less looked okay other than a couple fields in the METADATA file. When I untarred dist/git-filter-repo-2.23.0.tar.gz, the files I saw looked okay (to my untrained eye) other than PKG-INFO. That file, similar to the METADATA file in the other archive had multiple fields with a value of "UNKNOWN" -- License, Platforms, and Description. I fixed the first two by adding the lines

platforms = any
license = MIT

to the setup.cfg file, but the Description threw me for a loop. After digging for a while, I changed the long_description line into two lines reading

long-description = file: ../README.md
long_description_content_type = 'text/markdown'

but that failed with a Traceback, reporting

distutils.errors.DistutilsOptionError: `file:` directive can not access /home/newren/floss/git-filter-repo/README.md

That seems odd, since that is the correct full path to the filename. A google search on the error showed that the code checked if the filename was within os.getcwd(), which I guess is where the failure comes from since cwd is the release subdirectory. I haven't figured out how to get past that point. Any ideas?

[newren]

Okay, so adding:

platforms = any
license = MIT

to setup.cfg and

import shutil
shutil.copy("../README.md", ".")

to setup.py (before the call to setuptools.setup()) while leaving the long_description field as you had it seems to fix things for me. Do these look good to you? Any idea why it needs separate platforms and license toplevel fields when there are already Operating System and License fields within classifiers?

[Julian]

Sounds suspicious (like that I broke something while making a modification kind of suspicious) but will take me another day or two to have a chance to investigate. On the classifier question -- basically classifiers do nothing, they're opaque to pip (really setuptools), so pip still needs parameters like license specified separately. Yup it sucks.

…

On Tue, Dec 3, 2019, 12:07 Elijah Newren ***@***.***> wrote: Okay, so adding: platforms = any license = MIT to setup.cfg and ```import shutil shutil.copy("../README.md", ".") to setup.py (before the call to setuptools.setup()) while leaving the long_description field as you had it seems to fix things for me. Do these look good to you? Any idea why it needs separate platforms and license toplevel fields when there are already Operating System and License fields within classifiers? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#16?email_source=notifications&email_token=AACQQXVZ5WY7PL4AXCKKEZ3QW2G5BA5CNFSM4JOCR6X2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEF2DIYA#issuecomment-561263712>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACQQXXXUAYMBPVFOPUCBSDQW2G5BANCNFSM4JOCR6XQ> .

[Julian]

OK I didn't have a full moment to confirm 100% (will see if I can but have just started vacation), but I think actually what was happening was setuptools being unhappy with me specifying the source files relatively to the setup.cfg (having a subdirectory with the setup.py is uncommon, I can't really remember doing this before, but I was suspicious when it seemed to work :) -- the easiest way around that was to just symlink the files and not have them be relative (have a look at the new commit)

There still may be another way to do this, but yeah I think at least now the built distributions are correct.

[newren]

I tweaked the commit message to add 'release:' as the area (and added my Signed-off-by since I edited it). I also added another commit on top to fix a few issues (make sure the README.md actually gets included so that Description isn't "UNKNOWN", set the long_description_content_type so that it is parsed corrected, set the platform and LICENSE fields so they won't be "UNKNOWN", and a few others). I then merged this in an pushed it up, and also uploaded git-filter-repo-2.23.0 and git-filter-repo-2.24.0 to PyPI. Since git.git has done a 2.24.1 release, we can do a git-filter-repo-2.24.1 release and upload it to PyPI to correct any problems, but I think those versions should be working.

[newren]

Thanks for patch and all the explanations, @Julian!

[Julian]

Wow apologies, I apparently resolved this off my to-do list so completely forgot about that last issue, but glad you got it figured out and yeah happy to help, thanks for building this!

…

On Thu, Dec 26, 2019, 21:53 Elijah Newren ***@***.***> wrote: Thanks for patch and all the explanations, @Julian <https://github.com/Julian>! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16?email_source=notifications&email_token=AACQQXWJLRZHE5QFCSWKBPLQ2UKTPA5CNFSM4JOCR6X2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHWEG4Y#issuecomment-569131891>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACQQXREFZLUSUNJIJGRL33Q2UKTPANCNFSM4JOCR6XQ> .