-
Notifications
You must be signed in to change notification settings - Fork 8
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 SONAME for artifact produced in Gitlab #628
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #628 +/- ##
==========================================
+ Coverage 73.17% 73.19% +0.01%
==========================================
Files 252 252
Lines 35942 35942
==========================================
+ Hits 26300 26307 +7
+ Misses 9642 9635 -7
|
4e7f7d1
to
d0f7740
Compare
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.
👍 LGTM Thanks for getting to the bottom of this whole soname thing :)
build-profiling-ffi.sh
Outdated
if [[ "$target" == "aarch64-unknown-linux-gnu" ]]; then | ||
mkdir patchelf | ||
cd patchelf | ||
curl -OL https://github.com/NixOS/patchelf/releases/download/0.18.0/patchelf-0.18.0-aarch64.tar.gz | ||
tar zxvf patchelf-0.18.0-aarch64.tar.gz | ||
patchelf_bin=`pwd`"/bin/patchelf" | ||
cd .. | ||
fi |
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.
Minor: Maybe add here a echo "Downloading patchelf to workaround bug in centos patchelf"
in case someone gets surprised by this?
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.
I updated the Docker image instead so we only have the call to patchelf
d0f7740
to
b7140d8
Compare
BenchmarksComparisonBenchmark execution time: 2024-09-13 11:01:49 Comparing candidate commit b7140d8 in PR branch Found 4 performance improvements and 0 performance regressions! Performance is the same for 47 metrics, 2 unstable metrics. scenario:credit_card/is_card_number/37828224631000521389798
scenario:credit_card/is_card_number_no_luhn/37828224631000521389798
CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
BaselineOmitted due to size. |
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.
👍 🎉
What does this PR do?
Fix
SONAME
for artifacts produced in Gitlab.Motivation
Since in GitLab we do not use the new builder, we have to update the
build-profiling.sh
shell script in order to port this change.Additional Notes
🤷
How to test the change?
Download the artifact from Gitlab and run
readelf -d libdatadog_profiling.so