Fold geocouch in to couchdb formula #475

apjanke · 2016-04-20T05:13:59Z

Checklist

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?
Does your submission pass brew audit --strict --online <formula> (where <formula> is the name of the formula you're submitting)?
Have you built your formula locally prior to submission with brew install <formula>?

References

Closes #436
Fixes #471
Fixes "geocouch" item in #342

Description

Fixes issues with couchdb/geocouch's installation model not working under Homebrew sandboxing.

This makes geocouch an option of the couchdb formula instead of its own separate formula, because couchdb/geocouch's installation model really wants some of those files to be under couchdb's installation hierarchy.

Leaves a placeholder geocouch formula around. This is so users with an existing geocouch installation will have easy access to the caveats when it stops working and they're wondering what happened. Also so a brew update; brew upgrade results in removal of the old geocouch files, which may conflict with a new geocouch-enabled couchdb installation.

The geocouch option is off by default because GeoCouch is considered "experimental" (from what I can see while Googling) and is of lower quality/stability than CouchDB itself, so we probably don't want it on by default. Otherwise, I would have made it recommended, to provide continuity for existing users of geocouch, who would have then automatically picked it up in the upgrade to the next couchdb version at the same time geocouch became a placeholder or disappeared.

The geocouch placeholder formula can be boneyarded after a few months, once all GeoCouch users are likely to have gone through an upgrade cycle.

Testing

I'm not sure this actually works, because I couldn't find a simple way to test the GeoCouch components of CouchDB. The GeoCouch readme says to just run the test suite. But that won't really catch it, because if the installation didn't work, then the GeoCouch tests might not have been properly added to the CouchDB test suite, and you wouldn't really be testing it.

Is there a GeoCouch user who can explain how to do a "Hello World" exercising the GeoCouch extension?

xu-cheng · 2016-04-20T05:16:24Z

Formula/couchdb.rb

@@ -68,6 +77,52 @@ def install
    (var/"run/couchdb").mkpath


Probably want to put these (var/...).mkpath in post_install.

Moving to post_install.

xu-cheng · 2016-04-20T05:21:08Z

👍

MikeMcQuaid · 2016-04-20T07:18:29Z

Formula/couchdb.rb

  depends_on "spidermonkey"
  depends_on "icu4c"
  depends_on "erlang"
  depends_on "curl" if MacOS.version <= :leopard

+  resource "geocouch" do
+    url "https://codeload.github.com/couchbase/geocouch/tar.gz/couchdb1.3.x"


Prefer GitHub archive rather than codeload URLs. We should probably add an audit for that. Also, do we not have a tagged release we can use?

For this formula, the archive URL was redirecting to this one. If I didn't replace it for the resource usage, I was getting a SHA mismatch error.

I'll try the archive URL again; maybe it was something I was doing wrong locally.

I suspect that may be some weirdness using a branch tarball.

IMO it's cleaner to use a git checkout, branch and revision. It breaks our audit rule about using tags and revisions together but it should prevent the checksum breaking every time they push a new commit in that branch at least.

Agreed on checkout and revision if upstream don't provide tags we can use 👍

Might have been a transient issue or mistake on my part. The GitHub archive link is working again. Switched back to it.

I still think using the branch and revision is a better idea, personally. It's not a "real" archive tarball as such, and if they push another commit to that branch we'll get a checksum failure again. Just seems like unnecessary future hassle for something we can prevent today.

Makes sense. Switched to using git with branch/revision instead of the branch tarball.

apjanke · 2016-04-20T19:47:36Z

Amended to remove geocuch instead of making it a placeholder.

Most of the other feedback makes sense; will address that too.

DomT4 · 2016-04-23T16:24:08Z

Formula/couchdb.rb

@@ -78,13 +133,68 @@ def post_install
    end
  end

-  def caveats; <<-EOS.undent
+  def caveats
+    str = <<-EOS.undent


Annoying nit but we tend to use a simple s rather than str for caveats like these across formulae.

apjanke · 2016-04-24T07:04:07Z

Updated to address most of the feedback. Seems ready: it builds and passes test, though I wish we had a better test for the geocouch part.

MikeMcQuaid · 2016-04-24T16:14:53Z

Formula/couchdb.rb

-      <true/>
-    </dict>
-    </plist>
+    To see these instructions again, just run 'brew info couchdb'.


I think you can probably kill all these geocouch caveats; they either aren't Homebrew specific or apply to most Homebrew formulae.

True: the port 5984 is a couchdb default, and the rest is brew's normal functioning, now that we've folded geocouch in as an option and it's not doing the sandbox-violating cross-formula install. Removing.

I killed all the remaining caveats, too, since they were just instructions on how to test couchdb, and that should go in, well, the test definition. Converted them to actual test steps so now there's no caveats, and the test does more checking.

MikeMcQuaid · 2016-04-24T16:15:28Z

This is looking good; almost there.

apjanke · 2016-04-25T01:58:32Z

Test failed on Mavericks only. Which is weird, because that's what I tested it on locally, and it worked fine. Didn't try it under test-bot, though. Will look in to it.

Error Message

failed: brew test couchdb --verbose

Stacktrace

        Testing couchdb
==> Using the sandbox
/usr/bin/sandbox-exec -f /tmp/homebrew20160424-46760-1w3m6y3-0.sb /Users/brewadmin/Homebrew/Cellar/ruby187/1.8.7-p374_1/bin/ruby -W0 -I /usr/local/Library/Homebrew -- /usr/local/Library/Homebrew/test.rb /usr/local/Library/Taps/homebrew/homebrew-core/Formula/couchdb.rb --verbose
==> /usr/local/Cellar/couchdb/1.6.1_5/bin/couchjs -h
Usage: couchjs [FILE]

The couchjs command runs the Apache CouchDB JavaScript interpreter.

The exit status is 0 for success or 1 for failure.

Options:

  -h          display a short help message and exit
  -V          display version information and exit
  -H          enable couchjs cURL bindings (only avaiable
              if package was built with cURL available)
  -S SIZE     specify that the runtime should allow at
              most SIZE bytes of memory to be allocated
  -u FILE     path to a .uri file containing the address
              (or addresses) of one or more servers

Report bugs at <https://issues.apache.org/jira/browse/COUCHDB>.
Apache CouchDB 1.6.1 (LogLevel=info) is starting.
Apache CouchDB has started. Time to relax.
[info] [<0.32.0>] Apache CouchDB has started on http://127.0.0.1:5984/
==> curl --silent localhost:5984
[info] [<0.101.0>] 127.0.0.1 - - GET / 200
Error: couchdb: failed
<#<PkgVersion:0x106ea8fb0
 @revision=5,
 @version=#<Version::FromURL:0x106f17d48 @version="1.6.1">>> expected to be =~
</1\.6\.1_5/>.

MikeMcQuaid · 2016-06-20T12:27:53Z

@apjanke Any news on this? Would be great to get it merged in, even if restricted to Yosemite and above.

Fixes issues with couchdb/geocouch's installation model not working with Homebrew sandboxing. Closes Homebrew#436 Fixes Homebrew#471

apjanke · 2016-06-29T13:12:52Z

I took another look at this, and I don't even know why it ran the test the way it did. Here's the failing test message.

==> curl --silent localhost:5984
[info] [<0.101.0>] 127.0.0.1 - - GET / 200
Error: couchdb: failed
<#<PkgVersion:0x106ea8fb0
 @revision=5,
 @version=#<Version::FromURL:0x106f17d48 @version="1.6.1">>> expected to be =~
</1\.6\.1_5/>.

But the current test code isn't even matching against a version number; it's matching against /Homebrew/. And it's not passing --silent to curl.

      assert_match /Homebrew/, shell_output("curl -# localhost:5984")

Maybe it was something weird with the Mavericks test-bot's state. brew test-bot ... couchdb passed for me locally on Mavericks against this PR.

I amended this PR with a couple audit fixes and rebased on master. That'll kick off another test run and we'll see if the failure is repeatable.

apjanke · 2016-06-29T14:02:31Z

It passed this time. I'm going to tentatively chalk that previous failure up to some odd state on the Mavericks test bot.

Look okay to merge now?

MikeMcQuaid · 2016-06-29T21:26:16Z

Look okay to merge now?

👍

apjanke mentioned this pull request Apr 20, 2016

geocouch: fix multiple audit warnings (mostly concerning using system… #436

Closed

4 tasks

xu-cheng reviewed Apr 20, 2016
View reviewed changes

apjanke force-pushed the geocouch-as-couchdb-option branch 2 times, most recently from 0a4048d to 44e14a3 Compare April 20, 2016 05:47

xu-cheng added the maintainer feedback Additional maintainers' opinions may be needed label Apr 20, 2016

apjanke force-pushed the geocouch-as-couchdb-option branch from 44e14a3 to f68a780 Compare April 20, 2016 06:07

MikeMcQuaid reviewed Apr 20, 2016
View reviewed changes

apjanke added the needs response Needs a response from the issue/PR author label Apr 20, 2016

apjanke self-assigned this Apr 23, 2016

DomT4 reviewed Apr 23, 2016
View reviewed changes

apjanke force-pushed the geocouch-as-couchdb-option branch from f68a780 to d466f9c Compare April 24, 2016 01:40

ghost removed the needs response Needs a response from the issue/PR author label Apr 24, 2016

apjanke force-pushed the geocouch-as-couchdb-option branch 6 times, most recently from 7e8a4f6 to 682fc95 Compare April 24, 2016 06:57

MikeMcQuaid reviewed Apr 24, 2016
View reviewed changes

apjanke force-pushed the geocouch-as-couchdb-option branch 3 times, most recently from 209693a to e8b029c Compare April 24, 2016 18:46

apjanke added the needs response Needs a response from the issue/PR author label Apr 25, 2016

xu-cheng mentioned this pull request Jun 21, 2016

Formulae fail to build #342

Closed

53 tasks

apjanke force-pushed the geocouch-as-couchdb-option branch from e8b029c to 96a78a9 Compare June 29, 2016 12:41

ghost removed the needs response Needs a response from the issue/PR author label Jun 29, 2016

Fold geocouch in to couchdb formula

87bd094

Fixes issues with couchdb/geocouch's installation model not working with Homebrew sandboxing. Closes Homebrew#436 Fixes Homebrew#471

apjanke force-pushed the geocouch-as-couchdb-option branch from 96a78a9 to 87bd094 Compare June 29, 2016 13:02

apjanke closed this in 8c13fc2 Jul 2, 2016

apjanke deleted the geocouch-as-couchdb-option branch July 14, 2016 17:27

Homebrew locked and limited conversation to collaborators May 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fold geocouch in to couchdb formula #475

Fold geocouch in to couchdb formula #475

apjanke commented Apr 20, 2016 •

edited

xu-cheng Apr 20, 2016

apjanke Apr 20, 2016

xu-cheng commented Apr 20, 2016

MikeMcQuaid Apr 20, 2016

apjanke Apr 20, 2016

MikeMcQuaid Apr 20, 2016

DomT4 Apr 20, 2016 •

edited

MikeMcQuaid Apr 20, 2016

apjanke Apr 24, 2016

DomT4 Apr 24, 2016

MikeMcQuaid Apr 24, 2016

apjanke Apr 24, 2016

apjanke commented Apr 20, 2016

DomT4 Apr 23, 2016

apjanke Apr 24, 2016

apjanke commented Apr 24, 2016

MikeMcQuaid Apr 24, 2016

apjanke Apr 24, 2016

apjanke Apr 24, 2016

MikeMcQuaid Apr 24, 2016

MikeMcQuaid commented Apr 24, 2016

apjanke commented Apr 25, 2016

MikeMcQuaid commented Jun 20, 2016

apjanke commented Jun 29, 2016

apjanke commented Jun 29, 2016

MikeMcQuaid commented Jun 29, 2016

Fold geocouch in to couchdb formula #475

Fold geocouch in to couchdb formula #475

Conversation

apjanke commented Apr 20, 2016 • edited

Checklist

References

Description

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xu-cheng commented Apr 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DomT4 Apr 20, 2016 • edited

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

apjanke commented Apr 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apjanke commented Apr 24, 2016

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

MikeMcQuaid commented Apr 24, 2016

apjanke commented Apr 25, 2016

MikeMcQuaid commented Jun 20, 2016

apjanke commented Jun 29, 2016

apjanke commented Jun 29, 2016

MikeMcQuaid commented Jun 29, 2016

apjanke commented Apr 20, 2016 •

edited

DomT4 Apr 20, 2016 •

edited