zestsoftware / zest.releaser

Python software releasing made easy and repeatable
https://zestreleaser.readthedocs.io
GNU General Public License v2.0
198 stars 62 forks source link

Twine high level (with fixes) #309

Closed reinout closed 5 years ago

reinout commented 5 years ago

This is the branch from @htgoebel with some fixes, mostly test setup.

reinout commented 5 years ago

I especially hope that travis doesn't hang on some input anymore... :-)

reinout commented 5 years ago

@mauritsvanrees: OK with the change to effectively call twine's main() method? I do hope they keep it...

@htgoebel: OK with my changes to your branch? At least the tests run fine now. I guess twine asked something on the commandline.

Oh wait... if twine asked something on the commandline (possibly due to a missing pypirc on travis?), does the question show up when running zest.releaser if you don't have that file?

htgoebel commented 5 years ago

@reinout Okay with whatever is required to make get this merged :-)

reinout commented 5 years ago

Hmmm. We're effectively forbidden from using twine.cli.dispatch. See https://github.com/pypa/twine/pull/361#issuecomment-456804957

I'm thinking about just calling it on the commandline, but getting our hands on the path to the executable might be hard. Installing it as a dependency doesn't always mean the command line tool also gets installed (at least historically, that wouldn't be the default case for buildout).

This is taking hours of my time. We're told not to use setup.py upload. Then we start using twine and just like pip, there just isn't anything we're allowed to call or import.

htgoebel commented 5 years ago

No offense meant, but IMHO sigmavirus24 is not able (or willed) to respect user's needs but wants to implement cool stuff. So I would simply ignore https://github.com/pypa/twine/pull/361#issuecomment-456804957.

mauritsvanrees commented 5 years ago

Looks like with this PR we can throw a lot of code away, which is good. But we would be using a function that may get renamed, which is bad. I remember a threat by the same person in a similar situation about the twine.commands submodule:

The twine.commands submodule should not be used by anyone. It appears you still disagree with this, so if I have to, I will break that intentionally in 1.9 by renaming it.

Alternatively, we could call twine on the command line. We would need to find it first then. In zestreleaser.towncrier I have code to find the towncrier script. Something similar could be done to find twine. And we could warn early when twine is not found.

htgoebel commented 5 years ago

@mauritsvanrees If the function might be renamed (which I assume chances are low), it should be ease to fix. There always will be some way to access twine at this high level.

If you want to call twine using subprocess.call(), I'd simply assume it can be found on $PATH (might not be the case for windows, though).

Alternatively one could use pkg_resources.load_entry_point("twine", "console_scripts", "twine") to get teh main() function, then set sys.argv and call the gathered function. I would expect this to not change anytime soon.

Anyway, I would like to get this pull-request merged, as my change for uploading signed archives depends on this one.

reinout commented 5 years ago

I've merged master. The merge conflict was about the recent #319 ("also accept status code 201") change.

With the new try/except, I'm pretty sure that 201 doesn't raise an exception, so that ought to continue to work.

reinout commented 5 years ago

Well, the twine.cli.dispatch has been stable for six years, so I'm guessing we'll be OK. I've added a try/except ImportError with a quick explanation, pointing at this PR. So if it breaks, we'll hear soon enough.

reinout commented 5 years ago

I'll merge it. We can try the entry point or commandline-calling later if something breaks, right?

reinout commented 5 years ago

Well, releasing 6.18.0 didn't break horribly, so for now it works :-)