[vcpkg] Fix build under /W4

I was building under /W3, because CMake hadn't been set up to build
under /W4 -- therefore, I didn't see some warnings.

We also decided to remove the niebloids and instead break ADL by using
`= delete`, since otherwise we get warnings when we define a local
variable with the same name as a niebloid. I also removed `status` and
`symlink_status` from the `files` header, since it's unnecessary now,
and they're just implementation details of `RealFilesystem`.

I also removed some existing uses of unqualified `status(path)`, since
that no longer compiles. I also added `Filesystem::canonical`, to remove
another use of `fs::stdfs` in a function I was already working in.
This commit is contained in:
Nicole Mazzuca 2019-08-09 12:00:43 -07:00 committed by nicole mazzuca
parent 9dfab115aa
commit 52b2e740de
7 changed files with 164 additions and 186 deletions

View File

@ -51,8 +51,14 @@ namespace fs
#else
using stdfs::file_status;
using stdfs::file_type;
// to set up ADL correctly on `file_status` objects, we are defining
// this in our own namespace
struct file_status : private stdfs::file_status {
using stdfs::file_status::file_status;
using stdfs::file_status::type;
using stdfs::file_status::permissions;
};
#endif
@ -61,91 +67,49 @@ namespace fs
the presence of symlinks -- a symlink is treated as the object it points
to for `symlink_status` and `symlink_type`
*/
#if 0
#endif
// we want to poison ADL with these niebloids
namespace detail
inline bool is_symlink(file_status s) noexcept
{
struct status_t
{
file_status operator()(const path& p, std::error_code& ec) const noexcept;
file_status operator()(vcpkg::LineInfo li, const path& p) const noexcept;
file_status operator()(const path& p) const;
};
struct symlink_status_t
{
file_status operator()(const path& p, std::error_code& ec) const noexcept;
file_status operator()(vcpkg::LineInfo li, const path& p) const noexcept;
};
struct is_symlink_t
{
bool operator()(file_status s) const
{
#if defined(_WIN32)
return s.type() == file_type::directory_symlink || s.type() == file_type::symlink;
#else
return stdfs::is_symlink(s);
if (s.type() == file_type::directory_symlink) return true;
#endif
}
};
struct is_regular_file_t
{
inline bool operator()(file_status s) const
{
#if defined(_WIN32)
return s.type() == file_type::regular;
#else
return stdfs::is_regular_file(s);
#endif
}
};
struct is_directory_t
{
inline bool operator()(file_status s) const
{
#if defined(_WIN32)
return s.type() == file_type::directory;
#else
return stdfs::is_directory(s);
#endif
}
};
struct exists_t
{
inline bool operator()(file_status s) const
{
#if defined(_WIN32)
return s.type() != file_type::not_found && s.type() != file_type::none;
#else
return stdfs::exists(s);
#endif
}
};
return s.type() == file_type::symlink;
}
inline bool is_regular_file(file_status s)
{
return s.type() == file_type::regular;
}
inline bool is_directory(file_status s)
{
return s.type() == file_type::directory;
}
inline bool exists(file_status s)
{
return s.type() != file_type::not_found && s.type() != file_type::none;
}
constexpr detail::status_t status{};
constexpr detail::symlink_status_t symlink_status{};
constexpr detail::is_symlink_t is_symlink{};
constexpr detail::is_regular_file_t is_regular_file{};
constexpr detail::is_directory_t is_directory{};
constexpr detail::exists_t exists{};
}
/*
if someone attempts to use unqualified `symlink_status` or `is_symlink`,
they might get the ADL version, which is broken.
Therefore, put `symlink_status` in the global namespace, so that they get
our symlink_status.
Therefore, put `(symlink_)?status` as deleted in the global namespace, so
that they get an error.
We also want to poison the ADL on the other functions, because
we don't want people calling these functions on paths
*/
using fs::exists;
using fs::is_directory;
using fs::is_regular_file;
using fs::is_symlink;
using fs::status;
using fs::symlink_status;
void status(const fs::path& p) = delete;
void status(const fs::path& p, std::error_code& ec) = delete;
void symlink_status(const fs::path& p) = delete;
void symlink_status(const fs::path& p, std::error_code& ec) = delete;
void is_symlink(const fs::path& p) = delete;
void is_symlink(const fs::path& p, std::error_code& ec) = delete;
void is_regular_file(const fs::path& p) = delete;
void is_regular_file(const fs::path& p, std::error_code& ec) = delete;
void is_directory(const fs::path& p) = delete;
void is_directory(const fs::path& p, std::error_code& ec) = delete;
namespace vcpkg::Files
{
@ -192,6 +156,10 @@ namespace vcpkg::Files
virtual void copy_symlink(const fs::path& oldpath, const fs::path& newpath, std::error_code& ec) = 0;
virtual fs::file_status status(const fs::path& path, std::error_code& ec) const = 0;
virtual fs::file_status symlink_status(const fs::path& path, std::error_code& ec) const = 0;
fs::file_status status(LineInfo li, const fs::path& p) const noexcept;
fs::file_status symlink_status(LineInfo li, const fs::path& p) const noexcept;
virtual fs::path canonical(const fs::path& path, std::error_code& ec) const = 0;
fs::path canonical(LineInfo li, const fs::path& path) const;
virtual std::vector<fs::path> find_from_PATH(const std::string& name) const = 0;
};

View File

@ -145,7 +145,7 @@ namespace
CHECK_EC_ON_FILE(base, ec);
}
vcpkg::Files::Filesystem& setup(urbg_t& urbg)
vcpkg::Files::Filesystem& setup()
{
auto& fs = vcpkg::Files::get_real_filesystem();
@ -161,7 +161,7 @@ TEST_CASE ("remove all", "[files]")
{
auto urbg = get_urbg(0);
auto& fs = setup(urbg);
auto& fs = setup();
fs::path temp_dir = base_temporary_directory() / get_random_filename(urbg);
INFO("temp dir is: " << temp_dir);
@ -181,7 +181,7 @@ TEST_CASE ("remove all", "[files]")
TEST_CASE ("remove all -- benchmarks", "[files][!benchmark]")
{
auto urbg = get_urbg(1);
auto& fs = setup(urbg);
auto& fs = setup();
struct
{

View File

@ -141,7 +141,7 @@ namespace vcpkg::Test
std::filesystem::path targetp = target.native();
std::filesystem::path filep = file.native();
std::filesystem::create_symlink(targetp, filep);
std::filesystem::create_symlink(targetp, filep, ec);
}
else
{
@ -153,6 +153,7 @@ namespace vcpkg::Test
ec.assign(errno, std::system_category());
}
#else
std::ignore = ec;
vcpkg::Checks::exit_with_message(VCPKG_LINE_INFO, no_filesystem_message);
#endif
}
@ -165,7 +166,7 @@ namespace vcpkg::Test
std::filesystem::path targetp = target.native();
std::filesystem::path filep = file.native();
std::filesystem::create_directory_symlink(targetp, filep);
std::filesystem::create_directory_symlink(targetp, filep, ec);
}
else
{
@ -174,6 +175,7 @@ namespace vcpkg::Test
#elif FILESYSTEM_SYMLINK == FILESYSTEM_SYMLINK_UNIX
::vcpkg::Test::create_symlink(target, file, ec);
#else
std::ignore = ec;
vcpkg::Checks::exit_with_message(VCPKG_LINE_INFO, no_filesystem_message);
#endif
}

View File

@ -21,114 +21,87 @@
#include <copyfile.h>
#endif
namespace fs::detail
{
static file_status status_implementation(bool follow_symlinks, const path& p, std::error_code& ec)
{
#if defined(_WIN32)
WIN32_FILE_ATTRIBUTE_DATA file_attributes;
file_type ft = file_type::unknown;
perms permissions = perms::unknown;
if (!GetFileAttributesExW(p.c_str(), GetFileExInfoStandard, &file_attributes))
{
const auto err = GetLastError();
if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND)
{
ft = file_type::not_found;
}
else
{
ec.assign(err, std::system_category());
}
}
else if (!follow_symlinks && file_attributes.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)
{
// this also gives junctions file_type::directory_symlink
if (file_attributes.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
{
ft = file_type::directory_symlink;
}
else
{
ft = file_type::symlink;
}
}
else if (file_attributes.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
{
ft = file_type::directory;
}
else
{
// otherwise, the file is a regular file
ft = file_type::regular;
}
if (file_attributes.dwFileAttributes & FILE_ATTRIBUTE_READONLY)
{
constexpr auto all_write = perms::group_write | perms::owner_write | perms::others_write;
permissions = perms::all & ~all_write;
}
else if (ft != file_type::none && ft != file_type::none)
{
permissions = perms::all;
}
return file_status(ft, permissions);
#else
auto result = symlink ? stdfs::symlink_status(p, ec) : stdfs::status(p, ec);
// libstdc++ doesn't correctly not-set ec on nonexistent paths
if (ec.value() == ENOENT || ec.value() == ENOTDIR)
{
ec.clear();
result = file_status(file_type::not_found, perms::unknown);
}
return result;
#endif
}
file_status status_t::operator()(const path& p, std::error_code& ec) const noexcept
{
return status_implementation(false, p, ec);
}
file_status status_t::operator()(vcpkg::LineInfo li, const path& p) const noexcept
{
std::error_code ec;
auto result = (*this)(p, ec);
if (ec) vcpkg::Checks::exit_with_message(li, "error getting status of path %s: %s", p.string(), ec.message());
return result;
}
file_status status_t::operator()(const path& p) const
{
#if defined(_WIN32)
return (*this)(VCPKG_LINE_INFO, p);
#else
return fs::stdfs::status(p);
#endif
}
file_status symlink_status_t::operator()(const path& p, std::error_code& ec) const noexcept
{
return status_implementation(true, p, ec);
}
file_status symlink_status_t::operator()(vcpkg::LineInfo li, const path& p) const noexcept
{
std::error_code ec;
auto result = (*this)(p, ec);
if (ec) vcpkg::Checks::exit_with_message(li, "error getting status of path %s: %s", p.string(), ec.message());
return result;
}
}
namespace vcpkg::Files
{
static const std::regex FILESYSTEM_INVALID_CHARACTERS_REGEX = std::regex(R"([\/:*?"<>|])");
namespace
{
fs::file_status status_implementation(bool follow_symlinks, const fs::path& p, std::error_code& ec) noexcept
{
using fs::file_type;
using fs::perms;
#if defined(_WIN32)
WIN32_FILE_ATTRIBUTE_DATA file_attributes;
auto ft = file_type::unknown;
auto permissions = perms::unknown;
if (!GetFileAttributesExW(p.c_str(), GetFileExInfoStandard, &file_attributes))
{
const auto err = GetLastError();
if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND)
{
ft = file_type::not_found;
}
else
{
ec.assign(err, std::system_category());
}
}
else if (!follow_symlinks && file_attributes.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)
{
// this also gives junctions file_type::directory_symlink
if (file_attributes.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
{
ft = file_type::directory_symlink;
}
else
{
ft = file_type::symlink;
}
}
else if (file_attributes.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
{
ft = file_type::directory;
}
else
{
// otherwise, the file is a regular file
ft = file_type::regular;
}
if (file_attributes.dwFileAttributes & FILE_ATTRIBUTE_READONLY)
{
constexpr auto all_write = perms::group_write | perms::owner_write | perms::others_write;
permissions = perms::all & ~all_write;
}
else if (ft != file_type::none && ft != file_type::none)
{
permissions = perms::all;
}
return fs::file_status(ft, permissions);
#else
auto result = symlink ? stdfs::symlink_status(p, ec) : stdfs::status(p, ec);
// libstdc++ doesn't correctly not-set ec on nonexistent paths
if (ec.value() == ENOENT || ec.value() == ENOTDIR)
{
ec.clear();
result = fs::file_status(file_type::not_found, perms::unknown);
}
return result;
#endif
}
fs::file_status status(const fs::path& p, std::error_code& ec) noexcept
{
return status_implementation(false, p, ec);
}
fs::file_status symlink_status(const fs::path& p, std::error_code& ec) noexcept
{
return status_implementation(false, p, ec);
}
// does _not_ follow symlinks
void set_writeable(const fs::path& path, std::error_code& ec) noexcept
{
@ -220,6 +193,24 @@ namespace vcpkg::Files
return exists(path, ec);
}
fs::file_status Filesystem::status(vcpkg::LineInfo li, const fs::path& p) const noexcept
{
std::error_code ec;
auto result = this->status(p, ec);
if (ec) vcpkg::Checks::exit_with_message(li, "error getting status of path %s: %s", p.string(), ec.message());
return result;
}
fs::file_status Filesystem::symlink_status(vcpkg::LineInfo li, const fs::path& p) const noexcept
{
std::error_code ec;
auto result = this->symlink_status(p, ec);
if (ec) vcpkg::Checks::exit_with_message(li, "error getting status of path %s: %s", p.string(), ec.message());
return result;
}
void Filesystem::write_lines(const fs::path& path, const std::vector<std::string>& lines, LineInfo linfo)
{
std::error_code ec;
@ -244,6 +235,16 @@ namespace vcpkg::Files
}
}
fs::path Filesystem::canonical(LineInfo li, const fs::path& path) const
{
std::error_code ec;
const auto result = this->canonical(path, ec);
if (ec) Checks::exit_with_message(li, "Error getting canonicalization of %s: %s", path.string(), ec.message());
return result;
}
struct RealFilesystem final : Filesystem
{
virtual Expected<std::string> read_contents(const fs::path& file_path) const override
@ -463,7 +464,7 @@ namespace vcpkg::Files
static void do_remove(const fs::path& current_path, ErrorInfo& err)
{
std::error_code ec;
const auto path_status = fs::symlink_status(current_path, ec);
const auto path_status = Files::symlink_status(current_path, ec);
if (check_ec(ec, current_path, err)) return;
if (!fs::exists(path_status)) return;
@ -593,11 +594,11 @@ namespace vcpkg::Files
virtual fs::file_status status(const fs::path& path, std::error_code& ec) const override
{
return fs::status(path, ec);
return Files::status(path, ec);
}
virtual fs::file_status symlink_status(const fs::path& path, std::error_code& ec) const override
{
return fs::symlink_status(path, ec);
return Files::symlink_status(path, ec);
}
virtual void write_contents(const fs::path& file_path, const std::string& data, std::error_code& ec) override
{
@ -628,6 +629,11 @@ namespace vcpkg::Files
}
}
virtual fs::path canonical(const fs::path& path, std::error_code& ec) const override
{
return fs::stdfs::canonical(path, ec);
}
virtual std::vector<fs::path> find_from_PATH(const std::string& name) const override
{
#if defined(_WIN32)

View File

@ -623,7 +623,7 @@ namespace vcpkg::Build
std::vector<fs::path> port_files;
for (auto& port_file : fs::stdfs::recursive_directory_iterator(config.port_dir))
{
if (fs::is_regular_file(status(port_file)))
if (fs::is_regular_file(fs.status(VCPKG_LINE_INFO, port_file)))
{
port_files.push_back(port_file);
if (port_files.size() > max_port_file_count)

View File

@ -312,6 +312,7 @@ namespace vcpkg::Dependencies
const std::vector<std::string>* ports_dirs_paths)
: filesystem(paths.get_filesystem())
{
auto& fs = Files::get_real_filesystem();
if (ports_dirs_paths)
{
for (auto&& overlay_path : *ports_dirs_paths)
@ -326,7 +327,7 @@ namespace vcpkg::Dependencies
overlay.string());
Checks::check_exit(VCPKG_LINE_INFO,
fs::is_directory(status(overlay)),
fs::is_directory(fs.status(VCPKG_LINE_INFO, overlay)),
"Error: Path \"%s\" must be a directory",
overlay.string());

View File

@ -18,8 +18,9 @@ namespace vcpkg
const std::string& default_vs_path,
const std::vector<std::string>* triplets_dirs)
{
auto& fs = Files::get_real_filesystem();
std::error_code ec;
const fs::path canonical_vcpkg_root_dir = fs::stdfs::canonical(vcpkg_root_dir, ec);
const fs::path canonical_vcpkg_root_dir = fs.canonical(vcpkg_root_dir, ec);
if (ec)
{
return ec;
@ -42,7 +43,7 @@ namespace vcpkg
if (auto odp = overriddenDownloadsPath.get())
{
auto asPath = fs::u8path(*odp);
if (!fs::is_directory(status(asPath)))
if (!fs::is_directory(fs.status(VCPKG_LINE_INFO, asPath)))
{
Metrics::g_metrics.lock()->track_property("error", "Invalid VCPKG_DOWNLOADS override directory.");
Checks::exit_with_message(