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

feat: add version bumping for v1 recipes #3525

Merged
merged 41 commits into from
Jan 15, 2025
Merged

Conversation

wolfv
Copy link
Contributor

@wolfv wolfv commented Jan 6, 2025

Another version of version bumping for v1 recipes. This time it should handle all the cases :)

Needs: prefix-dev/rattler-build-conda-compat#57

With the pixi toml attached below, you can try this with:

mkdir trash
cd trash && git clone https://github.com/conda-forge/jolt-physics-feedstock && cd ../
pixi run bump ./trash/jolt-physics-feedstock/ 5.2.0
cd  trash/jolt-physics-feedstock && git diff

Should show you an updated recipe with version & hash bumped to 5.2.0

I am using this pixi.toml locally:

pixi.toml
[project]
authors = ["Wolf Vollprecht <[email protected]>"]
channels = ["conda-forge"]
description = "Add a short description here"
name = "cf-scripts"
platforms = ["osx-arm64"]
version = "0.1.0"

[tasks]
pip = { cmd = "pip install -e ../../rattler-build-conda-compat --no-deps", outputs = [".pixi.toml"] }
bump = { cmd = "python ./conda_forge_tick/update_recipe/version.py", depends-on = ["pip"] }

[dependencies]
attrs = "*"
backoff = "*"
black = "*"
cachecontrol = "*"
cachetools = "*"
click = "*"
conda = "*"
conda-forge-feedstock-check-solvable = ">=0.4.0"
conda-forge-pinning = "*"
conda-libmamba-solver = "*"
conda-forge-metadata = ">=0.3.0"
conda-package-handling = "*"
conda-smithy = "*"
conda-forge-feedstock-ops = ">=0.9.0"
conda-build = ">=3.24.0"
curl = "*"
depfinder = "*"
distributed = "*"
doctr = "*"
feedparser = "*"
frozendict = "*"
git = "*"
"github3.py" = "*"
grayskull = ">=2.5"
jinja2 = "*"
lockfile = "*"
mamba = ">=0.23"
msgpack-python = "*"
networkx = "!=2.8.1"
numpy = "*"
packaging = "*"
psutil = "*"
pygithub = "*"
pymongo = "*"
pynamodb = "*"
python-dateutil = "*"
python-graphviz = "*"
python-rapidjson = "*"
requests = "*"
rever = "*"
"ruamel.yaml" = "*"
"ruamel.yaml.jinja2" = "*"
scipy = "*"
setuptools = "*"
setuptools_scm = ">=7"
stopit = "*"
streamz = "*"
tar = "*"
tqdm = "*"
wurlitzer = "*"
yaml = "*"
pandas = "*"
xonsh = "*"

[pypi-dependencies]
conda-forge-tick = { path = ".", editable = true }
rattler-build-conda-compat = { path = "../../rattler-build-conda-compat", editable = true }

@wolfv
Copy link
Contributor Author

wolfv commented Jan 7, 2025

I just added a first test that demonstrates that talking to the pypi API also works for the new recipe format.

I'll start cutting a release over on rattler-build-conda-compat and then we can move forward here. We can port some more tests over.

@minrk
Copy link
Contributor

minrk commented Jan 7, 2025

This is the main blocker for me picking up v1 in some of the most expensive feedstocks on conda-forge, so I'm super excited about this one!

@wolfv
Copy link
Contributor Author

wolfv commented Jan 7, 2025

/relock-conda

Copy link
Contributor

@beckermr beckermr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left several comments that need addressing.

conda_forge_tick/migrators/version.py Outdated Show resolved Hide resolved
conda_forge_tick/migrators/version.py Outdated Show resolved Hide resolved
conda_forge_tick/migrators/version.py Outdated Show resolved Hide resolved
conda_forge_tick/update_recipe/version.py Show resolved Hide resolved
conda_forge_tick/update_recipe/version.py Outdated Show resolved Hide resolved
conda_forge_tick/update_recipe/version.py Show resolved Hide resolved
conda_forge_tick/url_transforms.py Outdated Show resolved Hide resolved
conda_forge_tick/update_recipe/version.py Outdated Show resolved Hide resolved
conda_forge_tick/migrators/version.py Show resolved Hide resolved
@wolfv
Copy link
Contributor Author

wolfv commented Jan 7, 2025

Thanks for the review @beckermr - I'll fix things up tomorrow morning. On a high-level, do things look alright to you?

@beckermr
Copy link
Contributor

beckermr commented Jan 7, 2025

Yes this looks good. Given how it works, we can switch the v0 recipes over to this algorithm as well and that will likely be more robust than what is in the bot now.

I just noticed you missed some of old vs new jinja2 stuff in the url transforms module. The current test doesn't hit those code paths which is why nothing weird happened.

@beckermr
Copy link
Contributor

beckermr commented Jan 7, 2025

It looks like the relock command didn't work on this PR from a fork? Let me try again to be sure.

/relock-conda

conda_forge_tick/migrators/version.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jan 8, 2025

Codecov Report

Attention: Patch coverage is 85.62874% with 24 lines in your changes missing coverage. Please review.

Project coverage is 75.73%. Comparing base (af6c59a) to head (eac71b0).
Report is 43 commits behind head on main.

Files with missing lines Patch % Lines
conda_forge_tick/update_recipe/version.py 84.81% 12 Missing ⚠️
conda_forge_tick/migrators/version.py 79.41% 7 Missing ⚠️
tests/test_migrators.py 87.17% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3525      +/-   ##
==========================================
- Coverage   77.11%   75.73%   -1.39%     
==========================================
  Files         132      132              
  Lines       14612    14728     +116     
==========================================
- Hits        11268    11154     -114     
- Misses       3344     3574     +230     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wolfv
Copy link
Contributor Author

wolfv commented Jan 8, 2025

One more issue over here: prefix-dev/rattler-build-conda-compat#61 will release asap

conda_forge_tick/migrators/version.py Outdated Show resolved Hide resolved
conda_forge_tick/migrators/version.py Outdated Show resolved Hide resolved
conda_forge_tick/update_recipe/version.py Show resolved Hide resolved
conda_forge_tick/update_recipe/version.py Outdated Show resolved Hide resolved
conda_forge_tick/update_recipe/version.py Outdated Show resolved Hide resolved
conda_forge_tick/update_recipe/version.py Outdated Show resolved Hide resolved
tests/test_migrators.py Outdated Show resolved Hide resolved
@ytausch
Copy link
Contributor

ytausch commented Jan 8, 2025

After this is merged, is there anything left for v1 updates to be processed by the bot? Asking because this only touches the auto-tick step.

@wolfv
Copy link
Contributor Author

wolfv commented Jan 8, 2025

@ytausch I believe that version updates should work once this migrator is enabled. Another high-value one would be the arch migrator: #3019

Unfortunately it was blocked because of mini-migrators ...

@h-vetinari h-vetinari dismissed their stale review January 8, 2025 13:18

addressed, thanks

Copy link
Contributor

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yet more recipe_* variable naming consistency issues.

conda_forge_tick/update_recipe/version.py Outdated Show resolved Hide resolved
conda_forge_tick/update_recipe/version.py Outdated Show resolved Hide resolved
conda_forge_tick/update_recipe/version.py Outdated Show resolved Hide resolved
conda_forge_tick/update_recipe/version.py Outdated Show resolved Hide resolved
conda_forge_tick/migrators/version.py Outdated Show resolved Hide resolved
@wolfv
Copy link
Contributor Author

wolfv commented Jan 8, 2025

/relock-conda

@ytausch
Copy link
Contributor

ytausch commented Jan 8, 2025

This might be helpful if the locking bot fails: conda-incubator/relock-conda#245 (comment)

@h-vetinari h-vetinari dismissed their stale review January 8, 2025 18:03

addressed

@h-vetinari h-vetinari force-pushed the add-version-bump-v1 branch from bc9c261 to 85c4173 Compare January 8, 2025 18:10
@beckermr
Copy link
Contributor

beckermr commented Jan 8, 2025

This PR is still missing changes to the url transforms in this file: https://github.com/regro/cf-scripts/blob/main/conda_forge_tick/url_transforms.py. If that code happens to run during a version update it will break the v1 recipe PRs.

@wolfv
Copy link
Contributor Author

wolfv commented Jan 8, 2025

@beckermr I used _get_new_url_tmpl_and_hash which calls the function you mentioned so I hope that this works :)

@wolfv
Copy link
Contributor Author

wolfv commented Jan 8, 2025

It does indeed miss the code about the R versions though, so that will have to be added back in. Will try to tackle this tomorrow if @mgorny doesn't get to it before me

@beckermr
Copy link
Contributor

beckermr commented Jan 8, 2025

@wolfv That is exactly the problem. We need to ensure get_transformed_urls doesn't break on jinja2 with ${{ instead of {{.

@wolfv wolfv force-pushed the add-version-bump-v1 branch from 1a755a0 to 6deaf29 Compare January 15, 2025 15:54
@wolfv
Copy link
Contributor Author

wolfv commented Jan 15, 2025

I just released rattler-build-conda-compat 1.3.2 which should fix the remaining errors. It will take a little more time to propagate through the CDNs etc. But I think then this should be ready :)

@wolfv
Copy link
Contributor Author

wolfv commented Jan 15, 2025

Nice, tests passed 🙏

@beckermr beckermr enabled auto-merge January 15, 2025 18:46
@beckermr beckermr merged commit d229020 into regro:main Jan 15, 2025
5 checks passed
@wolfv
Copy link
Contributor Author

wolfv commented Jan 15, 2025

Wow, thanks @beckermr!

Do you know if anything else needs to happen?

@beckermr
Copy link
Contributor

Not sure. We'll find out if we get some version bumps!

@wolfv
Copy link
Contributor Author

wolfv commented Jan 15, 2025

I asked the bot for one here: https://github.com/conda-forge/conda-forge-webservices/actions/runs/12795837076/job/35674099360

Do we need to update a docker image somewhere?

@beckermr
Copy link
Contributor

so to make that go live, you'll need to

  1. bump conda-forge-tick-feedstock to the latest version (make sure to adjust the constraints in the recipe on rattler build conda compat when doing that)
  2. update the webservices conda env

If you do 1, 2 will happen automatically.

@wolfv
Copy link
Contributor Author

wolfv commented Jan 15, 2025

OK, I bumped the conda-forge-tick-feedstock: conda-forge/conda-forge-tick-feedstock#60

@ytausch
Copy link
Contributor

ytausch commented Jan 16, 2025

appears to work: https://github.com/conda-forge/subfinder-feedstock/pull/1/files

@minrk
Copy link
Contributor

minrk commented Jan 16, 2025

absolutely wonderful, thanks folks!

@wolfv wolfv deleted the add-version-bump-v1 branch January 16, 2025 10:03
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.

7 participants