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

selectively add setuptools_git as a setup_requires #273

Closed
wants to merge 1 commit into from

Conversation

tisdall
Copy link
Contributor

@tisdall tisdall commented Jun 2, 2015

I needed to refactor my repo so I'm creating a new PR and closing #257. Please refer to #257 for the discussion.

@goodwillcoding
Copy link
Member

Are there any reasons other then those listen in #257 for this? I fully agree with @mmerickel that it is not setup.py's job to specify the "where". That is what requirement.txt is for.

@tisdall
Copy link
Contributor Author

tisdall commented Jan 11, 2016

I think this is a "what" issue, not a "where" issue...

The current stance is that people should install via pypi, but since no releases are being made, more and more people are going to have to install from git to have something working. As there are no instructions it's likely more and more people are going to end up with an install that's missing the templates and other static content. This was a simple patch to stop others from falling into the same pothole over and over again (and stop others from wasting hours like I and others ended up doing).

@stevepiercy
Copy link
Member

@goodwillcoding this issue should not have been closed, as it is dependent on the resolution of Pylons/pyramid#121

@stevepiercy stevepiercy reopened this Jan 11, 2016
@goodwillcoding
Copy link
Member

@stevepiercy ... it's not the same issue. And as far as I can tell Pylons/pyramid#121 is about a Manifest.in and setuptools_git is not considered a good alternative. See: Pylons/pyramid/issues/121#issuecomment-4747370

@tseaver
Copy link
Member

tseaver commented Jan 12, 2016

setup_requires will be injected into the static metadata for a release, when it is really only wanted in development mode.

@tisdall
Copy link
Contributor Author

tisdall commented Jan 13, 2016

@goodwillcoding - Correct me if I'm wrong, but it's my understanding is that setuptools_git is what is being used to generate the necessary MANIFEST when releasing to pypi. See: https://github.com/Pylons/pyramid/blob/master/RELEASING.txt#L48

@tseaver - Any suggests to work around that with respect to this patch? Is there a way to detect someone running setup.py install vs one of the "dist" commands?

@goodwillcoding
Copy link
Member

@tisdall that is correct.

However the real bit is that you are using 'setup.py install' which is not a supported method of installing deform. Currently supported method fro install is easy_install from a generated sdist. pip install also works because it is mostly easy_install compatible.

As @stevepiercy pointed we should probably have some docs that basically say "setup.py install" is not supported. However there are other things that are not supported (like let's say bdist_egg and so on) too and it can be a tad silly to have "negation" style docs as they are infinite

@tisdall
Copy link
Contributor Author

tisdall commented Jan 13, 2016

Well, it'd be nice if there was a way to detect setup.py install and throw an error instead of allowing a broken install instead. A broken install is really hard to diagnose if you don't already know why the files are missing and why. setup.py install works on every other project I've tried and Pylons' projects are the only exceptions that I've come across. I suspect a lot of people end up trying it not expecting to find a broken install and not expecting to hunt through github issues to find a reason (because there's no mention in the docs).

I've not really understood why this is such a contentious issue with Pylons projects when so many other Python projects seem to have no issue with allowing users to setup.py install. I'm not looking for a response to that statement as I've heard all the arguments already... My major point is I don't think you can expect people to "only use the pypi package" when you've completely stopped making releases. The last release is Oct 2013!

@goodwillcoding
Copy link
Member

setup.py install is pretty broken code. This is why the PYPA people gave up on in it and wrote new code with distribute/pip. It has all kinds of issues. It works on many projects cause they are simple but anything with even tad complexity breaks it. This is why it is not supported and a solution of adding one more dependency is not much of a solution.

@goodwillcoding
Copy link
Member

To note I agree it would be nice, but it puts more burden on the maintainers.

@stevepiercy
Copy link
Member

To resolve the underlying issue that precipitated this PR, I propose that documentation be written or modified that covers the supported installation methods and explicitly states that these are the ONLY supported methods at this time. We could explicitly exclude certain methods, such as python setup.py install, as well (I'm OK with listing the most common pitfalls so we don't have to revisit this issue).

Here's what I came up with off the top of my head:

We do not support installation of <package> using the following:

  • python setup.py install
  • other common pitfall

For the supported methods, scenarios for using a particular method should be included:

  • "To install a package in editable mode so you can edit its source, not have to reinstall what you've edited, and have your changes take immediate effect, use python setup.py develop."
  • "To install (typically third party) packages that you will not develop, edit, or debug, use pip install <package>."

I'll do the docs mods, provided I have some help and proofing for technical accuracy.

What do you all think?

@mmerickel
Copy link
Member

I'd tend toward what Pyramid already has... a RELEASING.txt with the couple steps necessary to bundle a release from a clean checkout.

Distributing a new release
==========================

- Make sure your Python has ``setuptools-git``, ``twine`` and ``wheel`` installed:

    $ $VENV/bin/easy_install setuptools-git twine wheel

- Do a platform test:

    $ tox -r

- Ensure all features of the release are documented (audit CHANGES.txt or communicate with contributors).

- Change setup.py version to the new version number.

- Change CHANGES.txt heading to reflect the new version number.

- Make sure PyPI long description renders:

    $ $VENV/bin/python setup.py check -r

- Build an sdist and a wheel:

    $ $VENV/bin/python setup.py sdist bdist_wheel

- Release the wheels to PyPI:

    $ $VENV/bin/twine upload dist/deform-X.Y*

- Upload a git tag for the release:

    $ git tag X.Y
    $ git push origin X.Y

- Update RTD to render a new version of the docs at that tag X.Y and set X.Y
  as the default branch such that /latest/ points to it.

- Announce the release on the mailing list.

- Announce the release on twitter.

This is the ONLY supported way of creating a new release of the package. Once you have an sdist or wheel you can use pip/easy_install to install that.

@tisdall
Copy link
Contributor Author

tisdall commented Jan 13, 2016

@mmerickel - I'm not sure I understand... A person wanting to use a repo copy of a project isn't going to need those steps.

@tisdall
Copy link
Contributor Author

tisdall commented Jan 13, 2016

@stevepiercy - Since this is sort of a "Pylons policy", I could see a single document being created in Pyramid and then the small projects linking to it. Something like "recommended way..." and "if you want to run the latest code from the repo then...". And then perhaps a "common pitfalls" explaining why a person would be missing .css files, etc.

@mmerickel
Copy link
Member

@tisdall sorry but they do need those steps. That's how you use our repository to create a distributable package.

@stevepiercy
Copy link
Member

@mmerickel I thought this issue is for installing a package, not distributing one?

@tisdall we still need to provide explicit instructions per repo, so linking to another repo would not really work. We could add a Pylons Project guideline in an abstract way, where the user can substitute the repo/package as needed. Probably on this page: http://www.pylonsproject.org/community/how-to-participate

@tisdall
Copy link
Contributor Author

tisdall commented Jan 13, 2016

@mmerickel - I think that would create confusion, not alleviate. People just need to know how to get an installed copy that includes the files that are usually excluded without being listed in a manifest.

@tisdall
Copy link
Contributor Author

tisdall commented Jan 13, 2016

@goodwillcoding - I tried pip install [deform directory] and the templates are excluded. I'm not sure what you're talking about. The setuptools docs also say "The setuptools install command is basically a shortcut to run the easy_install command on the current project". I suspect there's not much difference between setup.py install and pip install [repo directory].

@mmerickel
Copy link
Member

@stevepiercy @tisdall The entire question is "how do I create a distributable package"? The instructions for RELEASING are how to create one. You MUST install setuptools-git and run sdist or bdist_wheel. Those are your only options for distributing a package built from our repository. Optionally you can also use the repository as a source-level install via pip install -e . or python setup.py develop but your question is about installing a package that is decoupled from the source tree. That is called a distribution.

@tisdall
Copy link
Contributor Author

tisdall commented Jan 13, 2016

@mmerickel - I'm still not clear on what you're saying... Are you saying there should be a RELEASING.txt and people wanting to use the source version should read through that and figure out just the parts they need to get a working install? That document doesn't even explain how to make use of the packaging in someone's own project after creating it (just how to submit it to pypi). Even if the document exists, I'm not sure how people would know to look at it... I know I'd look at it and say to myself "I don't need this, I'm not releasing a new version".

@tisdall
Copy link
Contributor Author

tisdall commented Jan 13, 2016

So, it seems you can create your own setup.py install by adding cmdclass={'install': my_install_method} into setup(). What about adding a function that simply throws an exception with a useful message if anyone tries to do setup.py install?

@goodwillcoding
Copy link
Member

@tisdall I fully agree with @mmerickel. Again, you can only install a distributable package. A git checkout is not a distributable package. You first have to create one using the process that @mmerickel mentioned. We can add docs on that.

As mentioned before "setup.py install" is not anywhere in the docs as an approved method to install deform because it is a super broken in many ways. Adding yet one more dependency setuptool-git that fixes part of this broken code is not a good solution.

And yes setuptool-git is used currently for release, but that is a maintainers level tool.

@stevepiercy we can port the Releasing do to deform. Would you like me to do this.

@goodwillcoding
Copy link
Member

@tisdall there is no reason to add cmdclass={'install': my_install_method} ... again we are NOT planning to support setup.py install

@tisdall
Copy link
Contributor Author

tisdall commented Jan 13, 2016

@goodwillcoding - wait... whut? How are you planning on supporting setup.py install??

@goodwillcoding
Copy link
Member

@tisdall typo. Not planning to support it in anyway.

@mmerickel
Copy link
Member

@tisdall I'm sorry but it should be clear right now that we simply do not support installing directly from source no matter how many times you ask. You MUST create a distributable package and then install that, and to do that you must follow our RELEASING.txt guidelines (defined in pyramid's repo, but I've offered to create a copy of that in the deform repo to make it more clear).

$ easy_install setuptools-git
$ python setup.py sdist

From here you have a distributable which you can install from PyPI or copy it around yourself and install it.

$ easy_install dist/deform-X.Y.tar.gz

@tisdall
Copy link
Contributor Author

tisdall commented Jan 13, 2016

Okay, maybe you guys missed it... I'm saying FAIL to install someone doing setup.py install by explicitly giving people a message. The message could say something like "We don't support using setup.py install, try using setup.py develop". This would be instead of just installing a broken package with no message.

@goodwillcoding
Copy link
Member

@tisdall again no reason to document a negation or write code for it. For example, we also do not support installs by people using using "cp" command and copying file to dist-package but we are not going to have document that either. It does not matter if people decide to do that, it's up to them. shrug

Again it's not a supported method in anyway.

@rbu
Copy link
Contributor

rbu commented Jan 14, 2016

Please consider that there are users of deform that want to package/deploy a branch of your software, but are not committers to the Pylons project. I include myself in that.
Four years ago, I needed a few changes to deform. I created a git fork, developed changes, sent PRs upstream. Since I needed these changes in production-level projects, I did not want to git clone && setup.py develop. Rather, I tried to create a "local" package that I would distribute via our project's infrastructure.

I still assume this is a proper way to make use of and contribute to open source software. It's expected to find bugs and climb a learning curve in a process like this. I spent some hours figuring out what is wrong and submitted a pull request (#107), with the sole intent to avoid others would have to spend this time again. I find it incredibly sad that after almost four years, you are still refusing people's contributions and suggestions that one should "simply know" which build steps are supported in this project and which are not.

@tisdall
Copy link
Contributor Author

tisdall commented Jan 14, 2016

@rbu - thanks for replying... It's nice to know I'm not the only one trying to save a fellow developer a few hours I've already wasted.

I thought the MANFEST.in aversion was a Pylons project wide thing, but apparently it's not: https://github.com/Pylons/webob/blob/master/MANIFEST.in

goodwillcoding added a commit that referenced this pull request Jan 14, 2016
Should address some concerns in #273
@goodwillcoding
Copy link
Member

@rbu Lack of documentation of the release process for deform is a valid concern and we agreed wtih @tisdall on that. I just committed a RELEASING.txt which describes what the release process deform is using. I hope that helps address the same concern you have.

@goodwillcoding
Copy link
Member

@tisdall To note, WebOb project was adapted into Pylons, and we kept the process for it as is since it worked. It may change in the future.

@mmerickel mmerickel closed this Jan 14, 2016
@digitalresistor
Copy link
Member

On a separate note: I am thinking of removing it in WebOb because it has caused more issues in the past than it has been worth (specifically, I shipped a version of WebOb that was not just pure source for the sdist due to MANIFEST.in failures and "prune" no longer functioning as intended).

See: Pylons/webob#201

@tisdall
Copy link
Contributor Author

tisdall commented Jan 14, 2016

@rbu - I love how I'm being told I said we needed documentation for a releasing process... I'll let someone else tell me that I said this was the right solution to this problem, too.

@goodwillcoding
Copy link
Member

@tisdall I did not say you you said "that we need documentation". We discussed the problem you are trying to solve with your PR and from it came the notion of having clearer documentation of how release process works with deform. This process uses git checkout to create a distribution that can be installed by any.

Sometimes solutions that are implemented by maintainers won't match the solution you want. If you find it hard to accept and you want your EXACT solution please consider forking deform into your own library and maintain it anyway you see fit. This works well with the model for open source and that's why deform is licensed under such a permissive license.

Then you will no longer be encumbered by decisions of others, though you might then deal with other asking you to implement solutions you do not want in your library.

That said this issue is closed and there is no reason to use the thread it for unproductive comment. This is really is not the right venue for this.

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.

7 participants