-
Notifications
You must be signed in to change notification settings - Fork 102
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
Unpin requirements.txt #162
Comments
First, thanks for this powerful package! I would actually rename / invert the title and logic desired here. Unfortunately, constraining the packages pinned in the setup.py is not only bad practice as defined by pip, but creates also a lot of pain to many people when using gitlint with other dev tools. Not only with black, but it is actually unusable (because of incompatible versions) with most packaging tools like poetry or pip-tools if you have some other libraries defining click, arrow or dateutil.. Thus my suggestion would be this one:
What do you think about it? |
I’ve proposed a possible compromise solution in #220: provide the pinned requirements with |
I mean |
I understand the underlying frustration. IMO, it should be directed at the Python packaging system. Python does not allow installing multiple versions of a package, the way (say) Node.js does—in Node.js you could have different versions at But in Python, single versions are the reality we have to deal with, so most Python package maintainers will need to deal with a few bug reports about version incompatibilities.
Receiving these bug reports doesn’t mean your software is bad—it’s just part of the job description of a Python package maintainer. Although neither type of bug report may be theoretically avoidable, in practice, backwards-incompatible changes to Gitlint’s requirements have never affected Gitlint: if you remove the pins, the very newest versions of Gitlint’s requirements seem to work fine with the very oldest version of Gitlint that supports Python 3 at all. (I understand that some of the tests are more sensitive to the precise versions, but I’m not proposing to allow unpinning the versions used for running tests.) But version conflicts plainly do affect Gitlint: there were many complaints when Gitlint could not be co-installed with Black, and right now Zulip is unable to use the current version of Gitlint because it cannot be co-installed with the current version of Semgrep. By its nature as a linter tool to be used within other projects, many of which are also written in Python, Gitlint is most useful when it places as few restrictions as possible on the Python libraries it can be co-installed with. On the other hand, I understand the advantage of providing a recommended set of known-good versions. I’m trying to find a way to deliver this advantage while acknowledging the reality that users sometimes need to override these recommendations. No solution is perfect, but I’m hopeful that using an extra will work well enough. As far as extra names, I’m happy to go with whatever name you decide on. |
I want to re-emphasize that my position on this has little to do with what is the right/wrong technical decision. I think you make a strong argument @andersk, but in the end it all comes down to the same thing: my availability to timely merge changes and push releases out. That is not guaranteed at all (see #134 for additional context). As a result, I've made a decision that I believe will cause me the least pressure. I do like the idea of using extras as a way to facilitate us both - that hadn't occurred to me. However, for me to be comfortable with it, I'd want the logic to be reversed: use pinned dependencies when using the 'default' Having a quick look at |
Unfortunately, the Python packaging system doesn’t support extras that loosen dependencies rather than tightening them, nor does it support default extras (pypa/setuptools#1139). A possible workaround would be to rename the main package to (say) Alternatively, what do you think about simply adding a question to the GitHub issue template asking whether the user installed with |
@jorisroovers is there any chance you can resolve this, through one of removing the package pinning, merging a version of #220, or if you don't have time, through giving @andersk the necessary permissions execute a solution you're happy with? If this can't be resolved in the next few weeks, Zulip will likely need to either fork Gitlint or stop using it, neither of which feels like a happy resolution to this packaging issue. (We're currently working around this problem by using the Ubuntu package rather than using gitlint via pypi, but we need our development environment to work fully on Debian 10, which doesn't have said package). |
Ok, no guts, no glory - so let's try something :-) I've actually been pondering about @andersk suggestion of creating 2 packages, Any suggestions on how to best do this? Creating a separate repo or creating 2 separate packages from this source code repo? I've never done the latter. I want to avoid renaming this repository. On timeline, depends on how much work I need to do myself. Appreciate PRs :-) @sigmavirus24, please chime in. |
To summarize this suggestion:
I'd argue that the solution should be the inverse:
I've done something similar to this in the past.
If we want to be more conservative
As for practical implementation: We could use 2 repositories. There's not much reason to do that though, so we could instead have 2 package directories.
Otherwise, 2 repos could work well. We just need to figure out how to redirect people for PRs and other contributions |
gitlint’s pinned requirements make it difficult to use with other libraries and tools that have newer requirements, and may hold back important security updates. However, the maintainer prefers to expose pinned requirements by default. Reconcile this by splitting gitlint into two packages: * gitlint-core has unpinned requirements by default, but pinned requirements with the [trusted-deps] extra. * gitlint becomes an empty package that requires gitlint-core[trusted-deps]. Fixes jorisroovers#162. Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@sigmavirus24 No, this suggestion was exactly what you describe as “the inverse”, where Anyway, it sounds like we’re on roughly the same page, so I’ve put together a PR: #246. |
gitlint’s pinned requirements make it difficult to use with other libraries and tools that have newer requirements, and may hold back important security updates. However, the maintainer prefers to expose pinned requirements by default. Reconcile this by splitting gitlint into two packages: * gitlint-core has unpinned requirements by default, but pinned requirements with the [trusted-deps] extra. * gitlint becomes an empty package that requires gitlint-core[trusted-deps]. Fixes jorisroovers#162. Signed-off-by: Anders Kaseorg <andersk@mit.edu>
gitlint’s pinned requirements make it difficult to use with other libraries and tools that have newer requirements, and may hold back important security updates. However, the maintainer prefers to expose pinned requirements by default. Reconcile this by splitting gitlint into two packages: * gitlint-core has unpinned requirements by default, but pinned requirements with the [trusted-deps] extra. * gitlint becomes an empty package that requires gitlint-core[trusted-deps]. Fixes jorisroovers#162. Signed-off-by: Anders Kaseorg <andersk@mit.edu>
gitlint’s pinned requirements make it difficult to use with other libraries and tools that have newer requirements, and may hold back important security updates. However, the maintainer prefers to expose pinned requirements by default. Reconcile this by splitting gitlint into two packages: * gitlint-core has unpinned requirements by default, but pinned requirements with the [trusted-deps] extra. * gitlint becomes an empty package that requires gitlint-core[trusted-deps]. Fixes jorisroovers#162. Signed-off-by: Anders Kaseorg <andersk@mit.edu>
gitlint’s pinned requirements make it difficult to use with other libraries and tools that have newer requirements, and may hold back important security updates. However, the maintainer prefers to expose pinned requirements by default. Reconcile this by splitting gitlint into two packages: * gitlint-core has unpinned requirements by default, but pinned requirements with the [trusted-deps] extra. * gitlint becomes an empty package that requires gitlint-core[trusted-deps]. Fixes jorisroovers#162. Signed-off-by: Anders Kaseorg <andersk@mit.edu>
gitlint’s pinned requirements make it difficult to use with other libraries and tools that have newer requirements, and may hold back important security updates. However, the maintainer prefers to expose pinned requirements by default. Reconcile this by splitting gitlint into two packages: * gitlint-core has unpinned requirements by default, but pinned requirements with the [trusted-deps] extra. * gitlint becomes an empty package that requires gitlint-core[trusted-deps]. Fixes jorisroovers#162. Signed-off-by: Anders Kaseorg <andersk@mit.edu>
gitlint’s pinned requirements make it difficult to use with other libraries and tools that have newer requirements, and may hold back important security updates. However, the maintainer prefers to expose pinned requirements by default. Reconcile this by splitting gitlint into two packages: * gitlint-core has unpinned requirements by default, but pinned requirements with the [trusted-deps] extra. * gitlint becomes an empty package that requires gitlint-core[trusted-deps]. Fixes jorisroovers#162. Signed-off-by: Anders Kaseorg <andersk@mit.edu>
As we're getting close to getting #246 merged, I wanted to leave a note of appreciation for @andersk for getting us out of this impasse. You both came up with multiple suggestions (after taking the time to understand my perspective), and then also followed through and did the implementation. Thank you - gitlint is better because of it! And @sigmavirus24 of course, for perspective and expertise (always :-) ) Thank you! |
gitlint’s pinned requirements make it difficult to use with other libraries and tools that have newer requirements, and may hold back important security updates. However, the maintainer prefers to expose pinned requirements by default. Reconcile this by splitting gitlint into two packages: * gitlint-core has unpinned requirements by default, but pinned requirements with the [trusted-deps] extra. * gitlint becomes an empty package that requires gitlint-core[trusted-deps]. Fixes #162. Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Just released 0.17.0 with this change. For others referencing this issue in the future: you can now install gitlint with unpinned dependencies using: pip install gitlint-core @andersk I tested it myself, but appreciate if you can just confirm this is working from your end as well. Thanks! |
Looking good from here. Thanks so much! |
This issue can be used as a continuation of #133 which I inadvertently (and irreversibly) closed when deleting the
master
branch in favor of the newmain
default branch.Please read ALL of #133 before commenting here.
The text was updated successfully, but these errors were encountered: