kibana-4.3.0 #46593

jasontedor · 2015-12-02T19:01:53Z

This pull request updates the Kibana formula from version 4.1.1 to version
4.3.0, the latest stable version of Kibana as of 2015-11-24.

Relates #46349

c9n · 2015-12-03T03:46:15Z

👍 works

DomT4 · 2015-12-03T05:05:54Z

Library/Formula/kibana.rb

  homepage "https://www.elastic.co/products/kibana"
-  url "https://github.com/elastic/kibana/archive/v4.1.1.tar.gz"
-  sha256 "3f91e99e20e82d4e84ec141007822fea8f9454c71595551f9348ea2609c98284"
+  url "https://download.elastic.co/kibana/kibana/kibana-4.3.0-darwin-x64.tar.gz"


This looks like a binary release, rather than a source release?

@DomT4 It is binary release.

We'd prefer to build from source, per Homebrew submission rules. (Alternatively this could be shipped off to homebrew/binary or something).

@DomT4 It takes a long time to build kibana from source. Really hurts. I do agree moving it to homebrew/binary.

Won't be a huge problem for most of our users though, because we produce our own binaries for the three most recent OS X versions.

I want to contribute, so I am curious about how it really works.

Alright. Let us know if we can help there. The bottle generation logic for the CI specifically is stored in Library/Homebrew/cmd/test-bot.rb if you want to poke around locally.

Thanks very much. I will read the codes.

@DomT4 Thanks for the feedback here. I'll probably be looking at modifying this pull request to use a source release rather than a binary distribution but I might not get to it until next week.

Thanks! No worries, give us/me a shout when the change is made so we spot it.

jasontedor · 2015-12-12T18:18:35Z

Thanks! No worries, give us/me a shout when the change is made so we spot it.

@DomT4 I've spent a fair amount of time exploring some options here. Right now, the root problem is that Kibana is tested and supported against Node.js 0.12.7 (for version 4.3.0). While there is an open issue to get to an LTS release in elastic/kibana#5541, that's not slated for the 4.x line of Kibana.

Looking past that, I tried to get Kibana running against the latest Node.js (as would be provided by the current node formula), but it turns out we run into nodejs/node#3606 (actually nodejs/node#3922 but that issue is just marked as a duplicate of nodejs/node#3606). The seemingly unreliable workaround presented there did not work on my system, and even if it did we would still have the issue that Kibana is supported on Node.js 0.12.7.

The two proceedings paragraphs combined strongly suggest that it would be preferable to use Node.js 0.12.7 for the time being. That presents several immediate options.

Use the bundled Node.js that comes with the binary distribution (as submitted with this pull request) with the downside of not being a source distribution.
Depend on the versioned formula node012 with the downside of conflicting with existing formula node creating a mess for many users.
Depend on node using npm to install n to install Node.js 0.12.7 with the downside being this ultimately lands us back to depending on a binary install of Node.js.
Depend on the nvm formula to install Node.js 0.12.7 with the same downside of depending on a binary install of Node.js.
During the install of the kibana formula, download and compile Node.js 0.12.7 from source and install it alongside kibana (effectively how the binary distributions are packaged to begin with).

So we have three options that depend on a binary install, one that creates ugly conflicts with the node formula, and one that is going to duplicate the existing node012 formula to avoid conflicting with the node formula.

But before any firm conclusions are reached, I have a few questions. Is there an option that I'm not seeing? Is there an easy way to manage the conflicts in option 2. that doesn't create a mess for users? In particular, is there a way to just install the node012 formula inside the Kibana install?

If I had to rank order these preferences with the information I have now, I would rank them 1, 5, 4, 2, 3. I'm interested to hear your thoughts and if you see an option that I do not see.

MikeMcQuaid · 2015-12-18T15:07:36Z

During the install of the kibana formula, download and compile Node.js 0.12.7 from source and install it alongside kibana (effectively how the binary distributions are packaged to begin with).

This seems like the nicest option to me in terms of avoiding side-effects and being consistent with how we handle other formulae. One question: is that version of Node just needed at build time or also needed at run time? Thanks!

jasontedor · 2015-12-18T16:36:02Z

This seems like the nicest option to me in terms of avoiding side-effects and being consistent with how we handle other formulae.

Okay, I'll work on a version of the pull request that does this, but probably won't get to it until early next week.

One question: is that version of Node just needed at build time or also needed at run time?

It's needed at runtime.

Thanks!

Thanks for your help again.

MikeMcQuaid · 2015-12-18T17:03:26Z

@jasontedor Sounds good, thanks!

DomT4 · 2015-12-19T00:06:28Z

This seems like the nicest option to me in terms of avoiding side-effects and being consistent with how we handle other formulae.

👍. Should be dumped into libexec and only the kibana binary linked into the top-level prefix, though. I presume if they live inside the same "real" prefix and you use an exec script that shouldn't be problematic (?).

jasontedor · 2015-12-19T00:25:05Z

I presume if they live inside the same "real" prefix and you use an exec script that shouldn't be problematic (?).

Yes, I don't foresee that being a problem. :)

This commit modifies the Kibana formula to build Node.js from source. This is because the current versions of Kibana are not compatible with the latest versions of Node.js.

jasontedor · 2015-12-27T17:06:45Z

@MikeMcQuaid @DomT4 I've pushed b85a0f3 that builds Node.js from source alongside Kibana. One hacky thing I don't like is a change to the audit logic to exclude the kibana formula from the npm check so any suggestions there would be appreciated.

MikeMcQuaid · 2015-12-27T20:55:00Z

Library/Formula/kibana.rb

+    # do not download binary installs of Node.js
+    inreplace buildpath/"tasks/build/index.js" do |s|
+      s.gsub!(%r{('_build:downloadNodeBuilds:\w+',)}, "// \\1")
+    end


Can just use the inreplace function form when doing a single replacement in a file.

Thanks. Addressed in dfc3bc7.

MikeMcQuaid · 2015-12-27T20:59:57Z

Library/Formula/kibana.rb

+    inreplace "#{prefix}/config/kibana.yml" do |s|
+      s.sub!(%r{/var/run/kibana.pid}, "/usr/local/var/run/kibana.pid")
+    end
+    (etc/"kibana").install prefix/"config/kibana.yml" unless (etc/"kibana/kibana.yml").exist?


Doing an etc.install in install will prevent overwriting and then perhaps doing just a conditional inreplace in post_install if needed.

Hmm...I wonder why that's better? This feels like a post install task to me (but I'm still coming up to speed on Homebrew idioms).

@jasontedor In handles rewriting and writing out a default version in a slightly cleverer/more consistent way so it's preferable, if you don't mind.

Got it, thanks! I pushed 854bd6e.

jasontedor · 2015-12-28T04:19:13Z

@MikeMcQuaid Thanks for the thorough review; I've addressed or responded to all of your comments and think this is ready for another review cycle?

MikeMcQuaid · 2015-12-28T08:43:54Z

@jasontedor One final comment then 👍 from me!

This commit modifies the handling of installing the Kibana config so that it is consistent with the handling in other formulae.

jasontedor · 2015-12-28T17:04:18Z

One final comment then 👍 from me!

@MikeMcQuaid I've addressed this final comment, but also pushed some caveats in 8be0508. I think we're ready now contingent on the CI builds?

MikeMcQuaid · 2015-12-28T19:09:15Z

Thanks for the great work @jasontedor!

jasontedor · 2015-12-28T19:12:54Z

Thanks again @MikeMcQuaid and @DomT4!

jasontedor mentioned this pull request Dec 2, 2015

elasticsearch 2.1.0 #46349

Closed

jasontedor force-pushed the kibana-4.3.0 branch from d49d222 to 4497535 Compare December 2, 2015 19:05

kibana-4.3.0

4c0d3ac

jasontedor force-pushed the kibana-4.3.0 branch from 4497535 to 4c0d3ac Compare December 2, 2015 21:01

DomT4 reviewed Dec 3, 2015
View reviewed changes

DomT4 mentioned this pull request Dec 5, 2015

kibana-4.3.0 #46500

Closed

DomT4 mentioned this pull request Dec 22, 2015

Current kibana doesn't work with current elasticsearch #47261

Closed

Build Node.js from source when installing Kibana

b85a0f3

This commit modifies the Kibana formula to build Node.js from source. This is because the current versions of Kibana are not compatible with the latest versions of Node.js.

jasontedor force-pushed the kibana-4.3.0 branch from afc0f9f to b85a0f3 Compare December 27, 2015 16:55

jasontedor added 4 commits December 27, 2015 14:50

Install Node.js for Kibana in libexec

58d597d

Remove unnecessary binary Node.js downloads

660760b

Do not build Kibana packages for other platforms

a5f0d50

Do not build Kibana zip package

df33ce2

jasontedor force-pushed the kibana-4.3.0 branch from 08e99e6 to df33ce2 Compare December 27, 2015 19:50

MikeMcQuaid reviewed Dec 27, 2015
View reviewed changes

jasontedor added 3 commits December 27, 2015 16:14

Simplify substitutions in Kibana formula

dfc3bc7

Explicit whitelisting of Kibana formula for npm audit

d1141f3

Simplify handling of tar package in Kibana formula

92f9774

jasontedor force-pushed the kibana-4.3.0 branch 2 times, most recently from c37c460 to 40471e5 Compare December 28, 2015 03:13

Do not definitely break Linux builds of Kibana

ed5b6c1

jasontedor force-pushed the kibana-4.3.0 branch from 40471e5 to ed5b6c1 Compare December 28, 2015 03:19

jasontedor added 2 commits December 27, 2015 22:31

More explicit about supported platforms for installing Kibana

2f12e8a

Cleanup post_install for Kibana formula

49c146c

jasontedor added 2 commits December 28, 2015 11:55

Consistent handling of Kibana config

854bd6e

This commit modifies the handling of installing the Kibana config so that it is consistent with the handling in other formulae.

Add caveats for Kibana

8be0508

MikeMcQuaid closed this in bb46d03 Dec 28, 2015

jasontedor deleted the kibana-4.3.0 branch December 28, 2015 19:12

ches mentioned this pull request Feb 4, 2016

Kibana plugin installations fail due to not finding package's Node modules #48838

Closed

Homebrew locked and limited conversation to collaborators Jul 10, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kibana-4.3.0 #46593

kibana-4.3.0 #46593

jasontedor commented Dec 2, 2015

c9n commented Dec 3, 2015

DomT4 Dec 3, 2015

c9n Dec 4, 2015

DomT4 Dec 4, 2015

c9n Dec 4, 2015

DomT4 Dec 4, 2015

c9n Dec 4, 2015

DomT4 Dec 4, 2015

c9n Dec 4, 2015

jasontedor Dec 4, 2015

DomT4 Dec 10, 2015

jasontedor commented Dec 12, 2015

MikeMcQuaid commented Dec 18, 2015

jasontedor commented Dec 18, 2015

MikeMcQuaid commented Dec 18, 2015

DomT4 commented Dec 19, 2015

jasontedor commented Dec 19, 2015

jasontedor commented Dec 27, 2015

MikeMcQuaid Dec 27, 2015

jasontedor Dec 27, 2015

MikeMcQuaid Dec 27, 2015

jasontedor Dec 28, 2015

MikeMcQuaid Dec 28, 2015

jasontedor Dec 28, 2015

jasontedor commented Dec 28, 2015

MikeMcQuaid commented Dec 28, 2015

jasontedor commented Dec 28, 2015

MikeMcQuaid commented Dec 28, 2015

jasontedor commented Dec 28, 2015

kibana-4.3.0 #46593

kibana-4.3.0 #46593

Conversation

jasontedor commented Dec 2, 2015

c9n commented Dec 3, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasontedor commented Dec 12, 2015

MikeMcQuaid commented Dec 18, 2015

jasontedor commented Dec 18, 2015

MikeMcQuaid commented Dec 18, 2015

DomT4 commented Dec 19, 2015

jasontedor commented Dec 19, 2015

jasontedor commented Dec 27, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasontedor commented Dec 28, 2015

MikeMcQuaid commented Dec 28, 2015

jasontedor commented Dec 28, 2015

MikeMcQuaid commented Dec 28, 2015

jasontedor commented Dec 28, 2015