search

gulrak / filesystem

An implementation of C++17 std::filesystem for C++11 /C++14/C++17/C++20 on Windows, macOS, Linux and FreeBSD.
MIT License
1.34k stars 173 forks source link

Dynamic selection for more Apple platforms #167

Closed cpsauer closed 1 year ago

cpsauer commented 1 year ago

Hello Steffen!

Thanks so much for a wonderful library!

I'm moving my team's codebase over to your library from boost::filesystem--as a backport of std::filesystem on old Apple OSs. I love that yours is so much more compatible with std::filesystem and will try to get libCPR to move next. Along the way, I added conditionals for the other Apple OSs and fixed some things I saw, trying to propose ways to leave the code even better than I found it.

I'm hoping you might be willing to upstream the changes. The fastest review is probably to go commit-by-commit, since I tried to explain what I was up to in each commit message and keep changes independent, while batching them into one PR for fast review. But it might also be pretty fast to just skim the whole thing; there's nothing too crazy here.

Thanks again for all you do! Tschüß, Christopher

P.S. I've given you edit access, so feel free to just directly change anything you'd like differently!

gulrak commented 1 year ago

Hi Christopher, Thanks for the kind words, I'll look through them over the weekend, but first look is good. Thanks for contributing! Steffen/Gulrak

cpsauer commented 1 year ago

(Any idea what's up with the (delayed) failures? Look like instant-failures, with no log, and zero elapsed time.)

cpsauer commented 1 year ago

(Oh, some sort of CI issue? Looks like it's the same at HEAD.)

gulrak commented 1 year ago

Yeah, afaik the support for 18.04 in the workflows was dropped and only 20.04 and 22.04 are currently supported, so I need to update the build configuration.

cpsauer commented 1 year ago

Makes sense! Just wanted to make sure there wasn't something I needed to fix that'd be blocking you from merging.

gulrak commented 1 year ago

Okay, looks good to me, thank you again for the PR! Best wishes, Steffen/Gulrak

cpsauer commented 1 year ago

Thanks so much for vetting! I really appreciate your being great about it.

cpsauer commented 1 year ago

I'll next spin up some Bazel build rules to make using this great library of yours maximally easy for Bazel users. (We'll need to do it anyway internally and I bet it'll help others. And since Bazel has easy remote download and compile caching it makes sense to do a little wrapping.)

Would you be down to take a PR adding a one-liner to the README, linking Bazel users over to the setup code that'll make setup easy for them?