Skip to content

Conversation

Pospelove
Copy link
Contributor

In vcpkg_download_distfile.cmake vcpkg was using file(RENAME) to prevent error creating directory downloads/temp on Windows (see #3399).

Anyway, in some case file(RENAME) fails, which interrupt package installation.
The observed error messages are:
No such file or directory (#4245), Invalid cross-device link (#7079, #7997).

This PR proposes temp directory cleanup mechanism without file(RENAME).
We create temp once and remove all its contents when we need.

Here is the Dockerfile I built to test my changes.

FROM gcc:latest
RUN apt-get update && apt-get install -y git curl unzip tar
RUN git clone https://github.com/Pospelove/vcpkg.git && cd vcpkg && git reset --hard 32fa8b4c226129a98a3f62e92d17f47fe2122e55 && chmod 777 bootstrap-vcpkg.sh && ./bootstrap-vcpkg.sh
RUN cd vcpkg && ./vcpkg install cpp-httplib
RUN cd vcpkg && ./vcpkg install nlohmann-json

@ras0219-msft
Copy link
Collaborator

Thanks for the PR!

You mentioned that this error occurs "in some cases"; is it deterministic and reproducible in the cases it occurs or is it transient? If it's a transient error, some sort of retry logic may be a better approach.

Some alternative options:

  • Use cmake's "command mode" to perform the rename so we can catch errors and retry after a short sleep
    • execute_process(COMMAND ${CMAKE_COMMAND} -E rename <oldname> <newname>)
  • Remove the directory, check if it is still there, then loop after a delay until gone or retried N times

@Pospelove
Copy link
Contributor Author

@ras0219
It's absolutely deterministic and reproducible in my case. The error is reproduced in 100% of cases (#7997). I think repeating actions may help with #4245, but not with #7079/#7997. It will just fail all attempts.

@cbezault
Copy link
Contributor

cbezault commented Sep 11, 2019

CMake explicitly states that RENAME is only valid on a single partition and likely Docker is doing some weird filesystem things. There isn't any real downside to not using RENAME except for in-place renames, though I would argue that Kitware should fix this the same way mv handles cross partition moves (which is the same way we should handle them in this PR).

@vicroms vicroms merged commit 6c7f1c7 into microsoft:master Sep 23, 2019
@vicroms
Copy link
Member

vicroms commented Sep 23, 2019

Thanks for the PR!

@Pospelove Pospelove deleted the issue4245 branch June 11, 2020 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants