-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
ARROW-14839: [R] test-fedora-r-clang-sanitizer job failing due to snappy causing a sanitizer error #11875
Conversation
@github-actions crossbow submit test-fedora-r-clang-sanitizer |
1 similar comment
|
Revision: 4c426a5 Submitted crossbow builds: ursacomputing/crossbow @ actions-1251
|
@github-actions crossbow submit test-fedora-r-clang-sanitizer |
Revision: b141103 Submitted crossbow builds: ursacomputing/crossbow @ actions-1259
|
@github-actions crossbow submit test-fedora-r-clang-sanitizer |
@github-actions autotune |
Revision: e1599d3 Submitted crossbow builds: ursacomputing/crossbow @ actions-1260
|
@github-actions crossbow submit test-fedora-r-clang-sanitizer |
Revision: a9e77fd Submitted crossbow builds: ursacomputing/crossbow @ actions-1266
|
apt-get update | ||
apt-get install -y patch | ||
fi | ||
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.
This is not strictly necessary (see below where we disable snappy if we can't find patch
), but it makes sure that we do have patch
available so we do exercise this code in the sanitizer in our CI
if(patch) | ||
set(SNAPPY_PATCH_COMMAND "patch" "snappy.cc" | ||
"${CMAKE_SOURCE_DIR}/build-support/snappy-UBSAN.patch") | ||
endif() |
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.
Snappy works without the patch, though it has a UBSAN error, so this patches if we can and moves on if not so we don't break.
@@ -81,6 +81,7 @@ ARROW_RAPIDJSON_BUILD_VERSION=1a803826f1197b5e30703afe4b9c0e7dd48074f5 | |||
ARROW_RAPIDJSON_BUILD_SHA256_CHECKSUM=0b6b780b6c534bfb0b23d29910bfe361e486bcfeaf106db8bc8995792072905a | |||
ARROW_RE2_BUILD_VERSION=2021-02-02 | |||
ARROW_RE2_BUILD_SHA256_CHECKSUM=1396ab50c06c1a8885fb68bf49a5ecfd989163015fd96699a180d6414937f33f | |||
# 1.1.9 is patched to implement https://github.com/google/snappy/pull/148 if this is bumped, remove the patch |
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've tried to flag this, and if we pursue this path, I'll make a Jira that points to each of these lines that need to be deleted when > 1.1.9 is released
# patch available, so disable snappy in those cases. If the snappy version is bumped, we should remove this. | ||
if [ ! $(command -v patch) ]; then | ||
ARROW_WITH_SNAPPY=OFF | ||
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.
For our CI this should not be hit, but is important incase CRAN happens to not have patch
, it will disable snappy so we don't get a sanitizer error there.
@github-actions crossbow submit -g r |
Revision: a9e77fd Submitted crossbow builds: ursacomputing/crossbow @ actions-1269 |
The conda failures are known / on master. The test-r-depsource-system failure looks transient (and at the very least unrelated!) |
Benchmark runs are scheduled for baseline = cba23c4 and contender = c2a89e6. c2a89e6 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
This would replace (temporarily) #11796 in a way that might be ship-able as is until Snappy accepts google/snappy#148 and is released.