-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Support macOS Catalina #1032
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
Comments
Waiting for skade/leveldb-sys#12 to be merged. |
@a-rodin last I talked with florian he said that crate is pretty much unmaintained, maybe it makes sense for us to fork for the moment? |
and I also think this issue is a decently high priority since most mac users will probably switch over soon. |
I agree that having a fork would make it possible to iterate faster. I'm not sure is it better to have the fork in a separate repository or in |
Yeah, I want to hear what @lukesteensen has to say. This also leads me to the thing I've probably said a thousand times now maybe now is the time to move to something like sled? (may not be but just thought I'd bring it up) |
I wonder if we eventually end up with forks of large portion of our dependencies. ClickHouse seems to be doing this, with almost half of contrib dependencies being checked into the tree because of the need for additional patches. |
@a-rodin I think we can avoid this. It is usually possible to reach out or find people to reach out. We should only vendor things that we know the maintainer will not update. In this case, I had a chat with Florian at RustConf about it and this is pretty much what he said. I think also being apart of the rust open source communtiy I would like for us to focus on giving back as much as possible so trying to get the patches in. We've already done this a lot with tokio and tower. |
I stand corrected! https://twitter.com/Argorak/status/1185262666652237825?s=20 and happy that I was wrong :) |
I made a clean installation of Catalina (previously I was just upgrading from Mojave). Now I see the same issue with I've created a PR to the upstream to fix it: fede1024/rust-rdkafka#160 and #1063 to use our fork in the meantime. |
Also there are problems with linking to SSL the final Vector executable:
It looks like a conflict between different versions of OpenSSL. |
@a-rodin ah you need to add a few env vars I think if openssl is installed via homebrew
|
@LucioFranco So here is my take on this, for macOS Mojave 10.14.4 Vector used to build out of the box on Mojave prior to this pull request Now its broken, because the ssl feature was added to rdkafka rdkafka = { version = "0.20.0", features = ["ssl"], optional = true } and IMHO Vector is so cool that we should not limit people's experience in building Vector. And we are now REQUIRING people to have openssl installed on their Mac where My work around is simple... On macOS one has (2) choices... Either fix is a function of modifying Cargo.toml
rdkafka = { version = "0.20.0", features = ["ssl"], optional = true }
default = ["rdkafka", "leveldb", "jemallocator"] Not sure why rdkafka is enabled by default as many people probably never use kafka |
@LucioFranco It turns out that installing
@stormasm Thank you for your input. Vector had SSL dependency before that pull request too. It was used to, for example, enable HTTPS support for
It is also possible to build Vector with disabled
However, I think we need to add requirements for OpenSSL to CONTRIBUTING.md. #1066 does it. |
@a-rodin ah ok, does it work now for you then? |
I should note that I did not have any issues building vector on Sept 10 https://github.com/stormasm/vector I forked the repo on that date and everything worked at that time... So, I was curious about this issue some more and I did as you recommended... brew install openssl pkg-config Unfortunately, it is still broken if I do not modify Cargo.toml at all... ty" "-framework" "CoreFoundation" "-framework" "Security" "-lc++" "-lSystem" "-lresolv" "-lc" "-lm" error: aborting due to previous error error: Could not compile |
After doing the brew install above I added these lines to my .profile export LDFLAGS="-L/usr/local/opt/openssl/lib" export PKG_CONFIG_PATH="/usr/local/opt/openssl/lib/pkgconfig" Also note that I installed this as well via homebrew without any issues... |
It worked for me, but more by accident. Unfortunately in an arbitrary configuration we still need to export |
@LucioFranco @a-rodin thanks for looking into this further... I will be happy to test your solution on my mac once it gets worked out... No hurry on this end, rdkafka = { version = "0.20.0", optional = true } Plus, working on the bigger picture re: #1074 sounds like a good place It will also give me the opportunity to better understand native dependencies management |
@a-rodin Just sent you a file to your email with the whole stack trace for.... macOS Mojave Compiling rdkafka-sys v1.2.1 (https://github.com/timberio/rust-rdkafka#06ddd48d) Caused by: |
@stormasm From the logs it looks like you have non-empty environment variables Please try to unset |
@a-rodin Thanks that fixed the problem. macOS Mojave I ran all of the tests and they passed. |
At the moment builds with
leveldb
feature on macOS Catalina are broken becauseleveldb-sys
crate cannot be compiled. Builds on macOS Mojave still work.The text was updated successfully, but these errors were encountered: