This repository has been archived by the owner. It is now read-only.

zero-install 2.6.2 (new formula) #28771

Closed
wants to merge 15 commits into
from

Conversation

Projects
None yet
7 participants
@dbenamy

dbenamy commented Apr 27, 2014

This is my first homebrew contribution. Let me know if I did anything wrong.

This formula was mostly written by
Anders F Björklund afb@users.sourceforge.net and
Anil Madhavapeddy anil@recoil.org.

zero-install 2.6.2 (new formula)
This is my first homebrew contribution. Let me know if I did anything wrong.

This formula was mostly written by
Anders F Björklund <afb@users.sourceforge.net> and
Anil Madhavapeddy <anil@recoil.org>.

@adamv adamv added the new formula label Apr 27, 2014

@adamv

This comment has been minimized.

Show comment
Hide comment
@adamv

adamv Apr 27, 2014

Contributor

Thanks for the submission!

Looks like there is a small issue with the provided test:

Error Message

failed: brew test --verbose zero-install
Stacktrace

        Error: zero-install: failed
Testing zero-install
<"hello world"> expected but was
<"hello world\n">.
Contributor

adamv commented Apr 27, 2014

Thanks for the submission!

Looks like there is a small issue with the provided test:

Error Message

failed: brew test --verbose zero-install
Stacktrace

        Error: zero-install: failed
Testing zero-install
<"hello world"> expected but was
<"hello world\n">.
Library/Formula/zero-install.rb
@@ -0,0 +1,60 @@
+require 'formula'

This comment has been minimized.

@adamv

adamv Apr 27, 2014

Contributor

House style is to prefer double-quotes.

@adamv

adamv Apr 27, 2014

Contributor

House style is to prefer double-quotes.

This comment has been minimized.

@dbenamy

dbenamy Apr 27, 2014

Fixed.

Just FYI, I ran the style checker and it didn't warn me of this.

@dbenamy

dbenamy Apr 27, 2014

Fixed.

Just FYI, I ran the style checker and it didn't warn me of this.

Library/Formula/zero-install.rb
+ ENV.deparallelize # parellel builds fail for some of these libs
+
+ # Required for lablgtk2 to find Quartz X11 libs.
+ ENV.append_path 'PKG_CONFIG_PATH', '/opt/X11/lib/pkgconfig' if build.with? 'gui'

This comment has been minimized.

@adamv

adamv Apr 27, 2014

Contributor

If X is needed we need to declare a real x11 dependency.

@adamv

adamv Apr 27, 2014

Contributor

If X is needed we need to declare a real x11 dependency.

Library/Formula/zero-install.rb
+ end
+
+ test do
+ (testpath/"hello.py").write <<-EOPY.undent

This comment has been minimized.

@adamv

adamv Apr 27, 2014

Contributor

Please use EOS as the here-doc marker for both of these files

@adamv

adamv Apr 27, 2014

Contributor

Please use EOS as the here-doc marker for both of these files

@dbenamy

This comment has been minimized.

Show comment
Hide comment
@dbenamy

dbenamy Apr 27, 2014

Thanks for the review :-)

Whoops! I fixed the test.

dbenamy commented Apr 27, 2014

Thanks for the review :-)

Whoops! I fixed the test.

@dbenamy

This comment has been minimized.

Show comment
Hide comment
@dbenamy

dbenamy Apr 27, 2014

Should I create a new PR?

dbenamy commented Apr 27, 2014

Should I create a new PR?

@adamv

This comment has been minimized.

Show comment
Hide comment
@adamv

adamv Apr 27, 2014

Contributor

No, adding new commits to this one is great.

Contributor

adamv commented Apr 27, 2014

No, adding new commits to this one is great.

@dbenamy

This comment has been minimized.

Show comment
Hide comment
@dbenamy

dbenamy Apr 28, 2014

The build broke with this:

==> Installing dependencies for zero-install: gringo, clasp, aspcud, opam
==> Installing zero-install dependency: gringo
Error: gringo cannot be built with any available compilers.
To install this formula, you may need to:
  brew install gcc

Does that mean I should add a build dep on gcc?

dbenamy commented Apr 28, 2014

The build broke with this:

==> Installing dependencies for zero-install: gringo, clasp, aspcud, opam
==> Installing zero-install dependency: gringo
Error: gringo cannot be built with any available compilers.
To install this formula, you may need to:
  brew install gcc

Does that mean I should add a build dep on gcc?

@mistydemeo

This comment has been minimized.

Show comment
Hide comment
@mistydemeo

mistydemeo Apr 28, 2014

Contributor

@dbenamy No, that's fine. This only happens on Lion (where an appropriate C++11-compatible compiler isn't installed by Xcode).

Contributor

mistydemeo commented Apr 28, 2014

@dbenamy No, that's fine. This only happens on Lion (where an appropriate C++11-compatible compiler isn't installed by Xcode).

Library/Formula/zero-install.rb
+ depends_on 'gnupg'
+ depends_on 'glib' if build.without? 'gui'
+ depends_on 'gtk+' if build.with? 'gui'
+ depends_on :x11 if build.with? 'gui'

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 28, 2014

Member

depends_on :x11 => :optional and use build.with? "x11" everywhere instead.

@MikeMcQuaid

MikeMcQuaid Apr 28, 2014

Member

depends_on :x11 => :optional and use build.with? "x11" everywhere instead.

This comment has been minimized.

Library/Formula/zero-install.rb
+require "formula"
+
+class ZeroInstall < Formula
+ homepage 'http://0install.net/injector.html'

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 28, 2014

Member

Please read the Ruby Style Guide and update this pull request's changes to conform to it (i.e. use double quotes). Thanks!

@MikeMcQuaid

MikeMcQuaid Apr 28, 2014

Member

Please read the Ruby Style Guide and update this pull request's changes to conform to it (i.e. use double quotes). Thanks!

This comment has been minimized.

@dbenamy

dbenamy Apr 28, 2014

Thanks for the review!

I changed the quotes.

You might want to update "brew audit" to catch these.

@dbenamy

dbenamy Apr 28, 2014

Thanks for the review!

I changed the quotes.

You might want to update "brew audit" to catch these.

Library/Formula/zero-install.rb
+class ZeroInstall < Formula
+ homepage 'http://0install.net/injector.html'
+ url 'https://downloads.sf.net/project/zero-install/0install/2.6.2/0install-2.6.2.tar.bz2'
+ sha256 '5755226ef4b32f04723bcbe551f4694ddf78dffbb0f589c3140c2d7056370961'

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Apr 28, 2014

Member

Just leave this as sha1 unless the sha256 is prominent on the homepage (doesn't seem to be).

@MikeMcQuaid

MikeMcQuaid Apr 28, 2014

Member

Just leave this as sha1 unless the sha256 is prominent on the homepage (doesn't seem to be).

This comment has been minimized.

dbenamy added some commits Apr 28, 2014

@dbenamy

This comment has been minimized.

Show comment
Hide comment
@dbenamy

dbenamy Apr 28, 2014

Btw, the marketing name is "0install" but I can't make the recipe that because it starts with a number. Is there a way to add an alias or something so that "brew install zero-install" and "brew install 0install" will work?

dbenamy commented Apr 28, 2014

Btw, the marketing name is "0install" but I can't make the recipe that because it starts with a number. Is there a way to add an alias or something so that "brew install zero-install" and "brew install 0install" will work?

Library/Formula/zero-install.rb
@@ -0,0 +1,60 @@
+require "formula"

This comment has been minimized.

@dbenamy

dbenamy Apr 28, 2014

I just realized that the package on other systems is called zeroinstall (without the dash). Let me rename this to match.

@dbenamy

dbenamy Apr 28, 2014

I just realized that the package on other systems is called zeroinstall (without the dash). Let me rename this to match.

@dbenamy

This comment has been minimized.

Show comment
Hide comment
@dbenamy

dbenamy Apr 28, 2014

I figured out the alias bit and renamed the formula.

This should be good to go unless you find more problems :-)

dbenamy commented Apr 28, 2014

I figured out the alias bit and renamed the formula.

This should be good to go unless you find more problems :-)

Library/Formula/zeroinstall.rb
+ ENV.deparallelize
+
+ # Required for lablgtk2 to find Quartz X11 libs.
+ ENV.append_path "PKG_CONFIG_PATH", "/opt/X11/lib/pkgconfig" if build.with? "x11"

This comment has been minimized.

@jacknagel

jacknagel Apr 28, 2014

Contributor

This shouldn't be necessary, depending on x11 does this automatically.

@jacknagel

jacknagel Apr 28, 2014

Contributor

This shouldn't be necessary, depending on x11 does this automatically.

This comment has been minimized.

@dbenamy

dbenamy Apr 28, 2014

Neat! Done.

Thanks for the review!

@dbenamy

dbenamy Apr 28, 2014

Neat! Done.

Thanks for the review!

Library/Formula/zeroinstall.rb
+ depends_on :x11 => :optional
+ depends_on "glib" if build.without? "x11"
+ depends_on "gtk+" if build.with? "x11"
+ depends_on "gettext" => :build if build.head?

This comment has been minimized.

@jacknagel

jacknagel Apr 28, 2014

Contributor

head-specific dependencies should be in a block with the URL:

head do
  url "https://github.com/0install/0install.git"
  depends_on "gettext" => :build
end
@jacknagel

jacknagel Apr 28, 2014

Contributor

head-specific dependencies should be in a block with the URL:

head do
  url "https://github.com/0install/0install.git"
  depends_on "gettext" => :build
end

This comment has been minimized.

@dbenamy

dbenamy Apr 28, 2014

Done.

You may want to add this to brew audit.

@dbenamy

dbenamy Apr 28, 2014

Done.

You may want to add this to brew audit.

@jacknagel

This comment has been minimized.

Show comment
Hide comment
@jacknagel

jacknagel Apr 28, 2014

Contributor

As a historical note, this was previously removed from Homebrew in 5a15095 (I don't mind adding it back if things have changed since then).

Contributor

jacknagel commented Apr 28, 2014

As a historical note, this was previously removed from Homebrew in 5a15095 (I don't mind adding it back if things have changed since then).

Library/Formula/zeroinstall.rb
+
+ head "https://github.com/0install/0install"
+
+ depends_on "gnupg"

This comment has been minimized.

@jacknagel

jacknagel Apr 28, 2014

Contributor

We should use a requirement rather than depend on gpg directly; see the original version of this formula (5a15095) for how this can be done.

@jacknagel

jacknagel Apr 28, 2014

Contributor

We should use a requirement rather than depend on gpg directly; see the original version of this formula (5a15095) for how this can be done.

This comment has been minimized.

@dbenamy

This comment has been minimized.

Show comment
Hide comment
@dbenamy

dbenamy Apr 28, 2014

Thanks for the pointer to the old commit. 0install was rewritten in ocaml so it's no longer easy to pip install it.

It is possible to set up opam and install it that way, but that involves a few manual steps so this would still be nice.

dbenamy commented Apr 28, 2014

Thanks for the pointer to the old commit. 0install was rewritten in ocaml so it's no longer easy to pip install it.

It is possible to set up opam and install it that way, but that involves a few manual steps so this would still be nice.

Library/Formula/zeroinstall.rb
@@ -0,0 +1,88 @@
+require "formula"
+
+class Gnupg < Requirement

This comment has been minimized.

@adamv

adamv Apr 29, 2014

Contributor

Call this class GnupgRequirement

@adamv

adamv Apr 29, 2014

Contributor

Call this class GnupgRequirement

This comment has been minimized.

@dbenamy

dbenamy Apr 29, 2014

Then it won't match the other depends_on lines:

depends_on :x11
depends_on "glib"
depends_on "gtk+"
depends_on GnupgRequirement

But ok, it's your project :-)

@dbenamy

dbenamy Apr 29, 2014

Then it won't match the other depends_on lines:

depends_on :x11
depends_on "glib"
depends_on "gtk+"
depends_on GnupgRequirement

But ok, it's your project :-)

dbenamy added some commits Apr 29, 2014

@dbenamy

This comment has been minimized.

Show comment
Hide comment
@dbenamy

dbenamy Apr 30, 2014

I just fixed the patch's sha1, did a clean reinstall, and it looks good.

AFAIK, this is good to merge. Do you need anything else?

dbenamy commented Apr 30, 2014

I just fixed the patch's sha1, did a clean reinstall, and it looks good.

AFAIK, this is good to merge. Do you need anything else?

@dbenamy

This comment has been minimized.

Show comment
Hide comment
@dbenamy

dbenamy May 13, 2014

Hi there,

What can I do to move this along?

This went through about 4 rounds of review and changes and it's a little frustrating to have it just sitting here after all of that.

Thanks,
Dan

dbenamy commented May 13, 2014

Hi there,

What can I do to move this along?

This went through about 4 rounds of review and changes and it's a little frustrating to have it just sitting here after all of that.

Thanks,
Dan

@adamv

This comment has been minimized.

Show comment
Hide comment
@adamv

adamv May 21, 2014

Contributor

Sorry for the lack of correspondence here; I think the general uneasiness here is on this having a dependency on OPAM, which is itself a package manager. This suggests that this formula has other dependencies that aren't provided by Homebrew itself.

Is there a compelling reason to build this software from source rather than using the provided .pkg installer? http://0install.net/

Contributor

adamv commented May 21, 2014

Sorry for the lack of correspondence here; I think the general uneasiness here is on this having a dependency on OPAM, which is itself a package manager. This suggests that this formula has other dependencies that aren't provided by Homebrew itself.

Is there a compelling reason to build this software from source rather than using the provided .pkg installer? http://0install.net/

@adamv adamv added the o-caml label May 21, 2014

@afb

This comment has been minimized.

Show comment
Hide comment
@afb

afb May 22, 2014

Contributor

Actually it doesn't depend on OPAM, but on some modules available through it. This was the same reason the Python version was removed earlier, not being able to package all the dependencies.
If you want to handle the new 0install the same way as the old zeroinstall-injector, you could have the user install "0install" with OPAM. And then only use brew to install gnupg and ocaml/opam...

I think the main reason for using Homebrew for 0install was to use the gnupg/gnupg2 and glib/gtk+ that it provides, rather than the GnuPG.pkg and PyGTK.pkg ones that are bundled with ZeroInstall.pkg.
But there's also a tarball with generic binaries, that could be used instead of the bundled installation:
https://downloads.sourceforge.net/project/zero-install/0install/2.6.2/0install-darwin-x86_64-2.6.2.tar.bz2

Contributor

afb commented May 22, 2014

Actually it doesn't depend on OPAM, but on some modules available through it. This was the same reason the Python version was removed earlier, not being able to package all the dependencies.
If you want to handle the new 0install the same way as the old zeroinstall-injector, you could have the user install "0install" with OPAM. And then only use brew to install gnupg and ocaml/opam...

I think the main reason for using Homebrew for 0install was to use the gnupg/gnupg2 and glib/gtk+ that it provides, rather than the GnuPG.pkg and PyGTK.pkg ones that are bundled with ZeroInstall.pkg.
But there's also a tarball with generic binaries, that could be used instead of the bundled installation:
https://downloads.sourceforge.net/project/zero-install/0install/2.6.2/0install-darwin-x86_64-2.6.2.tar.bz2

@afb

This comment has been minimized.

Show comment
Hide comment
@afb

afb May 22, 2014

Contributor

Hmm, sorry. The tarball of 2.6.2 still depends on glib (in /opt/gtk), even though gtk+ was optional.
i.e. it requires http://downloads.sourceforge.net/project/macpkg/PyGTK/2.24.0/PyGTK.pkg

However, later 0install versions will be usable in CLI mode without installing glib: (GUI requires gtk+)
0install/0install@ab68a6d

Contributor

afb commented May 22, 2014

Hmm, sorry. The tarball of 2.6.2 still depends on glib (in /opt/gtk), even though gtk+ was optional.
i.e. it requires http://downloads.sourceforge.net/project/macpkg/PyGTK/2.24.0/PyGTK.pkg

However, later 0install versions will be usable in CLI mode without installing glib: (GUI requires gtk+)
0install/0install@ab68a6d

@dbenamy

This comment has been minimized.

Show comment
Hide comment
@dbenamy

dbenamy May 24, 2014

@adamv Thanks for the explanation. There may be more than one reason to install it via brew, but for me, it boils down to brew being an awesome way to install software. I guess it's the same reason that other programs, like mercurial and heroku-toolbelt, are in brew, even though they also provide standalone osx installers. It's just easier, more pleasant, and more secure to be able to install stuff from brew and be able to update it automatically. Is there any way that 0install is different from those?

dbenamy commented May 24, 2014

@adamv Thanks for the explanation. There may be more than one reason to install it via brew, but for me, it boils down to brew being an awesome way to install software. I guess it's the same reason that other programs, like mercurial and heroku-toolbelt, are in brew, even though they also provide standalone osx installers. It's just easier, more pleasant, and more secure to be able to install stuff from brew and be able to update it automatically. Is there any way that 0install is different from those?

@talex5

This comment has been minimized.

Show comment
Hide comment
@talex5

talex5 May 25, 2014

0install 2.7 has now been release and should work without glib (assuming you don't need a GUI).

talex5 commented May 25, 2014

0install 2.7 has now been release and should work without glib (assuming you don't need a GUI).

@afb

This comment has been minimized.

Show comment
Hide comment
@afb

afb May 29, 2014

Contributor

I updated to 2.7 (dropping the patch) and fixed the gui (it's gtk+, not x11) and curl cache, in afb@10f049e

Contributor

afb commented May 29, 2014

I updated to 2.7 (dropping the patch) and fixed the gui (it's gtk+, not x11) and curl cache, in afb@10f049e

@adamv

This comment has been minimized.

Show comment
Hide comment
@adamv

adamv Jul 19, 2014

Contributor

Would review an updated pull request for 2.7 or newer.

Contributor

adamv commented Jul 19, 2014

Would review an updated pull request for 2.7 or newer.

@adamv adamv closed this Jul 19, 2014

@dbenamy

This comment has been minimized.

Show comment
Hide comment
@dbenamy

dbenamy Jul 20, 2014

@adamv, I'm not terribly motivated to resubmit an updated PR :-/ This one was pretty frustrating: lots of requests to change pretty much every detail that could be changed, and then lots of delays and unclear resolution, eg no real answer to the May 24 comment advocating inclusion.

Maybe afb would like to push it forward.

Thanks anyway for the final follow up.

dbenamy commented Jul 20, 2014

@adamv, I'm not terribly motivated to resubmit an updated PR :-/ This one was pretty frustrating: lots of requests to change pretty much every detail that could be changed, and then lots of delays and unclear resolution, eg no real answer to the May 24 comment advocating inclusion.

Maybe afb would like to push it forward.

Thanks anyway for the final follow up.

@afb

This comment has been minimized.

Show comment
Hide comment
@afb

afb Jul 20, 2014

Contributor

I already updated to 2.7 (see above), feel free to merge. Unfortunately the handling of gnupg/gnupg2 stops bottling and makes it rather useless.

But the main showstopper is the opam module handling, the current local install hack isn't good and just erroring out with a message (like the built-in handling does) isn't too helpful either. So I guess we should (again) delete the formula and only use Homebrew for installing OCaml/OPAM and GTK+ ? I plan on removing the GnuPG dependency (with OpenPGP) anyway, for licensing reasons.

@dbenamy my experience was/is the same as yours, so you can close the Pull Request if you don't want to maintain it yourself... I'll handle it upstream

Contributor

afb commented Jul 20, 2014

I already updated to 2.7 (see above), feel free to merge. Unfortunately the handling of gnupg/gnupg2 stops bottling and makes it rather useless.

But the main showstopper is the opam module handling, the current local install hack isn't good and just erroring out with a message (like the built-in handling does) isn't too helpful either. So I guess we should (again) delete the formula and only use Homebrew for installing OCaml/OPAM and GTK+ ? I plan on removing the GnuPG dependency (with OpenPGP) anyway, for licensing reasons.

@dbenamy my experience was/is the same as yours, so you can close the Pull Request if you don't want to maintain it yourself... I'll handle it upstream

@afb afb referenced this pull request Aug 15, 2015

Closed

0install 2.8 #42405

@talex5

This comment has been minimized.

Show comment
Hide comment
@talex5

talex5 Aug 22, 2015

For those following this issue, the current attempt to get 0install in Homebrew is here: #42405

talex5 commented Aug 22, 2015

For those following this issue, the current attempt to get 0install in Homebrew is here: #42405

@Homebrew Homebrew locked and limited conversation to collaborators Feb 17, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.