Skip to content

Conversation

@Thomas1664
Copy link
Contributor

@Thomas1664 Thomas1664 commented Apr 20, 2025

Fixes microsoft/vcpkg#45130

Please note that there is already code in place to handle std::errc::cross_device_link for binary caching:

if (can_attempt_rename)
{
// if we are already on the same filesystem we can just rename into place
m_fs.rename_or_delete(zip_path, archive_path, ec);
}
if (!can_attempt_rename || (ec && ec == std::make_error_condition(std::errc::cross_device_link)))
{
// either we need to make a copy or the rename failed because buildtrees and the binary
// cache write target are on different filesystems, copy to a sibling in that directory and rename
// into place
m_fs.copy_file(zip_path, archive_temp_path, CopyOptions::overwrite_existing, ec);
if (!ec)
{
m_fs.rename_or_delete(archive_temp_path, archive_path, ec);
}
}

However, copy-and-delete was only tried after calling fs.rename_or_delete() which tries to move the file after increasing timeouts. I decided to not move the copy-and-delete behavior to fs.rename_or_delete() because the file is first copied to a temproary location in binarycaching.cpp.

Files on different filesystems can't be renamed:

Linux: https://man7.org/linux/man-pages/man2/rename.2.html

Windows seems to support moving files but not folders across devices: https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-movefile

std::errc::cross_device_link: https://en.cppreference.com/w/cpp/error/errno_macros

Stack Overflow: https://stackoverflow.com/questions/43206198/what-does-the-exdev-cross-device-link-not-permitted-error-mean

@Thomas1664 Thomas1664 changed the title Optimize rename_or_delete to stop retrying after std::errc::cross_device_link Optimize rename_or_delete to stop retrying after s EXDEV Apr 20, 2025
@Thomas1664 Thomas1664 changed the title Optimize rename_or_delete to stop retrying after s EXDEV Optimize rename_or_delete to stop retrying after EXDEV Apr 20, 2025
if (can_attempt_rename)
{
// if we are already on the same filesystem we can just rename into place
m_fs.rename_or_delete(zip_path, archive_path, ec);
Copy link
Contributor Author

@Thomas1664 Thomas1664 Apr 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before #802, files were copied to the binary cache, meaning the original file in zip_path did still exist after this function ended. However, this file is gone with rename_or_delete()

Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the improvement!

@BillyONeal BillyONeal merged commit d8ed13a into microsoft:main Apr 29, 2025
7 checks passed
@Thomas1664 Thomas1664 deleted the rename-or-delete branch April 29, 2025 21:02
this->remove_all(old_path, ec);
return false;
}
else if (ec == std::make_error_condition(std::errc::cross_device_link))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only that ec was already lost at this point, due to L2041.

#1669

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.

[vcpkg-tool] Slow binary cache submission due to rename() EXDEV retries

3 participants