Skip to content

BLD: set upper versions for build dependencies#17297

Merged
charris merged 1 commit intonumpy:masterfrom
rgommers:build-deps-upperlimit
Sep 12, 2020
Merged

BLD: set upper versions for build dependencies#17297
charris merged 1 commit intonumpy:masterfrom
rgommers:build-deps-upperlimit

Conversation

@rgommers
Copy link
Copy Markdown
Member

@rgommers rgommers commented Sep 11, 2020

This is something we really should always have done for all dependencies,
but I expect it to become more important now that build-related
packages are all starting to update regarding the upcoming
distutils deprecation.

And Cython has a major release coming up - just in case, avoid it.

This is something we really should do for all dependencies,
but I expect it to become more important now that build-related
packages are all starting to update regarding the upcoming
distutils deprecation.

And Cython has a major release coming up - just in case, avoid it.
@rgommers rgommers added this to the 1.20.0 release milestone Sep 11, 2020
@charris
Copy link
Copy Markdown
Member

charris commented Sep 11, 2020

Hmm. We certainly need to test the new Cython release when it comes out and the rapid Python release cycle is driving the upgrades. How do you suggest we handle that going forwards?

@rgommers
Copy link
Copy Markdown
Member Author

We certainly need to test the new Cython release when it comes out and the rapid Python release cycle is driving the upgrades. How do you suggest we handle that going forwards?

We can bump the version when it comes out. Alternatively, we can remove the upper bound from this PR and add it to the release instructions - and <=most_recent_released_cython_version at the time of creating a new release branch.

Maybe that is the lower-effort thing to do here. It's easy to forget though, and there's no good way to catch it if we do forget. What would you prefer?

@mattip
Copy link
Copy Markdown
Member

mattip commented Sep 12, 2020

If we set a specific version, wouldn’t dependabot tell us when there is a new release?

@charris
Copy link
Copy Markdown
Member

charris commented Sep 12, 2020

wouldn’t dependabot tell us when there is a new release?

Don't know, but I suppose putting this in is one way to find out :)

@charris
Copy link
Copy Markdown
Member

charris commented Sep 12, 2020

Pinning to a specific version is too strict and the current requirement may be too loose, then again this may make it too tight :) The problem is allowing sufficient flexibility for downstream projects who may have various Cython versions installed. OTOH, we want to make sure that bad Cython versions are forbidden, especially during the ongoing alpha and beta release cycle, but we also want to test those releases in order to provide feedback. Anyone know how dependabot handles alpha and beta releases? The idea of putting in an upper limit during the branch process is interesting, but maybe too easy to screw up.

@rgommers
Copy link
Copy Markdown
Member Author

The problem is allowing sufficient flexibility for downstream projects who may have various Cython versions installed.

That's not how this works, pyproject.toml is used when building from source with build isolation. Every project can put a different version in there, they won't affect each other.

Also note that this won't prevent installing a more recent version of Cython than the upper limit and running python setup.py install.

I'm not a fan of dependabot for this stuff - overly noisy for little benefit. I'd suggest always putting in the upper limit in pyproject.toml, and just updating it once in a while, e.g. right before creating a new release branch.

@charris charris merged commit 254a1f8 into numpy:master Sep 12, 2020
@charris
Copy link
Copy Markdown
Member

charris commented Sep 12, 2020

Let's give this a shot. Thanks Ralf.

@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Sep 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants