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

Adding option allowing to force using Boost.FileSystem #4081

Merged
merged 1 commit into from Sep 11, 2019

Conversation

@hkaiser
Copy link
Member

hkaiser commented Sep 7, 2019

  • this is useful in situations where other parts of an application rely on Boost.FileSystem
- this is useful in situations where other parts of an application rely on Boost.FileSystem
@hkaiser hkaiser force-pushed the module_filesystem branch from 1440504 to 473a16e Sep 7, 2019
@msimberg

This comment has been minimized.

Copy link
Contributor

msimberg commented Sep 9, 2019

@hkaiser would you mind explaining the motivation behind this? Shouldn't a user set up their Boost filesystem dependency themselves if they depend on it directly? Or do we actually expose std/boost filesystem somewhere in our ("public") API.

@hkaiser

This comment has been minimized.

Copy link
Member Author

hkaiser commented Sep 9, 2019

@msimberg We do not expose the filesystem in our documented API, AFAIK. However, sneaky as I am, I'm using HPX internals in other projects (like Phylanx) and there we have seen compatibility problems.

@msimberg

This comment has been minimized.

Copy link
Contributor

msimberg commented Sep 11, 2019

All right... Do you have an example of the problems you've had? Is it something you could eventually resolve in Phylanx itself?

@hkaiser

This comment has been minimized.

Copy link
Member Author

hkaiser commented Sep 11, 2019

All right... Do you have an example of the problems you've had? Is it something you could eventually resolve in Phylanx itself?

I'll see what I can do.

@hkaiser

This comment has been minimized.

Copy link
Member Author

hkaiser commented Sep 11, 2019

@msimberg: the issue we were running into (now I do remember) was that if HPX (silently) found the std::filesystem (while compiling in C++17, which is the default nowadays) then this forced our dependent libraries into using C++17 as well, but in a non-obvious way (mostly again because of the defaults we have there, which in that case was using C++14). In the end the users were suddenly facing compilation problems where there were no problems before. This is while we do not use the filesystem types in our APIs we still (indirectly) #include those files from our exposed headers.

Having a way to explicitly control which filesystem library to use would be helpful.

@msimberg

This comment has been minimized.

Copy link
Contributor

msimberg commented Sep 11, 2019

Fair point! Explicit is better. Thanks!

HPX modularization automation moved this from TODO to In Progress Sep 11, 2019
Copy link
Contributor

msimberg left a comment

Thanks!

@hkaiser hkaiser merged commit 1575769 into master Sep 11, 2019
15 of 19 checks passed
15 of 19 checks passed
pycicle daint-gcc-cuda Test errors 2
Details
pycicle daint-gcc-newest Test errors 1
Details
pycicle daint-gcc-oldest Build errors 50
Details
pycicle daint-gcc-oldest Test errors 761
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
build-and-test Workflow: build-and-test
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
pycicle daint-clang-newest Build errors 0
Details
pycicle daint-clang-newest Config errors 0
Details
pycicle daint-clang-newest Test errors 0
Details
pycicle daint-clang-oldest Build errors 0
Details
pycicle daint-clang-oldest Config errors 0
Details
pycicle daint-clang-oldest Test errors 0
Details
pycicle daint-gcc-cuda Build errors 0
Details
pycicle daint-gcc-cuda Config errors 0
Details
pycicle daint-gcc-newest Build errors 0
Details
pycicle daint-gcc-newest Config errors 0
Details
pycicle daint-gcc-oldest Config errors 0
Details
HPX modularization automation moved this from In Progress to Done Sep 11, 2019
@hkaiser hkaiser deleted the module_filesystem branch Sep 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
2 participants
You can’t perform that action at this time.