-
Notifications
You must be signed in to change notification settings - Fork 241
feat: remove vendored aws-lambda-cpp dependency #633
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
Changes from 11 commits
fe0331b
ddb2bdb
aa1a6f5
6cdbbdc
5f222d2
c3e1f95
f86bf64
27c207f
3f50af2
21b6c09
b9f6e34
d19c1b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,5 @@ | ||
| compile-flags.txt | ||
| ric-dev-environment/codeartifact-properties.mk | ||
|
|
||
| # aws-lambda-cpp prebuilt lib + headers, fetched and staged at build time | ||
| src/main/jni/deps/aws-lambda-cpp/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,6 @@ ARG CURL_VERSION | |
| RUN apk update && \ | ||
| apk add \ | ||
| openjdk11 \ | ||
| cmake \ | ||
| file \ | ||
| g++ \ | ||
| gcc \ | ||
|
|
@@ -31,17 +30,8 @@ RUN ./configure \ | |
| make && \ | ||
| make install | ||
|
|
||
| # Install aws-lambda-cpp dependency | ||
| ADD ./deps/aws-lambda-cpp-* /src/deps/aws-lambda-cpp | ||
| RUN mkdir -p /src/deps/aws-lambda-cpp/build | ||
| WORKDIR /src/deps/aws-lambda-cpp/build | ||
| RUN cmake .. \ | ||
| -DCMAKE_CXX_FLAGS="-fPIC -DBACKWARD_SYSTEM_UNKNOWN" \ | ||
| -DCMAKE_CXX_STANDARD=11 \ | ||
| -DCMAKE_INSTALL_PREFIX=$(pwd)/../../artifacts\ | ||
| -DCMAKE_MODULE_PATH=$(pwd)/../../artifacts/lib/pkgconfig && \ | ||
| make && \ | ||
| make install | ||
| COPY ./deps/aws-lambda-cpp/include /src/deps/artifacts/include | ||
| COPY ./deps/aws-lambda-cpp/lib/libaws-lambda-runtime.a /src/deps/artifacts/lib/ | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious how the alpine leg works with a glibc-built .a, since alpine doesn't have glibc (musl) — before this we compiled from source inside the container, so is musl's glibc symbol compat enough here or should upstream publish a musl variant too?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The prebuilt .a is just an archive of relocatable object files, so what matters isn't the libc it was built on but which external symbols its objects actually reference and none of them are glibc-specific. The final .so in the musl leg still links against the musl-built static curl and musl libc inside the Alpine container, with libstdc++/libgcc statically linked, so nothing glibc leaks in on that side either. Code used This is also why the tests are passing |
||
| # Build native client | ||
| ADD *.cpp *.h /src/ | ||
|
|
||
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
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 add a join job for this matrix like smoke-test has (
needs+if: always())? Branch protection against individual matrix job names gets painful. Also this runs all 11 combos on every PR with noneedsor path filter — intentional?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.
done, will add the branch protection once it reaches main not to block other PRs