qpid-proton 0.17.0 (new formula) #10111

crenz · 2017-02-19T01:33:25Z

New formula for Apache Qpid Proton AMQP library.
Provides C and Go language bindings.

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

New formula for Apache Qpid Proton AMQP library. Provides C and Go language bindings.

MikeMcQuaid · 2017-02-20T12:22:39Z

Formula/qpid-proton.rb

+      -DBUILD_RUBY=OFF
+    ]
+
+    # qpid-proton require build in a subdirectory


Can remove this comment.

Comment removed as requested.

MikeMcQuaid · 2017-02-20T12:22:56Z

Formula/qpid-proton.rb

+
+    # qpid-proton require build in a subdirectory
+    Dir.mkdir("build")
+    Dir.chdir("build")


Can instead do mkdir "build" do of the above two.

Changed as requested.

MikeMcQuaid · 2017-02-20T12:23:09Z

Formula/qpid-proton.rb

+  def install
+    # Javascript bindings switched off - leads to build errors
+    # Perl, Python, Ruby bindings switched off - leads to linking errors
+    cmake_args = %w[


Name this args for consistency with other formulae.

Renamed to args as requested.

MikeMcQuaid · 2017-02-20T12:23:37Z

Formula/qpid-proton.rb

+
+  test do
+    (testpath/"test.c").write <<-EOS.undent
+          #include "proton/message.h"


Indent this two characters right of the ( on the preceding line.

Indented as requested.

MikeMcQuaid · 2017-02-20T12:23:45Z

Formula/qpid-proton.rb

+              messenger = pn_messenger(NULL);
+              return 0;
+          }
+        EOS


Ident this with system below.

Indented as requested.

MikeMcQuaid · 2017-02-20T12:24:12Z

Formula/qpid-proton.rb

+              return 0;
+          }
+        EOS
+    system ENV.cc, "test.c", "-I#{include}", "-L#{lib}64", "-lqpid-proton", "-o", "test"


This should install libraries to lib not lib64.

Changed as requested.

MikeMcQuaid · 2017-02-20T12:24:54Z

Test failing on 10.10: https://bot.brew.sh/job/Homebrew%20Core/16695/version=el_capitan/testReport/junit/brew-test-bot/el_capitan/test_qpid_proton/

Will need some _clock_gettime surgery CC @ilovezfs for suggestions.

ilovezfs · 2017-02-25T05:35:57Z

Will need some _clock_gettime surgery CC @ilovezfs for suggestions.

    if MacOS.version == "10.11" && MacOS::Xcode.installed? && MacOS::Xcode.version >= "8.0"
      inreplace "proton-c/CMakeLists.txt",
        "CHECK_SYMBOL_EXISTS(clock_gettime \"time.h\" CLOCK_GETTIME_IN_LIBC)",
        "CHECK_SYMBOL_EXISTS(undefined_gibberish \"time.h\" CLOCK_GETTIME_IN_LIBC)"
    end

- Indentation - Installation to lib/ instead of lib64/ (fixing perl/python/ruby bindings issues in the process) - mkdir do - Attempted fix for El Capitan issue

As suggested by ilovezfs

crenz · 2017-02-27T21:33:12Z

Still doesn't work. Will need to take some time to review this in detail.

JCount · 2017-02-27T23:24:10Z

@crenz Your last commit seems to have built fine on the CI, including El Capitan. Is there some other issue besides the clock_gettime problem that you were trying to solve?

crenz · 2017-02-28T21:38:20Z

@JCount When I checked last night, the build failed - that was what I was referring to. Maybe I looked at the wrong build.

In any case, I did find one more issue: The build works with the system perl, but does not work with the Homebrew perl package. The reason is that the Homebrew build flags specify -arch-i386 -arch x86_64 (which is supported by system perl), but the Homebrew perl package is only a 64 bit package. The build works without the -arch-i386 flag, so I will look for a way to change the perl module build flags if the Homebrew perl package is installed.

The underlying issue is that Homebrew needs to be made aware I'm using the perl package. I addressed this with a 'depends' statement.

MikeMcQuaid · 2017-03-04T08:56:31Z

Formula/qpid-proton.rb

+  depends_on "cmake" => :build
+  depends_on "libuv"
+  depends_on "openssl"
+  depends_on "perl" if Formula["perl"].installed?


depends_on :perl without the if

@MikeMcQuaid – I'd prefer to not depend on perl and python. The thing is, this library works very well with the system perl and python. But IF the homebrew packages are installed, I need to reference them for the compilation to work correctly. It would be a nuisance to force people to install the perl and python packages for no good reason. Some people prefer to work with the system packages (for example since they want to develop software that should run on plain-vanilla macOs), so we should give them a choice.

Sorry, we don't support that use-case. depends_on :perl does what you suggest if they are building from source and uses the Homebrew perl when using a binary package.

MikeMcQuaid · 2017-03-04T08:56:46Z

Formula/qpid-proton.rb

+  depends_on "libuv"
+  depends_on "openssl"
+  depends_on "perl" if Formula["perl"].installed?
+  depends_on "python" if Formula["python"].installed?


depends_on :python without the if

MikeMcQuaid · 2017-03-04T08:57:06Z

Formula/qpid-proton.rb

+      The modules for the perl and python bindings can be found in
+        #{Formula["qpid-proton"].opt_prefix}/lib/proton/bindings
+    EOS
+  end


Can omit these caveats as this is the default location.

MikeMcQuaid · 2017-03-04T08:57:15Z

Formula/qpid-proton.rb

+        "CHECK_SYMBOL_EXISTS(undefined_gibberish \"time.h\" CLOCK_GETTIME_IN_LIBC)"
+    end
+
+    inreplace "proton-c/bindings/perl/CMakeLists.txt",


Can you please submit this patch to the upstream developers of this project and add a link to the upstream patch submission and explanation of why the patch is needed in a comment in the formula file. Thanks!

MikeMcQuaid · 2017-03-04T08:57:20Z

Formula/qpid-proton.rb

+    ]
+
+    if MacOS.version == "10.11" && MacOS::Xcode.installed? && MacOS::Xcode.version >= "8.0"
+      inreplace "proton-c/CMakeLists.txt",


Can you please submit this patch to the upstream developers of this project and add a link to the upstream patch submission and explanation of why the patch is needed in a comment in the formula file. Thanks!

MikeMcQuaid · 2017-03-04T08:57:33Z

Formula/qpid-proton.rb

+    ENV["OPENSSL_ROOT_DIR"] = Formula["openssl"].opt_prefix
+
+    # Enforce installation into lib/ instead of lib64/
+    system "sed", "-i.orig", "1s/^/set(LIB_SUFFIX \"\") /", "CMakeLists.txt"


Can you please submit this patch to the upstream developers of this project and add a link to the upstream patch submission and explanation of why the patch is needed in a comment in the formula file. Thanks!

Also, use inreplace here instead.

MikeMcQuaid · 2017-03-04T08:57:45Z

Formula/qpid-proton.rb

@@ -0,0 +1,64 @@
+class QpidProton < Formula
+  desc "is a high-performance, lightweight AMQP 1.0 messaging library."


Remove is a

crenz · 2017-03-27T13:24:49Z

Sorry, I was rather busy with another project. Will attempt to get to this soon.

JCount · 2017-04-20T02:47:43Z

Closing this as stalled because there has been essentially no activity since Mar 2.

kbrock · 2017-07-21T00:41:05Z

@crenz Thanks for the recipe. Playing with it and seeing what I can fix

gberginc · 2017-10-13T04:21:05Z

@kbrock as we are discussing this right now with @Fryguy I just want to check if you managed to push this any further? I am also keen on updating it for Qpid Proton v0.18 which is about to happen.

kbrock · 2017-10-20T23:39:24Z

@gberginc see the referenced PR from Fryguy

qpid-proton 0.17.0 (new formula)

a2562f3

New formula for Apache Qpid Proton AMQP library. Provides C and Go language bindings.

MikeMcQuaid requested changes Feb 20, 2017

View reviewed changes

ilovezfs added the clock_gettime CI fails due to clock_gettime symbol label Feb 25, 2017

crenz added 2 commits February 26, 2017 21:41

Implemented requested changes

35f2315

- Indentation - Installation to lib/ instead of lib64/ (fixing perl/python/ruby bindings issues in the process) - mkdir do - Attempted fix for El Capitan issue

Implemented fix for El Capitan

3d6269d

As suggested by ilovezfs

crenz added 2 commits March 1, 2017 01:37

Fixed build errors with homebrew perl package

f16bc04

Fixed indentation

fecd7f1

MikeMcQuaid requested changes Mar 4, 2017

View reviewed changes

dunn added the needs response Needs a response from the issue/PR author label Mar 8, 2017

bfontaine added the new formula PR adds a new formula to Homebrew/homebrew-core label Mar 10, 2017

BrewTestBot removed the needs response Needs a response from the issue/PR author label Mar 27, 2017

dunn added the needs response Needs a response from the issue/PR author label Apr 1, 2017

JCount closed this Apr 20, 2017

BrewTestBot removed the needs response Needs a response from the issue/PR author label Jul 21, 2017

Fryguy mentioned this pull request Oct 13, 2017

qpid-proton 0.18.0 (new formula) #19414

Closed

4 tasks

Homebrew locked and limited conversation to collaborators May 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

qpid-proton 0.17.0 (new formula) #10111

qpid-proton 0.17.0 (new formula) #10111

crenz commented Feb 19, 2017

MikeMcQuaid Feb 20, 2017

crenz Mar 3, 2017

MikeMcQuaid Feb 20, 2017

crenz Mar 3, 2017

MikeMcQuaid Feb 20, 2017

crenz Mar 3, 2017

MikeMcQuaid Feb 20, 2017

crenz Mar 3, 2017

MikeMcQuaid Feb 20, 2017

crenz Mar 3, 2017

MikeMcQuaid Feb 20, 2017

crenz Mar 3, 2017

MikeMcQuaid commented Feb 20, 2017

ilovezfs commented Feb 25, 2017

crenz commented Feb 27, 2017

JCount commented Feb 27, 2017

crenz commented Feb 28, 2017 •

edited

MikeMcQuaid Mar 4, 2017

crenz Mar 27, 2017

MikeMcQuaid Mar 28, 2017

MikeMcQuaid Mar 4, 2017

MikeMcQuaid Mar 4, 2017

MikeMcQuaid Mar 4, 2017

MikeMcQuaid Mar 4, 2017

MikeMcQuaid Mar 4, 2017

MikeMcQuaid Mar 4, 2017

crenz commented Mar 27, 2017

JCount commented Apr 20, 2017

kbrock commented Jul 21, 2017

gberginc commented Oct 13, 2017

kbrock commented Oct 20, 2017

		@@ -0,0 +1,64 @@
		class QpidProton < Formula
		desc "is a high-performance, lightweight AMQP 1.0 messaging library."

qpid-proton 0.17.0 (new formula) #10111

qpid-proton 0.17.0 (new formula) #10111

Conversation

crenz commented Feb 19, 2017

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeMcQuaid commented Feb 20, 2017

ilovezfs commented Feb 25, 2017

crenz commented Feb 27, 2017

JCount commented Feb 27, 2017

crenz commented Feb 28, 2017 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crenz commented Mar 27, 2017

JCount commented Apr 20, 2017

kbrock commented Jul 21, 2017

gberginc commented Oct 13, 2017

kbrock commented Oct 20, 2017

crenz commented Feb 28, 2017 •

edited