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

[email protected] 1.1.0 (new formula) #971

Closed
wants to merge 1 commit into from
Closed

[email protected] 1.1.0 (new formula) #971

wants to merge 1 commit into from

Conversation

DomT4
Copy link
Member

@DomT4 DomT4 commented May 8, 2016

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same formula update/change?
  • Have you built your formula locally prior to submission with brew install <formula> (where <formula> is the name of the formula you're submitting)?
  • Does your submission pass brew audit --strict --online <formula> (after doing brew install <formula>)?
    (Apart from one issue, which I've deliberately ignored for consistency).

It's going to take us a long time to transition every formula across to the new OpenSSL release (when it's stable) given the not insignificant API changes, as well as upstream deprecation of zlib compression and SSLv3.

Consequently, we're going to be running two OpenSSL releases for the foreseeable. The OpenSSL formula will remain for now the 1.0.2 branch which is supported until 2019, this formula will track 1.1.0 which is supported upstream till ~2018 under current plans.

Some things in this formula are on by default, such as the overdue death of SSLv3 and zlib compression, but since those are fairly big departures from OpenSSL releases to date it's explicitly stated in-formula so when people report "This is broken using this" some of the very obvious breakage points are highlighted at cursory glance.

This OpenSSL release also introduces a fairly high Perl requirement. I'd like to note at this point this isn't our choice and is something we're trying to minimise, but to be somewhat blunt if you're running 10.8 or less you're going to have to accept the system utilities on those OS X versions are increasingly problematic today.

This formula is a fairly large departure from the way people have come to expect Homebrew to behave in terms of always running the latest. OpenSSL is a particularly punchy example of this, but going forwards incompatible releases are going to be given a little more leeway to remain around until a transition can happen in a mildly smoother manner than we've perhaps managed previously at times.

@DomT4 DomT4 added do not merge in progress Stale bot should stay away maintainer feedback Additional maintainers' opinions may be needed new formula PR adds a new formula to Homebrew/homebrew-core labels May 8, 2016
@DomT4
Copy link
Member Author

DomT4 commented May 8, 2016

This is still very much a WIP. I need to work out the system Perl bug and whether that's an upstream problem (and then go report it if it is), but here's floating a discussion 🎈.

@UniqMartin
Copy link
Contributor

Given what I read on the OpenSSL homepage about their release policy and compatibility guarantees, I wonder whether this formula should be named openssl11 instead. (A release of, e.g., OpenSSL 1.1.1 should in theory not break any formulae compiled against 1.1.0 unless we happen to disable features in a future release that are currently enabled.)

#ifdef __#{arch}__
#{(buildpath/"build-#{arch}/opensslconf.h").read}
#endif
EOS
Copy link
Contributor

Choose a reason for hiding this comment

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

This EOS seems to be misaligned.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it is 👍. I'm not sure who put it there originally, but I missed that alignment issue when I re-used it here. Will fix.

@geoff-nixon
Copy link
Contributor

Yeah. This sucks. I gotta ask, what is the impetus for 1.1.x at all? Why are they doing this? What's being gained? Sure, zlib and SSLv3 should have probably been disabled a while ago actually, upstream, and/or in the homebrew formula, but that's all water under the bridge at this point. But I definitely think this should be openssl110 (and be relegated to -versions to boot).

I wonder how much stuff would break if the openssl deps were all changed to libressl.
That's really "the future" of this library, IMHO.

if File.read(f).include?(Formula["perl"].bin)
inreplace f, Formula["perl"].bin, Formula["perl"].opt_bin
end
end
Copy link
Member

Choose a reason for hiding this comment

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

IMO, this part should be in install instead of post_install.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that shouldn't be a problem. I dumped it here for easier testing, and then didn't move it, but install makes sense to me.

@DomT4
Copy link
Member Author

DomT4 commented May 8, 2016

I gotta ask, what is the impetus for 1.1.x at all? Why are they doing this?

1.1.x is a (relative to OpenSSL) cleaner & more consistently-styled codebase, which long-term should make life a bit easier for the upstream devs and people who have to package it downstream, and less "cruft" brings with it the chances of less vulnerabilities from areas of the codebase not touched for years. They're also introducing new ciphers that other crypto libraries have had for a while.

The 1.1.x release branch is, IMHO, a step forwards for OpenSSL and will eventually make life easier for a whole bunch of other tools and libraries that require it. But yeah, short-term, this is going to be hell for most package managers whilst we wait for the OpenSSL ecosystem to catch up on 1.1.x and patch their projects for it.

But I definitely think this should be openssl110 (and be relegated to -versions to boot).

Versions, as a tap, is more or less a dying pony at this point. There's already a fairly major cleanup going on behind the scenes there but long-term the core problem is that almost nobody in terms of either contributors or maintainers either wants or has the time to promise to look after versioned formulae adequately.

This sort of thing is going to become more commonplace, where upstream release major breaking changes the old version sticks around in the core for a while beside the new version. It'll likely happen with the next Boost release, for example.

I wonder how much stuff would break if the openssl deps were all changed to libressl.

Lots. Lots. The LibreSSL team have made their fair share of breaking changes themselves, and for now is a less stable library than OpenSSL, as fond as I am of LibreSSL's work.

@geoff-nixon
Copy link
Contributor

1.1.x is a (relative to OpenSSL) cleaner & more consistently-styled codebase, which long-term should make life a bit easier for the upstream devs and people who have to package it downstream, and less "cruft" brings with it the chances of less vulnerabilities from areas of the codebase not touched for years. They're also introducing new ciphers that other crypto libraries have had for a while.

I see. Well, that's good news!

Versions, as a tap, is more or less a dying pony at this point.

That makes sense. My, a lot has changed while I was gone for a bit.

This sort of thing is going to become more commonplace, where upstream release major breaking changes the old version sticks around in the core for a while beside the new version.

I think that's really quite prudent, honestly. Personally, I think that bleeding-edge culture homebrew had going for a while may have been useful to a point, but its probably time to settle down a little bit. Perfectly fine with me.

Lots. Lots. The LibreSSL team have made their fair share of breaking changes themselves, and for now is a less stable library than OpenSSL, as fond as I am of LibreSSL's work.

Yeah, I suspected as much. Bummer.

A CA file has been bootstrapped using certificates from the system
keychain. To add additional certificates, place .pem files in
#{openssldir}/certs

Copy link
Contributor

Choose a reason for hiding this comment

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

While we're kinda on the topic: are you guys aware that this doesn't actually work?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you elaborate? It seems to work for me locally.

Copy link
Contributor

@geoff-nixon geoff-nixon May 8, 2016

Choose a reason for hiding this comment

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

If you put .pems in #{openssldir}/certs, they do nothing. For me, anyway. Maybe it's because I'm not in /usr/local.

@DomT4
Copy link
Member Author

DomT4 commented May 9, 2016

Updated to reflect Xu's comments on post_install and Martin's comments on the naming and EOS misplacement. Still working on the Perl issues and the other raised points.

@UniqMartin UniqMartin changed the title openssl110 1.1.0-beta5 (new formula) openssl11 1.1.0-beta5 (new formula) May 9, 2016
@vszakats
Copy link
Contributor

vszakats commented May 10, 2016

How is migration planned to be done for openssl dependent formulae?

Ideally, the switch to openssl11 from openssl should be made once support is available for a given formula. But, this may still break dependent projects which depend on that given formula and some other ones as well, that don't yet have 1.1.x support. What would be the solution to handle this scenario?

Mixing old and new openssls in those projects doesn't seem like a very safe idea. Holding off OpenSSL upgrades until every dependent formulae and all their related subcomponents support it, may mean a very slow upgrade process. Using alternate names for openssl11 dependent formulae would look quite messy (f.e. libssh2-openssl11), plus it'd need heavy patching to avoid colliding names.

[ Apologize if this is an already resolved problem. ]

@UniqMartin
Copy link
Contributor

@vszakats This hasn't been completely formalized and we don't have a coherent document yet we can point people at. This is partially a result of the discussion in Homebrew/brew#60. A summary is in the top post and it's probably worth reading the few comments from Homebrew/brew#60 (comment) to the end. Though if you're curious, I'd say reading the whole thread for more context is worthwhile.

@vszakats
Copy link
Contributor

Thanks for the pointers @UniqMartin!

@DomT4
Copy link
Member Author

DomT4 commented May 10, 2016

Although for full disclosure, even I haven't read that thread yet, knowing full well once I do I'll end up writing a minor dissertation 😅.

Mixing old and new openssls in those projects don't seem like a very safe idea.

Agreed. They shouldn't end up in the same recursive dependency tree in an ideal world.

Holding off OpenSSL upgrades until every dependent formulae and all their related subcomponents support it, may mean a very slow upgrade process.

I'm not even going to set a timetable for this, to be honest. 1.0.2 is supported until 2019 and it wouldn't surprise me if it takes that long for our entire package ecosystem that needs OpenSSL to be shunted across to what will presumably by then by 1.1.1 or something.

If we're lucky Debian will take the jump sooner rather than later, significant chunks of the rest of Linux will follow and the ecosystem will go "Linux has moved over, we need to get on with fixing any incompatibility sharpish". We saw something like this happen when Debian killed off SSLv3, although on a smaller scale than would be required here.

The primary goal of this package isn't necessarily to move everything right out of the gate; it's to provide an easy way to test what works and what doesn't locally & also fulfil the loose promise that somewhere, somehow Homebrew supplies the latest and greatest.

Using alternate names for openssl11 dependent formulae would look quite messy (f.e. libssh2-openssl11), plus it'd need heavy patching to avoid colliding names.

Yeah, have no desire to go down this path whatsoever. Even if Homebrew had 50 maintainers and a large CI farm I'd have my concerns about this, let alone as we are, heh.

@vszakats
Copy link
Contributor

Thank you @DomT4, I've also read into the other thread and now have a picture of how this is planned.

Here's hoping the 1.1.x upgrade can happen relatively fast and smooth. It's an important upgrade, with clear benefits. I could drop some ugly old cruft from existing code while updating, and most attention needed to go into keeping compatibility with previous/current OpenSSL versions.

@geoff-nixon
Copy link
Contributor

Hey - I was wondering if anyone knew where the original discussion that led from from changing from using the Curl CA bundle to using the certs generated by the security. lives these days? It's probably several years old by now. I know that @mistydemeo's tigerbrew still uses this still uses this in some cases. I'm not necessarily advocating that approach exactly per se, but https://www.apple.com/certificateauthority (with documents like https://www.apple.com/certificateauthority/ca_program.html) are pretty interesting.

@DomT4
Copy link
Member Author

DomT4 commented May 12, 2016

@geoff-codes Your comments were investigated further & resulted in changes:

Thanks for highlighting the potential issue there.

@geoff-nixon
Copy link
Contributor

geoff-nixon commented May 12, 2016

@DomT4 No problem!
Thanks for taking it seriously (I appreciate the rev. bump), and taking care of it so thoroughly.

I'm considering opening an issue in brew for discussion of possible robust, long-term approaches to this sort of thing. While I wouldn't consider it at all critical, its still not ideal how we're reliant on the Apple CA list at the time of install; when Apple updates the list with a software update, brew's list doesn't get updated without reinstalling OpenSSL, GnuTLS, etc. Which is to say nothing of older versions of OS X, which don't get updated at all. And it would be nice to have some type of mechanism/game-plan in place for if and when something like this happens again.

@Owners, would you guys be open to some type of discussion on this?

@DomT4
Copy link
Member Author

DomT4 commented May 13, 2016

I'm considering opening an issue in brew for discussion of possible robust, long-term approaches to this sort of thing.

Don't worry about the Issue. I've thought about this before and did so again after re-reading the original discussion a few days back. I have 90% of a PR ready to go already. Give me today to file that, PRs generate more discussion than Issues by their nature, especially once I threaten to start merging mine as-is 😜.

@geoff-nixon
Copy link
Contributor

@DomT4 Fantastic. 😉

@DomT4
Copy link
Member Author

DomT4 commented May 13, 2016

^ Homebrew/brew#241

@geoff-nixon
Copy link
Contributor

You read my mind. 💯

@DomT4
Copy link
Member Author

DomT4 commented Jun 10, 2016

Pushed a bunch of changes here, contacted OpenSSL about the Perl hashbang oddities, etc. PR may be slow, but it's not dead yet 😉.

@DomT4 DomT4 mentioned this pull request Aug 2, 2016
4 tasks
@CamJN
Copy link
Contributor

CamJN commented Aug 13, 2016

@DomT4
Copy link
Member Author

DomT4 commented Aug 13, 2016

Yeah, I'm aware, I'm on the mailing list, heh. Thanks for the unintentional nudge here though!

Keep meaning to bump this to the current beta; need to check the status around the Perl hashbang, which by reporting upstream I managed to spark a wider debate which I quietly backed away from & left them to decide the best path forwards 😄.

@DomT4 DomT4 changed the title openssl11 1.1.0-beta5 (new formula) openssl11 1.1.0 (new formula) Aug 25, 2016
@DomT4
Copy link
Member Author

DomT4 commented Aug 25, 2016

This be stable now. PR updated.


# This isn't technically needed where Clang exists, but is a small
# dep and is hard to reliably express non-Clang-only in dependency format.
depends_on "makedepend" => :build
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't clang automatically exist if either the CLT or Xcode is installed? If so, can we just drop this?

OTOH something rings a bell that makedepend entered the picture for reasons totally unconnected to compilers.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you changed the default algorithms you needed to run make depend to rejiggle (technical term that) things to build correctly before. I think it would still be necessary if you've got a dirty source. Documentation upstream is kind of "eh" on the makedepend situation. There's a commit here that adds some detail, but not much.

I'll try poking it locally. If it breaks I guess we could always add the dep in later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think makedepend was brought in because we activated the ec-nistp stuff. I think that without the make depend step we silently weren't getting the ec-nistp.

But then how could it be true that makedepend is unnecessary when clang is present?

Copy link
Member Author

Choose a reason for hiding this comment

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

openssl/openssl@30752dd
openssl/openssl@09aa263

Apparently Clang (since always) and some GCC versions support dependency generation natively without having to lean on makedepend to do that magic.

It's going to take us a long time to transition every formula across to the
new OpenSSL release (when it's stable) given the not insignificant API
changes, as well as upstream deprecation of zlib compression and SSLv3.

Consequently, we're going to be running two OpenSSL releases for the foreseeable.
The OpenSSL formula will remain for now the 1.0.2 branch which is supported until 2019,
this formula will track 1.1.0 which is supported upstream till ~2018 under
current plans.

Some things in this formula are on by default, such as the overdue death of SSLv3
and zlib compression, but since those are fairly big departures from OpenSSL releases
to date it's explicitly stated in-formula so when people report "This is broken using this"
some of the very obvious breakage points are highlighted at cursory glance.

This OpenSSL release also introduces a fairly high Perl requirement. I'd like to
note at this point this isn't our choice and is something we're trying to minimise,
but to be somewhat blunt if you're running 10.8 or less you're going to have to accept
the system utilities on those OS X versions are increasingly problematic today.

This formula is a fairly large departure from the way people have come to expect
Homebrew to behave in terms of always running the latest. OpenSSL is a particularly
punchy example of this, but going forwards incompatible releases are going to be
given a little more leeway to remain around until a transition can happen in
a mildly smoother manner than we've perhaps managed previously at times.
@mithrandi
Copy link
Contributor

If we're lucky Debian will take the jump sooner rather than later, significant chunks of the rest of Linux will follow and the ecosystem will go "Linux has moved over, we need to get on with fixing any incompatibility sharpish". We saw something like this happen when Debian killed off SSLv3, although on a smaller scale than would be required here.

Debian has had OpenSSL 1.1.0 prereleases in experimental for a while now, and there is a transition tracking bug open (the lengthy list of blocking bugs there are packages that failed to rebuild against 1.1.0). It'll probably take a while to sort out the broken packages, but I expect the next stable release of Debian to have 1.1.0. So at worst, you can probably borrow Debian's patches in many cases, but hopefully most of these problems will be fixed upstream, rather than requiring distribution-specific patches.

@DomT4
Copy link
Member Author

DomT4 commented Aug 26, 2016

and there is a transition tracking bug open

Yeah, I've been keeping an eye on some of them. It's a few months old now so less useful but https://breakpoint.cc/openssl-1.1-rebuild-2016-05-29/ is interesting & an easier format to read through.

So at worst, you can probably borrow Debian's patches in many cases, but hopefully most of these problems will be fixed upstream, rather than requiring distribution-specific patches.

Yeah, I don't have a lot of interest in carrying around what is essentially a feature patch (support for 1.1.0) given OpenSSL are supporting 1.0.2 as an LTS branch. Not planning to leap over upstream releases unless absolutely necessary for whatever reason.

Transition is also complicated by potentially overlapping linkage, so for example, if you brew install curl --with-openssl against OpenSSL 1.1.0 but you add --with-libssh2 as well which remains linked against OpenSSL 1.0.2, we're not really sure how predictably that'll behave. Ideally, the plan is:

  • Anything standalone (i.e. nothing in brew uses) that supports OpenSSL 1.1.0 can use this new formula as the dependency.
  • Anything else needs to be upgraded by tree, so to update curl you'd want libssh2 to be using OpenSSL 1.1.0 first, etc.

We may well change that plan as time passes; If I could I'd have the entire ecosystem using OpenSSL 1.1.0 tomorrow but it really depends on how quickly we can get this done without breaking significant things for users.

@DomT4
Copy link
Member Author

DomT4 commented Aug 26, 2016

This is blocked on Homebrew/brew#812 temporarily.

@DomT4 DomT4 changed the title openssl11 1.1.0 (new formula) [email protected] 1.1.0 (new formula) Aug 28, 2016
@DomT4
Copy link
Member Author

DomT4 commented Aug 28, 2016

Now blocked on Homebrew/brew#832. Making progress!

@MikeMcQuaid
Copy link
Member

@DomT4 :shipit:!

@DomT4
Copy link
Member Author

DomT4 commented Sep 3, 2016

Can't quite yet. There's a new blocker around pull, which might actually be a deeper issue.

No available formula with the name "homebrew/core/[email protected]"

I poked at it a bit in irb but haven't had the chance to see whether it's a purely pull issue or pointing at a deeper problem yet:

irb(main):058:0> orig_revision = "05256bca5b18a84f98258e3da847c2bb8c4f53ec"
=> "05256bca5b18a84f98258e3da847c2bb8c4f53ec"
irb(main):059:0> changed_formulae_names = []
=> []
irb(main):060:0>  Utils.popen_read("git", "diff-tree", "-r", "--name-only","--diff-filter=AM", orig_revision, "HEAD", "--", tap.formula_dir.to_s).each_line do |line|
irb(main):061:1* next unless line.end_with? ".rb\n"
irb(main):062:1> name = "#{tap.name}/#{File.basename(line.chomp, ".rb")}"
irb(main):063:1> changed_formulae_names << name
irb(main):064:1> end
=> "Formula/[email protected]\n"
irb(main):065:0> changed_formulae_names
=> ["homebrew/core/[email protected]"]
irb(main):072:0> changed_formulae_names.each do |name|
irb(main):073:1* begin
irb(main):074:2* f = Formula[name]
irb(main):075:2> end
irb(main):076:1> end
FormulaUnavailableError: No available formula with the name "homebrew/core/[email protected]"
    from /usr/local/Library/Homebrew/formulary.rb:188:in `get_formula'
    from /usr/local/Library/Homebrew/formulary.rb:216:in `factory'
    from /usr/local/Library/Homebrew/formula.rb:1279:in `[]'
irb(main):089:0> changed_formulae_names.to_s.gsub!(/@/, "AT") { |f| Formula[f].name }
=> "[\"homebrew/core/opensslAT1.1\"]"

@MikeMcQuaid
Copy link
Member

@DomT4 Weirdly worked fine for me the second time I did a brew pull after a git reset --hard. Can you confirm if that works for you? If not, can I see the brew pull --debug output?

@DomT4
Copy link
Member Author

DomT4 commented Sep 3, 2016

It brew pulls okay initially, but you can't get a bottle from it, even though there's one sat on Bintray.

~> brewpb https://github.com/Homebrew/homebrew-core/pull/971
==> Fetching patch
Patch: https://github.com/Homebrew/homebrew-core/pull/971.patch
==> Applying patch
Applying: [email protected] 1.1.0 (new formula)
==> Patch closes issue #971
Error: No available formula with the name "homebrew/core/[email protected]"

~> gloga
* 3313499 (HEAD -> master) [email protected] 1.1.0 (new formula)
~> brewpb https://github.com/Homebrew/homebrew-core/pull/971 --debug
==> Fetching patch
Patch: https://github.com/Homebrew/homebrew-core/pull/971.patch
==> Applying patch
Applying: [email protected] 1.1.0 (new formula)
==> Patch closes issue #971
Error: No available formula with the name "homebrew/core/[email protected]"
/usr/local/Library/Homebrew/formulary.rb:188:in `get_formula'
/usr/local/Library/Homebrew/formulary.rb:216:in `factory'
/usr/local/Library/Homebrew/formula.rb:1279:in `[]'
/usr/local/Library/Homebrew/cmd/pull.rb:171:in `block in pull'
/usr/local/Library/Homebrew/cmd/pull.rb:49:in `each'
/usr/local/Library/Homebrew/cmd/pull.rb:49:in `pull'
/usr/local/Library/Homebrew/brew.rb:91:in `<main>'

@DomT4 DomT4 closed this in 663e479 Sep 4, 2016
@BrewTestBot BrewTestBot removed the in progress Stale bot should stay away label Sep 4, 2016
@DomT4
Copy link
Member Author

DomT4 commented Sep 4, 2016

Because someone will notice & ask, the postinstall step is hilariously verbose now. That's an upstream change, not something on our end. I'll look into trying to do that step a bit more quietly, but this PR has sat for a long, long time now so let's get it merged & let people play with it and the new core-versioned formulae system.

Merged in 663e479.

@DomT4 DomT4 deleted the openssl_future branch September 4, 2016 18:36
@MikeMcQuaid
Copy link
Member

@DomT4 Hooray! 👏

@DomT4
Copy link
Member Author

DomT4 commented Sep 4, 2016

@MikeMcQuaid Thanks for your help finishing up the @ support! Couldn't have got it merged without the final piece in the puzzle Homebrew/brew@0c44ce2.

I'll try and spin up a VM soon locally to rebuild as much as possible in the core that currently uses openssl & we can track what's horribly broken & what works already.

mistydemeo pushed a commit to mistydemeo/tigerbrew that referenced this pull request May 10, 2017
Follow-up from #812 to fix handling fully-qualified versioned formulae
names.

Allows pulling Homebrew/homebrew-core#971.
mistydemeo pushed a commit to mistydemeo/tigerbrew that referenced this pull request May 10, 2017
Follow-up from #812 to fix handling fully-qualified versioned formulae
names.

Allows pulling Homebrew/homebrew-core#971.
@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
maintainer feedback Additional maintainers' opinions may be needed new formula PR adds a new formula to Homebrew/homebrew-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants