diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp index 47d26d5ac4..7611af7195 100644 --- a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp +++ b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -181,6 +181,68 @@ namespace AppInstaller::CLI::Workflow return args; } + + // Complicated rename algorithm due to somewhat arbitrary failures. + // 1. First, try to rename. + // 2. Then, create an empty file for the target, and attempt to rename. + // 3. Then, try repeatedly for 500ms in case it is a timing thing. + // 4. Attempt to use a hard link if available. + // 5. Copy the file if nothing else has worked so far. + void RenameFile(const std::filesystem::path& from, const std::filesystem::path& to) + { + // 1. First, try to rename. + try + { + // std::filesystem::rename() handles motw correctly if applicable. + std::filesystem::rename(from, to); + return; + } + CATCH_LOG(); + + // 2. Then, create an empty file for the target, and attempt to rename. + // This seems to fix things in certain cases, so we do it. + try + { + { + std::ofstream targetFile{ to }; + } + std::filesystem::rename(from, to); + return; + } + CATCH_LOG(); + + // 3. Then, try repeatedly for 500ms in case it is a timing thing. + for (int i = 0; i < 5; ++i) + { + try + { + std::this_thread::sleep_for(100ms); + std::filesystem::rename(from, to); + return; + } + CATCH_LOG(); + } + + // 4. Attempt to use a hard link if available. + if (Runtime::SupportsHardLinks(from)) + { + try + { + // Create a hard link to the file; the installer will be left in the temp directory afterward + // but it is better to succeed the operation and leave a file around than to fail. + // First we have to remove the target file as the function will not overwrite. + std::filesystem::remove(to); + std::filesystem::create_hard_link(from, to); + return; + } + CATCH_LOG(); + } + + // 5. Copy the file if nothing else has worked so far. + // Create a copy of the file; the installer will be left in the temp directory afterward + // but it is better to succeed the operation and leave a file around than to fail. + std::filesystem::copy_file(from, to, std::filesystem::copy_options::overwrite_existing); + } } void ShellExecuteInstallImpl(Execution::Context& context) @@ -242,23 +304,7 @@ namespace AppInstaller::CLI::Workflow break; } - // If rename fails, don't give up quite yet. - bool retry = true; - - try - { - // std::filesystem::rename() handles motw correctly if applicable. - std::filesystem::rename(installerPath, renamedDownloadedInstaller); - retry = false; - } - CATCH_LOG(); - - if (retry) - { - std::this_thread::sleep_for(250ms); - // If it fails again, let that one throw - std::filesystem::rename(installerPath, renamedDownloadedInstaller); - } + RenameFile(installerPath, renamedDownloadedInstaller); installerPath.assign(renamedDownloadedInstaller); AICLI_LOG(CLI, Info, << "Successfully renamed downloaded installer. Path: " << installerPath); diff --git a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h index 584571c611..59aa953d78 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h @@ -60,4 +60,7 @@ namespace AppInstaller::Runtime // Checks if the file system is NTFS bool IsNTFS(const std::filesystem::path& filePath); + + // Checks if the file system at path supports hard links + bool SupportsHardLinks(const std::filesystem::path& path); } diff --git a/src/AppInstallerCommonCore/Runtime.cpp b/src/AppInstallerCommonCore/Runtime.cpp index 7875cee0e1..b14ed091c9 100644 --- a/src/AppInstallerCommonCore/Runtime.cpp +++ b/src/AppInstallerCommonCore/Runtime.cpp @@ -409,6 +409,9 @@ namespace AppInstaller::Runtime return wil::test_token_membership(nullptr, SECURITY_NT_AUTHORITY, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS); } + // TODO: Replace this function with proper checks for supported functionality rather + // than simply relying on "is it NTFS?", even if those functions delegate to + // this one for the answer. bool IsNTFS(const std::filesystem::path& filePath) { wil::unique_hfile fileHandle{ CreateFileW( @@ -436,6 +439,11 @@ namespace AppInstaller::Runtime return _wcsicmp(fileSystemName, L"NTFS") == 0; } + bool SupportsHardLinks(const std::filesystem::path& path) + { + return IsNTFS(path); + } + #ifndef AICLI_DISABLE_TEST_HOOKS void TestHook_SetPathOverride(PathName target, const std::filesystem::path& path) {