Fix errors when linking fdbserver on macOS due to boost::filesystem #11566

sepeth · 2024-08-09T10:44:07Z

Hi there,

This was one of the issues when compiling on macOS. See here: https://forums.foundationdb.org/t/building-on-macos/4491/3

This replaces boost::filesystem with std::filesystem.std::filesystem::recursive_directory_iterator is supported since C++17 and std::string::ends_with is since C++20. FoundationDB is configured to C++20.

Code-Reviewer Section

The general pull request guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

The PR has a description, explaining both the problem and the solution.
The description mentions which forms of testing were done and the testing seems reasonable.
Every function/class/actor that was touched is reasonably well documented.

For Release-Branches

If this PR is made against a release-branch, please also check the following:

This change/bugfix is a cherry-pick from the next younger branch (younger release-branch or main if this is the youngest branch)
There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)

giorgiozoppi · 2024-08-09T14:52:38Z

fdbserver/tester.actor.cpp

@@ -2566,7 +2561,7 @@ void encryptionAtRestPlaintextMarkerCheck() {
 	bool success = true;
 	// Enumerate all files in the "simfdb/" folder and look for "marker" string
 	for (fs::recursive_directory_iterator itr(p); itr != end; ++itr) {
-		if (boost::filesystem::is_regular_file(itr->path())) {
+		if (fs::is_regular_file(itr->path())) {
 			std::ifstream f(itr->path().string().c_str());
 			if (f) {


naming is bad. can we more explicit. Also do we really to c_str for creating a file. Can we explicit in check if the file is open or not. A better name instead of f is currentFileStream

Yeah, probably string() is also unnecessary for ifstream constructor.

About naming, do you mean the f here? Although I agree, the scope is also not too big. TBH, I am new in FDB code base, so I try to keep the changes minimal to the functionality, instead of refactoring in bigger chunks. That's why I didn't go beyond the issue much. It is also easier for the reviewer I think to keep the scope to one thing.

spraza · 2024-08-09T17:08:54Z

@xis19 Since you were discussing about using std::filesystem, it'd be good if you can have a look.

foundationdb-ci · 2024-08-09T20:51:35Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: d72ecd4
Duration 0:06:22
Result: ❌ FAILED
Error: Error while executing command: ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ${HOME}/.ssh_key ec2-user@${MAC_EC2_HOST} /opt/homebrew/bin/bash --login -c ./build_pr_macos.sh. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-08-09T20:56:22Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: d72ecd4
Duration 0:11:08
Result: ❌ FAILED
Error: Error while executing command: ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ${HOME}/.ssh_key ec2-user@${MAC_EC2_HOST} /usr/local/bin/bash --login -c ./build_pr_macos.sh. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-08-09T21:11:20Z

Result of foundationdb-pr-clang-ide on Linux CentOS 7

Commit ID: d72ecd4
Duration 0:26:03
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-08-09T21:11:58Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: d72ecd4
Duration 0:26:38
Result: ❌ FAILED
Error: Error while executing command: ninja -v -C build_output -j ${NPROC} all packages strip_targets. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2024-08-09T21:14:55Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: d72ecd4
Duration 0:29:37
Result: ❌ FAILED
Error: Error while executing command: ninja -v -C build_output -j ${NPROC} all packages strip_targets. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-08-09T21:37:02Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: d72ecd4
Duration 0:51:46
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-08-09T22:00:51Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: d72ecd4
Duration 1:15:36
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

ghost · 2024-08-15T14:29:13Z

Seems to be related to this #11564

Currently trying to use std::filesystem over what we have currently. This is still a long-effort since there's a lot to refactor through. Leaving this here as a note

sepeth · 2024-08-15T17:25:18Z

Seems to be related to this #11564

Currently trying to use std::filesystem over what we have currently. This is still a long-effort since there's a lot to refactor through. Leaving this here as a note

Awesomeness ^-^ I remember seeing your change, I wanted to do this separately though, as it is tiny in scope, and solves something painful for me on macOS. I had to keep stashing/commenting this bit to be able to compile on macOS.

sepeth · 2024-08-22T14:02:50Z

Hmm, looks like foundationdb-pr check fails with:

fdbserver/tester.actor.cpp: In function 'void encryptionAtRestPlaintextMarkerCheck()':
fdbserver/tester.actor.cpp:2558:6: error: 'path' is not a member of 'fs'
fdbserver/tester.actor.cpp:2559:6: error: 'recursive_directory_iterator' is not a member of 'fs'
fdbserver/tester.actor.cpp:2563:11: error: 'recursive_directory_iterator' is not a member of 'fs'
fdbserver/tester.actor.cpp:2563:48: error: 'itr' was not declared in this scope
fdbserver/tester.actor.cpp:2563:55: error: 'end' was not declared in this scope

Is it ok to use something like the following here?

#ifdef __APPLE__ // or clang
using fs = std::filesystem;
#else
using fs = boost::filesystem;
#end

xis19 · 2024-08-22T17:41:45Z

From https://libcxx.llvm.org/Status/Cxx17.html#note-p0156 it seems filesystem is fully supported in clang libc++ since version 7.0. Can you post full compiler error? Really hate to compile FDB using a Mac laptop, takes forever

sepeth · 2024-08-22T18:55:07Z

From https://libcxx.llvm.org/Status/Cxx17.html#note-p0156 it seems filesystem is fully supported in clang libc++ since version 7.0. Can you post full compiler error? Really hate to compile FDB using a Mac laptop, takes forever

Oh, the last error I shared above is from GCC on Linux from the checks above. See this foundationdb-pr check log: #11566 (comment)

Clang supports std::filesystem, but linking boost::filesystem, which the current code uses, fails on macOS for some reason. I mentioned this in the forum a while ago here: https://forums.foundationdb.org/t/building-on-macos/4491/3

For that reason, I thought I can switch this code to use std::filesystem, but now GCC is not happy.

But this is still curious, as it should be supported by GCC 11 / libstdc++ as well if I am reading this correctly: https://en.cppreference.com/w/cpp/compiler_support/17 and this: https://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html#status.iso.2017

sepeth · 2024-08-22T18:58:19Z

Oh 🙈 I think the problem is that I forgot to #include <filesystem>.

sepeth · 2024-08-22T19:54:01Z

fdbmonitor/fdbmonitor.h

@@ -31,6 +31,7 @@
 #include <string>
 #include <sstream>
 #include <random>
+#include <unordered_map>


This seems to be forgotten as well. See it is used later in the file:

foundationdb/fdbmonitor/fdbmonitor.h

Line 86 in bf59b54

std::unordered_map<int, std::unordered_set<std::string>> set_watches(std::string path, int ifd);

And fails in another PR check:

In file included from /Users/ec2-user/foundationdb/fdbmonitor/fdbmonitor.cpp:32: /Users/ec2-user/foundationdb/fdbmonitor/fdbmonitor.h:86:6: error: no template named 'unordered_map' in namespace 'std'; did you mean 'unordered_set'? std::unordered_map<int, std::unordered_set<std::string>> set_watches(std::string path, int ifd); ~~~~~^~~~~~~~~~~~~ unordered_set /Library/Developer/CommandLineTools/SDKs/MacOSX13.1.sdk/usr/include/c++/v1/unordered_set:411:28: note: 'unordered_set' declared here class _LIBCPP_TEMPLATE_VIS unordered_set ^ /Users/ec2-user/foundationdb/fdbmonitor/fdbmonitor.cpp:66:13: error: no template named 'unordered_map' in namespace 'std'; did you mean 'unordered_set'? extern std::unordered_map<ProcessID, std::unique_ptr<Command>> id_command; ~~~~~^~~~~~~~~~~~~ unordered_set /Library/Developer/CommandLineTools/SDKs/MacOSX13.1.sdk/usr/include/c++/v1/unordered_set:411:28: note: 'unordered_set' declared here class _LIBCPP_TEMPLATE_VIS unordered_set ^ /Users/ec2-user/foundationdb/fdbmonitor/fdbmonitor.cpp:67:13: error: no template named 'unordered_map' in namespace 'std'; did you mean 'unordered_set'? extern std::unordered_map<pid_t, ProcessID> pid_id; ~~~~~^~~~~~~~~~~~~ unordered_set /Library/Developer/CommandLineTools/SDKs/MacOSX13.1.sdk/usr/include/c++/v1/unordered_set:411:28: note: 'unordered_set' declared here class _LIBCPP_TEMPLATE_VIS unordered_set ^ /Users/ec2-user/foundationdb/fdbmonitor/fdbmonitor.cpp:68:13: error: no template named 'unordered_map' in namespace 'std'; did you mean 'unordered_set'? extern std::unordered_map<ProcessID, pid_t> id_pid; ~~~~~^~~~~~~~~~~~~

#11574 (comment)

sepeth · 2024-08-23T19:16:12Z

May you please kick off PR checks again? I believe they will pass this time ^-^

This replaces boost::filesystem with std::filesystem. `std::filesystem::recursive_directory_iterator` is supported since C++17 and `std::string::ends_with` is since C++20. FoundationDB is configured to C++20. Without this change, linking fails with: ``` [1183/1697] Linking CXX executable bin/fdbserver FAILED: bin/fdbserver ... Undefined symbols for architecture arm64: "boost::filesystem::detail::dir_itr_imp::~dir_itr_imp()", referenced from: encryptionAtRestPlaintextMarkerCheck() in tester.actor.g.cpp.o boost::filesystem::recursive_directory_iterator::~recursive_directory_iterator() in tester.actor.g.cpp.o boost::filesystem::recursive_directory_iterator::recursive_directory_iterator(boost::filesystem::path const&) (.cold.1) in tester.actor.g.cpp.o "boost::filesystem::detail::dir_itr_imp::operator delete(void*)", referenced from: encryptionAtRestPlaintextMarkerCheck() in tester.actor.g.cpp.o boost::filesystem::recursive_directory_iterator::~recursive_directory_iterator() in tester.actor.g.cpp.o boost::filesystem::recursive_directory_iterator::recursive_directory_iterator(boost::filesystem::path const&) (.cold.1) in tester.actor.g.cpp.o "boost::filesystem::detail::recursive_directory_iterator_construct(boost::filesystem::recursive_directory_iterator&, boost::filesystem::path const&, boost::filesystem::directory_options, boost::system::error_code*)", referenced from: boost::filesystem::recursive_directory_iterator::recursive_directory_iterator(boost::filesystem::path const&) in tester.actor.g.cpp.o ld: symbol(s) not found for architecture arm64 clang: error: linker command failed with exit code 1 (use -v to see invocation) ``` This also fixes a forgotten include (unordered_map) in fdbmonitor.h.

sepeth · 2024-09-17T16:18:05Z

Hi @spraza and @xis19, ICYMI ^-^

May you please kick off PR checks again? I believe they will pass this time ^-^

spraza · 2024-09-17T18:11:40Z

Hi @spraza and @xis19, ICYMI ^-^

May you please kick off PR checks again? I believe they will pass this time ^-^

@sepeth LGTM. Triggered CI. Once it passes, will accept and merge the PR.

foundationdb-ci · 2024-09-17T18:33:44Z

Result of foundationdb-pr-clang-ide on Linux CentOS 7

Commit ID: b614ba7
Duration 0:22:21
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-09-17T18:54:03Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: b614ba7
Duration 0:42:42
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-09-17T19:01:25Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: b614ba7
Duration 0:50:03
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-09-17T19:04:07Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: b614ba7
Duration 0:52:48
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-09-17T19:04:37Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: b614ba7
Duration 0:53:16
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-09-17T19:05:54Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: b614ba7
Duration 0:54:35
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2024-09-17T19:10:50Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: b614ba7
Duration 0:59:32
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

giorgiozoppi reviewed Aug 9, 2024

View reviewed changes

spraza requested a review from xis19 August 9, 2024 17:07

jzhou77 closed this Aug 9, 2024

jzhou77 reopened this Aug 9, 2024

sepeth mentioned this pull request Aug 20, 2024

Fix warnings for long long or int64_t format specifiers by switching to fmt::print* #11574

Merged

5 tasks

sepeth force-pushed the Fix_linking_errors_in_fdbserver/tester.actor.cpp branch from d72ecd4 to a552282 Compare August 22, 2024 19:48

sepeth commented Aug 22, 2024

View reviewed changes

sepeth force-pushed the Fix_linking_errors_in_fdbserver/tester.actor.cpp branch from a552282 to b614ba7 Compare September 17, 2024 16:16

spraza closed this Sep 17, 2024

spraza reopened this Sep 17, 2024

spraza approved these changes Sep 17, 2024

View reviewed changes

spraza merged commit bf429a3 into apple:main Sep 17, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix errors when linking fdbserver on macOS due to boost::filesystem #11566

Fix errors when linking fdbserver on macOS due to boost::filesystem #11566

sepeth commented Aug 9, 2024

giorgiozoppi Aug 9, 2024 •

edited

Loading

sepeth Aug 9, 2024

spraza commented Aug 9, 2024

foundationdb-ci commented Aug 9, 2024

foundationdb-ci commented Aug 9, 2024

foundationdb-ci commented Aug 9, 2024

foundationdb-ci commented Aug 9, 2024

foundationdb-ci commented Aug 9, 2024

foundationdb-ci commented Aug 9, 2024

foundationdb-ci commented Aug 9, 2024

ghost commented Aug 15, 2024

sepeth commented Aug 15, 2024

sepeth commented Aug 22, 2024 •

edited

Loading

xis19 commented Aug 22, 2024

sepeth commented Aug 22, 2024

sepeth commented Aug 22, 2024 •

edited

Loading

sepeth Aug 22, 2024

sepeth commented Aug 23, 2024

sepeth commented Sep 17, 2024

spraza commented Sep 17, 2024

foundationdb-ci commented Sep 17, 2024

foundationdb-ci commented Sep 17, 2024

foundationdb-ci commented Sep 17, 2024

foundationdb-ci commented Sep 17, 2024

foundationdb-ci commented Sep 17, 2024

foundationdb-ci commented Sep 17, 2024

foundationdb-ci commented Sep 17, 2024

Fix errors when linking fdbserver on macOS due to boost::filesystem #11566

Fix errors when linking fdbserver on macOS due to boost::filesystem #11566

Conversation

sepeth commented Aug 9, 2024

Code-Reviewer Section

For Release-Branches

giorgiozoppi Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

sepeth Aug 9, 2024

Choose a reason for hiding this comment

spraza commented Aug 9, 2024

foundationdb-ci commented Aug 9, 2024

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

foundationdb-ci commented Aug 9, 2024

Result of foundationdb-pr-macos on macOS Ventura 13.x

foundationdb-ci commented Aug 9, 2024

Result of foundationdb-pr-clang-ide on Linux CentOS 7

foundationdb-ci commented Aug 9, 2024

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

foundationdb-ci commented Aug 9, 2024

Result of foundationdb-pr on Linux CentOS 7

foundationdb-ci commented Aug 9, 2024

Result of foundationdb-pr-clang-arm on Linux CentOS 7

foundationdb-ci commented Aug 9, 2024

Result of foundationdb-pr-clang on Linux CentOS 7

ghost commented Aug 15, 2024

sepeth commented Aug 15, 2024

sepeth commented Aug 22, 2024 • edited Loading

xis19 commented Aug 22, 2024

sepeth commented Aug 22, 2024

sepeth commented Aug 22, 2024 • edited Loading

sepeth Aug 22, 2024

Choose a reason for hiding this comment

sepeth commented Aug 23, 2024

sepeth commented Sep 17, 2024

spraza commented Sep 17, 2024

foundationdb-ci commented Sep 17, 2024

Result of foundationdb-pr-clang-ide on Linux CentOS 7

foundationdb-ci commented Sep 17, 2024

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

foundationdb-ci commented Sep 17, 2024

Result of foundationdb-pr-macos on macOS Ventura 13.x

foundationdb-ci commented Sep 17, 2024

Result of foundationdb-pr-clang-arm on Linux CentOS 7

foundationdb-ci commented Sep 17, 2024

Result of foundationdb-pr-clang on Linux CentOS 7

foundationdb-ci commented Sep 17, 2024

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

foundationdb-ci commented Sep 17, 2024

Result of foundationdb-pr on Linux CentOS 7

giorgiozoppi Aug 9, 2024 •

edited

Loading

sepeth commented Aug 22, 2024 •

edited

Loading

sepeth commented Aug 22, 2024 •

edited

Loading