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

rdiff-backup: revert from 2.4.0 to 2.2.6 #156900

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

samford
Copy link
Member

@samford samford commented Dec 10, 2023

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

rdiff-backup 2.4.0 was erroneously published and removed sometime after the formula was upgraded. At the moment, 2.2.6 is the latest available version on PyPI. The main developer of the project has acknowledged this issue and requested the formula be reverted to 2.2.6, as there won't be a 2.4.0 or higher release in the foreseeable future.

With that in mind, this PR downgrades the formula from 2.4.0 to 2.2.6. Per our internal discussion about how we handle downgrades, I haven't bumped the version_scheme here. Any users on 2.4.0 who wish to downgrade will need to run brew reinstall rdiff-backup after this is merged.

@github-actions github-actions bot added the python Python use is a significant feature of the PR or issue label Dec 10, 2023
@samford
Copy link
Member Author

samford commented Dec 10, 2023

brew audit is predictably failing with stable version should not decrease (from 2.4.0 to 2.2.6).

How should we resolve this these days without using version_scheme?

@cho-m
Copy link
Member

cho-m commented Dec 11, 2023

Only option I can think of other than version_scheme would be to revert to 4603ab2 in syntax PR. And then create separate PR to apply any in-between changes (e.g. dd801cd) perhaps with revision bump.

Cons are that it won't downgrade users and may cause issues when 2.4.0 is actually released.

@MikeMcQuaid
Copy link
Member

brew audit is predictably failing with stable version should not decrease (from 2.4.0 to 2.2.6).

How should we resolve this these days without using version_scheme?

I think we can just try to merge without this and it won't break master.

If it does: we should fix this with a label tweak or something.

@SMillerDev
Copy link
Member

Would users ever get that version @MikeMcQuaid ? Since it's lower than what they have?

@MikeMcQuaid
Copy link
Member

@SMillerDev If they haven't already upgraded: yes. If they have: not unless they brew reinstall.

@samford
Copy link
Member Author

samford commented Dec 11, 2023

I think we can just try to merge without this and it won't break master.

If there weren't any changes between the version bump and now, we could seemingly just do a straightforward revert and use the bottles for 2.2.6 (correct me if I'm wrong). The trouble is that we need new bottles for 2.2.6 because there were changes to the rdiff-backup formula in the interim time (#151123). What @cho-m suggested above is also the only solution that I could think of within our current constraints.

If it does: we should fix this with a label tweak or something.

I was thinking that it may be nice to have a CI-downgrade label to skip this specific audit and allow CI to run when we need new bottles. It wouldn't be necessary when we can do a simple revert but it would save us a lot of trouble in situations like this (i.e., we can handle it in one PR instead of doing a syntax-only PR for the downgrade and then creating a follow-up PR to incorporate interim changes and create new bottles).

Looking at formula_auditor.rb, the "stable version should not decrease" check is part of the #audit_revision_and_version_scheme method. If we want to only skip this particular check (while continuing to run the others), we would presumably have to split that logic out into a separate method.

@MikeMcQuaid
Copy link
Member

I was thinking that it may be nice to have a CI-downgrade label to skip this specific audit and allow CI to run when we need new bottles.

Sounds good 👍🏻

I suppose the question is whether it's possible to skip that particular audit while allowing all the others to run (or how much of a challenge it would be to make that possible).

No(t yet), I think. We can in exceptional cases merge 🔴 CI but would be good to get a better solution here.

@samford samford added the CI-version-downgrade Pass --skip-stable-version-audit to brew test-bot. label Dec 17, 2023
@samford
Copy link
Member Author

samford commented Dec 17, 2023

All the pieces are now in place to skip the "stable version should not decrease" audit, so I applied the new CI-version-downgrade label and rebased this (to pick up the workflow change).

I bumped the revision in a previous PR, so this should produce 2.2.6_1 bottles at this point. Anyone on 2.4.0 who wants to downgrade after this is merged will need to run brew reinstall rdiff-backup.


Past that, I'm not sure if we will need to manually remove the 2.4.0 bottles after this. Will there be bottle issues if/when a 2.4.0 version is released in the future? I'm also not sure how removing the bottles would impact users but I figured it was worth mentioning, at least.

Alternatively, I can ask upstream to simply skip version 2.4.0 in the future. Using something like 2.4.1 would also ensure that users on the removed 2.4.0 would be upgraded to the proper newer version.

@samford samford added the ready to merge PR can be merged once CI is green label Dec 17, 2023
Copy link
Contributor

🤖 An automated task has requested bottles to be published to this PR.

@github-actions github-actions bot added the CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. label Dec 18, 2023
@BrewTestBot BrewTestBot added this pull request to the merge queue Dec 18, 2023
Merged via the queue into Homebrew:master with commit b3afb36 Dec 18, 2023
12 checks passed
@MikeMcQuaid
Copy link
Member

Thanks @samford! @Homebrew/maintainers If you can keep your eye out for any other times we might need to merge a 🔴 PR: it would be good to create an issue (or ideally do the work yourself) to be like @samford has been here and improve our workflow ❤️

@samford samford deleted the rdiff-backup-2.2.6-downgrade branch December 18, 2023 13:18
@github-actions github-actions bot added the outdated PR was locked due to age label Jan 18, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. CI-version-downgrade Pass --skip-stable-version-audit to brew test-bot. outdated PR was locked due to age python Python use is a significant feature of the PR or issue ready to merge PR can be merged once CI is green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants