Skip to content
This repository has been archived by the owner on Jun 11, 2022. It is now read-only.

Handle C++17 std::filesystem::path #164

Closed
egorpugin opened this issue May 7, 2018 · 5 comments
Closed

Handle C++17 std::filesystem::path #164

egorpugin opened this issue May 7, 2018 · 5 comments

Comments

@egorpugin
Copy link
Contributor

Gcc 8.1, VS15.7 (2017) are released with std::filesystem support.
Boost::process must learn how to handle them in all places where it could handle boost::filesystem.
bp::search_path(std::filesystem::path("b2"));

@klemens-morgenstern
Copy link
Owner

Boost.process is still a C++11 library and should remain backwards-compatible. I'd accept any PR that doesn't break said compatibility though.

@egorpugin
Copy link
Contributor Author

egorpugin commented May 8, 2018

I'm thinking about how it's possible to make both boost::fs and std::fs co-exist in the code.
I asked on the boost ML, we'll see if there are any good ideas.

@klemens-morgenstern
Copy link
Owner

That's the problem, I don't think they can. But converting std::fs to boost::fs should be a simple copy. So having a frontend might be the way to go.

Btw.: if the networking TS makes it into the standard, a standalone process library could be interesting, maybe even going for standardization.

@barcharcraz
Copy link

Looks like Boost::DLL added BOOST_DLL_USE_STD_FS I'm not sure what the compat limitations are with that though.

fanquake added a commit to bitcoin-core/gui that referenced this issue Jan 20, 2022
…ternal signing on Windows

e2ab9f8 build: disable external signer on Windows (fanquake)

Pull request description:

  This change explicitly disables support for external signing when targeting Windows and OpenBSD. The driver for this is that Boost Process uses boost::filesystem internally, when targeting Windows, which gets in the way of removing our usage of it (#20744). While we could adjust #20744 to still link against the Boost libs when building for Windows, that would be disappointing, as we wouldn't have cleanly removed the Boost usage we're trying too (including the build infrastructure), and, we'd be in a position where we would be building releases differently depending on the platform, which is something I want to avoid.

  After discussion with Sjors, Achow and Hebasto, this seemed like a reasonable step to move #20744 forward (as-is). Note that support for external signing ([while already being experimental](https://github.com/bitcoin/bitcoin/blob/master/doc/external-signer.md#example-usage)), could be considered even more experimental on Windows. Also, oddly, we have external-signing [explicitly disabled in our Windows (cross-compile) CI](https://github.com/bitcoin/bitcoin/blob/807169e10b4a18324356ed6ee4d69587b96a7c70/ci/test/00_setup_env_win64.sh#L16), it's not clear why this is the case, as, if it's a feature being built into releases, it should be being built and tested in the CI which is most-like the release process.

  There is an [issue open upstream](boostorg/process#207), in regards to migrating Boost Process to std::filesystem, or having an option to use it. However there hasn't been much discussion since it was opened ~9 months ago. There is another related issue here: klemens-morgenstern/boost-process#164.

  Resolves #24036.

ACKs for top commit:
  Sjors:
    utACK e2ab9f8
  achow101:
    ACK e2ab9f8
  kallewoof:
    utACK e2ab9f8
  hebasto:
    ACK e2ab9f8, tested on Linux Mint 20.2 (x86_64).

Tree-SHA512: 36fcfc0e1a008a8271dc76b8e12e93d3e1d1e528bf668e95a559e9f6fd7d5f031bd7a6a6bc8b9fa9d057b2cd56f9ec8838c7f74e87899bf9a6aeb787afbd112c
@klemens-morgenstern
Copy link
Owner

Done, #define BOOST_PROCESS_USE_STD_FS which will come with the next release.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants