Skip to content

Commit

Permalink
[filesystem] Make remove_all remove directories that aren't writable …
Browse files Browse the repository at this point in the history
…on POSIX. (#227)

* [filesystem] Make remove_all remove directories that aren't writable on POSIX.

Observe this before and after:

```
@BillyONeal ➜ /workspaces/vcpkg (download-script) $ ./vcpkg install crossguid
Computing installation plan...
The following packages will be built and installed:
    crossguid[core]:x64-linux -> 0.2.2-2018-06-16#2
  * libuuid[core]:x64-linux -> 1.0.3#8
Additional packages (*) will be modified to complete this operation.
Detecting compiler hash for triplet x64-linux...
Restored 2 packages from /home/codespace/.cache/vcpkg/archives in 5.092 ms. Use --debug to see more details.
Starting package 1/2: libuuid:x64-linux
Installing package libuuid[core]:x64-linux...
Elapsed time for package libuuid:x64-linux: 1.517 ms
Starting package 2/2: crossguid:x64-linux
Installing package crossguid[core]:x64-linux...
Elapsed time for package crossguid:x64-linux: 1.12 ms

Total elapsed time: 352.8 ms

The package crossguid provides CMake targets:

    find_package(crossguid CONFIG REQUIRED)
    target_link_libraries(main PRIVATE crossguid)

@BillyONeal ➜ /workspaces/vcpkg (download-script) $ ./vcpkg remove crossguid
The following packages will be removed:
    crossguid:x64-linux
Removing package crossguid:x64-linux...
Failure to remove_all("/workspaces/vcpkg/packages/crossguid_x64-linux") due to file "/workspaces/vcpkg/packages/crossguid_x64-linux/include/crossguid/guid.hpp": Permission denied
@BillyONeal ➜ /workspaces/vcpkg (download-script) $ ../vcpkg-tool/build/vcpkg remove crossguid
The following packages are not installed, so not removed:
    crossguid:x64-linux
Package crossguid:x64-linux is not installed
@BillyONeal ➜ /workspaces/vcpkg (download-script) $ ../vcpkg-tool/build/vcpkg x-ci-clean
Starting vcpkg CI clean
Clearing contents of /workspaces/vcpkg/buildtrees
Clearing contents of /workspaces/vcpkg/installed
Clearing contents of /workspaces/vcpkg/packages
Completed vcpkg CI clean
@BillyONeal ➜ /workspaces/vcpkg (download-script) $ ./vcpkg install crossguid
Computing installation plan...
The following packages will be built and installed:
    crossguid[core]:x64-linux -> 0.2.2-2018-06-16#2
  * libuuid[core]:x64-linux -> 1.0.3#8
Additional packages (*) will be modified to complete this operation.
Detecting compiler hash for triplet x64-linux...
Restored 2 packages from /home/codespace/.cache/vcpkg/archives in 5.771 ms. Use --debug to see more details.
Starting package 1/2: libuuid:x64-linux
Installing package libuuid[core]:x64-linux...
Elapsed time for package libuuid:x64-linux: 1.752 ms
Starting package 2/2: crossguid:x64-linux
Installing package crossguid[core]:x64-linux...
Elapsed time for package crossguid:x64-linux: 1.334 ms

Total elapsed time: 347.1 ms

The package crossguid provides CMake targets:

    find_package(crossguid CONFIG REQUIRED)
    target_link_libraries(main PRIVATE crossguid)

@BillyONeal ➜ /workspaces/vcpkg (download-script) $ ../vcpkg-tool/build/vcpkg remove crossguid
The following packages will be removed:
    crossguid:x64-linux
Removing package crossguid:x64-linux...
@BillyONeal ➜ /workspaces/vcpkg (download-script) $
```

* add `::`

* you need execute bit

* clang-format

Co-authored-by: nicole mazzuca <83086508+strega-nil-ms@users.noreply.github.com>
Co-authored-by: nicole mazzuca <mazzucan@outlook.com>
  • Loading branch information
3 people authored Oct 21, 2021
1 parent 8db809c commit 470ef33
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 9 deletions.
12 changes: 12 additions & 0 deletions src/vcpkg-test/files.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,18 @@ namespace
{
create_directory_tree(urbg, fs, base / get_random_filename(urbg), remaining_depth - 1);
}

#if !defined(_WIN32)
if (urbg() & 1u)
{
const auto chmod_result = ::chmod(base.c_str(), 0444);
if (chmod_result != 0)
{
const auto failure_message = std::generic_category().message(errno);
FAIL("chmod failed with " << failure_message);
}
}
#endif // ^^^ !_WIN32
}

REQUIRE(exists(fs.symlink_status(base, ec)));
Expand Down
56 changes: 47 additions & 9 deletions src/vcpkg/base/files.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,10 @@ namespace
ec.assign(errno, std::generic_category());
}

void vcpkg_remove_all_directory(const Path& base, std::error_code& ec, Path& failure_point);
void vcpkg_remove_all_directory(const Path& base,
std::error_code& ec,
Path& failure_point,
struct stat& base_lstat);

void vcpkg_remove_all(const Path& base,
std::error_code& ec,
Expand All @@ -749,13 +752,29 @@ namespace

if (S_ISDIR(s.st_mode))
{
vcpkg_remove_all_directory(base, ec, failure_point);
vcpkg_remove_all_directory(base, ec, failure_point, s);
return;
}
}
else if (base_dtype == PosixDType::Directory)
{
vcpkg_remove_all_directory(base, ec, failure_point);
// establish `base` being writable before calling vcpkg_remove_all_directory
if (::lstat(base.c_str(), &s) != 0)
{
// no ENOENT check here since we were supposed to be visiting a directory
mark_recursive_error(base, ec, failure_point);
return;
}

if (!S_ISDIR(s.st_mode))
{
// if it isn't still a directory something is racy
ec = std::make_error_code(std::errc::device_or_resource_busy);
mark_recursive_error(base, ec, failure_point);
return;
}

vcpkg_remove_all_directory(base, ec, failure_point, s);
return;
}

Expand All @@ -765,9 +784,21 @@ namespace
}
}

void vcpkg_remove_all_directory(const Path& base, std::error_code& ec, Path& failure_point)
void vcpkg_remove_all_directory(const Path& base, std::error_code& ec, Path& failure_point, struct stat& base_lstat)
{
// it was a directory, so delete everything inside
// ensure that the directory is writable and executable
// NOTE: the execute bit on directories is needed to allow opening files inside of that directory
if ((base_lstat.st_mode & (S_IWUSR | S_IXUSR)) != (S_IWUSR | S_IXUSR))
{
if (::chmod(base.c_str(), base_lstat.st_mode | S_IWUSR | S_IXUSR) != 0)
{
ec.assign(errno, std::generic_category());
mark_recursive_error(base, ec, failure_point);
return;
}
}

// delete everything inside
ReadDirOp op{base, ec};
if (ec)
{
Expand All @@ -787,7 +818,7 @@ namespace
return;
}

// no more entries left, fall down to remove below
// no more entries left, fall down to rmdir below
break;
}

Expand Down Expand Up @@ -2315,16 +2346,23 @@ namespace vcpkg

return result;
#else // ^^^ _WIN32 // !_WIN32 vvv
if (::remove(target.c_str()) == 0 || errno == ENOENT)
if (::remove(target.c_str()) == 0)
{
ec.clear();
return true;
}

const auto remove_errno = errno;
if (remove_errno == ENOENT)
{
ec.clear();
}
else
{
ec.assign(errno, std::generic_category());
return false;
ec.assign(remove_errno, std::generic_category());
}

return false;
#endif // _WIN32
}
virtual void remove_all(const Path& base, std::error_code& ec, Path& failure_point) override
Expand Down

0 comments on commit 470ef33

Please sign in to comment.