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
[C++17] Check whether the output directory exists. #12080
Conversation
@@ -144,6 +144,7 @@ namespace LA | |||
#endif | |||
|
|||
#include <cmath> | |||
#include <filesystem> |
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.
That'll be some fun - For libstdc++
we will have to figure out a sane way to decide whether we have to link against libstdc++fs
as well or not.
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.
Shouldn't -std=c++17
imply that?
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.
@bangerth gcc version 9 onwards should just require -std=c++17
, gcc version 8 and older require manual linking with libstdc++fs
. At least if I recall correctly 😸
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 remember that I had similar issues with gcc8. I solved this by substitution with boost
libraries:
#ifdef __cpp_lib_filesystem
# include <filesystem>
namespace filesystem
{
using namespace std::filesystem;
}
#else
# include <boost/filesystem.hpp>
namespace filesystem
{
using namespace boost::filesystem;
}
#endif
I'm not sure if the __cpp_lib_filesystem
flag only exists for gcc
.
We could move this code snippet into the std_cxx17
namespace. What do you think?
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.
The problem is that we don't include the boost filesystem parts in the bundled/
directory :-( There is also the issue that this is a tutorial and I really didn't want to introduce some complicated workaround in tutorials -- I'd rather just leave things as they are if we can't do it right....
b5f6ffd
to
834a87f
Compare
This is now open for merge :-) |
834a87f
to
59a32fb
Compare
The minimal version we test and thus meaningfully support now is gcc-9 so this can go in as is. |
tester-ng still uses gcc-7.5, though, see https://cdash.dealii.org/viewSite.php?siteid=3&project=1¤ttime=1688518800. |
Fixes #10281 . We can only apply this once we support C++17, but I had this ready so I'll post for whenever the time comes.