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

Avoid arch migrator PRs to maintenance branches that already have the arches #2500

Open
h-vetinari opened this issue Apr 23, 2024 · 4 comments · Fixed by #2503
Open

Avoid arch migrator PRs to maintenance branches that already have the arches #2500

h-vetinari opened this issue Apr 23, 2024 · 4 comments · Fixed by #2503

Comments

@h-vetinari
Copy link
Contributor

I thought the bot opened these just if there's non-empty diff, but it turns out it'll even open a PR for a completely empty diff, c.f. conda-forge/openjdk-feedstock#160 / conda-forge/openjdk-feedstock#161

On feedstocks where we regularly need to create new branches for LTS versions, this becomes annoying eventually (aside from wasting CI resources). It can also create confusion for people who deal with this less frequently

@beckermr
Copy link
Contributor

This might be a bug related to the latest rerendering changes

@h-vetinari
Copy link
Contributor Author

Thanks for the quick response! I only see permission-related bits in #2503 though, so I think that'll likely only cover the corner case mentioned in the OP (empty diff, presumably nonempty permission changes).

However, as soon as the creation of a maintenance branch causes any kind of rerendering change (which is very often the case, as the old state of the feedstock at which we branch is not often 100% up to date), it'll still causes new arch PRs.

Since the arch migrations are special anyway, I'd really like to avoid that case - e.g. we could check if there are already osx-arm64 files under .ci_support and abort if so.

In general, we could probably improve the situation around spurious PRs for all migrations (if we can spend the compute resources to rerender twice) as follows:

  • Rerender feedstock first (to create baseline to compare against)
  • Actually add the migrator file
  • rerender again
    • if there's a diff, amend the previous rerender commit and open PR
    • if there's no diff, don't open a PR.

@h-vetinari h-vetinari reopened this Apr 23, 2024
@beckermr
Copy link
Contributor

Two rerenders will slow the bot down a ton and so is not feasible at the moment.

@h-vetinari
Copy link
Contributor Author

That's what I expected, but detecting files in .ci_support should be cheap

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 a pull request may close this issue.

2 participants