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

Setuptools_scm incorrect version and Conda environments #8201

Closed
nabobalis opened this issue May 20, 2021 · 17 comments · Fixed by #9016
Closed

Setuptools_scm incorrect version and Conda environments #8201

nabobalis opened this issue May 20, 2021 · 17 comments · Fixed by #9016
Assignees
Labels
Support Support question

Comments

@nabobalis
Copy link

nabobalis commented May 20, 2021

Details

This comes from sunpy/sunpy#5225 where we have been debugging an issue with setuptools_scm in our RTD builds with the help of @astrojuanlu.

He figured out this for us:

Long story short: on March 30th the CONDA_APPEND_CORE_REQUIREMENTS feature flag became enabled for all projects on Read the Docs, and that causes a change in rtd-environment.yml which is taken by setuptools_scm as a dirty
checkout.

This will affect every project that uses a conda environment for their RTD build.

Our pyflct build that uses conda also has this issue: https://pyflct.readthedocs.io/en/stable/
Whereas our non-conda build work fine: https://docs.sunpy.org/projects/sunkit-image/en/latest/

I guess the question is, is it possible to disable this flag or a way to avoid having the "rtd-environment.yml" file from being edited? Or anything else we could do in our git repo to work around this?

I will be looking at "setuptools_scm" to see if I can get around this as an alternative in our config.

Expected Result

The correct "clean" version in our documentation sidebar.

Actual Result

We get a "dirty" version in our documentation sidebar due to this change.
You can see on these recent tagged builds:

https://readthedocs.org/projects/sunpy/builds/13747239/ v1.1.4, is 1.1.5.dev0+g5f04bbdd9.d20210512
https://readthedocs.org/projects/sunpy/builds/13747236/ v2.1.4, is 2.1.5.dev0+g9245ce003.d20210512

@stsewd
Copy link
Member

stsewd commented May 20, 2021

Hi, we are trying to do fewer changes to user's configs/files #8103, I think this would be solved when implemented. But we may still generate or edit some files in the build process, so I think a good solution is having setuptools_scm ignore the edited files and always use the no distance and clean or distance and clean forms https://github.com/pypa/setuptools_scm#default-versioning-scheme

@humitos
Copy link
Member

humitos commented Jun 29, 2021

@nabobalis Hi! Were you able to solve this problem with the solution that @stsewd posted?

I don't think there is anything actionable we can do from our side to solve this.

@humitos humitos added the Needed: more information A reply from issue author is required label Jun 29, 2021
@nabobalis
Copy link
Author

For now we fixed the version in our docs/conf.py for any release.

@no-response no-response bot removed the Needed: more information A reply from issue author is required label Jun 29, 2021
@nabobalis nabobalis reopened this Oct 13, 2021
@nabobalis
Copy link
Author

I've decided to reopen this as I wanted to check if there was any progress on this from the RTD side?

We are looking at seeing how to change our setuptools_scm setup to avoid it.

@Cadair
Copy link

Cadair commented Oct 14, 2021

👋

We ran back into this as we had to explicitly bump our version shown in our docs due to this bug and I forgot.

I was wondering if it would be possible to come up with a workaround for this, as it really breaks our workflow (and presumably other projects which are using setuptools_scm + conda) in a super frustrating way.

Rather than overwriting the environment file in place inside the checkout would it be possible for rtd to save the modified file to a temporary file and then build the env from that file rather than the one in the checkout? If that sounds like something that would be tractable I would be willing to dive into the code and see if I could make it happen.

Otherwise, would it be possible to get this feature flag turned off for the sunpy and pyflct repos?

@stsewd
Copy link
Member

stsewd commented Oct 14, 2021

Rather than overwriting the environment file in place inside the checkout would it be possible for rtd to save the modified file to a temporary file and then build the env from that file rather than the one in the checkout?

I think we should push to not require doing changes to those files at all (#8103, #8190), instead of doing more workarounds.

I think the feature flag is enabled for all projects now. If anything, we can invert that flag

if self.project.has_feature(Feature.CONDA_APPEND_CORE_REQUIREMENTS):
so is easy to opt projects out.

@Cadair
Copy link

Cadair commented Oct 14, 2021

instead of doing more workarounds.

I will leave it to you to decide what the best short-term solution is then.

If anything, we can invert that flag

Do you mean by this that you can just turn this flag off for us? or that we need to change the code?

@stsewd
Copy link
Member

stsewd commented Oct 14, 2021

Do you mean by this that you can just turn this flag off for us? or that we need to change the code?

We need to change the code to be able to do that.

@stsewd
Copy link
Member

stsewd commented Oct 14, 2021

I think the feature flag is enabled for all projects now.

I just confirmed this in our db.

@astrojuanlu
Copy link
Contributor

Today I discovered that this also affects projects using versioneer rather than setuptools_scm, like geopandas: geopandas/geopandas#1831

@adrien-berchet
Copy link

Hello,
I also have this issue for this package: https://morphology-workflows.readthedocs.io/en/stable/
And in my case I use a theme that checks that the version from the conf.py file and from the metadata of the package should be the same, so I can't just remove the dirty part of the version in the conf.py file. So I am quite stuck for now.
I am wondering: wouldn't it be possible to change the build process so that the package is installed in the conda environment BEFORE the files are changed? So the package metadata would be correct and thus getting the version using pkg_resources.get_distribution() would remain correct, even after the files are modified (and thus the versions would be the same in the package metadata and in the conf.py file).

@Cadair
Copy link

Cadair commented Dec 21, 2021

I "solved" this issue by not using conda any more! I think that #8585 is probably the easy short term fix, but I am not particularly motivated to work on it any more, and I now have a load of other more urgent things occupying my time.

@mathause
Copy link

As an alternative to #8585 readthedocs could potentially call

git update-index --assume-unchanged environment.yml docs/conf.py

before the installation step.

https://git-scm.com/docs/git-update-index#_using_assume_unchanged_bit

@humitos
Copy link
Member

humitos commented Jan 21, 2022

@mathause that is definitely a nice hack ☺️. I wasn't aware of that command. I think it's a good solution for our case. I'll give it a try next week

@humitos
Copy link
Member

humitos commented Mar 16, 2022

I tested this solution, #8201 (comment), with the PR that I'm working on at #9016 and it did work properly! 💯 So, I think that would be the suggested way to work around this problem.

@humitos humitos moved this to Planned in 📍Roadmap Mar 29, 2022
Repository owner moved this from Planned to Done in 📍Roadmap Apr 4, 2022
@humitos
Copy link
Member

humitos commented Apr 5, 2022

Hi! In a few hours, we are deploying a new feature that will allow people to call arbitrary commands at different moments in the build process (see #9016). For example,

build:
  os: ubuntu-20.04
  jobs:
    pre_install:
      - git update-index --assume-unchanged environment.yml docs/conf.py

I think this new config key, build.jobs, will cover this case. Take a look at the documentation at https://docs.readthedocs.io/en/latest/config-file/v2.html#build-jobs and please let me know if this is useful for you.

@adrien-berchet
Copy link

Interesting! Thank you very for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Support Support question
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants