Skip to content

Conversation

@robacarp
Copy link
Contributor

@robacarp robacarp commented Aug 15, 2018

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

fixes amberframework/amber#928
unblocks #31086

The Amber project is going to back out of brew for now to let the crystal-lang community run faster. To support brew, amber will need to create its own tap again.

I wish brew had a way around this, but that's a discussion for the maintainers of brew and not me. If there's a misconfiguration in the way the Amber recipe is defined, let me know and I'll sort that out instead.

cc @drujensen @elorest @eliasjpr @paulcsmith @asterite

@DomT4 DomT4 added the maintainer feedback Additional maintainers' opinions may be needed label Aug 16, 2018
@DomT4
Copy link
Contributor

DomT4 commented Aug 17, 2018

Would it be a complete nightmare to vendor an older version of crystal into the amber formula and then write a wrapper around amber so it finds that vendored version of crystal first?

I don't necessarily have an objection to this but I think that's worth a discussion, perhaps.

@RX14
Copy link
Contributor

RX14 commented Aug 17, 2018

It's been 10 days with no activity on merging the amber pull request to update to 0.26.0, and no response from any amber maintainers on any PR in that time, let's just call amber unmaintained and remove it.

@Sija
Copy link
Contributor

Sija commented Aug 17, 2018

It's extremely weird to see a language recipe blocked by a framework, which might or might not support every new version, and does it in timely manner. How that happenned in the first place?

@DomT4
Copy link
Contributor

DomT4 commented Aug 17, 2018

How that happenned in the first place?

Because Homebrew is a very large project policed by a relatively very small team of people who do this entirely in what spare time we can find each day and we don't always have the time to get into deep philosophical debates about the long-term implications of packaging or not packing a specific piece of software?

It passed the notability check, it passed CI, the person who submitted the formula was responsive on making requested changes and it built against the version of crystal that was present in Homebrew at the time. It has also since been accepted by MacPorts, so Homebrew is hardly unique in looking at amber and going "Yeah, looks fine".

Let's try and keep the discussion here based around resolving the issue, ideally in a way that works for as many people as possible.

@robacarp
Copy link
Contributor Author

@DomT4 The Amber project (of which I am now the most available maintainer and core team member) is going to put up a Tap to replace this formula hopefully this weekend. Notable or not, the Amber project is currently very undermaintained.

@DomT4
Copy link
Contributor

DomT4 commented Aug 17, 2018

@robacarp Thanks, that's a helpful answer.

Is there value in waiting for the weekend and that tap creation so we can formally migrate the formula over to you, rather than leave a bunch of users with essentially a dead (in the sense that their installed amber will no longer work against the new crystal with no obvious recourse for fixing that) formula installed?

@robacarp
Copy link
Contributor Author

@DomT4 that’s a great idea, I will make that happen today. Can you point me to any documentation on how to make that happen to speed up the process?

@paulcsmith
Copy link

paulcsmith commented Aug 17, 2018 via email

@DomT4
Copy link
Contributor

DomT4 commented Aug 17, 2018

There's some advice here. In your case can more or less be summarised as:

Create a GitHub repository called something like homebrew-amber inside the amberframework org, copy amber.rb from homebrew/core, remove the bottle block, commit that formula over to your new tap, add the line "amber": "amberframework/amber", (or whatever you wish to call the tap) to tap_migrations.json in this PR.

(I appreciate most of that is wildly obvious but figure that's more helpful than being vague).

Existing users should roll over to you automatically and anyone who types brew install amber without having your tap installed in future will be directed to it like this:

~> brew install helios
Error: No available formula with the name "helios"
It was migrated from homebrew/core to spotify/public.
You can access it again by running:
  brew tap spotify/public
And then you can install it by running:
  brew install helios

@paulcsmith
Copy link

@DomT4 thanks for the super quick responses and for home brew

I think @sijas question was more about the reasoning behind packages blocking other packages. In other words, why Amber is stopping Crystal from being updated. Is there a way around that in case it happens again? Like a way to force update Crystal even if other packages don’t work with it?

I realize that would be bad for those other packages, but it would be beneficial to everyone else using crystal so sometimes it might be worth doing.

@robacarp
Copy link
Contributor Author

@DomT4 thanks for the instructions. I have (re)created a amber community maintained tap and updated this pull to include the redirect json.

I was able to brew tap the new tap and install Amber from it, see attached command output. The tap is obviously quite minimal, but I think it's sufficient for now.

I cannot emphasize enough how much time homebrew has saved me in the last decade, and I appreciate the project and the attitude of the maintainers and volunteers greatly. Thank you for your time, effort, and kindness.

Hulk-Tsunami-Horror /u/l/H/L/T/h/homebrew-core> brew tap amberframework/amber
Updating Homebrew...
==> Auto-updated Homebrew!
Updated Homebrew from 880f6fa5 to a11039d8.
Updated 3 taps (homebrew/cask-versions, homebrew/core, homebrew/cask).

# [snip]

==> Tapping amberframework/amber
Cloning into '/usr/local/Homebrew/Library/Taps/amberframework/homebrew-amber'...
remote: Counting objects: 4, done.
remote: Compressing objects: 100% (2/2), done.
Unpacking objects: 100% (4/4), done.
remote: Total 4 (delta 0), reused 4 (delta 0), pack-reused 0
Tapped 1 formula (27 files, 21.6KB).
 master amzn: bulb 62059d37 Aug-17 06:56:10 ∆t=30s
Hulk-Tsunami-Horror /u/l/H/L/T/h/homebrew-core> brew install amberframework/amber
Error: No available formula with the name "amberframework/amber" 
==> Searching for a previously deleted formula (in the last month)...
Error: No previously deleted formula found.
==> Searching for similarly named formulae...
This similarly named formula was found:
amberframework/amber/amber ✔
To install it, run:
  brew install amberframework/amber/amber ✔

Hulk-Tsunami-Horror /u/l/H/L/T/h/homebrew-core> brew reinstall amberframework/amber/amber
==> Reinstalling amberframework/amber/amber 
==> Downloading https://github.com/amberframework/amber/archive/v0.8.0.tar.gz
Already downloaded: /Users/robert/Library/Caches/Homebrew/amber--0.8.0.tar.gz
==> shards install
==> make install PREFIX=/usr/local/Cellar/amber/0.8.0
Error: The `brew link` step did not complete successfully
The formula built, but is not symlinked into /usr/local
Could not symlink bin/amber
Target /usr/local/bin/amber
already exists. You may want to remove it:
  rm '/usr/local/bin/amber'

To force the link and overwrite all conflicting files:
  brew link --overwrite amber

To list all files that would be deleted:
  brew link --overwrite --dry-run amber

Possible conflicting files are:
/usr/local/bin/amber
==> Summary
🍺  /usr/local/Cellar/amber/0.8.0: 5 files, 6.2MB, built in 59 seconds

Hulk-Tsunami-Horror /u/l/H/L/T/h/homebrew-core> brew link --overwrite amber
Linking /usr/local/Cellar/amber/0.8.0... 1 symlinks created

@felixbuenemann
Copy link
Contributor

I think @sijas question was more about the reasoning behind packages blocking other packages. In other words, why Amber is stopping Crystal from being updated.

@paulcsmith That's because the homebrew ci checks the reverse dependencies of a package and tries to rebuild them all with the new version to make sure the updated formula doesn't break any other packages.

For most cases that's a good approach, it just doesn't work well with crystal because it still breaks backwards compatibility a lot due to the pre-1.0 stage.

@sijas
Copy link

sijas commented Aug 17, 2018

You are tagging the wrong person here. @Sija is the right user.

@DomT4
Copy link
Contributor

DomT4 commented Aug 17, 2018

thanks for the super quick responses and for home brew

I cannot emphasize enough how much time homebrew has saved me in the last decade, and I appreciate the project and the attitude of the maintainers and volunteers greatly. Thank you for your time, effort, and kindness.

Thank you both for the kind words ❤️. We definitely appreciate your willingness to contribute, without which Homebrew would rapidly grind to a halt.

I think @Sija question was more about the reasoning behind packages blocking other packages. In other words, why Amber is stopping Crystal from being updated

I apologise if I did misinterpret @Sija's tone on that and consequently gave them an undeservedly tetchy reply. @felixbuenemann's comment does a perfect job of explaining the technical reasoning behind CI red flagging the other PR repeatedly (Thanks for that Felix!).

Error: The `brew link` step did not complete successfully
The formula built, but is not symlinked into /usr/local
Could not symlink bin/amber
Target /usr/local/bin/amber
already exists. You may want to remove it:
  rm '/usr/local/bin/amber'

To force the link and overwrite all conflicting files:
  brew link --overwrite amber

To list all files that would be deleted:
  brew link --overwrite --dry-run amber

Possible conflicting files are:
/usr/local/bin/amber
==> Summary
🍺  /usr/local/Cellar/amber/0.8.0: 5 files, 6.2MB, built in 59 seconds

It's been a while since we've done a cross-tap migration so my memory is a little suboptimal on this but IIRC this shouldn't be an issue once this PR is merged. Homebrew should treat them as the same formula that still lives in the same prefix and therefore can overwrite the links it already "owns".

@DomT4 DomT4 removed the maintainer feedback Additional maintainers' opinions may be needed label Aug 17, 2018
@DomT4 DomT4 closed this in f8ee800 Aug 17, 2018
@DomT4
Copy link
Contributor

DomT4 commented Aug 17, 2018

Merged in f8ee800. Thank you for your contribution to Homebrew @robacarp; we appreciate it! 😺

@robacarp
Copy link
Contributor Author

@DomT4 thanks! Full disclosure, I believe the symlink issue was because the link was pointed to a local build of amber, not the brew-core version.

@lock lock bot added the outdated PR was locked due to age label Sep 16, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Sep 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider moving Homebrew package to custom tap

7 participants