-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add installed files and startup link metadata collection in Installer Metadata collection util #2985
Conversation
constexpr std::string_view s_ShellLinkFileExtension = ".lnk"sv; | ||
const std::vector<std::pair<std::filesystem::path, std::string>> s_CandidateInstallLocationRoots = | ||
{ | ||
{ Filesystem::GetExpandedPath("%LOCALAPPDATA%"), "%LOCALAPPDATA%" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer using known folders APIs over environment variables, but this will probably do.
if (relativePath) | ||
{ | ||
// TODO: Here we assume the install location is the top directory of relative path. | ||
auto installLocation = baseFolder / *relativePath->begin(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for any of the base locations it is not uncommon for there to be a structure like base\publisher\application
.
I think the only way to be able to know the real location is to watch these base locations and then score new directories based on new file count, where links are pointing, etc.
Of course, there isn't a good way to distinguish between the publisher
and publisher\application
directories unless the publisher directory is new and contains no other directories/files itself.
{ | ||
// Try to match the candidate install location roots first. | ||
std::string installLocationString = installLocation.string(); | ||
for (auto const& entry : s_CandidateInstallLocationRoots) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a function to do this: ReplaceCommonPathPrefix
in Filesystem.h
. Could make it return a bool to indicate whether it replaced or not for your use here.
|
||
InstalledFilesCorrelation::InstalledFilesCorrelation() | ||
{ | ||
m_fileWatchers.emplace_back(Filesystem::GetKnownFolderPath(FOLDERID_CommonStartMenu), std::string{ s_ShellLinkFileExtension }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a convenience or are we truly required to have two watchers to watch two locations?
fileEntry.RelativeFilePath = relativePath->string(); | ||
std::ifstream in{ linkInfo->Path, std::ifstream::binary }; | ||
fileEntry.FileSha256 = Utility::SHA256::ComputeHash(in); | ||
fileEntry.InvocationParameter = linkInfo->Args; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that we will want to simply invoke the shortcuts rather than directly running the executables, so I don't think that collecting this is necessary. But at the same time, it probably doesn't hurt.
Tested manually and added unit tests for merging related logic.
Microsoft Reviewers: Open in CodeFlow