Skip to content

No longer builds on OS X #52

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

Closed
webbnh opened this issue Jul 5, 2019 · 5 comments
Closed

No longer builds on OS X #52

webbnh opened this issue Jul 5, 2019 · 5 comments

Comments

@webbnh
Copy link

webbnh commented Jul 5, 2019

I do development for a variety of targets (mostly Docker-based). I do my actual deployment builds on Ubuntu, but it's been really handy to be able to do test builds in my local environment which is Mac OS X.

I just started setting up a new development environment, and I pulled fresh aws-lambda-cpp sources, and, I'm sad to report, they no longer build on OS X. The last time I pulled the code for aws-lambda-cpp was in March, and they built fine then. That was prior to the v0.2.0 changes for stack trace support.

I realize that that OS X is not quite Linux (i.e., it's Darwin, which I believe is a BSD derivative), and that aws-lambda-cpp is targeted at Linux. But, it would be really nice if I could at least build locally.

Looking at the aws-lambda-cpp source, I see what look like provisions for Darwin in include/backward.h, but they don't seem to be being used. Here's what I get when I try to build the current aws-lambda-cpp commit:

[ 20%] Building CXX object CMakeFiles/aws-lambda-runtime.dir/src/backward.cpp.o
In file included from /Users/webb/URSA/aws-lambda-cpp/src/backward.cpp:26:
/Users/webb/URSA/aws-lambda-cpp/include/backward.h:188:22: fatal error: 
      'link.h' file not found
#            include <link.h>
                     ^~~~~~~~
1 error generated.
make[2]: *** [CMakeFiles/aws-lambda-runtime.dir/src/backward.cpp.o] Error 1
make[1]: *** [CMakeFiles/aws-lambda-runtime.dir/all] Error 2
make: *** [all] Error 2

I presume that this is because the compiler is being forced down the Linux path by line 46.

Commenting that line out and allowing the file to auto-detect the platform allows my build on OS X to get far enough to hit a complaint about converting size_t to an int in the invocation of backtrace_symbols() in TraceResolverDarwinImpl<>::load_stacktrace().

If I add the same typecast there which was added to TraceResolverLinuxImpl<>::load_stacktrace(), then the build completes.

Was there a reason for forcing the compilation down the Linux path? (E.g., does the auto-detection code not work reliably?) If not, would you be willing to accept a change to allow the code to build on OS X? (The two changes are trivial -- I could submit a pull request if you don't want to do them by hand.)

@webbnh
Copy link
Author

webbnh commented Jul 5, 2019

In case it's helpful, here's the patch:

diff --git a/include/backward.h b/include/backward.h
index b8ca68e..393b364 100644
--- a/include/backward.h
+++ b/include/backward.h
@@ -43,7 +43,7 @@
 
 // You can define one of the following (or leave it to the auto-detection):
 //
-#    define BACKWARD_SYSTEM_LINUX
+// #    define BACKWARD_SYSTEM_LINUX
 //     - specialization for linux
 //
 // #define BACKWARD_SYSTEM_DARWIN
@@ -3057,7 +3057,7 @@ public:
         if (st.size() == 0) {
             return;
         }
-        _symbols.reset(backtrace_symbols(st.begin(), st.size()));
+        _symbols.reset(backtrace_symbols(st.begin(), (int)st.size()));
     }
 
     ResolvedTrace resolve(ResolvedTrace trace)

@webbnh
Copy link
Author

webbnh commented Jul 5, 2019

Actually, to get master to build, one more tiny change is required:

diff --git a/src/runtime.cpp b/src/runtime.cpp
index 0af68b2..08a93b1 100644
--- a/src/runtime.cpp
+++ b/src/runtime.cpp
@@ -316,7 +316,7 @@ runtime::next_outcome runtime::get_next()
         req.deadline += std::chrono::milliseconds(ms);
         logging::log_info(
             LOG_TAG,
-            "Received payload: %s\nTime remaining: %ld",
+            "Received payload: %s\nTime remaining: %lld",
             req.payload.c_str(),
             req.get_time_remaining().count());
     }

This one is in common code; I don't know whether it works on Linux.

@marcomagdy
Copy link
Contributor

Was there a reason for forcing the compilation down the Linux path?

Yes. Lambda only runs on Linux. So why bother with maintaining the code on macOS?
You can think of it as a fail-fast mechanism to prevent people from building and packaging a Lambda on macOS only to then see it fail when deployed.

see #34 for a similar question/answer.

@marcomagdy
Copy link
Contributor

If anything, I should add a better compiler error when building on macOS.
Something like:

#ifdef __APPLE__
#error "You should not be using macOS for building Lambda. Only Linux binaries will work on AWS Lambda."
#endif

@webbnh
Copy link
Author

webbnh commented Jul 30, 2019

I should add a better compiler error when building on macOS.

Yes, that would be good; the code as it stands looks like it should support Mac OS when in fact your intention is quite the opposite, and that is confusing, at best.

Thanks for the response! (And, thanks for supporting C++-based Lambda functions!)

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

No branches or pull requests

2 participants