Move all references to stdfs:: into files.cpp #141
Conversation
BillyONeal
force-pushed
the
depath
branch
4 times, most recently
from
f9e14ca
to
7ceea10
Compare
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
# Conflicts: # include/vcpkg/base/downloads.h # src/vcpkg/base/downloads.cpp
# Conflicts: # src/vcpkg/base/files.cpp
| Ty unaligned_load(const void* pv) noexcept | ||
| { | ||
| static_assert(std::is_trivial<Ty>::value, "Unaligned loads require trivial types"); | ||
| Ty tmp; | ||
| memcpy(&tmp, pv, sizeof(tmp)); | ||
| return tmp; | ||
| } | ||
|
|
||
| bool is_drive_prefix(const char* const first) noexcept | ||
| { | ||
| // test if first points to a prefix of the form X: | ||
| // pre: first points to at least 2 char instances | ||
| // pre: Little endian | ||
| auto value = unaligned_load<unsigned short>(first); | ||
| value &= 0xFFDFu; // transform lowercase drive letters into uppercase ones | ||
| value -= (static_cast<unsigned short>(':') << CHAR_BIT) | 'A'; | ||
| return value < 26; | ||
| } | ||
|
|
||
| bool has_drive_letter_prefix(const char* const first, const char* const last) noexcept | ||
| { | ||
| // test if [first, last) has a prefix of the form X: | ||
| return last - first >= 2 && is_drive_prefix(first); | ||
| } |
There was a problem hiding this comment.
this seems... extremely bit-bashy and overcomplex for vcpkg. Can we do something like:
return ((first[0] & 0xDF) - 'A' < 26) && (first[1] == ':')the assembler is slightly different, but not like... that much different.
There was a problem hiding this comment.
Given that this is commented as a copy-pasta from microsoft/STL I would prefer to leave it as close to that in form as possible?
src/vcpkg/base/files.cpp
Outdated
| #if defined(__clang__) | ||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wunused-const-variable" |
There was a problem hiding this comment.
We should be using clang_diagnostic macros here; see vcpkg/base/pragmas.h
| #if defined(__clang__) | |
| #pragma clang diagnostic push | |
| #pragma clang diagnostic ignored "-Wunused-const-variable" | |
| VCPKG_CLANG_DIAGNOSTIC(push) | |
| VCPKG_CLANG_DIAGNOSTIC(ignored -Wunused-const-variable) |
There was a problem hiding this comment.
There is no VCPKG_CLANG_DIAGNOSTIC although I can add one
src/vcpkg/base/files.cpp
Outdated
| #endif // __clang__ | ||
| constexpr char preferred_separator = VCPKG_PREFERED_SEPARATOR[0]; | ||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop |
Co-authored-by: nicole mazzuca <83086508+strega-nil-ms@users.noreply.github.com>
|
I think this change resulted in some include problem on our builds for conda-forge: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=363464&view=logs&jobId=58ac6aab-c4bc-5de2-2894-98e408cc8ec9&j=58ac6aab-c4bc-5de2-2894-98e408cc8ec9&t=933f325c-924e-533d-4d95-e93b5843ce8b Would you have a second to check that? |
The Windows build failures there are probably expected; now that we ship binary releases we only worry about building vcpkg itself with current (e.g. 2019) on Windows (which also means we don't have to workaround std::experimental::filesystem's many Many MANY bugs). The MacOS one is more concerning; I'm currently working on making binary releases possible for older versions of that too by ripping out the std::filesystem dependency, but I didn't expect intermediate broken there. Is that attempting to use "macOS-10.14" Pipelines machines and that's the trigger or is something else going on? |
|
Great, I can upgrade the conda-forge config to use VS2019 if that's fixing it. On conda-forge we're using the 10.9 SDK for macOS, but a recent clang version. |
|
Missing include is entirely possible |
|
Yes, this looks exactly like a missing include -- we aren't including |
|
Could you try #158 and confirm if it solves the problem for MacOS? |
|
hey, sorry, I was off on the weekend :) I applied the patch from #158 and that works on macOS! On Windows, I still have issues after switching to VS2019, although those seem to be in optional: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=364375&view=logs&jobId=a70f640f-cc53-5cd3-6cdc-236a1aa90802&j=a70f640f-cc53-5cd3-6cdc-236a1aa90802&t=f5d15007-a01c-5ad8-c9ce-4d519d3b275f |
|
I was just making sure you were notified/ tagged since you weren't @'d before not being impatient :) |
|
Thanks, no worries at all :) |
That log still says 2017
|
|
you're right. sorry. I just pushed another set of modifications that should properly activate VS 2019. Fingers crossed! |
|
No problem, I broke you after all :) |
|
all green now! thx |
This change makes us treat std::filesystem as if it were a system API, and making paths follow our "internals are UTF-8" conventions. This fixes numerous path encoding bugs where we forgot to say `vcpkg::u8path", and makes it easier to add a completely POSIX backend so that we can run on old Linuxes.
Adds a new type,
vcpkg::Path, which is an analogue ofstd::filesystem::path, except which always is UTF-8 internally. This means we can't represent everything on the file system, but we don't consider creating file system objects with invalid unicode names reasonable behavior.