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

ARROW-16141: [R] Update rhub/fedora-clang-devel for upstreamed changes #12824

Merged
merged 5 commits into from
Aug 24, 2022

Conversation

paleolimbot
Copy link
Member

In ARROW-15857 (#12734) we fixed the nightly failures on rhub/fedora-clang-devel by a kludge modifying the default makefile, but also upstreamed the fixes (rstudio/sass#104 and r-hub/rhub-linux-builders#60). These upstreams are now both released, so we can remove the kludge from modification of the docker image.

@paleolimbot
Copy link
Member Author

@github-actions crossbow submit test-r-linux-as-cran

@github-actions
Copy link

github-actions bot commented Apr 7, 2022

Revision: fa02aa5

Submitted crossbow builds: ursacomputing/crossbow @ actions-1827

Task Status
test-r-linux-as-cran Github Actions

@paleolimbot paleolimbot marked this pull request as draft April 7, 2022 14:16
@paleolimbot
Copy link
Member Author

(This is failing because the latest release of the 'sass' package didn't actually contain the upstreamed fix despite the NEWS bullet indicating that it was, so we have to wait for 0.4.2 to be released: https://github.com/rstudio/sass/releases).

@paleolimbot
Copy link
Member Author

As per ARROW-16310, we also need to make sure that test-fedora-r-clang-sanitizer passes since the current Makefile modifications might be affecting the install of tzdb.

@pitrou
Copy link
Member

pitrou commented Aug 9, 2022

@paleolimbot What's the status on this? Should this PR be kept open?

@paleolimbot
Copy link
Member Author

@github-actions crossbow submit test-r-linux-as-cran

@github-actions
Copy link

github-actions bot commented Aug 9, 2022

Revision: d6cfe59

Submitted crossbow builds: ursacomputing/crossbow @ actions-32da4d031b

Task Status
test-r-linux-as-cran Github Actions

@paleolimbot paleolimbot marked this pull request as ready for review August 9, 2022 14:52
@paleolimbot
Copy link
Member Author

This looks like it works...I was waiting on a release of the 'sass' package that didn't properly pick up linker flags from the CXXFLAGS environment variable after I'd upstreamed the fix to rhub and the sass package. I don't spend a lot of time in the CI so it should probably have a look from @assignUser before merging.

@paleolimbot
Copy link
Member Author

@github-actions crossbow submit test-r-linux-as-cran

@github-actions
Copy link

Revision: 3c439de

Submitted crossbow builds: ursacomputing/crossbow @ actions-c95ef40d0f

Task Status
test-r-linux-as-cran Github Actions

@paleolimbot
Copy link
Member Author

@github-actions crossbow submit test-r-linux-as-cran

@github-actions
Copy link

Revision: 45b7556

Submitted crossbow builds: ursacomputing/crossbow @ actions-c9fd68f714

Task Status
test-r-linux-as-cran Github Actions

@assignUser
Copy link
Member

@github-actions crossbow submit test-r-linux-as-cran

@github-actions
Copy link

Revision: 45b7556

Submitted crossbow builds: ursacomputing/crossbow @ actions-7509e9f2b3

Task Status
test-r-linux-as-cran Github Actions

@assignUser
Copy link
Member

@paleolimbot I re-ran the failed job with arrow_r_dev but it worked so I started another run which also passed, maybe a change in the docker image or some other flakiness?

@paleolimbot
Copy link
Member Author

I can't personally explain the previous failure (the code chunk I added back in seems pretty well fenced to the one image). Thanks for taking a look! I need to open up the image locally to make sure that gnu extensions are not being loaded with those Makevars (I can't see why they would be with those flags but seeing as the image has one job...)

@paleolimbot
Copy link
Member Author

@github-actions crossbow submit test-r-linux-as-cran

@paleolimbot
Copy link
Member Author

I investigated this, and the rhub image doesn't need anything additional to ensure that libc++ is getting used even with the -std=gnu++XXX flag:

# docker run --rm -it rhub/fedora-clang-devel bash
printf "#include <ciso646>\n#ifdef _LIBCPP_VERSION\n#error Using libc++\n#endif" > test.cpp
/opt/R-devel/bin/R CMD SHLIB test.cpp
# /usr/bin/clang++ -std=gnu++14 -stdlib=libc++ -I"/opt/R-devel/lib64/R/include" -DNDEBUG   -I/usr/local/include    -fpic   -g -O2  -c test.cpp -o test.o
# test.cpp:3:2: error: Using libc++
# #error Using libc++
#  ^
# 1 error generated.
/usr/bin/clang++ -stdlib=libc++ -I/usr/local/include -g -O2 -std=gnu++11 -E test.cpp | head -n 1
# test.cpp:3:2: error: Using libc++
# #error Using libc++
#  ^
# 1 error generated.

@github-actions
Copy link

Revision: 1748b23

Submitted crossbow builds: ursacomputing/crossbow @ actions-d762bd3886

Task Status
test-r-linux-as-cran Github Actions

@paleolimbot paleolimbot merged commit c90d07f into apache:master Aug 24, 2022
@paleolimbot paleolimbot deleted the r-rhub-fedora-clang branch August 24, 2022 00:47
@ursabot
Copy link

ursabot commented Aug 24, 2022

Benchmark runs are scheduled for baseline = 3c13dc4 and contender = c90d07f. c90d07f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.0% ⬆️0.0%] test-mac-arm
[Failed ⬇️0.82% ⬆️0.0%] ursa-i9-9960x
[Failed ⬇️2.78% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] c90d07f7 ec2-t3-xlarge-us-east-2
[Failed] c90d07f7 test-mac-arm
[Failed] c90d07f7 ursa-i9-9960x
[Failed] c90d07f7 ursa-thinkcentre-m75q
[Finished] 3c13dc4e ec2-t3-xlarge-us-east-2
[Failed] 3c13dc4e test-mac-arm
[Failed] 3c13dc4e ursa-i9-9960x
[Failed] 3c13dc4e ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

anjakefala pushed a commit to anjakefala/arrow that referenced this pull request Aug 31, 2022
apache#12824)

In ARROW-15857 (apache#12734) we fixed the nightly failures on rhub/fedora-clang-devel by a kludge modifying the default makefile, but also upstreamed the fixes (rstudio/sass#104 and r-hub/rhub-linux-builders#60). These upstreams are now both released, so we can remove the kludge from modification of the docker image.

Lead-authored-by: Dewey Dunnington <dewey@voltrondata.com>
Co-authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
zagto pushed a commit to zagto/arrow that referenced this pull request Oct 7, 2022
apache#12824)

In ARROW-15857 (apache#12734) we fixed the nightly failures on rhub/fedora-clang-devel by a kludge modifying the default makefile, but also upstreamed the fixes (rstudio/sass#104 and r-hub/rhub-linux-builders#60). These upstreams are now both released, so we can remove the kludge from modification of the docker image.

Lead-authored-by: Dewey Dunnington <dewey@voltrondata.com>
Co-authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
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.

4 participants