Normalize nested installer paths - #6552
Kaleb Luedtke (Trenly) wants to merge 4 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
This comment has been minimized.
This comment has been minimized.
| for (const auto& nestedInstallerFile : installer.NestedInstallerFiles) | ||
| { | ||
| const std::filesystem::path& nestedInstallerPath = targetInstallerPath / ConvertToUTF16(nestedInstallerFile.RelativeFilePath); | ||
| std::filesystem::path nestedInstallerPath = targetInstallerPath / ConvertToUTF16(nestedInstallerFile.RelativeFilePath); |
There was a problem hiding this comment.
I would prefer that we normalize them centrally; preferably in a way that makes it impossible to get the original version.
One of:
- Only store as a
pathand normalize when doing so. - Create a wrapper type that effectively forces the above.
- Make the field private and require access through a method that returns the normalized form.
There was a problem hiding this comment.
Created the wrapper that forces storage as path and normalizes when doing so
There was a problem hiding this comment.
This still requires the call site to do the right thing. With a wrapper, I would replace the type of RelativeFilePath so that the normalization is forced at construction (alternately it could be done lazily through the getter).
Also, your wrapper is far more complex than needed. It doesn't need to be a path, just hold a path.
There was a problem hiding this comment.
I guess I'm not fully understanding your suggestion here. Are you saying to make the actual field value in ManifestCommon for RelativeFilePath the NormalizedPath wrapper type?
There was a problem hiding this comment.
That, and to make it a simpler wrapper constructed from a string and holding a path that is normalized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2 participants
📖 Description
Introduces
AppInstaller::Utility::NormalizedPath, astd::filesystem::pathwrapper that converts UTF-8 inputs to UTF-16 and normalizes directory separators using the platform-preferred separator.Uses the wrapper for nested installer paths in archive, font, and portable installer workflows, replacing repeated manual calls to
make_preferred(). The wrapper is structured so additional normalization can be added later.🔗 References
Resolves #6551
🔍 Validation
msbuild src\AppInstallerCommonCore\AppInstallerCommonCore.vcxproj /p:Configuration=Debug /p:Platform=x64 /p:SolutionDir=E:\winget-cli\src\ /m:1 /nodeReuse:false /nologo /v:minimalmsbuild src\AppInstallerCLITests\AppInstallerCLITests.vcxproj /p:Configuration=Debug /p:Platform=x64 /p:SolutionDir=E:\winget-cli\src\ /m:1 /nodeReuse:false /nologo /v:minimalbecause Visual Studio held a lock onAppInstallerCLICore.idb(C1033). Shared and common library compilation completed successfully before the lock failure.NormalizedPathunit coverage for UTF-8 to UTF-16 conversion, preferred separator normalization,std::filesystem::pathcompatibility, and reassignment.✅ Checklist
🤖 AI Assistance
📋 Issue Type