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

Fix build on macOS Catalina #160

Closed
wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 21, 2019

This PR fixes #159, as the problem seems to be related to the change of the default shell from bash to zsh in Catalina.

Signed-off-by: Alexander Rodin <rodin.alexander@gmail.com>
@ghost ghost force-pushed the macos-catalina-fix branch from 0062781 to 924c52b Compare October 21, 2019 12:20
@benesch
Copy link
Collaborator

benesch commented Oct 21, 2019

Hey, thanks for the PR. I'm a bit confused though, because the error message in the issue you filed (#159) doesn't line up with your fix here. Based on the output there, configure ran successfully, since stdout starts with "Configuring librdkafka" and ends with "Now type 'make' to build". Also, the backtrace starts in "build_script_build::make_librdkafka":

run_command_or_fail("librdkafka", "make", &["-j", &num_cpus::get().to_string(), "libs"]);

Is it possible that you didn't have make installed?

@ghost
Copy link
Author

ghost commented Oct 21, 2019

@benesch Thank you for quick reply.

The error is still related to ./configure because

Running command: "./configure --disable-sasl --disable-ssl --disable-lz4" in dir: librdkafka

is the last logged item and also because the stack trace contains line 112, which is the one fixed in this PR:

   0: std::panicking::default_hook::{{closure}}
   1: std::panicking::default_hook
   2: <std::panicking::begin_panic::PanicPayload<A> as core::panic::BoxMeUp>::get
   3: std::panicking::continue_panic_fmt
   4: std::thread::local::fast::Key<T>::try_initialize
   5: build_script_build::run_command_or_fail
             at ./build.rs:25
   6: build_script_build::build_librdkafka
             at ./build.rs:112

Basically, the problem is that although ./configure does all the configuration, for some reason it returns non-zero exit code. It returns 0 as the exit code if invoked from shell manually or from Rust as sh -c ./configure, and this PR uses the latter approach as a workaround.

As a reference, levedb-sys crate had similar issue, which was fixed in a similar way: skade/leveldb-sys#11.

@benesch
Copy link
Collaborator

benesch commented Oct 21, 2019 via email

@benesch
Copy link
Collaborator

benesch commented Oct 21, 2019

Ok, this reproduced instantly. Thanks again for the report. I dug in pretty deep, and came to the conclusion that while the proposed fix here does fix the issue, it obscures the real issue, which is that using relative paths with std::process::Command::current_dir is documented to be broken, and is particularly broken on macOS Catalina: rust-lang/rust#37868 (comment)

I submitted #161, which more clearly works around the problem, IMO. Let me know what you think!

@ghost
Copy link
Author

ghost commented Oct 21, 2019

I like #161, I think it solves the problem in a cleaner way. Closing this one.

@ghost ghost closed this Oct 21, 2019
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The build is failing on macOS Catalina
2 participants