From 8e3c9ecbf339438b12335f3a11d5182dc3a12ee7 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 11 Nov 2026 11:49:50 -0800 Subject: [PATCH 1/8] initial impl --- doc/Settings.md | 6 + .../1.31/2036-shorter-installer-log-paths.md | 7 + src/AppInstallerCLICore/Resources.h | 1 + .../ShellExecuteInstallerHandler.cpp | 180 +++++++++----- .../Shared/Strings/en-us/winget.resw | 4 + src/AppInstallerCLITests/InstallFlow.cpp | 230 ++++++++++++++++++ 6 files changed, 370 insertions(+), 58 deletions(-) create mode 100644 doc/release-notes/1.31/2036-shorter-installer-log-paths.md diff --git a/doc/Settings.md b/doc/Settings.md index 6aa29dc770..f9700fba58 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -320,6 +320,12 @@ In addition, there are special values that cover multiple channels. `default` i Sets the default strategy for naming log files for installers that support it. `manifest` is the default and uses the manifest name. `timestamp` uses the date and time. `guid` uses a generated GUID. `shortguid` uses the first 8 characters of a generated GUID. Invalid values will revert to `manifest`. +The `manifest` strategy targets a complete log path of at most 150 UTF-16 code units, including the directory and `.log` extension. Existing names are retained when they fit. Otherwise, WinGet omits the package version and uses a compact `YYMMDDhhmmss` timestamp. If necessary, it retains only the end of the package ID, with a short hash to distinguish abbreviated identities, or falls back to an eight-character random name. The complete package identity and selected path remain available in WinGet's diagnostic log. + +This leaves additional room for installers that append text to the supplied log name, but suffix compatibility is best effort, not guaranteed. The log directory is unchanged. Other configured naming strategies retain their formats when they fit within the Windows path limit. Any generated path that reaches `MAX_PATH` falls back to the eight-character random name. Shortening a path or exceeding the preferred length does not produce a user-visible warning. If even the minimal name cannot fit within `MAX_PATH`, WinGet warns and continues without passing the installer logging switch. + +Explicit log paths, such as those supplied with `--log`, are not shortened or omitted by this policy. For configuration runs, `WinGetUserSettings` can select `logging.fileNameStrategy` for the execution user's settings; `WinGetPackage` does not expose a per-package log-path property. + ```json "logging": { "fileNameStrategy": "manifest" | "timestamp" | "guid" | "shortguid" diff --git a/doc/release-notes/1.31/2036-shorter-installer-log-paths.md b/doc/release-notes/1.31/2036-shorter-installer-log-paths.md new file mode 100644 index 0000000000..363bcbe9dc --- /dev/null +++ b/doc/release-notes/1.31/2036-shorter-installer-log-paths.md @@ -0,0 +1,7 @@ +## Bug Fixes + +### Shorter automatic installer log paths + +WinGet now shortens default installer log names as needed to target a complete path of 150 characters, leaving more room for installers that append their own log names. Names retain the package version and timestamp when they fit; otherwise, WinGet uses a compact timestamp and omits the version, abbreviates the package ID, or uses a short random name. + +This is a best-effort mitigation, not a guarantee for installer-added suffixes. The log directory and explicit log paths are unchanged. If even a minimal automatic name cannot fit within the Windows path limit, WinGet warns and continues without requesting an installer log file. diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 2e378e4463..a821582edb 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -396,6 +396,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(InstallerHashMismatchOverrideRequired); WINGET_DEFINE_RESOURCE_STRINGID(InstallerHashVerified); WINGET_DEFINE_RESOURCE_STRINGID(InstallerLogAvailable); + WINGET_DEFINE_RESOURCE_STRINGID(InstallerLogUnavailable); WINGET_DEFINE_RESOURCE_STRINGID(InstallerProhibitsElevation); WINGET_DEFINE_RESOURCE_STRINGID(InstallerRequiresInstallLocation); WINGET_DEFINE_RESOURCE_STRINGID(InstallersAbortTerminal); diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp index b4674f7f2f..cb5a938a80 100644 --- a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp +++ b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -5,6 +5,7 @@ #include #include #include +#include using namespace AppInstaller::CLI; using namespace AppInstaller::Utility; @@ -73,14 +74,14 @@ namespace AppInstaller::CLI::Workflow } // Gets the escaped installer args. - std::string GetInstallerArgsTemplate(Execution::Context& context) + std::vector GetInstallerArgsTemplate(Execution::Context& context) { bool isUpdate = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate); bool isRepair = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseRepair); const auto& installer = context.Get(); const auto& installerSwitches = installer->Switches; - std::string installerArgs = {}; + std::vector installerArgs; // Construct install experience arg. // SilentWithProgress is default, so look for it first. @@ -106,13 +107,13 @@ namespace AppInstaller::CLI::Workflow if (experienceArgsItr != installerSwitches.end()) { - installerArgs += experienceArgsItr->second; + installerArgs.emplace_back(experienceArgsItr->second); } // Construct log path arg. if (installerSwitches.find(InstallerSwitchType::Log) != installerSwitches.end()) { - installerArgs += ' ' + installerSwitches.at(InstallerSwitchType::Log); + installerArgs.emplace_back(' ' + installerSwitches.at(InstallerSwitchType::Log)); } // Construct repair arg. Custom switches and other args are not applicable for repair scenario so we can return here. @@ -120,7 +121,7 @@ namespace AppInstaller::CLI::Workflow { if (installerSwitches.find(InstallerSwitchType::Repair) != installerSwitches.end()) { - installerArgs += ' ' + installerSwitches.at(InstallerSwitchType::Repair); + installerArgs.emplace_back(' ' + installerSwitches.at(InstallerSwitchType::Repair)); } return installerArgs; @@ -129,7 +130,7 @@ namespace AppInstaller::CLI::Workflow // Construct custom arg. if (installerSwitches.find(InstallerSwitchType::Custom) != installerSwitches.end()) { - installerArgs += ' ' + installerSwitches.at(InstallerSwitchType::Custom); + installerArgs.emplace_back(' ' + installerSwitches.at(InstallerSwitchType::Custom)); } // Construct custom arg passed in by cli arg @@ -139,87 +140,146 @@ namespace AppInstaller::CLI::Workflow // Since these arguments are appended to the installer at runtime, it doesn't make sense to append them if empty or whitespace if (!Utility::IsEmptyOrWhitespace(customSwitches)) { - installerArgs += ' ' + std::string{ customSwitches }; + installerArgs.emplace_back(' ' + std::string{ customSwitches }); } } // Construct update arg if applicable if (isUpdate && installerSwitches.find(InstallerSwitchType::Update) != installerSwitches.end()) { - installerArgs += ' ' + installerSwitches.at(InstallerSwitchType::Update); + installerArgs.emplace_back(' ' + installerSwitches.at(InstallerSwitchType::Update)); } // Construct install location arg if necessary. if (context.Args.Contains(Execution::Args::Type::InstallLocation) && installerSwitches.find(InstallerSwitchType::InstallLocation) != installerSwitches.end()) { - installerArgs += ' ' + installerSwitches.at(InstallerSwitchType::InstallLocation); + installerArgs.emplace_back(' ' + installerSwitches.at(InstallerSwitchType::InstallLocation)); } return installerArgs; } - // Applies values to the template. - void PopulateInstallerArgsTemplate(Execution::Context& context, std::string& installerArgs) + std::optional GetAutomaticInstallerLogPath(Execution::Context& context) { - // Populate with value from command line or temp path. - std::string logPath; - if (context.Args.Contains(Execution::Args::Type::Log)) + // Chosen to allow a reported 108 character suffix from installer. + constexpr size_t preferredPathLength = 150; + auto directory = Runtime::GetPathTo(Runtime::PathName::DefaultLogLocation); + auto makePath = [&](const std::filesystem::path& stem) { - logPath = context.Args.GetArg(Execution::Args::Type::Log); - } - else + auto result = directory / stem; + result += Logging::FileLogger::DefaultExt(); + return result; + }; + auto shortGuidPath = [&]() + { + return makePath(Utility::CreateNewGuidNameWString().substr(0, 8)); + }; + const auto time = std::chrono::system_clock::now(); + const auto timestamp = Utility::TimePointToString(time, Utility::TimeFacet::ShortYearSecondPrecision | Utility::TimeFacet::Filename); + std::filesystem::path result; + switch (Settings::User().Get()) + { + case Logging::LogNameStrategy::Manifest: { const auto& manifest = context.Get(); - const Logging::LogNameStrategy logNameStrategy = Settings::User().Get(); - auto path = Runtime::GetPathTo(Runtime::PathName::DefaultLogLocation); - - switch (logNameStrategy) + result = makePath(GetPathPart(manifest).native() + L'-' + Utility::ConvertToUTF16(timestamp)); + if (result.native().size() > preferredPathLength) { - case Logging::LogNameStrategy::Manifest: - // Use manifest ID and version for log file name - // Results in \.-.log - path /= GetPathPart(manifest); - path += '-'; - path += Utility::GetCurrentTimeForFilename(true); - break; - case Logging::LogNameStrategy::Timestamp: - // Use only timestamp for log file name - // Results in \.log - path /= Utility::GetCurrentTimeForFilename(true); - break; - case Logging::LogNameStrategy::Guid: - // Use a GUID for log file name - // Results in \.log - path /= Utility::CreateNewGuidNameWString(); - break; - case Logging::LogNameStrategy::ShortGuid: - // Use the first 8 characters of a GUID for log file name - // Results in \.log - path /= Utility::CreateNewGuidNameWString().substr(0, 8); - break; - default: - // This should never happen due to validation when reading settings, but handle it just in case. - AICLI_LOG(CLI, Error, << "Unknown log naming strategy."); - THROW_HR(E_UNEXPECTED); - break; + auto compactTimestamp = timestamp; + compactTimestamp.erase(std::remove(compactTimestamp.begin(), compactTimestamp.end(), '-'), compactTimestamp.end()); + const auto suffix = L"-" + Utility::ConvertToUTF16(compactTimestamp); + const auto id = Utility::MakeSuitablePathPart(manifest.Id); + result = makePath(Utility::ConvertToUTF16(id) + suffix); + if (result.native().size() > preferredPathLength) + { + const auto hash = SHA256::ConvertToString(SHA256::ComputeHash(manifest.Id + '\0' + manifest.Version)).substr(0, 6); + // A hash prefix also prevents a truncated tail from becoming a reserved device name. + const auto prefix = Utility::ConvertToUTF16(hash) + L"~"; + const auto fixedLength = makePath(prefix + suffix).native().size(); + std::string_view tail = id; + // Keep whole grapheme clusters, but budget their Windows UTF-16 representation. + while (!tail.empty() && fixedLength + Utility::ConvertToUTF16(tail).size() > preferredPathLength) + { + tail = Utility::UTF8Substring(tail, 1, std::string_view::npos); + } + result = tail.empty() ? shortGuidPath() : makePath(prefix + Utility::ConvertToUTF16(tail) + suffix); + } } + AICLI_LOG(CLI, Info, << "Installer log for " << manifest.Id << " [" << manifest.Version << "]: " << result.u8string()); + break; + } + case Logging::LogNameStrategy::Timestamp: + result = makePath(timestamp); + break; + case Logging::LogNameStrategy::Guid: + result = makePath(Utility::CreateNewGuidNameWString()); + break; + case Logging::LogNameStrategy::ShortGuid: + result = shortGuidPath(); + break; + default: + AICLI_LOG(CLI, Error, << "Unknown log naming strategy."); + THROW_HR(E_UNEXPECTED); + } - // Add the extension to the log file regardless of naming strategy - path += Logging::FileLogger::DefaultExt(); - - logPath = path.u8string(); + if (result.native().size() >= MAX_PATH) + { + result = shortGuidPath(); + } + if (result.native().size() >= MAX_PATH) + { + AICLI_LOG(CLI, Warning, << "No automatic installer log path fits within MAX_PATH in " << directory.u8string()); + context.Reporter.Warn() << Resource::String::InstallerLogUnavailable << std::endl; + return std::nullopt; } + if (result.native().size() > preferredPathLength) + { + AICLI_LOG(CLI, Warning, << "Installer log path exceeds the preferred length of " << preferredPathLength << ": " << result.u8string()); + } + + return result; + } - if (Utility::FindAndReplace(installerArgs, std::string(ARG_TOKEN_LOGPATH), logPath)) + // Applies values to the template, retaining fragments so an unavailable log switch can be omitted. + void PopulateInstallerArgsTemplate(Execution::Context& context, std::vector& installerArgs) + { + const auto usesLogPath = [](const std::string& arg) { return arg.find(ARG_TOKEN_LOGPATH) != std::string::npos; }; + if (std::any_of(installerArgs.begin(), installerArgs.end(), usesLogPath)) { - context.Add(Utility::ConvertToUTF16(logPath)); + std::optional logPath; + if (context.Args.Contains(Execution::Args::Type::Log)) + { + logPath = Utility::ConvertToUTF16(context.Args.GetArg(Execution::Args::Type::Log)); + } + else + { + logPath = GetAutomaticInstallerLogPath(context); + } + + if (logPath) + { + auto logPathString = logPath->u8string(); + for (auto& arg : installerArgs) + { + Utility::FindAndReplace(arg, ARG_TOKEN_LOGPATH, logPathString); + } + context.Add(std::move(*logPath)); + } + else + { + installerArgs.erase(std::remove_if(installerArgs.begin(), installerArgs.end(), usesLogPath), installerArgs.end()); + } } // Populate with value from command line. if (context.Args.Contains(Execution::Args::Type::InstallLocation)) { - Utility::FindAndReplace(installerArgs, std::string(ARG_TOKEN_INSTALLPATH), context.Args.GetArg(Execution::Args::Type::InstallLocation)); + auto locationArg = context.Args.GetArg(Execution::Args::Type::InstallLocation); + for (auto& arg : installerArgs) + { + Utility::FindAndReplace(arg, ARG_TOKEN_INSTALLPATH, locationArg); + } } // Todo: language token support will be implemented later @@ -334,9 +394,13 @@ namespace AppInstaller::CLI::Workflow return; } - std::string installerArgs = GetInstallerArgsTemplate(context); - - PopulateInstallerArgsTemplate(context, installerArgs); + auto installerArgsTemplate = GetInstallerArgsTemplate(context); + PopulateInstallerArgsTemplate(context, installerArgsTemplate); + std::string installerArgs; + for (const auto& arg : installerArgsTemplate) + { + installerArgs += arg; + } AICLI_LOG(CLI, Info, << "Installer args: " << installerArgs); context.Add(std::move(installerArgs)); diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 53e3bba57d..069a915921 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1370,6 +1370,10 @@ Do you agree to the terms? Installer log is available at: {0} {Locked="{0}"} Message displayed to inform the user about the system path of a diagnostic files containing information about the installer. {0} is a placeholder replaced by the diagnostic file system path. + + The installer log path is too long. Continuing without requesting an installer log file. + Warning when even a minimal automatically generated installer log file name cannot fit within the Windows path limit. The operation continues with the logging switch omitted; the installer may still produce its own logs. + The following packages were found among the working sources. Please specify one of them using the --source option to proceed. diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index d424fac669..99ea5959ee 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -33,6 +33,28 @@ using namespace AppInstaller::Settings; using namespace AppInstaller::Utility; using namespace AppInstaller::Utility::literals; +namespace +{ + struct InstallerLogDirectoryOverride + { + InstallerLogDirectoryOverride(size_t length) : + Original(AppInstaller::Runtime::GetPathDetailsFor(AppInstaller::Runtime::PathName::DefaultLogLocation)) + { + AppInstaller::Filesystem::PathDetails details; + details.Path = L"C:\\" + std::wstring(length - 3, L'a'); + details.Create = false; + AppInstaller::Runtime::TestHook_SetPathOverride(AppInstaller::Runtime::PathName::DefaultLogLocation, details); + } + + ~InstallerLogDirectoryOverride() + { + AppInstaller::Runtime::TestHook_SetPathOverride(AppInstaller::Runtime::PathName::DefaultLogLocation, Original); + } + + AppInstaller::Filesystem::PathDetails Original; + }; +} + void OverrideForDirectMsi(TestContext& context) { OverrideForCheckExistingInstaller(context); @@ -1030,6 +1052,8 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]") TEST_CASE("ShellExecuteHandlerInstallerArgs_LogNamingStrategy", "[InstallFlow][workflow]") { + InstallerLogDirectoryOverride logDirectory{ 7 }; + SECTION("Manifest") { TestUserSettings testSettings; @@ -1122,6 +1146,212 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogNamingStrategy", "[InstallFlow][w } } +TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBudget", "[InstallFlow][workflow][InstallerLogPath]") +{ + TestUserSettings testSettings; + testSettings.Set(LogNameStrategy::Manifest); + auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_WithSwitches.yaml")); + manifest.Id = "Microsoft.DotNet.SDK.10"; + manifest.Version = "10.0.404"; + size_t directoryLength = 95; + bool shortened = false; + bool hashed = false; + + SECTION("ExactlyPreferredLength") + { + } + SECTION("JustAbovePreferredLength") + { + directoryLength = 96; + shortened = true; + } + SECTION("ReportedNestedSuffixes") + { + directoryLength = 119; + shortened = hashed = true; + } + SECTION("LongVersion") + { + manifest.Version = std::string(200, '1'); + shortened = true; + } + SECTION("UnicodePackageTail") + { + std::string id = "Publisher."; + for (size_t i = 0; i < 30; ++i) + { + id += "\xF0\x9F\x98\x80"; + } + manifest.Id = id + ".SDK"; + directoryLength = 119; + shortened = hashed = true; + } + SECTION("LongPackageIdentity") + { + manifest.Id = std::string(200, 'a') + ".SDK"; + directoryLength = 119; + shortened = hashed = true; + } + SECTION("ReservedDeviceTail") + { + manifest.Id = "Publisher.CON.ab"; + directoryLength = 119; + shortened = hashed = true; + } + + InstallerLogDirectoryOverride logDirectory{ directoryLength }; + std::ostringstream installOutput; + TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.SetFlags(ContextFlag::InstallerExecutionUseUpdate); + context.Add(manifest); + context.Add(manifest.Installers.at(0)); + context << GetInstallerArgs; + + REQUIRE_FALSE(context.IsTerminated()); + REQUIRE(installOutput.str().empty()); + REQUIRE(context.Contains(Data::LogPath)); + const auto& path = context.Get(); + INFO(path.u8string()); + REQUIRE(path.native().size() <= 150); + REQUIRE(path.extension() == L".log"); + REQUIRE(MakeSuitablePathPart(path.filename().u8string()) == path.filename().u8string()); + const auto stem = path.stem().u8string(); + const auto timestampStart = stem.rfind('-'); + REQUIRE(timestampStart != std::string::npos); + REQUIRE(ConvertToUTF16(ConvertToUTF8(path.native())) == path.native()); + if (shortened) + { + const auto timestamp = stem.substr(timestampStart + 1); + REQUIRE(timestamp.size() == 12); + REQUIRE(std::all_of(timestamp.begin(), timestamp.end(), [](char c) { return c >= '0' && c <= '9'; })); + REQUIRE(stem.find(manifest.Version) == std::string::npos); + } + else + { + REQUIRE(path.native().size() == 150); + REQUIRE(stem.find(manifest.Id + '.' + manifest.Version) == 0); + } + if (hashed) + { + REQUIRE(stem.find('~') != std::string::npos); + auto otherManifest = manifest; + otherManifest.Id = "Other." + manifest.Id; + std::ostringstream otherOutput; + TestContext otherContext{ otherOutput, std::cin }; + otherContext.Add(otherManifest); + otherContext.Add(otherManifest.Installers.at(0)); + otherContext << GetInstallerArgs; + const auto otherStem = otherContext.Get().stem().u8string(); + REQUIRE(stem.substr(0, timestampStart) != otherStem.substr(0, otherStem.rfind('-'))); + } + + const auto& args = context.Get(); + REQUIRE(args.find("/mylog=\"" + path.u8string() + '"') != std::string::npos); + REQUIRE(args.find("/mycustom") != std::string::npos); + for (const auto& suffix : { + L"_013_Microsoft.NET.Sdk.Android.Manifest_6.0.100.31.0.101_preview.9.16_x64.msi", + L"_018_{106C9C71-2390-4214-9E7E-7BA47FFB1FAB}_010_dotnet_sdk_internal_10.0.401_servicing.26423.113_win_x64.msi" }) + { + auto derivedPath = path.parent_path() / (path.stem().native() + suffix + path.extension().native()); + REQUIRE(derivedPath.native().size() < MAX_PATH); + } +} + +TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathFallback", "[InstallFlow][workflow][InstallerLogPath]") +{ + const auto strategy = GENERATE(LogNameStrategy::Manifest, LogNameStrategy::Timestamp, LogNameStrategy::Guid, LogNameStrategy::ShortGuid); + const size_t directoryLength = GENERATE(137, 138, 246, 247, 300); + CAPTURE(strategy, directoryLength); + InstallerLogDirectoryOverride logDirectory{ directoryLength }; + TestUserSettings testSettings; + testSettings.Set(strategy); + std::ostringstream installOutput; + TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_WithSwitches.yaml")); + context.Add(manifest); + context.Add(manifest.Installers.at(0)); + context.Args.AddArg(Execution::Args::Type::InstallLocation, "MyDir"sv); + context.Override({ ShellExecuteInstallImpl, [](TestContext& context) + { + REQUIRE(context.Get().find("/mycustom") != std::string::npos); + context.Add(0); + } }); + context << GetInstallerArgs << ShellExecuteInstallImpl; + + REQUIRE_FALSE(context.IsTerminated()); + REQUIRE(context.Get() == 0); + const auto& args = context.Get(); + REQUIRE(args.find("") == std::string::npos); + REQUIRE(args.find("/mysilentwithprogress") != std::string::npos); + REQUIRE(args.find("/mycustom") != std::string::npos); + REQUIRE(args.find("/myinstalldir=\"MyDir\"") != std::string::npos); + if (directoryLength + 1 + 12 >= MAX_PATH) + { + REQUIRE_FALSE(context.Contains(Data::LogPath)); + REQUIRE(args.find("/mylog") == std::string::npos); + REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::InstallerLogUnavailable).get()) != std::string::npos); + } + else + { + const auto& path = context.Get(); + const size_t stemLength = strategy == LogNameStrategy::Guid ? 36 : (strategy == LogNameStrategy::Timestamp ? 17 : 8); + const size_t configuredLength = directoryLength + 1 + stemLength + 4; + REQUIRE(path.native().size() == (configuredLength < MAX_PATH ? configuredLength : directoryLength + 1 + 12)); + REQUIRE(args.find("/mylog=\"" + path.u8string() + '"') != std::string::npos); + REQUIRE(installOutput.str().empty()); + } +} + +TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBypass", "[InstallFlow][workflow][InstallerLogPath]") +{ + InstallerLogDirectoryOverride logDirectory{ 300 }; + std::ostringstream installOutput; + TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_WithSwitches.yaml")); + auto installer = manifest.Installers.at(0); + const std::string explicitPath = "C:\\" + std::string(270, 'a') + ".log"; + bool hasExplicitPath = false; + bool overridden = false; + SECTION("ExplicitLog") + { + context.Args.AddArg(Execution::Args::Type::Log, explicitPath); + hasExplicitPath = true; + } + SECTION("Override") + { + context.Args.AddArg(Execution::Args::Type::Override, "/override"sv); + overridden = true; + } + SECTION("NoLogToken") + { + installer.Switches[InstallerSwitchType::Log] = "/fixedlog"; + } + context.Add(manifest); + context.Add(installer); + context << GetInstallerArgs; + + REQUIRE_FALSE(context.IsTerminated()); + REQUIRE(context.Contains(Data::LogPath) == hasExplicitPath); + const auto& args = context.Get(); + if (hasExplicitPath) + { + REQUIRE(context.Get().u8string() == explicitPath); + REQUIRE(args.find("/mylog=\"" + explicitPath + '"') != std::string::npos); + } + else if (overridden) + { + REQUIRE(args == "/override"); + } + else + { + REQUIRE(args.find("/fixedlog") != std::string::npos); + } + REQUIRE(installOutput.str().empty()); +} + TEST_CASE("InstallFlow_SearchAndInstall", "[InstallFlow][workflow]") { TestCommon::TempFile installResultPath("TestExeInstalled.txt"); From 5e16d907408710671627111a892b9c8acbe5b6c0 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 11 Nov 2026 11:58:21 -0800 Subject: [PATCH 2/8] use short guid strategy instead of hashing --- doc/Settings.md | 2 +- .../1.31/2036-shorter-installer-log-paths.md | 2 +- .../Workflows/DownloadFlow.cpp | 2 +- .../ShellExecuteInstallerHandler.cpp | 21 +++------- src/AppInstallerCLITests/InstallFlow.cpp | 41 ++++++------------- src/AppInstallerCLITests/YamlManifest.cpp | 19 ++++++--- .../Manifest/Manifest.cpp | 19 ++++++++- .../Public/winget/Manifest.h | 11 ++++- 8 files changed, 62 insertions(+), 55 deletions(-) diff --git a/doc/Settings.md b/doc/Settings.md index f9700fba58..aa073ae367 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -320,7 +320,7 @@ In addition, there are special values that cover multiple channels. `default` i Sets the default strategy for naming log files for installers that support it. `manifest` is the default and uses the manifest name. `timestamp` uses the date and time. `guid` uses a generated GUID. `shortguid` uses the first 8 characters of a generated GUID. Invalid values will revert to `manifest`. -The `manifest` strategy targets a complete log path of at most 150 UTF-16 code units, including the directory and `.log` extension. Existing names are retained when they fit. Otherwise, WinGet omits the package version and uses a compact `YYMMDDhhmmss` timestamp. If necessary, it retains only the end of the package ID, with a short hash to distinguish abbreviated identities, or falls back to an eight-character random name. The complete package identity and selected path remain available in WinGet's diagnostic log. +The `manifest` strategy targets a complete log path of at most 150 UTF-16 code units, including the directory and `.log` extension. Existing names are retained when they fit. Otherwise, WinGet omits the package version and uses a compact `YYMMDDhhmmss` timestamp. If that still exceeds the preferred length, it falls back to an eight-character random name. The complete package identity and selected path remain available in WinGet's diagnostic log. This leaves additional room for installers that append text to the supplied log name, but suffix compatibility is best effort, not guaranteed. The log directory is unchanged. Other configured naming strategies retain their formats when they fit within the Windows path limit. Any generated path that reaches `MAX_PATH` falls back to the eight-character random name. Shortening a path or exceeding the preferred length does not produce a user-visible warning. If even the minimal name cannot fit within `MAX_PATH`, WinGet warns and continues without passing the installer logging switch. diff --git a/doc/release-notes/1.31/2036-shorter-installer-log-paths.md b/doc/release-notes/1.31/2036-shorter-installer-log-paths.md index 363bcbe9dc..248a93cc8c 100644 --- a/doc/release-notes/1.31/2036-shorter-installer-log-paths.md +++ b/doc/release-notes/1.31/2036-shorter-installer-log-paths.md @@ -2,6 +2,6 @@ ### Shorter automatic installer log paths -WinGet now shortens default installer log names as needed to target a complete path of 150 characters, leaving more room for installers that append their own log names. Names retain the package version and timestamp when they fit; otherwise, WinGet uses a compact timestamp and omits the version, abbreviates the package ID, or uses a short random name. +WinGet now shortens default installer log names as needed to target a complete path of 150 characters, leaving more room for installers that append their own log names. Names retain the package version and timestamp when they fit; otherwise, WinGet uses a compact timestamp and omits the version, or falls back to a short random name. This is a best-effort mitigation, not a guarantee for installer-added suffixes. The log directory and explicit log paths are unchanged. If even a minimal automatic name cannot fit within the Windows path limit, WinGet warns and continues without requesting an installer log file. diff --git a/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp b/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp index 0a0db10b27..172a551887 100644 --- a/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp @@ -751,7 +751,7 @@ namespace AppInstaller::CLI::Workflow } const auto& manifest = context.Get(); - context.Add(downloadsDirectory / GetPathPart(manifest, '_', true)); + context.Add(downloadsDirectory / GetPathPart(manifest, '_', PathPartVersionStrategy::DropUnknownVersion)); } } diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp index cb5a938a80..2a2e3e9b16 100644 --- a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp +++ b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -5,7 +5,6 @@ #include #include #include -#include using namespace AppInstaller::CLI; using namespace AppInstaller::Utility; @@ -178,32 +177,24 @@ namespace AppInstaller::CLI::Workflow const auto time = std::chrono::system_clock::now(); const auto timestamp = Utility::TimePointToString(time, Utility::TimeFacet::ShortYearSecondPrecision | Utility::TimeFacet::Filename); std::filesystem::path result; + switch (Settings::User().Get()) { case Logging::LogNameStrategy::Manifest: { const auto& manifest = context.Get(); result = makePath(GetPathPart(manifest).native() + L'-' + Utility::ConvertToUTF16(timestamp)); + if (result.native().size() > preferredPathLength) { auto compactTimestamp = timestamp; compactTimestamp.erase(std::remove(compactTimestamp.begin(), compactTimestamp.end(), '-'), compactTimestamp.end()); const auto suffix = L"-" + Utility::ConvertToUTF16(compactTimestamp); - const auto id = Utility::MakeSuitablePathPart(manifest.Id); - result = makePath(Utility::ConvertToUTF16(id) + suffix); - if (result.native().size() > preferredPathLength) + result = makePath(GetPathPart(manifest, '.', PathPartVersionStrategy::NeverIncludeVersion).native() + suffix); + + if (result.native().size() >= preferredPathLength) { - const auto hash = SHA256::ConvertToString(SHA256::ComputeHash(manifest.Id + '\0' + manifest.Version)).substr(0, 6); - // A hash prefix also prevents a truncated tail from becoming a reserved device name. - const auto prefix = Utility::ConvertToUTF16(hash) + L"~"; - const auto fixedLength = makePath(prefix + suffix).native().size(); - std::string_view tail = id; - // Keep whole grapheme clusters, but budget their Windows UTF-16 representation. - while (!tail.empty() && fixedLength + Utility::ConvertToUTF16(tail).size() > preferredPathLength) - { - tail = Utility::UTF8Substring(tail, 1, std::string_view::npos); - } - result = tail.empty() ? shortGuidPath() : makePath(prefix + Utility::ConvertToUTF16(tail) + suffix); + result = shortGuidPath(); } } AICLI_LOG(CLI, Info, << "Installer log for " << manifest.Id << " [" << manifest.Version << "]: " << result.u8string()); diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index 99ea5959ee..4ac6c5e336 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -1155,7 +1155,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBudget", "[InstallFlow][workf manifest.Version = "10.0.404"; size_t directoryLength = 95; bool shortened = false; - bool hashed = false; + bool shortGuid = false; SECTION("ExactlyPreferredLength") { @@ -1168,14 +1168,14 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBudget", "[InstallFlow][workf SECTION("ReportedNestedSuffixes") { directoryLength = 119; - shortened = hashed = true; + shortGuid = true; } SECTION("LongVersion") { manifest.Version = std::string(200, '1'); shortened = true; } - SECTION("UnicodePackageTail") + SECTION("UnicodePackageId") { std::string id = "Publisher."; for (size_t i = 0; i < 30; ++i) @@ -1184,19 +1184,13 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBudget", "[InstallFlow][workf } manifest.Id = id + ".SDK"; directoryLength = 119; - shortened = hashed = true; + shortGuid = true; } SECTION("LongPackageIdentity") { manifest.Id = std::string(200, 'a') + ".SDK"; directoryLength = 119; - shortened = hashed = true; - } - SECTION("ReservedDeviceTail") - { - manifest.Id = "Publisher.CON.ab"; - directoryLength = 119; - shortened = hashed = true; + shortGuid = true; } InstallerLogDirectoryOverride logDirectory{ directoryLength }; @@ -1217,11 +1211,16 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBudget", "[InstallFlow][workf REQUIRE(path.extension() == L".log"); REQUIRE(MakeSuitablePathPart(path.filename().u8string()) == path.filename().u8string()); const auto stem = path.stem().u8string(); - const auto timestampStart = stem.rfind('-'); - REQUIRE(timestampStart != std::string::npos); REQUIRE(ConvertToUTF16(ConvertToUTF8(path.native())) == path.native()); - if (shortened) + if (shortGuid) + { + REQUIRE(stem.size() == 8); + REQUIRE(stem.find('-') == std::string::npos); + } + else if (shortened) { + const auto timestampStart = stem.rfind('-'); + REQUIRE(timestampStart != std::string::npos); const auto timestamp = stem.substr(timestampStart + 1); REQUIRE(timestamp.size() == 12); REQUIRE(std::all_of(timestamp.begin(), timestamp.end(), [](char c) { return c >= '0' && c <= '9'; })); @@ -1232,20 +1231,6 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBudget", "[InstallFlow][workf REQUIRE(path.native().size() == 150); REQUIRE(stem.find(manifest.Id + '.' + manifest.Version) == 0); } - if (hashed) - { - REQUIRE(stem.find('~') != std::string::npos); - auto otherManifest = manifest; - otherManifest.Id = "Other." + manifest.Id; - std::ostringstream otherOutput; - TestContext otherContext{ otherOutput, std::cin }; - otherContext.Add(otherManifest); - otherContext.Add(otherManifest.Installers.at(0)); - otherContext << GetInstallerArgs; - const auto otherStem = otherContext.Get().stem().u8string(); - REQUIRE(stem.substr(0, timestampStart) != otherStem.substr(0, otherStem.rfind('-'))); - } - const auto& args = context.Get(); REQUIRE(args.find("/mylog=\"" + path.u8string() + '"') != std::string::npos); REQUIRE(args.find("/mycustom") != std::string::npos); diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index 1454c85d10..5cc29fb88b 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -1641,7 +1641,7 @@ TEST_CASE("PackageIdentifierAndVersionPathValidation", "[ManifestValidation]") RequireSingleError(errors, ManifestError::FieldEscapesDirectory, "PackageVersion", manifest.Version); } -TEST_CASE("ManifestGetPathPart", "[ManifestValidation]") +TEST_CASE("ManifestGetPathPart", "[ManifestValidation][InstallerLogPath]") { Manifest manifest; manifest.Id = "Foo.Bar"; @@ -1650,22 +1650,28 @@ TEST_CASE("ManifestGetPathPart", "[ManifestValidation]") // The common case must not alter the value, as the resulting paths are persisted. REQUIRE(GetPathPart(manifest) == std::filesystem::path{ L"Foo.Bar.1.0.0" }); REQUIRE(GetPathPart(manifest, '_') == std::filesystem::path{ L"Foo.Bar_1.0.0" }); + REQUIRE(GetPathPart(manifest, '.', PathPartVersionStrategy::AlwaysIncludeVersion) == std::filesystem::path{ L"Foo.Bar.1.0.0" }); + REQUIRE(GetPathPart(manifest, '.', PathPartVersionStrategy::NeverIncludeVersion) == std::filesystem::path{ L"Foo.Bar" }); + REQUIRE(GetPathPart(manifest, '_', PathPartVersionStrategy::NeverIncludeVersion) == std::filesystem::path{ L"Foo.Bar" }); // An unknown version is only dropped when the caller asks for it. manifest.Version = "Unknown"; REQUIRE(GetPathPart(manifest) == std::filesystem::path{ L"Foo.Bar.Unknown" }); REQUIRE(GetPathPart(manifest, '_') == std::filesystem::path{ L"Foo.Bar_Unknown" }); - REQUIRE(GetPathPart(manifest, '.', true) == std::filesystem::path{ L"Foo.Bar" }); - REQUIRE(GetPathPart(manifest, '_', true) == std::filesystem::path{ L"Foo.Bar" }); + REQUIRE(GetPathPart(manifest, '.', PathPartVersionStrategy::AlwaysIncludeVersion) == std::filesystem::path{ L"Foo.Bar.Unknown" }); + REQUIRE(GetPathPart(manifest, '.', PathPartVersionStrategy::DropUnknownVersion) == std::filesystem::path{ L"Foo.Bar" }); + REQUIRE(GetPathPart(manifest, '_', PathPartVersionStrategy::DropUnknownVersion) == std::filesystem::path{ L"Foo.Bar" }); + REQUIRE(GetPathPart(manifest, '.', PathPartVersionStrategy::NeverIncludeVersion) == std::filesystem::path{ L"Foo.Bar" }); - // A known version is kept regardless of the drop request. + // Dropping only unknown versions keeps known versions. manifest.Version = "1.0.0"; - REQUIRE(GetPathPart(manifest, '.', true) == std::filesystem::path{ L"Foo.Bar.1.0.0" }); + REQUIRE(GetPathPart(manifest, '.', PathPartVersionStrategy::DropUnknownVersion) == std::filesystem::path{ L"Foo.Bar.1.0.0" }); // Values that validation would have rejected are sanitized rather than used as given. manifest.Id = "a\\b"; manifest.Version = "c/d"; REQUIRE(GetPathPart(manifest) == std::filesystem::path{ L"a_b.c_d" }); + REQUIRE(GetPathPart(manifest, '.', PathPartVersionStrategy::NeverIncludeVersion) == std::filesystem::path{ L"a_b" }); manifest.Id = "Foo.Bar"; manifest.Version = "C:"; @@ -1684,6 +1690,9 @@ TEST_CASE("ManifestGetPathPart", "[ManifestValidation]") manifest.Id = id; REQUIRE_THROWS_HR(GetPathPart(manifest), APPINSTALLER_CLI_ERROR_INVALID_MANIFEST); } + + manifest.Id = "CON"; + REQUIRE_THROWS_HR(GetPathPart(manifest, '.', PathPartVersionStrategy::NeverIncludeVersion), APPINSTALLER_CLI_ERROR_INVALID_MANIFEST); } TEST_CASE("PortableFileTypeValidation", "[ManifestValidation]") diff --git a/src/AppInstallerCommonCore/Manifest/Manifest.cpp b/src/AppInstallerCommonCore/Manifest/Manifest.cpp index 31eb41b26c..ac9d5dd401 100644 --- a/src/AppInstallerCommonCore/Manifest/Manifest.cpp +++ b/src/AppInstallerCommonCore/Manifest/Manifest.cpp @@ -326,11 +326,26 @@ namespace AppInstaller::Manifest return result; } - std::filesystem::path GetPathPart(const Manifest& manifest, char separator, bool dropUnknownVersion) + std::filesystem::path GetPathPart(const Manifest& manifest, char separator, PathPartVersionStrategy versionStrategy) { std::string value = manifest.Id; - if (!dropUnknownVersion || !Utility::Version{ manifest.Version }.IsUnknown()) + bool includeVersion = false; + switch (versionStrategy) + { + case PathPartVersionStrategy::AlwaysIncludeVersion: + includeVersion = true; + break; + case PathPartVersionStrategy::DropUnknownVersion: + includeVersion = !Utility::Version{ manifest.Version }.IsUnknown(); + break; + case PathPartVersionStrategy::NeverIncludeVersion: + break; + default: + THROW_HR(E_INVALIDARG); + } + + if (includeVersion) { value += separator; value += manifest.Version; diff --git a/src/AppInstallerCommonCore/Public/winget/Manifest.h b/src/AppInstallerCommonCore/Public/winget/Manifest.h index 2eb107d28d..63d8398576 100644 --- a/src/AppInstallerCommonCore/Public/winget/Manifest.h +++ b/src/AppInstallerCommonCore/Public/winget/Manifest.h @@ -81,10 +81,17 @@ namespace AppInstaller::Manifest std::function extractStringFromAppsAndFeaturesEntry = {}) const; }; + enum class PathPartVersionStrategy + { + AlwaysIncludeVersion, + DropUnknownVersion, + NeverIncludeVersion, + }; + // Creates a file system path part for the manifest in the form ``, - // or just `` when the version is unknown and the drop is requested. + // or just `` according to the version strategy. // Manifest validation rejects values that are not safe to use in a path, but the values can also come from // sources that do not go through it (for instance, installed package data), so they are sanitized here as // well. Throws if the result would point outside of the directory that it is used in. - std::filesystem::path GetPathPart(const Manifest& manifest, char separator = '.', bool dropUnknownVersion = false); + std::filesystem::path GetPathPart(const Manifest& manifest, char separator = '.', PathPartVersionStrategy versionStrategy = PathPartVersionStrategy::AlwaysIncludeVersion); } From 83c4ddfba23f5cb5610d253c70f2e1f8d4b9b0f5 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 11 Nov 2026 13:44:43 -0800 Subject: [PATCH 3/8] add one more file name reduction strategy --- doc/Settings.md | 2 +- .../1.31/2036-shorter-installer-log-paths.md | 2 +- .../ShellExecuteInstallerHandler.cpp | 27 +++++++++- src/AppInstallerCLITests/InstallFlow.cpp | 52 +++++++++++++++++-- 4 files changed, 75 insertions(+), 8 deletions(-) diff --git a/doc/Settings.md b/doc/Settings.md index aa073ae367..9e795b3284 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -320,7 +320,7 @@ In addition, there are special values that cover multiple channels. `default` i Sets the default strategy for naming log files for installers that support it. `manifest` is the default and uses the manifest name. `timestamp` uses the date and time. `guid` uses a generated GUID. `shortguid` uses the first 8 characters of a generated GUID. Invalid values will revert to `manifest`. -The `manifest` strategy targets a complete log path of at most 150 UTF-16 code units, including the directory and `.log` extension. Existing names are retained when they fit. Otherwise, WinGet omits the package version and uses a compact `YYMMDDhhmmss` timestamp. If that still exceeds the preferred length, it falls back to an eight-character random name. The complete package identity and selected path remain available in WinGet's diagnostic log. +The `manifest` strategy targets a complete log path of at most 150 UTF-16 code units, including the directory and `.log` extension. Existing names are retained when they fit. Otherwise, WinGet omits the package version and uses a compact `YYMMDDhhmmss` timestamp. If that still exceeds the preferred length, it uses an `hhmmss` timestamp and retains as much of the start of the package ID as fits, preserving whole Unicode characters. If no package-ID prefix fits, it falls back to an eight-character random name. The complete package identity and selected path remain available in WinGet's diagnostic log. This leaves additional room for installers that append text to the supplied log name, but suffix compatibility is best effort, not guaranteed. The log directory is unchanged. Other configured naming strategies retain their formats when they fit within the Windows path limit. Any generated path that reaches `MAX_PATH` falls back to the eight-character random name. Shortening a path or exceeding the preferred length does not produce a user-visible warning. If even the minimal name cannot fit within `MAX_PATH`, WinGet warns and continues without passing the installer logging switch. diff --git a/doc/release-notes/1.31/2036-shorter-installer-log-paths.md b/doc/release-notes/1.31/2036-shorter-installer-log-paths.md index 248a93cc8c..2a1e4e5dd8 100644 --- a/doc/release-notes/1.31/2036-shorter-installer-log-paths.md +++ b/doc/release-notes/1.31/2036-shorter-installer-log-paths.md @@ -2,6 +2,6 @@ ### Shorter automatic installer log paths -WinGet now shortens default installer log names as needed to target a complete path of 150 characters, leaving more room for installers that append their own log names. Names retain the package version and timestamp when they fit; otherwise, WinGet uses a compact timestamp and omits the version, or falls back to a short random name. +WinGet now shortens default installer log names as needed to target a complete path of 150 characters, leaving more room for installers that append their own log names. Names retain the package version and timestamp when they fit; otherwise, WinGet omits the version and uses a compact timestamp, then a package-ID prefix with a time-only timestamp, before falling back to a short random name. This is a best-effort mitigation, not a guarantee for installer-added suffixes. The log directory and explicit log paths are unchanged. If even a minimal automatic name cannot fit within the Windows path limit, WinGet warns and continues without requesting an installer log file. diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp index 2a2e3e9b16..9a8ec5c36d 100644 --- a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp +++ b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -190,9 +190,32 @@ namespace AppInstaller::CLI::Workflow auto compactTimestamp = timestamp; compactTimestamp.erase(std::remove(compactTimestamp.begin(), compactTimestamp.end(), '-'), compactTimestamp.end()); const auto suffix = L"-" + Utility::ConvertToUTF16(compactTimestamp); - result = makePath(GetPathPart(manifest, '.', PathPartVersionStrategy::NeverIncludeVersion).native() + suffix); + const auto id = GetPathPart(manifest, '.', PathPartVersionStrategy::NeverIncludeVersion); + result = makePath(id.native() + suffix); - if (result.native().size() >= preferredPathLength) + if (result.native().size() > preferredPathLength) + { + const auto timeSuffix = L"-" + Utility::ConvertToUTF16(compactTimestamp.substr(6)); + const auto fixedLength = makePath(timeSuffix).native().size(); + if (fixedLength < preferredPathLength && !id.empty()) + { + const auto idString = id.u8string(); + const auto idBudget = preferredPathLength - fixedLength; + auto prefixLength = idBudget; + auto prefix = Utility::ConvertToUTF16(Utility::UTF8Substring(idString, 0, prefixLength)); + // Substring counts graphemes, but the path budget counts UTF-16 code units. + while (prefix.size() > idBudget) + { + prefix = Utility::ConvertToUTF16(Utility::UTF8Substring(idString, 0, --prefixLength)); + } + if (!prefix.empty()) + { + result = makePath(prefix + timeSuffix); + } + } + } + + if (result.native().size() > preferredPathLength) { result = shortGuidPath(); } diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index 4ac6c5e336..401c5fff01 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -1155,7 +1155,9 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBudget", "[InstallFlow][workf manifest.Version = "10.0.404"; size_t directoryLength = 95; bool shortened = false; + bool timeOnly = false; bool shortGuid = false; + std::wstring expectedPrefix; SECTION("ExactlyPreferredLength") { @@ -1165,10 +1167,22 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBudget", "[InstallFlow][workf directoryLength = 96; shortened = true; } + SECTION("ExactlyPreferredCompactLength") + { + directoryLength = 109; + shortened = true; + } + SECTION("TimeOnlyWithoutTruncation") + { + directoryLength = 110; + shortened = timeOnly = true; + expectedPrefix = L"Microsoft.DotNet.SDK.10"; + } SECTION("ReportedNestedSuffixes") { directoryLength = 119; - shortGuid = true; + shortened = timeOnly = true; + expectedPrefix = L"Microsoft.DotNet.SD"; } SECTION("LongVersion") { @@ -1184,12 +1198,33 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBudget", "[InstallFlow][workf } manifest.Id = id + ".SDK"; directoryLength = 119; - shortGuid = true; + shortened = timeOnly = true; + expectedPrefix = L"Publisher.\U0001F600\U0001F600\U0001F600\U0001F600"; } SECTION("LongPackageIdentity") { manifest.Id = std::string(200, 'a') + ".SDK"; directoryLength = 119; + shortened = timeOnly = true; + expectedPrefix = std::wstring(19, L'a'); + } + SECTION("CombiningCharacterBoundary") + { + manifest.Id = "Publisher.e\xCC\x81.SDK"; + directoryLength = 127; + shortened = timeOnly = true; + expectedPrefix = L"Publisher."; + } + SECTION("OneCharacterPrefix") + { + directoryLength = 137; + shortened = timeOnly = true; + expectedPrefix = L"M"; + } + SECTION("NoRoomForFirstGrapheme") + { + manifest.Id = "\xF0\x9F\x98\x80.SDK"; + directoryLength = 137; shortGuid = true; } @@ -1222,9 +1257,18 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBudget", "[InstallFlow][workf const auto timestampStart = stem.rfind('-'); REQUIRE(timestampStart != std::string::npos); const auto timestamp = stem.substr(timestampStart + 1); - REQUIRE(timestamp.size() == 12); + REQUIRE(timestamp.size() == (timeOnly ? 6 : 12)); REQUIRE(std::all_of(timestamp.begin(), timestamp.end(), [](char c) { return c >= '0' && c <= '9'; })); REQUIRE(stem.find(manifest.Version) == std::string::npos); + if (timeOnly) + { + REQUIRE(stem.substr(0, timestampStart) == ConvertToUTF8(expectedPrefix)); + REQUIRE(path.native().size() == directoryLength + 1 + expectedPrefix.size() + 11); + } + else + { + REQUIRE(stem.substr(0, timestampStart) == GetPathPart(manifest, '.', PathPartVersionStrategy::NeverIncludeVersion).u8string()); + } } else { @@ -1250,7 +1294,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathFallback", "[InstallFlow][wor CAPTURE(strategy, directoryLength); InstallerLogDirectoryOverride logDirectory{ directoryLength }; TestUserSettings testSettings; - testSettings.Set(strategy); + testSettings.Set(LogNameStrategy{ strategy }); std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; auto previousThreadGlobals = context.SetForCurrentThread(); From 40b82078456d430f25da75fe4bd126cdb551a926 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 11 Nov 2026 13:57:08 -0800 Subject: [PATCH 4/8] test fix --- src/AppInstallerCLITests/InstallFlow.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index 401c5fff01..ac8ada6c7f 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -1208,11 +1208,19 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBudget", "[InstallFlow][workf shortened = timeOnly = true; expectedPrefix = std::wstring(19, L'a'); } - SECTION("CombiningCharacterBoundary") + SECTION("NormalizedPackageIdPrefix") { manifest.Id = "Publisher.e\xCC\x81.SDK"; directoryLength = 127; shortened = timeOnly = true; + expectedPrefix = L"Publisher.\u00E9"; + } + SECTION("CombiningCharacterBoundary") + { + // Unlike e + acute, x + acute remains two code units after normalization. + manifest.Id = "Publisher.x\xCC\x81.SDK"; + directoryLength = 127; + shortened = timeOnly = true; expectedPrefix = L"Publisher."; } SECTION("OneCharacterPrefix") From be139cff958ec6c4d1f6660ec72e688a4dda2845 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 11 Nov 2026 14:34:00 -0800 Subject: [PATCH 5/8] spelling --- .github/actions/spelling/patterns.txt | 3 +++ src/AppInstallerCLITests/InstallFlow.cpp | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/actions/spelling/patterns.txt b/.github/actions/spelling/patterns.txt index 529cbc6a5b..280a58d2b4 100644 --- a/.github/actions/spelling/patterns.txt +++ b/.github/actions/spelling/patterns.txt @@ -9,6 +9,9 @@ REQUIRE(?:[_A-Z]*)\(.*\) 8wekyb3d8bbwe _tisf_sqliteReturnValue +# Compact installer log timestamp format +\bYYMMDDhhmmss\b + El proyecto .* diferentes # Package family names and package full names diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index ac8ada6c7f..3d711ad798 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -1364,7 +1364,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBypass", "[InstallFlow][workf } SECTION("NoLogToken") { - installer.Switches[InstallerSwitchType::Log] = "/fixedlog"; + installer.Switches[InstallerSwitchType::Log] = "/fixedLog"; } context.Add(manifest); context.Add(installer); @@ -1384,7 +1384,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBypass", "[InstallFlow][workf } else { - REQUIRE(args.find("/fixedlog") != std::string::npos); + REQUIRE(args.find("/fixedLog") != std::string::npos); } REQUIRE(installOutput.str().empty()); } From 6540534a870be304beaa408fd0dc7c47701413db Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 11 Nov 2026 17:03:06 -0800 Subject: [PATCH 6/8] build fix and always attempt with shortGuid --- doc/Settings.md | 2 +- .../1.31/2036-shorter-installer-log-paths.md | 2 +- src/AppInstallerCLICore/Resources.h | 1 - .../ShellExecuteInstallerHandler.cpp | 65 ++++++------------- .../Shared/Strings/en-us/winget.resw | 4 -- src/AppInstallerCLITests/InstallFlow.cpp | 27 ++++---- 6 files changed, 33 insertions(+), 68 deletions(-) diff --git a/doc/Settings.md b/doc/Settings.md index 9e795b3284..9967c0d1ac 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -322,7 +322,7 @@ Sets the default strategy for naming log files for installers that support it. ` The `manifest` strategy targets a complete log path of at most 150 UTF-16 code units, including the directory and `.log` extension. Existing names are retained when they fit. Otherwise, WinGet omits the package version and uses a compact `YYMMDDhhmmss` timestamp. If that still exceeds the preferred length, it uses an `hhmmss` timestamp and retains as much of the start of the package ID as fits, preserving whole Unicode characters. If no package-ID prefix fits, it falls back to an eight-character random name. The complete package identity and selected path remain available in WinGet's diagnostic log. -This leaves additional room for installers that append text to the supplied log name, but suffix compatibility is best effort, not guaranteed. The log directory is unchanged. Other configured naming strategies retain their formats when they fit within the Windows path limit. Any generated path that reaches `MAX_PATH` falls back to the eight-character random name. Shortening a path or exceeding the preferred length does not produce a user-visible warning. If even the minimal name cannot fit within `MAX_PATH`, WinGet warns and continues without passing the installer logging switch. +This leaves additional room for installers that append text to the supplied log name, but suffix compatibility is best effort, not guaranteed. The log directory is unchanged. Other configured naming strategies retain their formats when they fit within the Windows path limit. Any generated path that reaches `MAX_PATH` falls back to the eight-character random name. WinGet passes that fallback path to the installer even if it still reaches `MAX_PATH`; it does not omit arguments or display a warning because of the path length. Explicit log paths, such as those supplied with `--log`, are not shortened or omitted by this policy. For configuration runs, `WinGetUserSettings` can select `logging.fileNameStrategy` for the execution user's settings; `WinGetPackage` does not expose a per-package log-path property. diff --git a/doc/release-notes/1.31/2036-shorter-installer-log-paths.md b/doc/release-notes/1.31/2036-shorter-installer-log-paths.md index 2a1e4e5dd8..b9cd0bd32a 100644 --- a/doc/release-notes/1.31/2036-shorter-installer-log-paths.md +++ b/doc/release-notes/1.31/2036-shorter-installer-log-paths.md @@ -4,4 +4,4 @@ WinGet now shortens default installer log names as needed to target a complete path of 150 characters, leaving more room for installers that append their own log names. Names retain the package version and timestamp when they fit; otherwise, WinGet omits the version and uses a compact timestamp, then a package-ID prefix with a time-only timestamp, before falling back to a short random name. -This is a best-effort mitigation, not a guarantee for installer-added suffixes. The log directory and explicit log paths are unchanged. If even a minimal automatic name cannot fit within the Windows path limit, WinGet warns and continues without requesting an installer log file. +This is a best-effort mitigation, not a guarantee for installer-added suffixes. The log directory and explicit log paths are unchanged. WinGet still passes the shortest fallback path to the installer if it exceeds the Windows path limit, without removing installer arguments. diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index a821582edb..2e378e4463 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -396,7 +396,6 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(InstallerHashMismatchOverrideRequired); WINGET_DEFINE_RESOURCE_STRINGID(InstallerHashVerified); WINGET_DEFINE_RESOURCE_STRINGID(InstallerLogAvailable); - WINGET_DEFINE_RESOURCE_STRINGID(InstallerLogUnavailable); WINGET_DEFINE_RESOURCE_STRINGID(InstallerProhibitsElevation); WINGET_DEFINE_RESOURCE_STRINGID(InstallerRequiresInstallLocation); WINGET_DEFINE_RESOURCE_STRINGID(InstallersAbortTerminal); diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp index 9a8ec5c36d..62dbc9b346 100644 --- a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp +++ b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -73,14 +73,14 @@ namespace AppInstaller::CLI::Workflow } // Gets the escaped installer args. - std::vector GetInstallerArgsTemplate(Execution::Context& context) + std::string GetInstallerArgsTemplate(Execution::Context& context) { bool isUpdate = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate); bool isRepair = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseRepair); const auto& installer = context.Get(); const auto& installerSwitches = installer->Switches; - std::vector installerArgs; + std::string installerArgs; // Construct install experience arg. // SilentWithProgress is default, so look for it first. @@ -106,13 +106,13 @@ namespace AppInstaller::CLI::Workflow if (experienceArgsItr != installerSwitches.end()) { - installerArgs.emplace_back(experienceArgsItr->second); + installerArgs += experienceArgsItr->second; } // Construct log path arg. if (installerSwitches.find(InstallerSwitchType::Log) != installerSwitches.end()) { - installerArgs.emplace_back(' ' + installerSwitches.at(InstallerSwitchType::Log)); + installerArgs += ' ' + installerSwitches.at(InstallerSwitchType::Log); } // Construct repair arg. Custom switches and other args are not applicable for repair scenario so we can return here. @@ -120,7 +120,7 @@ namespace AppInstaller::CLI::Workflow { if (installerSwitches.find(InstallerSwitchType::Repair) != installerSwitches.end()) { - installerArgs.emplace_back(' ' + installerSwitches.at(InstallerSwitchType::Repair)); + installerArgs += ' ' + installerSwitches.at(InstallerSwitchType::Repair); } return installerArgs; @@ -129,7 +129,7 @@ namespace AppInstaller::CLI::Workflow // Construct custom arg. if (installerSwitches.find(InstallerSwitchType::Custom) != installerSwitches.end()) { - installerArgs.emplace_back(' ' + installerSwitches.at(InstallerSwitchType::Custom)); + installerArgs += ' ' + installerSwitches.at(InstallerSwitchType::Custom); } // Construct custom arg passed in by cli arg @@ -139,27 +139,27 @@ namespace AppInstaller::CLI::Workflow // Since these arguments are appended to the installer at runtime, it doesn't make sense to append them if empty or whitespace if (!Utility::IsEmptyOrWhitespace(customSwitches)) { - installerArgs.emplace_back(' ' + std::string{ customSwitches }); + installerArgs += ' ' + std::string{ customSwitches }; } } // Construct update arg if applicable if (isUpdate && installerSwitches.find(InstallerSwitchType::Update) != installerSwitches.end()) { - installerArgs.emplace_back(' ' + installerSwitches.at(InstallerSwitchType::Update)); + installerArgs += ' ' + installerSwitches.at(InstallerSwitchType::Update); } // Construct install location arg if necessary. if (context.Args.Contains(Execution::Args::Type::InstallLocation) && installerSwitches.find(InstallerSwitchType::InstallLocation) != installerSwitches.end()) { - installerArgs.emplace_back(' ' + installerSwitches.at(InstallerSwitchType::InstallLocation)); + installerArgs += ' ' + installerSwitches.at(InstallerSwitchType::InstallLocation); } return installerArgs; } - std::optional GetAutomaticInstallerLogPath(Execution::Context& context) + std::filesystem::path GetAutomaticInstallerLogPath(Execution::Context& context) { // Chosen to allow a reported 108 character suffix from installer. constexpr size_t preferredPathLength = 150; @@ -241,12 +241,6 @@ namespace AppInstaller::CLI::Workflow { result = shortGuidPath(); } - if (result.native().size() >= MAX_PATH) - { - AICLI_LOG(CLI, Warning, << "No automatic installer log path fits within MAX_PATH in " << directory.u8string()); - context.Reporter.Warn() << Resource::String::InstallerLogUnavailable << std::endl; - return std::nullopt; - } if (result.native().size() > preferredPathLength) { AICLI_LOG(CLI, Warning, << "Installer log path exceeds the preferred length of " << preferredPathLength << ": " << result.u8string()); @@ -255,13 +249,12 @@ namespace AppInstaller::CLI::Workflow return result; } - // Applies values to the template, retaining fragments so an unavailable log switch can be omitted. - void PopulateInstallerArgsTemplate(Execution::Context& context, std::vector& installerArgs) + // Applies values to the template. + void PopulateInstallerArgsTemplate(Execution::Context& context, std::string& installerArgs) { - const auto usesLogPath = [](const std::string& arg) { return arg.find(ARG_TOKEN_LOGPATH) != std::string::npos; }; - if (std::any_of(installerArgs.begin(), installerArgs.end(), usesLogPath)) + if (installerArgs.find(ARG_TOKEN_LOGPATH) != std::string::npos) { - std::optional logPath; + std::filesystem::path logPath; if (context.Args.Contains(Execution::Args::Type::Log)) { logPath = Utility::ConvertToUTF16(context.Args.GetArg(Execution::Args::Type::Log)); @@ -271,29 +264,16 @@ namespace AppInstaller::CLI::Workflow logPath = GetAutomaticInstallerLogPath(context); } - if (logPath) - { - auto logPathString = logPath->u8string(); - for (auto& arg : installerArgs) - { - Utility::FindAndReplace(arg, ARG_TOKEN_LOGPATH, logPathString); - } - context.Add(std::move(*logPath)); - } - else - { - installerArgs.erase(std::remove_if(installerArgs.begin(), installerArgs.end(), usesLogPath), installerArgs.end()); - } + auto logPathString = logPath.u8string(); + Utility::FindAndReplace(installerArgs, ARG_TOKEN_LOGPATH, logPathString); + context.Add(std::move(logPath)); } // Populate with value from command line. if (context.Args.Contains(Execution::Args::Type::InstallLocation)) { auto locationArg = context.Args.GetArg(Execution::Args::Type::InstallLocation); - for (auto& arg : installerArgs) - { - Utility::FindAndReplace(arg, ARG_TOKEN_INSTALLPATH, locationArg); - } + Utility::FindAndReplace(installerArgs, ARG_TOKEN_INSTALLPATH, locationArg); } // Todo: language token support will be implemented later @@ -408,13 +388,8 @@ namespace AppInstaller::CLI::Workflow return; } - auto installerArgsTemplate = GetInstallerArgsTemplate(context); - PopulateInstallerArgsTemplate(context, installerArgsTemplate); - std::string installerArgs; - for (const auto& arg : installerArgsTemplate) - { - installerArgs += arg; - } + auto installerArgs = GetInstallerArgsTemplate(context); + PopulateInstallerArgsTemplate(context, installerArgs); AICLI_LOG(CLI, Info, << "Installer args: " << installerArgs); context.Add(std::move(installerArgs)); diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 069a915921..53e3bba57d 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1370,10 +1370,6 @@ Do you agree to the terms? Installer log is available at: {0} {Locked="{0}"} Message displayed to inform the user about the system path of a diagnostic files containing information about the installer. {0} is a placeholder replaced by the diagnostic file system path. - - The installer log path is too long. Continuing without requesting an installer log file. - Warning when even a minimal automatically generated installer log file name cannot fit within the Windows path limit. The operation continues with the logging switch omitted; the installer may still produce its own logs. - The following packages were found among the working sources. Please specify one of them using the --source option to proceed. diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index 3d711ad798..5575b13ec4 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -1265,7 +1265,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBudget", "[InstallFlow][workf const auto timestampStart = stem.rfind('-'); REQUIRE(timestampStart != std::string::npos); const auto timestamp = stem.substr(timestampStart + 1); - REQUIRE(timestamp.size() == (timeOnly ? 6 : 12)); + REQUIRE(timestamp.size() == (timeOnly ? 6u : 12u)); REQUIRE(std::all_of(timestamp.begin(), timestamp.end(), [](char c) { return c >= '0' && c <= '9'; })); REQUIRE(stem.find(manifest.Version) == std::string::npos); if (timeOnly) @@ -1307,6 +1307,8 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathFallback", "[InstallFlow][wor TestContext context{ installOutput, std::cin }; auto previousThreadGlobals = context.SetForCurrentThread(); auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_WithSwitches.yaml")); + manifest.Installers.at(0).Switches[InstallerSwitchType::Log] = "/before /mylog=\"\" /after"; + manifest.Installers.at(0).Switches[InstallerSwitchType::Custom] = "/mycustom /log=\"\""; context.Add(manifest); context.Add(manifest.Installers.at(0)); context.Args.AddArg(Execution::Args::Type::InstallLocation, "MyDir"sv); @@ -1324,21 +1326,14 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathFallback", "[InstallFlow][wor REQUIRE(args.find("/mysilentwithprogress") != std::string::npos); REQUIRE(args.find("/mycustom") != std::string::npos); REQUIRE(args.find("/myinstalldir=\"MyDir\"") != std::string::npos); - if (directoryLength + 1 + 12 >= MAX_PATH) - { - REQUIRE_FALSE(context.Contains(Data::LogPath)); - REQUIRE(args.find("/mylog") == std::string::npos); - REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::InstallerLogUnavailable).get()) != std::string::npos); - } - else - { - const auto& path = context.Get(); - const size_t stemLength = strategy == LogNameStrategy::Guid ? 36 : (strategy == LogNameStrategy::Timestamp ? 17 : 8); - const size_t configuredLength = directoryLength + 1 + stemLength + 4; - REQUIRE(path.native().size() == (configuredLength < MAX_PATH ? configuredLength : directoryLength + 1 + 12)); - REQUIRE(args.find("/mylog=\"" + path.u8string() + '"') != std::string::npos); - REQUIRE(installOutput.str().empty()); - } + REQUIRE(context.Contains(Data::LogPath)); + const auto& path = context.Get(); + const size_t stemLength = strategy == LogNameStrategy::Guid ? 36 : (strategy == LogNameStrategy::Timestamp ? 17 : 8); + const size_t configuredLength = directoryLength + 1 + stemLength + 4; + REQUIRE(path.native().size() == (configuredLength < MAX_PATH ? configuredLength : directoryLength + 1 + 12)); + REQUIRE(args.find("/before /mylog=\"" + path.u8string() + "\" /after") != std::string::npos); + REQUIRE(args.find("/mycustom /log=\"" + path.u8string() + '"') != std::string::npos); + REQUIRE(installOutput.str().empty()); } TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathBypass", "[InstallFlow][workflow][InstallerLogPath]") From 493dba8775a050e6c02d05ee288be1b0342d9100 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 11 Nov 2026 17:14:26 -0800 Subject: [PATCH 7/8] spells --- src/AppInstallerCLITests/InstallFlow.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index 5575b13ec4..370413ef93 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -1307,14 +1307,14 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathFallback", "[InstallFlow][wor TestContext context{ installOutput, std::cin }; auto previousThreadGlobals = context.SetForCurrentThread(); auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_WithSwitches.yaml")); - manifest.Installers.at(0).Switches[InstallerSwitchType::Log] = "/before /mylog=\"\" /after"; - manifest.Installers.at(0).Switches[InstallerSwitchType::Custom] = "/mycustom /log=\"\""; + manifest.Installers.at(0).Switches[InstallerSwitchType::Log] = "/before /myLog=\"\" /after"; + manifest.Installers.at(0).Switches[InstallerSwitchType::Custom] = "/myCustom /log=\"\""; context.Add(manifest); context.Add(manifest.Installers.at(0)); context.Args.AddArg(Execution::Args::Type::InstallLocation, "MyDir"sv); context.Override({ ShellExecuteInstallImpl, [](TestContext& context) { - REQUIRE(context.Get().find("/mycustom") != std::string::npos); + REQUIRE(context.Get().find("/myCustom") != std::string::npos); context.Add(0); } }); context << GetInstallerArgs << ShellExecuteInstallImpl; @@ -1324,15 +1324,15 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs_LogPathFallback", "[InstallFlow][wor const auto& args = context.Get(); REQUIRE(args.find("") == std::string::npos); REQUIRE(args.find("/mysilentwithprogress") != std::string::npos); - REQUIRE(args.find("/mycustom") != std::string::npos); + REQUIRE(args.find("/myCustom") != std::string::npos); REQUIRE(args.find("/myinstalldir=\"MyDir\"") != std::string::npos); REQUIRE(context.Contains(Data::LogPath)); const auto& path = context.Get(); const size_t stemLength = strategy == LogNameStrategy::Guid ? 36 : (strategy == LogNameStrategy::Timestamp ? 17 : 8); const size_t configuredLength = directoryLength + 1 + stemLength + 4; REQUIRE(path.native().size() == (configuredLength < MAX_PATH ? configuredLength : directoryLength + 1 + 12)); - REQUIRE(args.find("/before /mylog=\"" + path.u8string() + "\" /after") != std::string::npos); - REQUIRE(args.find("/mycustom /log=\"" + path.u8string() + '"') != std::string::npos); + REQUIRE(args.find("/before /myLog=\"" + path.u8string() + "\" /after") != std::string::npos); + REQUIRE(args.find("/myCustom /log=\"" + path.u8string() + '"') != std::string::npos); REQUIRE(installOutput.str().empty()); } From 630ead99285f2b5636110c03cf6942941e434f08 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Thu, 12 Nov 2026 10:39:18 -0800 Subject: [PATCH 8/8] test fixes --- src/AppInstallerCLIE2ETests/Pinning.cs | 7 +++---- src/AppInstallerCLITests/InstallFlow.cpp | 10 +++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/AppInstallerCLIE2ETests/Pinning.cs b/src/AppInstallerCLIE2ETests/Pinning.cs index 5711a0e911..81414ef515 100644 --- a/src/AppInstallerCLIE2ETests/Pinning.cs +++ b/src/AppInstallerCLIE2ETests/Pinning.cs @@ -25,7 +25,7 @@ public void Setup() // All tests use TestExeInstaller; try to clean it up for failure cases, // then install the base version for pinning TestCommon.RunAICLICommand("uninstall", "AppInstallerTest.TestExeInstaller"); - TestCommon.RunAICLICommand("install", "AppInstallerTest.TestExeInstaller -v 1.0.0.0"); + TestCommon.RunAICLICommand("install", "AppInstallerTest.TestExeInstaller -v 1.0.0.0 --custom \"/Version 1.0.0.0\""); TestCommon.RunAICLICommand("pin remove", "AppInstallerTest.TestExeInstaller"); } @@ -71,8 +71,7 @@ public void UpgradeWithPinningPin() RunCommandResult result; string installDir = Path.GetTempPath(); - // The base version of this app does not log /Version, but it still includes the version number in the log file name. - Assert.That(TestCommon.VerifyTestExeInstalled(installDir, "1.0.0.0"), Is.True, "Base version installed"); + Assert.That(TestCommon.VerifyTestExeInstalled(installDir, "/Version 1.0.0.0"), Is.True, "Base version installed"); result = TestCommon.RunAICLICommand("pin add", "AppInstallerTest.TestExeInstaller"); Assert.That(result.ExitCode, Is.EqualTo(Constants.ErrorCode.S_OK)); @@ -89,7 +88,7 @@ public void UpgradeWithPinningPin() result = TestCommon.RunAICLICommand("upgrade", "--all"); Assert.That(result.ExitCode, Is.EqualTo(Constants.ErrorCode.S_OK), "Upgrade succeeds with nothing to upgrade"); - Assert.That(TestCommon.VerifyTestExeInstalled(installDir, "1.0.0.0"), Is.True, "No newer version installed"); + Assert.That(TestCommon.VerifyTestExeInstalled(installDir, "/Version 1.0.0.0"), Is.True, "No newer version installed"); result = TestCommon.RunAICLICommand("upgrade", "--include-pinned"); Assert.That(result.ExitCode, Is.EqualTo(Constants.ErrorCode.S_OK)); diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index 370413ef93..69a62968ff 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -885,8 +885,12 @@ TEST_CASE("PortableInstallFlow_MachineScope", "[InstallFlow][workflow]") REQUIRE(std::filesystem::exists(portableInstallResultPath.GetPath())); } -TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]") +TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow][InstallerLogPath]") { + InstallerLogDirectoryOverride logDirectory{ 7 }; + TestUserSettings testSettings; + testSettings.Set(LogNameStrategy::Manifest); + { std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; @@ -899,7 +903,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]") context << GetInstallerArgs; std::string installerArgs = context.Get(); REQUIRE(installerArgs.find("/passive") != std::string::npos); - REQUIRE(installerArgs.find(FileLogger::DefaultPrefix()) != std::string::npos); + REQUIRE(installerArgs.find(context.Get().u8string()) != std::string::npos); REQUIRE(installerArgs.find(manifest.Id) != std::string::npos); REQUIRE(installerArgs.find(manifest.Version) != std::string::npos); } @@ -953,7 +957,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]") context << GetInstallerArgs; std::string installerArgs = context.Get(); REQUIRE(installerArgs.find("/SILENT") != std::string::npos); - REQUIRE(installerArgs.find(FileLogger::DefaultPrefix()) != std::string::npos); + REQUIRE(installerArgs.find(context.Get().u8string()) != std::string::npos); REQUIRE(installerArgs.find(manifest.Id) != std::string::npos); REQUIRE(installerArgs.find(manifest.Version) != std::string::npos); }