Avoid suggesting to upload dist/* in documentations

[McSinyx]

Some background info into this: So a few days ago I asked my collaborator to handle our project's release for the first time, and he blindly followed twine's README, especially the part of twine upload dist/*, which lead to a bunch of eggs having been uploaded to our index. Of course the fault is in our communication (I told him to only upload the latest release's sdist, but he was careless), but I wonder if we can avoid documenting dangerous copy-pastable commands in the docs.

For more experienced maintainers who know what they're doing, it's obvious that the command is there to only suggest that packages usually in dist, andone can use Twine to upload things in there. However, for newbies, the amount of information might be overwhelming to figure that out at their first glance.

I don't have any specific suggestion to replace the current instruction though.

[sigmavirus24]

I wonder if we can avoid documenting dangerous copy-pastable commands in the docs.

What's dangerous about it?

This is the first I've seen anyone suggest that our quickstart guide examples are dangerous. It's also not easy to document in a way that I expect would have overcome the fact that you asked a task to be done and still blame the person who made the mistake for your not giving them clear enough instructions or automation.

Generally speaking, unless someone has customized their project differently, dist/ should only be the output of whatever build command was run (generally speaking with setuptools). And if one is customizing their project that way, I'd expect them to be documenting their release process. https://flake8.pycqa.org/en/latest/internal/releases.html is an example that documents the release process we have a far less verbose release process documented https://twine.readthedocs.io/en/latest/contributing.html#making-a-new-release as another example.

[McSinyx]

What's dangerous about it?

Sorry for the use of strong word 😄

dist/ should only be the output of whatever build command was run (generally speaking with setuptools)

I am aware of that, and I am referring to the eggs of extension modules. While I admit it's our team's communication issue, it is a fact that I repeated the instructions multiple times, and somehow my teammate decided to copy-paste everything instead (well he is not a very careful person I guess).

Thank you for the suggestion of canonical documentation of the release process though. And just to be clear, I opened this issue because I think that it is something that can be improved/guarded. If Twine's maintainers think that's not something to be done and other measures (e.g. facilitate the quality of communication) should be taken instead, feel free to close the issue. Cheers!

[SanketDG]

There definitely seems to be a place for a check here.

Why not extend twine check functionality to do this?

[McSinyx]

@SanketDG, I don't think I understand what you mean, could you please elaborate? tl;dr of the above: twine upload dist/* might not be a safe suggestion for people new to Python packaging, since the directory may contain, e.g. eggs or packages of unintended releases.

[SanketDG]

twine upload dist/* might not be a safe suggestion for people new to Python packaging, since the directory may contain, e.g. eggs or packages of unintended releases.

I see. I was of the idea that this could somehow be checked and reported before uploading anything, but seems like there can be an arbitrary amount of things that can be uploaded wronfully.

[McSinyx]

that this could somehow be checked and reported before uploading anything

I believe it is not a very good idea to break backward-compatibility for such a simple thing (I assume you're talking about showing packages and ask for users' confirmation), unless there's any way to determine if Twine is running for CD (which AFAIK isn't if keyring is used).

@sigmavirus24, I've thought a bit more on this, and this is the change I meant

diff --git a/README.rst b/README.rst
index 5cb1c45..96f9242 100644
--- a/README.rst
+++ b/README.rst
@@ -72,18 +72,22 @@ Installation
 Using Twine
 -----------
 
-1. Create some distributions in the normal way:
+1. Create some distributions in the normal way.  For example,
 
    .. code-block:: console
 
        $ python setup.py sdist bdist_wheel
 
+   creates a source distribution and a wheel package in ``dist/``, e.g.
+   ``dist/spam-1.2.0.zip`` and ``dist/spam-1.2.0-py3-none-any.whl``.
+
 2. Upload with ``twine`` to `Test PyPI`_ and verify things look right.
    Twine will automatically prompt for your username and password:
 
    .. code-block:: console
 
-       $ twine upload --repository-url https://test.pypi.org/legacy/ dist/*
+       $ twine upload --repository-url https://test.pypi.org/legacy/ \
+       > dist/spam-1.2.0.zip dist/spam-1.2.0-py3-none-any.whl
        username: ...
        password:
        ...
@@ -92,7 +96,7 @@ Using Twine
 
    .. code-block:: console
 
-       $ twine upload dist/*
+       $ twine upload dist/spam-1.2.0.zip dist/spam-1.2.0-py3-none-any.whl
 
 4. Done!
 

Note that the shell break is POSIX so probably it's not what we want finally. If you agree, I can open a new PR for this, with help output adjusted accordingly.

[bhrutledge]

dist/ should only be the output of whatever build command was run (generally speaking with setuptools)

I am aware of that, and I am referring to the eggs of extension modules.

the directory may contain, e.g. eggs or packages of unintended releases.

@McSinyx I haven't yet built a package that involves extension modules. Can you give an example of the commands you (or your team) are running that results in those showing up in dist/?

I have experienced the "unintended releases" caveat. I think that's one of the reasons for the --skip-existing option. My resolution is to have the equivalent of rm -rf dist/ as the first step of my release process, prior to building the package. Twine also does this via tox -e release:

twine/tox.ini

Lines 76 to 79 in 653e82f

	commands =
	python -c "import path; path.Path('dist').rmtree_p()"
	python -m pep517.build .
	python -m twine upload dist/*

[sigmavirus24]

So, I'll reiterate, if we had more in-depth documentation that went through things like the Packaging Guide does (creating a project named spam, setting up the metadata, building an artefact, testing the artefact, registering a PyPI account, and then uploading the artefact(s) to PyPI) then I'd be totally fine clarifying those docs for this case.

For the readme, however, which is not our canonical documentation location anyway but instead a quick "Hi there person new to this project, here's why you might want to use it" I think that leaving it generic enough makes sense. If there is someone brand new to the world of packaging who is trying to follow just our README, I think naming the files spam with arbitrary version numbers is far more confusing. Add to that the fact that someone new wouldn't have unrelated eggs downloaded in that directory.

I'd be willing to include a link to the proper packaging guide, but I don't think that obviates the "Here's why you should use twine" aspect of our README.

---

Aside from the merits of changing the README here, I think there's a larger problem in how you treat your colleagues that is becoming evident in how you talk about the person who uploaded eggs.

it is a fact that I repeated the instructions multiple times, and somehow my teammate decided to copy-paste everything instead (well he is not a very careful person I guess).

It sounds like you verbally said to the person "twine upload blah blah blah" and when they couldn't take notes fast enough, they didn't want to bother you because you've already made yourself unapproachable to them. They did their best with what very little you gave them to work on, and you're shifting all of the blame on them as if you have no responsibility here. That:

A) Seems unrealistic (that you're blameless)
B) Is incredibly harmful to that colleague if they hear you speaking of them this way. It sounds like they're less experienced than you, and it seems you're making whole character judgements about them based on 1 mistake

I really think you're trying to solve the wrong problem with this issue and I think you need to step back and reflect on your own responsibility in the incident you experienced.

[sigmavirus24]

And to be clear, I'm stuck on this point because I won't be complicit in your placing the blame on this person. This is clearly someone you're either on-boarding, mentoring, or coaching and when you haven't helped that person learn how to learn or given them a safe way to learn, you've clearly failed them. The mistake is not theirs, and I won't let the result of this be an implicit blame game that gives you ammunition to blame the person who made a mistake.

[McSinyx]

@bhrutledge, the following is the content of my dist (I remove it quite frequently and I have not been in charge of last release hence no sdist):

dist/
├── palace-0.1.1-py3.7-linux-x86_64.egg
├── palace-0.1.2-py3.7-linux-x86_64.egg
├── palace-0.1.3-py3.7-linux-x86_64.egg
└── palace-0.1.3-py3.8-linux-x86_64.egg

@sigmavirus24, I'm sorry for the time and energy you have to spend on this (somehow I feel like you're feeling uncomfortable dealing with me). Anyway, back to the discussion

So, I'll reiterate, if we had more in-depth documentation that went through things like the Packaging Guide [...] then I'd be totally fine clarifying those docs for this case.

Python packaging guide also suggests twine upload dist/*. Based on the output of this discussion I might file a request at bpo to elaborate the guide, if you all at Twine think it's a good idea.

If there is someone brand new to the world of packaging who is trying to follow just our README, I think naming the files spam with arbitrary version numbers is far more confusing.

Got it.

Is incredibly harmful to that colleague if they hear you speaking of them this way. It sounds like they're less experienced than you, and it seems you're making whole character judgements about them based on 1 mistake

I really thought careless would be understood as sometimes making mistakes, as we all do when we're not 100% focus. Really hope @9a24f0 is OK with it. Edit: this also means that I and the majority of your users are careless, and that's why I filed this issue in the first place--so even if we are careless, unintended things won't happen.

your placing the blame on this person. This is clearly someone you're either on-boarding, mentoring, or coaching and when you haven't helped that person learn how to learn or given them a safe way to learn, you've clearly failed them. The mistake is not theirs, and I won't let the result of this be an implicit blame game that gives you ammunition to blame the person who made a mistake.

I guess I'm totally in capable of communication at this point 😄 No, it was not just his fault, and very likely to be my inability to deliver any information clearly, as well as my will to give them all permissions to develop, deploy and make mistakes (our project is not that important, if something goes wrong I'll be able to clean up just fine). See McSinyx/palace#70 (comment) for the instruction I gave and McSinyx/palace#74 for my brake down (sorry for the strong language). Including the original instruction, there are at least two times I mentioned to only upload the source distribution (at the 3rd probably they already ran the command), but I guess the entire release process is a bit overwhelming.

Long and meaningless emotional ebolarations

And to be clear, I'm not opening this with the intention to put the blame somewhere--whatever happened in our project got solved in like 10 minutes or so (I'm not in the place with the best Internet ever) and we have moved on from that. What I tried to communicate here is basically hey, there is something that might be possible to be made clearer, and if the it is not possible, it has zero impact on our team. I am not in your shoes, so I don't know if the suggestion is plausible, I just want you to know that there was that incident, and maybe we can avoid it for others.

Thank you for the human relationship advise though, I'm in the awkward position of working with my friends, sometimes I don't want to repeat things over and over or giving too details as if I am lecturing them; on the other hand if I handle them a few pieces of info somehow one of the info will get slipped away. Maybe I'm pushing them too hard out of their comfort zone and everything is overwhelming, from VCS to deployment )-: I just make them people I can rely on and share the workload with.

[bhrutledge]

Quick note: we do link to the Python Packaging User Guide in the "Using" instructions.

Also, @McSinyx, it's still not clear to me how the *.egg files ended up in dist/ (which is probably due to my inexperience). If you don't mind, can you share the command that generates those files? My limited understanding is that Eggs have been superseded by Wheels.

[McSinyx]

@bhrutledge, the eggs are generated as artifact of ./setup.py install --user. Yes, we are aware of pip install . -v --no-build-isolation which pretty much does the same thing, just that the log of direct call to setuptools looks a bit cleaner at the moment. Now all of my mates are comfortable with the workflow and the release process, it doesn't matter which command they use for local installation and testing. (However, for the matter of completeness, I'll link and summarize this discussion to them based on what I learned or think that they may find useful.)

Additionally, with GH-593 in mind, I think the intuitive logic for an average user like me is to be able to build and distribute the current version, i.e. something like flit publish. Since the confusing spam suggestion is rejected, I think we can add a small note after the instructions in README saying that dist/ may contain more than what one wants to upload to the package index.

[bhrutledge]

I think we can add a small note after the instructions in README saying that dist/ may contain more than what one wants to upload to the package index.

As I think was suggested earlier, I'm inclined to keep Twine's instructions simple, and lean on the PyPA guides. I also think that in general, the instructions as written (using setup.py sdist bdist_wheel) are safe.

Closing this issue, but further suggestions for documentation improvements are welcome via PR's or issues.

[sigmavirus24]

I really thought careless would be understood as sometimes making mistakes, as we all do when we're not 100% focus.

There's a difference between someone has been careless, versus what you said which is that "they are not a careful person". One allows for it to have been a mistake. Everyone has been inattentive to detail (a.k.a., careless) at one point or another, and often for reasonably good reason. Saying someone is "not a careful person" instead says they are intrinsically flawed in a way which is irredeemable and you are passing judgement on them. It shows that you have a low opinion of this person you call a collaborator (a fancy term for a peer) and that you don't actually view them as an equal.

Edit: this also means that I and the majority of your users are careless, and that's why I filed this issue in the first place--so even if we are careless, unintended things won't happen.

I don't understand how you think it makes it any better to say "the majority of twine's users are careless". The majority automate their release process and document it according to best practices.

Thank you for the human relationship advise though, I'm in the awkward position of working with my friends, sometimes I don't want to repeat things over and over or giving too details as if I am lecturing them; on the other hand if I handle them a few pieces of info somehow one of the info will get slipped away.

I think this is honestly the most important piece here. It seems as though the fact that you've added your friends to your project as collaborators is making it harder for you to work with them and trust them. One thing all teams (professional and open source) rely upon is trust and psychological safety.

Take this project as an example, @bhrutledge is a relatively new maintainer but he built trust with the existing maintenance team and earned his spot as a maintainer of the project. He's proposed ideas for how to improve the project and regardless I still feel comfortable asking him hard questions about those proposals. I'd suggest you take time to build up trust with your friends around your project

[McSinyx]

@sigmavirus24, thank you for your advises on working with others. I'm pretty content with my team at the moment after incrementally get them used to new stuffs. That being said, were I in this position again, I'd take it slowlier for the transition periods to be less rough. We (my mates any I) are just students and are still learning how to work and to work with each others--I hope our project stays a pleasant experience for all of us where we discover lots of cool things.