-
-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
apache-pulsar 2.8.0 (new formula) #83238
Conversation
This commit adds a formula for the Apache Pulsar message broker (https://pulsar.apache.org/). The broker will operate in its minimal/ default "standalone" mode when started by this formula, which is roughly equivalent to the behavior of ZooKeeper and other similar formulae.
The arm64 CI failure is due to missing support upstream: os72/protoc-jar#93 |
I'm unsure what the ubuntu-latest CI failure indicates. Does anyone know what this means?
|
That means it shouldn't be installing powerpc binaries for Linux because we only support x86_64 on Linux |
After spending some time trying to get this working on M1, it seems like there are multiple significant issues blocking it working on that platform: grpc, protoc-maven, protoc itself, and rocksdb all have open issues for M1 support. Some of those have workarounds, but threading those workarounds down through Pulsar's (complicated) build system seems questionable and would require programmatically editing at least the build system, if not some of Pulsar's code. Attempting to override the architecture to build x86_64 artifacts on ARM causes other build failures, as it appears some compiled artifacts used by this project do not honor the arch override and thus fail to link/load on M1. That leads me to the question: |
Sorry, didn't mean to close, just comment. |
Formula/apache-pulsar.rb
Outdated
def install | ||
chmod "+x", "src/rename-netty-native-libs.sh" | ||
with_env( | ||
"JAVA_HOME" => Formula["openjdk@11"].opt_prefix, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't the usual location for JAVA_HOME
so I don't think this does anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍, addressed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How exactly? The line is still the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately removing this caused tests to fail; I restored it in the latest commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, the test block was failing? That means that it's compiling in this value. In that case especially this shouldn't be opt_prefix
but overridable_java_home
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the install step was failing because it was building on a newer, incompatible version of Java. Evidence from the CI failure in the first linked commit:
==> mvn -X clean package -DskipTests -Pcore-modules
Picked up _JAVA_OPTIONS: -Duser.home=/Users/brew/Library/Caches/Homebrew/java_cache
Apache Maven 3.8.2 (ea98e05a04480131370aa0c110b8c54cf726c06f)
Maven home: /usr/local/Cellar/maven/3.8.2/libexec
Java version: 16.0.2, vendor: Homebrew, runtime: /usr/local/Cellar/openjdk/16.0.2/libexec/openjdk.jdk/Contents/Home
Default locale: en_GB, platform encoding: UTF-8
OS name: "mac os x", version: "11.3.1", arch: "x86_64", family: "mac"
[DEBUG] Created new class realm maven.api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the confusion; by "tests" I meant "formula CI in general", not brew test
.
Are there additional changes that should be made to this formula before it can be merged? Unrelated to this formula, I have a question: there is another distribution of this project, streamnative pulsar, which leads ahead of the Apache release (though they try to upstream most of their work to Apache eventually) and releases on a more regular cadence. If I wanted to make a formula for that distribution, should I a) make a tap, b) make another formula with a different name (e.g. |
@SMillerDev any further changes needed here? Apologies for the nagging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this formula, I have a question: there is another distribution of this project, streamnative pulsar, which leads ahead of the Apache release (though they try to upstream most of their work to Apache eventually) and releases on a more regular cadence.
Is that a fork of this one? If so, I'd guess that that would belong in an external tap, unless it's very popular.
Formula/apache-pulsar.rb
Outdated
system( | ||
"mvn", | ||
"-X", | ||
"clean", | ||
"package", | ||
"-DskipTests", | ||
"-Pcore-modules", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we turn this into one (or maybe two) lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 fixed!
@carlocab @SMillerDev what additional changes are needed here? |
Co-authored-by: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
Co-authored-by: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @zbentley.
Thanks @carlocab and @SMillerDev for your time and assistance! |
No problem. Thanks for your patience here! |
This commit adds a formula for the Apache Pulsar message broker
(https://pulsar.apache.org/). The broker will operate in its minimal/
default "standalone" mode when started by this formula, which is
roughly equivalent to the behavior of ZooKeeper and other similar
formulae.
brew install --build-from-source <formula>
, where<formula>
is the name of the formula you're submitting?brew test <formula>
, where<formula>
is the name of the formula you're submitting?brew audit --strict <formula>
(after doingbrew install --build-from-source <formula>
)? If this is a new formula, does it passbrew audit --new <formula>
?