Update to use std::filesystem rather than Boost #238

mjcarroll · 2021-08-10T19:32:05Z

🎉 Remove FilesystemBoost.cc in favor of std::filesystem

Summary

Remove the vendored FilesystemBoost.cc file in favor of using std::filesystem.

Test it

Functionality should remain the same across the stack, and all local unit/integration tests should pass on all platforms.

Checklist

Signed all commits for DCO
Added tests
Added example and/or tutorial
Updated documentation (as needed)
Updated migration guide (as needed)
codecheck passed (See contributing)
All tests passed (See test coverage)
While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

ahcorde

I made some minor comments and I noticed that it's still a draft.

Can you also test ign-fuel-tools ? just to check that we are not breaking things on Windows

include/ignition/common/TempDirectory.hh

src/DirIter.cc

mjcarroll · 2021-09-02T20:46:39Z

ignition_common-ci-pr_any-homebrew-amd64 is expected to fail because std::filesystem isn't supported on 10.14

codecov · 2021-09-02T20:47:53Z

Codecov Report

Merging #238 (e404a82) into main (9b790e6) will increase coverage by 0.10%.
The diff coverage is 95.94%.

@@            Coverage Diff             @@
##             main     #238      +/-   ##
==========================================
+ Coverage   76.43%   76.53%   +0.10%     
==========================================
  Files          75       75              
  Lines       10463    10327     -136     
==========================================
- Hits         7997     7904      -93     
+ Misses       2466     2423      -43

Impacted Files	Coverage Δ
include/ignition/common/Filesystem.hh	`100.00% <ø> (ø)`
src/Util.cc	`84.23% <66.66%> (+0.23%)`	⬆️
src/Filesystem.cc	`96.09% <97.29%> (+22.20%)`	⬆️
src/DirIter.cc	`100.00% <100.00%> (ø)`
src/SystemPaths.cc	`85.96% <100.00%> (-0.13%)`	⬇️
src/TempDirectory.cc	`76.56% <100.00%> (-1.70%)`	⬇️

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 9b790e6...e404a82. Read the comment docs.

ahcorde

Conflicts

ahcorde

Mac and Windows are failing

mjcarroll · 2021-09-13T16:18:11Z

Mac and Windows are failing

Windows is failing because the ign-utils dependency introduced in #244.

It is expected that the Jenkins macOS build will fail, as it is using 10.14. The new GitHub actions-based macOS build is the one we should be referring to from this PR forward.

mjcarroll · 2021-09-13T19:25:16Z

@osrf-jenkins retest this please

chapulina · 2021-09-13T22:27:36Z

Homebrew is not happy yet:

/Users/jenkins/workspace/ignition_common-ci-pr_any-homebrew-amd64/ign-common/src/DirIter.cc:31:15: error: 'directory_iterator' is unavailable: introduced in macOS 10.15
  public: fs::directory_iterator it;

mjcarroll · 2021-09-14T17:04:44Z

Homebrew is not happy yet:

Homebrew is expected to fail starting with this PR. std::filesystem is not supported on macOS 10.14. This is why I added the second actions-based CI against macOS 10.15

chapulina · 2021-09-15T01:57:28Z

Homebrew is expected to fail starting with this PR.

So should we disable that job, or update it to use 10.15?

scpeters · 2022-01-20T10:05:22Z

down to three really suspicious failures here:

Suspicious indeed. They all seem to segfault even before the test starts, so maybe it's something within GTest? Could the CXX_FILESYSTEM_LIBRARIES have affected it? I can't tell what these tests have in common that's specific to them. They all use TEST and don't have a main, but other tests do too and they're passing.

weird that the tests only fail in jenkins, not in Github actions

those failures occurred on the r2d2 machine, which I have taken offline for testing. I see the following when I try to load one of the failing tests in gdb on r2d2:

Reading symbols from bin/UNIT_Pbr_TEST...
(gdb) r
Starting program: /var/lib/jenkins/workspace/ignition_common-ci-pr_any-ubuntu_auto-amd64/build/bin/UNIT_Pbr_TEST 
warning: Error disabling address space randomization: Operation not permitted
warning: Could not trace the inferior process.
warning: ptrace: Operation not permitted
During startup program exited with code 127.

scpeters · 2022-01-27T20:06:02Z

I downloaded the Dockerfile and build.sh script from the jenkins job and with a few modifications was able to get it running on my local machine. I collected the following backtrace from UNIT_Pbr_TEST:

Reading symbols from bin/UNIT_Pbr_TEST...
(gdb) r
Starting program: /home/jenkins/workspace/ignition_common-ci-pr_any-ubuntu_auto-amd64/build/bin/UNIT_Pbr_TEST 
warning: Error disabling address space randomization: Operation not permitted
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
std::filesystem::__cxx11::path::~path (this=0x0, __in_chrg=<optimized out>)
   at /usr/include/c++/8/bits/fs_path.h:209
209      ~path() = default;
(gdb) thread apply all bt

Thread 1 (Thread 0x7fb0423e1c40 (LWP 4110)):
#0 std::filesystem::__cxx11::path::~path (this=0x0, __in_chrg=<optimized out>) at /usr/include/c++/8/bits/fs_path.h:209
#1 0x00007fb0438d6434 in std::filesystem::__cxx11::path::_Cmpt::~_Cmpt (this=0x0, __in_chrg=<optimized out>) at /usr/include/c++/8/bits/fs_path.h:644
#2 std::_Destroy<std::filesystem::__cxx11::path::_Cmpt> (__pointer=0x0) at /usr/include/c++/8/bits/stl_construct.h:98
#3 std::_Destroy_aux<false>::__destroy<std::filesystem::__cxx11::path::_Cmpt*> (__last=<optimized out>, __first=0x0) at /usr/include/c++/8/bits/stl_construct.h:108
#4 std::_Destroy<std::filesystem::__cxx11::path::_Cmpt*> (__last=<optimized out>, __first=<optimized out>) at /usr/include/c++/8/bits/stl_construct.h:137
#5 std::_Destroy<std::filesystem::__cxx11::path::_Cmpt*, std::filesystem::__cxx11::path::_Cmpt> (__last=0x2, __first=<optimized out>) at /usr/include/c++/8/bits/stl_construct.h:206
#6 std::vector<std::filesystem::__cxx11::path::_Cmpt, std::allocator<std::filesystem::__cxx11::path::_Cmpt> >::~vector (this=0x55e9e44c34b0, __in_chrg=<optimized out>) at /usr/include/c++/8/bits/stl_vector.h:567
#7 std::filesystem::__cxx11::path::~path (this=0x55e9e44c3490, __in_chrg=<optimized out>) at /usr/include/c++/8/bits/fs_path.h:209
#8 std::filesystem::__cxx11::path::_Cmpt::~_Cmpt (this=0x55e9e44c3490, __in_chrg=<optimized out>) at /usr/include/c++/8/bits/fs_path.h:644
#9 std::_Destroy<std::filesystem::__cxx11::path::_Cmpt> (__pointer=0x55e9e44c3490) at /usr/include/c++/8/bits/stl_construct.h:98
#10 std::_Destroy_aux<false>::__destroy<std::filesystem::__cxx11::path::_Cmpt*> (__last=<optimized out>, __first=0x55e9e44c3490) at /usr/include/c++/8/bits/stl_construct.h:108
#11 std::_Destroy<std::filesystem::__cxx11::path::_Cmpt*> (__last=<optimized out>, __first=<optimized out>) at /usr/include/c++/8/bits/stl_construct.h:137
#12 std::_Destroy<std::filesystem::__cxx11::path::_Cmpt*, std::filesystem::__cxx11::path::_Cmpt> (__last=0x0, __first=0x55e9e44c3490) at /usr/include/c++/8/bits/stl_construct.h:206
--Type <RET> for more, q to quit, c to continue without paging--c
#13 std::vector<std::filesystem::__cxx11::path::_Cmpt, std::allocator<std::filesystem::__cxx11::path::_Cmpt> >::~vector (this=<synthetic pointer>, __in_chrg=<optimized out>) at /usr/include/c++/8/bits/stl_vector.h:567
#14 std::vector<std::filesystem::__cxx11::path::_Cmpt, std::allocator<std::filesystem::__cxx11::path::_Cmpt> >::_M_move_assign (__x=..., this=0x7ffe85f32920) at /usr/include/c++/8/bits/stl_vector.h:1679
#15 std::vector<std::filesystem::__cxx11::path::_Cmpt, std::allocator<std::filesystem::__cxx11::path::_Cmpt> >::operator= (__x=..., this=0x7ffe85f32920) at /usr/include/c++/8/bits/stl_vector.h:601
#16 std::filesystem::__cxx11::path::operator= (__p=..., this=0x7ffe85f32900) at /usr/include/c++/8/bits/fs_path.h:797
#17 std::filesystem::__cxx11::path::operator= (__p=..., this=0x7ffe85f32900) at /usr/include/c++/8/bits/fs_path.h:791
#18 ignition::common::joinPaths (_path1="/home/jenkins", _path2=".ignition") at /home/jenkins/workspace/ignition_common-ci-pr_any-ubuntu_auto-amd64/ign-common/src/Filesystem.cc:143
#19 0x00007fb0438e3cf9 in ignition::common::SystemPaths::SystemPaths (this=0x55e9e44c3460) at /usr/include/c++/8/bits/basic_string.h:252
#20 0x00007fb0438cccc5 in __static_initialization_and_destruction_0 (__initialize_p=1, __priority=65535) at /home/jenkins/workspace/ignition_common-ci-pr_any-ubuntu_auto-amd64/ign-common/src/Util.cc:564
#21 _GLOBAL__sub_I_Util.cc(void) () at /home/jenkins/workspace/ignition_common-ci-pr_any-ubuntu_auto-amd64/ign-common/src/Util.cc:564
#22 0x00007fb043e2eb8a in ?? () from /lib64/ld-linux-x86-64.so.2
#23 0x00007fb043e2ec91 in ?? () from /lib64/ld-linux-x86-64.so.2
#24 0x00007fb043e1e13a in ?? () from /lib64/ld-linux-x86-64.so.2
#25 0x0000000000000001 in ?? ()
#26 0x00007ffe85f34d24 in ?? ()
#27 0x0000000000000000 in ?? ()
(gdb)

Signed-off-by: Michael Carroll <[email protected]>

src/Util.cc

Signed-off-by: Michael Carroll <[email protected]>

mjcarroll · 2022-01-28T21:17:10Z

Waiting on osrf/homebrew-simulation#1796

scpeters · 2022-01-28T22:17:51Z

Waiting on osrf/homebrew-simulation#1796

I just merged it and restarted the homebrew build

scpeters

a few debug messages can be removed

src/SystemPaths.cc

test/performance/logging.cc

Signed-off-by: Michael Carroll <[email protected]>

Signed-off-by: Steve Peters <[email protected]>

scpeters

two comments about Filesystem.cc

src/Filesystem.cc

Signed-off-by: Michael Carroll <[email protected]>

github-actions bot added 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress labels Aug 10, 2021

ahcorde requested changes Aug 10, 2021

View reviewed changes

include/ignition/common/TempDirectory.hh Outdated Show resolved Hide resolved

include/ignition/common/TempDirectory.hh Outdated Show resolved Hide resolved

src/DirIter.cc Show resolved Hide resolved

mjcarroll changed the base branch from ign-common4 to mjcarroll/temp_directory August 31, 2021 18:27

mjcarroll changed the title ~~Filesystem updates~~ Update to use std::filesystem rather than Boost Aug 31, 2021

mjcarroll force-pushed the remove_boost branch 2 times, most recently from e666988 to ec7048b Compare August 31, 2021 18:30

mjcarroll marked this pull request as ready for review August 31, 2021 18:30

mjcarroll force-pushed the remove_boost branch 2 times, most recently from be50dc4 to 93b2650 Compare September 2, 2021 20:39

mjcarroll mentioned this pull request Sep 2, 2021

Add functions and objects for Temporary Directories #244

Merged

8 tasks

chapulina added the beta Targeting beta release of upcoming collection label Sep 8, 2021

ahcorde requested changes Sep 13, 2021

View reviewed changes

mjcarroll force-pushed the remove_boost branch from 50bd4ad to dae56b8 Compare September 13, 2021 15:45

mjcarroll requested a review from ahcorde September 13, 2021 15:46

ahcorde previously requested changes Sep 13, 2021

View reviewed changes

mjcarroll requested a review from ahcorde September 13, 2021 19:36

Base automatically changed from mjcarroll/temp_directory to ign-common4 September 13, 2021 22:40

mjcarroll force-pushed the remove_boost branch from dae56b8 to 9957c49 Compare September 14, 2021 18:15

Merge branch 'main' into remove_boost

7acb8e0

Prevent initialization/destruction order fiasco

020abd3

Signed-off-by: Michael Carroll <[email protected]>

mjcarroll force-pushed the remove_boost branch from 1d9ffc7 to 020abd3 Compare January 28, 2022 20:07

Make macOS happy

583b421

Signed-off-by: Michael Carroll <[email protected]>

azeey reviewed Jan 28, 2022

View reviewed changes

src/Util.cc Outdated Show resolved Hide resolved

mjcarroll mentioned this pull request Jan 28, 2022

Prepare for 1.2.0 release gazebosim/gz-utils#35

Merged

7 tasks

Cleaner static SystemPaths implementation

c5bea16

Signed-off-by: Michael Carroll <[email protected]>

scpeters reviewed Feb 1, 2022

View reviewed changes

src/SystemPaths.cc Outdated Show resolved Hide resolved

src/SystemPaths.cc Outdated Show resolved Hide resolved

src/SystemPaths.cc Outdated Show resolved Hide resolved

src/SystemPaths.cc Outdated Show resolved Hide resolved

scpeters mentioned this pull request Feb 1, 2022

Merge 4 -> main #302

Merged

scpeters reviewed Feb 1, 2022

View reviewed changes

test/performance/logging.cc Outdated Show resolved Hide resolved

mjcarroll and others added 2 commits February 1, 2022 12:55

Remove extra debug output

dbdb0e9

Signed-off-by: Michael Carroll <[email protected]>

Remove one more debug message

6ab5b60

Signed-off-by: Steve Peters <[email protected]>

scpeters reviewed Feb 2, 2022

View reviewed changes

src/Filesystem.cc Show resolved Hide resolved

src/Filesystem.cc Show resolved Hide resolved

mjcarroll added 3 commits February 2, 2022 15:35

Merge branch 'main' into remove_boost

4f01f9e

Signed-off-by: Michael Carroll <[email protected]>

Clarify discarding return on create_directories

66f716c

Signed-off-by: Michael Carroll <[email protected]>

Address reviewer feedback

e404a82

Signed-off-by: Michael Carroll <[email protected]>

scpeters approved these changes Feb 2, 2022

View reviewed changes

mjcarroll merged commit fdcb378 into main Feb 4, 2022

mjcarroll deleted the remove_boost branch February 4, 2022 13:06

jennuine mentioned this pull request Feb 9, 2022

Fixed parentPath() return when input has no parent #307

Merged

7 tasks

j-rivero mentioned this pull request Feb 11, 2022

Mix of packages built with gcc-8 and gcc-9 lead to crashes when using std::filesystem gazebo-tooling/release-tools#639

Closed

chapulina mentioned this pull request May 2, 2022

Use common filesystem gazebosim/sdformat#929

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to use std::filesystem rather than Boost #238

Update to use std::filesystem rather than Boost #238

mjcarroll commented Aug 10, 2021 •

edited

Loading

ahcorde left a comment

mjcarroll commented Sep 2, 2021

codecov bot commented Sep 2, 2021 •

edited

Loading

ahcorde left a comment

ahcorde left a comment

mjcarroll commented Sep 13, 2021

mjcarroll commented Sep 13, 2021

chapulina commented Sep 13, 2021

mjcarroll commented Sep 14, 2021

chapulina commented Sep 15, 2021 •

edited

Loading

scpeters commented Jan 20, 2022

scpeters commented Jan 27, 2022

mjcarroll commented Jan 28, 2022

scpeters commented Jan 28, 2022

scpeters left a comment

scpeters left a comment

Update to use std::filesystem rather than Boost #238

Update to use std::filesystem rather than Boost #238

Conversation

mjcarroll commented Aug 10, 2021 • edited Loading

🎉 Remove FilesystemBoost.cc in favor of std::filesystem

Summary

Test it

Checklist

ahcorde left a comment

Choose a reason for hiding this comment

mjcarroll commented Sep 2, 2021

codecov bot commented Sep 2, 2021 • edited Loading

Codecov Report

ahcorde left a comment

Choose a reason for hiding this comment

ahcorde left a comment

Choose a reason for hiding this comment

mjcarroll commented Sep 13, 2021

mjcarroll commented Sep 13, 2021

chapulina commented Sep 13, 2021

mjcarroll commented Sep 14, 2021

chapulina commented Sep 15, 2021 • edited Loading

scpeters commented Jan 20, 2022

scpeters commented Jan 27, 2022

mjcarroll commented Jan 28, 2022

scpeters commented Jan 28, 2022

scpeters left a comment

Choose a reason for hiding this comment

scpeters left a comment

Choose a reason for hiding this comment

mjcarroll commented Aug 10, 2021 •

edited

Loading

codecov bot commented Sep 2, 2021 •

edited

Loading

chapulina commented Sep 15, 2021 •

edited

Loading