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

rustup: renamed from rustup-init #177840

Merged
merged 5 commits into from
Jul 22, 2024
Merged

Conversation

branchvincent
Copy link
Member

  • 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>?

With #177582, rustup-init now provides the full rustup installation (so using the rustup-init installer is no longer necessary)

Rename the formula to just rustup to reflect these changes (which also hopefully gives users some indication of these changes)

@github-actions github-actions bot added the automerge-skip `brew pr-automerge` will skip this pull request label Jul 19, 2024
@chenrui333
Copy link
Member

maybe split the PR into two one depends_on rustup for test, one depends_on rustup

@branchvincent
Copy link
Member Author

What are you thinking? But the audit will fail if this isn't renamed all in one PR:

$ brew audit rustfmt
rustfmt
  * Can't find dependency 'rustup-int'.
Error: 1 problem in 1 formula detected.

@chenrui333
Copy link
Member

aha, that sounds about right. Yeah, the transition for such thing is not easy.

@chenrui333
Copy link
Member

also, this might need a long build label

@branchvincent
Copy link
Member Author

Gonna follow #165649 instead so this can be a syntax-only PR: add rustup as a new formula first, migrate existing dependents to call rustup directly instead of rustup-init, and then update formula_renames.json here

@branchvincent branchvincent added CI-syntax-only Change only affects brew syntax, not the install. Only run syntax CI. ready to merge PR can be merged once CI is green and removed automerge-skip `brew pr-automerge` will skip this pull request long build Set a long timeout for formula testing labels Jul 22, 2024
@BrewTestBot BrewTestBot added this pull request to the merge queue Jul 22, 2024
Merged via the queue into Homebrew:master with commit 983477e Jul 22, 2024
21 checks passed
@branchvincent branchvincent deleted the rustup branch July 22, 2024 16:30
@AurevoirXavier
Copy link

AurevoirXavier commented Jul 24, 2024

Shit. This breaks all my CIs.

Such an important update without any warning and migration test.

==> Migrating formula rustup-init to rustup
==> Unlinking rustup-init
==> Moving rustup-init versions to /opt/homebrew/Cellar/rustup
==> Relinking rustup
==> Downloading https://ghcr.io/v2/homebrew/core/create-dmg/manifests/1.2.2
==> Fetching create-dmg
==> Downloading https://ghcr.io/v2/homebrew/core/create-dmg/blobs/sha256:ed92cefd6df282057e6c1a162eb453d4c8d3b34a4a8637bf292813817badef81
==> Pouring create-dmg--1.2.2.all.bottle.tar.gz
🍺  /opt/homebrew/Cellar/create-dmg/1.2.2: 13 files, [70](https://github.com/hack-ink/AiR/actions/runs/10078744161/job/27864368210#step:4:71).6KB
/Users/runner/work/_temp/11525461-2136-42d7-8eca-b1dd4a94ee41.sh: line 4: cargo: command not found
Error: Process completed with exit code 127.

AurevoirXavier added a commit to hack-ink/AiR that referenced this pull request Jul 24, 2024
AurevoirXavier added a commit to hack-ink/AiR that referenced this pull request Jul 24, 2024
AurevoirXavier added a commit to hack-ink/AiR that referenced this pull request Jul 24, 2024
@branchvincent
Copy link
Member Author

branchvincent commented Jul 24, 2024

@AurevoirXavier this was tested and was expected to be backwards compatible. The only noticeable change is we now link rustup on PATH by default

can you share more about what exactly is breaking for you (i.e. a reproducible example)?

EDIT: Looks like this is from brew update moving the formulae to keg-only but not triggering our post_install method (which is where we ensure rustup-init remains linked). Created #178322 which will fix this for brew upgrade. In your case, a short-term workaround would be brew update && brew reinstall rustup (and of course long-term, it'll be resolved when github updates their image to include these changes)

@AurevoirXavier
Copy link

can you share more about what exactly is breaking for you (i.e. a reproducible example)?

I think there were already some links to this PR. Such as edgedb/edgedb#7590, hack-ink/AiR@3583c95. Please take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-syntax-only Change only affects brew syntax, not the install. Only run syntax CI. ready to merge PR can be merged once CI is green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants