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

spidermonkey @ 1486087028 #9607

Closed
wants to merge 1 commit into from

Conversation

mathiasbynens
Copy link
Contributor

@mathiasbynens mathiasbynens commented Feb 3, 2017

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

The existing SpiderMonkey formula builds a heavily outdated version from source. This new formula installs a more recent version by downloading a precompiled binary off the official servers.

@mathiasbynens
Copy link
Contributor Author

Do not merge this yet!

Let’s see where the discussion on https://bugzilla.mozilla.org/show_bug.cgi?id=1336514 goes first.

bin.install "shell/js"
end
lib.install "libmozglue.dylib", "libnss3.dylib"
bin.install "js" => "spidermonkey"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally, this would be aliased as sm in addition to spidermonkey. What’s the best way to do that within a formula? bin.install "js" => "spidermonkey", "js" => "sm" doesn’t seem to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bin.install "js" => "spidermonkey"
bin.install_symlink "spidermonkey" => "sm"

@mathiasbynens mathiasbynens mentioned this pull request Feb 4, 2017
4 tasks
@@ -1,54 +1,17 @@
class Spidermonkey < Formula
desc "JavaScript-C Engine"
desc "SpiderMonkey is Mozilla's JavaScript engine, as used in Firefox"
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use desc "Mozilla's JavaScript engine, as used in Firefox" not to repeat the formula’s name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Updated.


depends_on "readline"
depends_on "nspr"
url "https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-release-macosx64/1486087028/jsshell-mac.zip"
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to build from source.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How strong is that requirement? Building SpiderMonkey takes a long time, and building other JavaScript engines like V8 takes even longer. (For V8 specifically, the build process is quite cumbersome and it changes all the time — see #9392 for more background. CC @pinepain)

Copy link
Member

Choose a reason for hiding this comment

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

It's a hard blocker.

Copy link
Contributor Author

@mathiasbynens mathiasbynens Feb 4, 2017

Choose a reason for hiding this comment

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

Ah, I found https://github.com/Homebrew/brew/blob/master/docs/Acceptable-Formulae.md#we-dont-like-binary-formulae which explains this requirement a bit further. Feel free to close this PR — I’ll create a tap instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tap with formulae for ECMAScript/JavaScript engines (downloaded from official sources instead of built from source, for performance reasons): https://github.com/mathiasbynens/homebrew-ecmascript

Copy link
Member

Choose a reason for hiding this comment

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

Why is performance better when not build from source?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MikeMcQuaid I meant brew install spidermonkey is much faster if all you need to do is download a tarball and extract it, compared to building the exact same binary from source.

Copy link
Member

Choose a reason for hiding this comment

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

We build binary packages for people for this same reason.

Copy link
Member

Choose a reason for hiding this comment

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

Which in this case is cellar :any so can be used by anyone using the latest three versions of macOS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants