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

Remove version pinning #432

Open
dvzrv opened this issue Sep 27, 2018 · 17 comments

Comments

@dvzrv
Copy link

commented Sep 27, 2018

The Pipfile lists many version pinned requirements.
IMHO requirements should ideally never be version pinned (only if really really required).
Please remove the version pinning, where not needed!
It's hard to do proper packaging with this (especially for distributions, that follow upstream closely, such as Arch Linux).

@tony

This comment has been minimized.

Copy link
Member

commented Sep 29, 2018

@dvzrv Can you create a PR example and go into greater detail.

Does Arch packaging rely on Pipfile, or setup.py -> requirements/base.txt?

@tony

This comment has been minimized.

Copy link
Member

commented Sep 29, 2018

It'd be helpful to clarify if Pipfile plays a role in the packaging (I only really use Pipfile for development).

And second, what would a requirements/*.txt / Pipfile look like that didn't conflict downstream. I'd like to be helpful as possible to packagers, but would like an example.

Some considerations: click < 7 isn't supported yet, and we need a minimum version of pyyaml for python 3.7 support.

@dvzrv

This comment has been minimized.

Copy link
Author

commented Sep 30, 2018

@tony sorry for the misunderstanding: I meant this more as a general recommendation!
Pipfiles are not used in our scenario, as we are building with setuptools (for the filesystem integration and all that).

However, I thought the Pipfile displayed the current use of all requirements (and I was kinda right).
When looking at requirements/base.txt, we have e.g. colorama == 3.9.0. Is that really required? If there is an update to colorama, the automated travis builds will not pick that up (and for better or worse your build won't break).
The same counts for the version pinning in requirements/{dev,doc,test}.txt, which basically prevents a more fluent integration with the dependencies. After all, that's what reproducible builds are for! It's good that those break from time to time (if something with a more recent version can't be integrated), so the maintainer can know instantly and react if need be.

Arch, being rolling release, updates to latest stable pretty fast (with whatever breakage that might bring). That is an obvious downside to slow moving projects, but on the other hand there really is no upside to version pinning all your requirements (with libtmux probably being the exception here, as long as it's always released ahead of tmuxp).

@dvzrv

This comment has been minimized.

Copy link
Author

commented Sep 30, 2018

On the click topic: So tmuxp 1.4.1 is not yet compatible to click >= 7.0, correct?

@tony

This comment has been minimized.

Copy link
Member

commented Sep 30, 2018

On the click topic: So tmuxp 1.4.1 is not yet compatible to click >= 7.0, correct?

Correct

We will target 1.5.0 to be click 7.0 compatible

I'll release 1.4.2 which will includes tests in the sdist.

@dvzrv

This comment has been minimized.

Copy link
Author

commented Oct 1, 2018

@tony please consider making this a higher priority, as tmuxp is now not working anymore:

Traceback (most recent call last):
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 578, in _build_master
    ws.require(__requires__)
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 895, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 786, in resolve
    raise VersionConflict(dist, req).with_context(dependent_req)
pkg_resources.ContextualVersionConflict: (Click 7.0 (/usr/lib/python3.7/site-packages), Requirement.parse('click==6.7'), {'tmuxp'})

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/bin/tmuxp", line 6, in <module>
    from pkg_resources import load_entry_point
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 3112, in <module>
    @_call_aside
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 3096, in _call_aside
    f(*args, **kwargs)
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 3125, in _initialize_master_working_set
    working_set = WorkingSet._build_master()
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 580, in _build_master
    return cls._build_from_requirements(__requires__)
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 593, in _build_from_requirements
    dists = ws.resolve(reqs, Environment())
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 781, in resolve
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'click==6.7' distribution was not found and is required by tmuxp

To get back on topic: More hard version pinned requirements will break tmuxp, even if that dependency works in a higher version.
In this case it's click that really breaks it, but still... it's unnecessary, as it keeps the project on a specific version of some library for too long.

@tony

This comment has been minimized.

Copy link
Member

commented Oct 1, 2018

@dvzrv
I agree with loosening pin constraints and am happy to accommodate that.

There's also a reason why we enforce constraints, for instance: Click 7 will break tmuxp. That's why that's pinned.

Click 7 PR: #434

Re:

pkg_resources.DistributionNotFound: The 'click==6.7'

I believe 1.4.1 and 1.4.2 is click < 7.

In this case it's click that really breaks it, but still... it's unnecessary, as it keeps the project on a specific version of some library for too long.

I've had the opposite issue. I've had the issue before where not pinning ended up breaking tmuxp for a lot of users: #399 (comment)

So see where I'm coming from? If I don't have some constraint, it's possible that a dependency could have an API break that would otherwise not be a holdup. When I create a pin (sometimes I may be wrong and we should remove the pin) but the idea is we're preventing a potential break.

So I think, maybe, there's three issues to distinguish from: pin that are bad that cause breakages, pins that help protect from breaking (e.g. enforcing click <7 until we support it) and catching up with downstream dependency versions used in package managers. Is arch using click 7 and us not being updated blocking? (Shouldn't we be expected to have some time to update? We didn't know click 7 was going to be released, let alone it'd break API's and not even have a migration guide to explain why)

@rfoliva

This comment has been minimized.

Copy link
Member

commented Oct 4, 2018

Hi @tony,

I think what we need is a middle ground. The current pipfile freezes every dependency. What I think we need is to freeze only ones that break tmuxp for one reason or another, until cycles to address the problems comes or based on release cycle.

Here is an example of Pipfile, with this in mind:
https://github.com/requests/requests/blob/master/Pipfile

This is from requests package, which is from the same author as pipenv. I think we can understand / accept this as best practice.

If everyone is ok with this approach, I would be happy to put in pull request by tomorrow.

Please let me know.

@rfoliva

This comment has been minimized.

Copy link
Member

commented Oct 4, 2018

#436

Feel free to assign this issue to me. Happy to help.

@rfoliva

This comment has been minimized.

Copy link
Member

commented Oct 6, 2018

#436 has been merged.

@tony - I think we can close this issue.

@rfoliva

This comment has been minimized.

Copy link
Member

commented Oct 8, 2018

@dvzrv - Believe this has been addressed now, in a recent merge. Can you please try again and let us know so we can close this issue?

Thanks!

@rfoliva

This comment has been minimized.

Copy link
Member

commented Oct 14, 2018

Closing this issue due to inactivity.

We believe the issue / request has been addressed. If there are still any problems around this, please advise us by create a new issue, possibly referencing this one.

Thanks!

@rfoliva rfoliva closed this Oct 14, 2018
@dvzrv

This comment has been minimized.

Copy link
Author

commented Oct 20, 2018

@rfoliva sorry for not getting back earlier. I have many packages and life ;-)

I guess this issue is not really solved, as there are still hard version requirements, that break tmuxp (see #445 for current issues with colorama, which I even mentioned earlier in this issue).

I think I can only remove all hard version pinning for this package to function properly in the future on the OS level. Even if that potentially breaks functionality, when a dependency is updated, it at least attempts to use it in the first place.

@rfoliva

This comment has been minimized.

Copy link
Member

commented Oct 23, 2018

Thanks @dvzrv.

What version are you tracking for this Arch package? Any specific tag?

I have pushed the unpinning on to master using Pipfile / Pipfile.lock, so I believe it has not been applied to version you are tracking.

How are you building your package? Using OS repos (stable / dev). I have been meaning to reach out to package maintainers to find out how they build their packages and see how we can help, make your like easier.

If I have this information, we can keep it in mind and try to help.

Thanks,
Ricardo

@rfoliva rfoliva reopened this Oct 23, 2018
@rfoliva

This comment has been minimized.

Copy link
Member

commented Oct 23, 2018

@dvzrv from what I could see from this link you provided on another issue, you are using the requirements files to build your package and this is where you did the version unpinning. I can do the same on our side, but we need to make sure 1.5.0a takes these changes as well.

Please confirm I am on the right track and I will make this happen.

Thanks,
Ricardo

@dvzrv

This comment has been minimized.

Copy link
Author

commented Oct 23, 2018

@rfoliva yes, as mentioned here, we use setuptools.
Currently, I build 1.5.0a (as you rightfully pointed out in your comment), because we are on a higher click version in the repos already.

I do understand, that hard version pinning makes sense in a development process, but it is also a bit painful for Arch maintainers ;-)

The way it is right now (with sed'ing the pinned versions) it works for me, but it will definitely break tmuxp, if there are incompatibilities.

In general, I'd rather have that though, than an enforced upper limit, as if things break, people can actually give you useful stacktraces (my stuff breaks with tmuxp and dependency in version x.y.z).
In the case there are python developers amongst those, they might even fix the issue at hand for you and you have more functional code (in an ideal world).

@rfoliva rfoliva self-assigned this Feb 12, 2019
@rfoliva

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

I think this should be addressed officially with release of 1.5.0. No need for alpha anymore.

Can you please confirm @dvzrv so we can close it?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.