Need a option to sign when uploading (pass --sign to twine upload)

[htgoebel]

For PyInstaller, we are using a releaser.before_upload hook to sign releases.
Now twine no longer allows uploading the additional file but fail (see below) with

  Unknown distribution format: 'PyInstaller-3.3.tar.gz.asc'

Proposed solution

twine upload has an option sign, so all we need is a way to pass this option to twine upload.

Traceback

$ release
Tag needed to proceed, you can use the following command:
git tag 3.3 -m "Tagging 3.3"
Run this command (Y/n)? 
Check out the tag (for tweaks or pypi/distutils server upload) (Y/n)? 
INFO: Doing a checkout...
…
Writing PyInstaller-3.3/setup.cfg
creating dist
Creating tar archive
removing 'PyInstaller-3.3' (and everything under it)

Signing file /tmp/PyInstaller-3.3-8mqhmxfv/gitclone/dist/PyInstaller-3.3.tar.gz

… [Enter passphrese for gpgp key]
Upload to testpypi (Y/n)?     
Traceback (most recent call last):
  File "/tmp/py-web/bin/release", line 11, in <module>
    sys.exit(main())
  File "/tmp/py-web/lib64/python3.5/site-packages/zest/releaser/release.py", line 332, in main
    releaser.run()
  File "/tmp/py-web/lib64/python3.5/site-packages/zest/releaser/baserelease.py", line 353, in run
    self.execute()
  File "/tmp/py-web/lib64/python3.5/site-packages/zest/releaser/release.py", line 71, in execute
    self._release()
  File "/tmp/py-web/lib64/python3.5/site-packages/zest/releaser/release.py", line 317, in _release
    self._upload_distributions(package)
  File "/tmp/py-web/lib64/python3.5/site-packages/zest/releaser/release.py", line 167, in _upload_distributions
    self._retry_twine('upload', server, filename)
  File "/tmp/py-web/lib64/python3.5/site-packages/zest/releaser/release.py", line 185, in _retry_twine
    package_file = PackageFile.from_filename(filename, comment=None)
  File "/tmp/py-web/lib64/python3.5/site-packages/twine/package.py", line 93, in from_filename
    os.path.basename(filename)
ValueError: Unknown distribution format: 'PyInstaller-3.3.tar.gz.asc'

[reinout]

Weird. If I look at the 1.9.1 twine code, the part of the code that prints out "unkown distribution format" loops over a list where *.asc has been filtered out!

So I assume it still ought to work. I've checked the current master (https://github.com/pypa/twine/blob/master/twine/commands/upload.py#L87) and it still looks fine, there. It should automatically pick up a .asc signature.

Can you debug a bit further? I've looked at passing in sign=True or so, but it looks like you need to pass in an identity, too, then. So it looks a bit more complex than I expected.

[htgoebel]

So I assume it still ought to work.

I tried the latest release and it did not work. I have no time for debugging this, sorry.

[htgoebel]

Instead of spending time on debugging this, the time could be used to add the requested option and we could even drop our plugin :-)

[mauritsvanrees]

It goes wrong because zest.releaser is calling Packagefile.from_filename on all files, including the .asc file, and that one fails.

We could filter out those .asc files so twine does not choke on them. And for any file that we upload, we could check if there is an accompanying .asc file that we pass to twine in one command.

[htgoebel]

I started implementing this and while analyzing how to implement it, I (presumably) found the root-cause of this issue:

As @mauritsvanrees write, twine contains code for filtering .asc-files and adding them as signature for the respective file. This is done in twine.commands.upload.upload(). But zest.releaser is using a low-level function, Repository.upload(), passing files one by one. Thus the aforementioned code in commands.upload is not executed.

What about switching to use the high-level functions? As far as I understood releaser's code, we could even get rid of quite some code, e.g. for handling repositories, and maybe even more for reading the .pypyprc-file

[reinout]

Note: we use the .pypirc file also for some defaults. I don't think it is used very much in practice for that purpose, however.

[reinout]

Getting rid of code is always good, in principle.

Twine: we'll have to make sure the higher-level functions are actually intended to be used as an API. If I remember correctly, twine once changed its internal API, breaking zest.releaser. At that time, it wasn't intended to be used as an API yet (or we probably picked the wrong abstraction level and used functions that were too low-level).

Summary: if we're going to use twine on a different level, we'll have to talk to the twine people first :-)

[htgoebel]

It should be reasonable save to "run" the commands, e.g.
twine.commands.upload.main("--repository", server, *files_in_dist).

Anyway talking to the twine people would be a good choice. Are you going to do this?

[mauritsvanrees]

Nope. We were using the upload and register functions before, but the twine people have made it abundantly clear in the past that those should not be used as an API. See #189

[htgoebel]

I'm not suggesting to use upload(), but main() and passing it the command line arguments. So it's nearly the same as running twine via `subcommand.Popen()?. ID#d expect the command-line arguments not to break without prior notice. YMMV.

[reinout]

Our mileage already did vary :-)

But it might be OK to call main(), though I'm not sure, seeing the discussion @mauritsvanrees had.

It sounds a bit the same as pip, that one also has no API. "The only API is the command line". So tools like buildout cannot use it except through the command line (if buildout wanted).

[mauritsvanrees]

Easiest still seems what I suggested earlier:

We could filter out those .asc files so twine does not choke on them. And for any file that we upload, we could check if there is an accompanying .asc file that we pass to twine in one command.

That seems doable without much extra code and without a complete restructuring of how we call twine.

[htgoebel]

We could filter out those .asc files so twine does not choke on them. And for any file that we upload, we could check if there is an accompanying .asc file that we pass to twine in one command.

I tried this and found myself re-implementing quite some parts of twine.commands.upload#upload(). Not only one would need to filter out the .asc files first, but also add_gpg_signature to each file to be uploaded, if the signature exists. Although this would still be only a view lines, keeping up with twine features and PyPI API changes would become a cat-and-mice-game in he long run.

I asked for another high-level api (see pypa/twine#194 (comment)), one of which twine.cli.dispatch() could be. This would allow something like:

dispatch(twine_command, '-r', server, *files_in_dist)

[reinout]

I think we have this fixed now in 6.18.0, right?

[htgoebel]

I think we have this fixed now in 6.18.0, right?

I can't see how 6.18.0 solved this. Did I miss something?

[reinout]

You said this in one of the comments:

What about switching to use the high-level functions? As far as I understood releaser's code, we could even get rid of quite some code, e.g. for handling repositories, and maybe even more for reading the .pypyprc-file

So I guessed that, now that we call twine.cli.script(), twine does all the work for us, including dealing with .asc files.

I thought that "just let twine do the right thing" was one of the goals of the recent change :-)
I might be mixing things up, though!

[htgoebel]

We still need to pass --sign? to twine. Please reopen this issue. Thanks.

[htgoebel]

PyPI no longer accepts GPG signature files: they are simply discarded — and the developer gets an e-mail about this. So this can be closed.