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

Add dynamic version from environment variable #3885

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

IlyaMichlin
Copy link

@IlyaMichlin IlyaMichlin commented Apr 6, 2023

Based on this request from StackOverflow.

Summary of changes

  • Adding the ability to set the dynamic version using an environment variable.

Pull Request Checklist

@sinoroc
Copy link

sinoroc commented Apr 28, 2023

I like the idea in principle. Is there any prior art? Was it already discussed somewhere?

Maybe the documentation should make it clear that the environment variable will be read only when building the sdist but not the wheel. As far as I understood, this is the expected behavior, isn't it?

@abravalheri
Copy link
Contributor

abravalheri commented Apr 28, 2023

I am not sure about this one...
What happens if a newbie clones the original repo and tries to build the package without setting the environment variable? Will they see an error and know what to do next?

Isn't it this a bit of an edge-case1 that we not necessarily need to optimise for?
It should be very straightforward to achieve the desired outcome with the tools that are already available today. Considering that all the relevant config and metadata is already in pyproject.toml and/or setup.cfg, the only thing necessary is to add a simple setup.py2:

import os, setuptools

setup(version=os.getenv("MY_VERSION_ENV_VAR"))

Footnotes

  1. I am sure this is a perfect valid use case and there are circumstances out there that require this kind of approach.

  2. Even if directly running python setup.py ... is deprecated, setup.py follows being a perfectly valid configuration file that interplays well with both setup.cfg and pyproject.toml.

@sinoroc
Copy link

sinoroc commented Apr 28, 2023

Isn't it this a bit of an edge-case1 that we not necessarily need to optimise for?

Based on my monitoring of StackOverflow, it does come up, not often but quite regularly.

I guess there are multiple reasons:

  • thinking that using setup.py is now forbidden, that support for it has completely disappeared
  • wanting to have everything in one file and one file only: pyproject.toml
  • wanting to follow the declarative path all the way (pyproject.toml or setup.cfg does not matter which one)

@IlyaMichlin
Copy link
Author

IlyaMichlin commented Apr 29, 2023

I am not sure about this one... What happens if a newbie clones the original repo and tries to build the package without setting the environment variable? Will they see an error and know what to do next?

An error message will be displayed, but the content can be improved.

@abravalheri, I appreciate your input and agree that there may be some workarounds that could be implemented. However, as @sinoroc has pointed out, these are precisely the underlying concerns that prompted me to create this solution in the first place.

My goal with this solution is to add a feature that may prevent developers from moving from setup.py to pyproject.toml. Based on the feedback and engagement on StackOverflow, it appears that there is an interest in such a feature.

@abravalheri
Copy link
Contributor

I don't know @IlyaMichlin... Personally I am a bit unease with this FR1.

Maybe the best would be asking for the other maintainers if they are OK with that. (@jaraco do you have any thoughts?)

I appreciate your input and agree that there may be some workarounds that could be implemented.

Please note that using setup.py is not a workaround, instead it is a very genuine implementation 😅.

setup.py is a perfectly valid configuration file and a first-class citizen when it comes to dynamically computing build parameters/metadata.

My goal with this solution is to add a feature that may prevent developers from moving from setup.py to pyproject.toml.

setup.py and pyproject.toml can be mixed together and having a use case where dynamic computation of metadata/build config is required should not be an impediment for extracting all the static metadata/config into pyproject.toml, while leaving the dynamic bits behind in setup.py. It is already possible to do that with the implementation of setuptools available today.

Based on the feedback and engagement on StackOverflow, it appears that there is an interest in such a feature.

The question in the StackOverflow seems to be a bit more nuanced than simply extracting the version from an environment variable, and I genuinely believe that the best answer for that particular question is complementing pyproject.toml with setup.py (I added a reply there trying to clarify a few topics).

It is very unlikely that setuptools implementation for pyproject.toml will be ever be able to cover every single use case out there, but thanks to setup.py users can still achieve a great degree of customisation.

Footnotes

  1. Please note that this is not a criticism about the implementation. Thank you very much for contributing to the project, btw!

@jaraco
Copy link
Member

jaraco commented Aug 30, 2023

I don't feel strongly either way.

On one hand, as abravalheri points out, setup.py is supported and satisfies the need.

On the other hand, if there's broad desire for such a feature (more than one org or more than a few individual users), it'd be nice to support it in pyproject.toml and obviate even more use-cases that rely on setup.py.

My biggest concern is that the dynamic option is setuptools-specific. If there were just one other build backend considering supporting this feature, I'd feel a lot better about it.

I too have wished I could supply/override the version with an environment variable, but that was in situations where the SCM metadata wasn't present, but in that case, setuptools_scm was able to solicit the environment variable.

The question in the StackOverflow seems to be a bit more nuanced than simply extracting the version from an environment variable

I just read the StackOverflow question and I now agree that this proposed change doesn't even satisfy that user's use-case (having an environment variable override, but defaulting to an attribute).

Are there cases that can really benefit from this feature as drafted? Can you point to them? If so and they're numerous, we should consider merging the feature. Otherwise, I'm -1.

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.

None yet

4 participants