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

Remove boost dependency #7932

Merged
merged 11 commits into from
May 10, 2021
Merged

Conversation

codereport
Copy link
Contributor

@codereport codereport commented Apr 10, 2021

Once we remove Boost filesystem dependency and use std::filesystem, we can remove the boost dependency.

This change requires dropping support for GCC < 9.

@codereport codereport added 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 10, 2021
@codereport codereport self-assigned this Apr 10, 2021
@codereport codereport added this to PR-WIP in v21.06 Release via automation Apr 10, 2021
@codereport codereport added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Apr 11, 2021
@codereport codereport marked this pull request as ready for review April 11, 2021 01:08
@codereport codereport requested review from a team as code owners April 11, 2021 01:08
@davidwendt
Copy link
Contributor

Boost is also installed in the docker_build Docker file

libboost-filesystem-dev \
libboost-system-dev \
libboost-regex-dev \

I'm not sure if the libboost-system-dev and libboost-regex-dev are needed for some other purpose.

@github-actions github-actions bot added conda Java Affects Java cuDF API. labels Apr 12, 2021
@codereport
Copy link
Contributor Author

rerun tests

Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If LICENSE-bundled is being deleted then the corresponding <resource> node needs to be removed in java/pom.xml

java/README.md Show resolved Hide resolved
v21.06 Release automation moved this from PR-WIP to PR-Needs review Apr 20, 2021
@codereport codereport requested review from a team as code owners April 20, 2021 16:24
@codereport codereport requested a review from jlowe April 21, 2021 00:16
java/README.md Outdated Show resolved Hide resolved
java/ci/Dockerfile.centos7 Outdated Show resolved Hide resolved
@codereport codereport requested a review from jlowe April 29, 2021 02:12
@jrhemstad
Copy link
Contributor

rerun tests

@harrism
Copy link
Member

harrism commented May 6, 2021

07:16:12 FAILED: CMakeFiles/cudf.dir/src/jit/cache.cpp.o 
07:16:13 ccache /usr/bin/g++ -DCUDF_VERSION=0.20.0 -DCUFILE_FOUND -DJITIFY_PRINT_LOG=0 -DJITIFY_USE_CACHE -DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_INFO -DTHRUST_DEVICE_SYSTEM=THRUST_DEVICE_SYSTEM_CUDA -DTHRUST_HOST_SYSTEM=THRUST_HOST_SYSTEM_CPP -Dcudf_EXPORTS -I_deps/jitify-src -I_deps/libcudacxx-src/include -I../include -Iinclude -I../src -I_deps/thrust-src -I_deps/thrust-src/dependencies/cub -isystem $PREFIX/include -isystem $BUILD_PREFIX/include -isystem /usr/local/cuda/include -isystem /usr/local/cuda/lib64 -O3 -DNDEBUG -fPIC -Wall -Werror -Wno-unknown-pragmas -Wno-error=deprecated-declarations -Wno-deprecated-declarations -pthread -std=gnu++1z -MD -MT CMakeFiles/cudf.dir/src/jit/cache.cpp.o -MF CMakeFiles/cudf.dir/src/jit/cache.cpp.o.d -o CMakeFiles/cudf.dir/src/jit/cache.cpp.o -c ../src/jit/cache.cpp
07:16:13 ../src/jit/cache.cpp:22:10: fatal error: filesystem: No such file or directory
07:16:13  #include <filesystem>
07:16:13           ^~~~~~~~~~~~
07:16:13 compilation terminated.

So it appears that gcc is not finding the filesystem header. What is weird to me is this: -std=gnu++1z instead of -std=c++17.

Copy link
Collaborator

@kkraus14 kkraus14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmake / conda lgtm

v21.06 Release automation moved this from PR-Needs review to PR-Reviewer approved May 10, 2021
@codecov
Copy link

codecov bot commented May 10, 2021

Codecov Report

Merging #7932 (db60e37) into branch-0.20 (51336df) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.20    #7932      +/-   ##
===============================================
- Coverage        82.88%   82.88%   -0.01%     
===============================================
  Files              103      104       +1     
  Lines            17668    17907     +239     
===============================================
+ Hits             14645    14843     +198     
- Misses            3023     3064      +41     
Impacted Files Coverage Δ
python/cudf/cudf/core/tools/datetimes.py 80.42% <0.00%> (-4.11%) ⬇️
python/cudf/cudf/core/column/decimal.py 90.20% <0.00%> (-2.72%) ⬇️
python/cudf/cudf/core/column/datetime.py 88.03% <0.00%> (-1.88%) ⬇️
python/cudf/cudf/core/column/struct.py 94.73% <0.00%> (-1.56%) ⬇️
python/cudf/cudf/utils/dtypes.py 82.20% <0.00%> (-1.24%) ⬇️
python/dask_cudf/dask_cudf/groupby.py 91.28% <0.00%> (-0.88%) ⬇️
python/cudf/cudf/core/series.py 91.17% <0.00%> (-0.56%) ⬇️
python/cudf/cudf/core/index.py 92.52% <0.00%> (-0.55%) ⬇️
python/cudf/cudf/core/column/column.py 88.20% <0.00%> (-0.44%) ⬇️
python/cudf/cudf/core/column/lists.py 86.98% <0.00%> (-0.43%) ⬇️
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99df69f...db60e37. Read the comment docs.

@codereport
Copy link
Contributor Author

@gpucibot merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team CMake CMake build issue improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

8 participants