-
Notifications
You must be signed in to change notification settings - Fork 383
Correct Package Family Name Construction for Main and Singleton Packages #5729
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
base: main
Are you sure you want to change the base?
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
address PR comments
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
dev/Deployment/DeploymentManager.cpp
Outdated
} | ||
|
||
unsigned int numberBeforeUnderscore; | ||
if (swscanf_s(versionTag.c_str(), L"%*[^0-9]%u", &numberBeforeUnderscore) == 1) |
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.
What's the [^0-9]
do here? Wondering how this differs from "%u"
dev/Deployment/DeploymentManager.cpp
Outdated
else | ||
{ | ||
formattedVersionTag = L""; | ||
} | ||
} | ||
else | ||
{ | ||
// Other version types not supported. | ||
FAIL_FAST_HR(HRESULT_FROM_WIN32(ERROR_UNSUPPORTED_TYPE)); |
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.
FAIL_FAST will crash the process. Seems pretty harsh considering this is an inproc API running in some app's process space, and giving them no chance to note the issue or recover.
Why not THROW_HR_MSG(HRESULT_FROM_WIN32(ERROR_UNSUPPORTED_TYPE), "%d", static_cast<int>(package.versionType));
?
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Summary
This change corrects the construction of the packageFamilyName for the main and singleton packages in DeploymentManager.cpp. Previously, the package family names were being built incorrectly, which caused the DeploymentManager to incorrectly determine that these packages were not installed. As a result, the deployment status was reported as PackageInstallRequired instead of Ok.
Details
• Fixed the logic that formats the version tag and constructs the packageFamilyName for main and singleton packages.
• With the corrected names, DeploymentManager now accurately detects installed packages and returns the correct deployment status.
Testing
The fix was validated in the debugger: after the change, the deployment status is Ok when the packages are present.
A microsoft employee must use /azp run to validate using the pipelines below.
WARNING:
Comments made by azure-pipelines bot maybe inaccurate.
Please see pipeline link to verify that the build is being ran.
For status checks on the main branch, please use TransportPackage-Foundation-PR
(https://microsoft.visualstudio.com/ProjectReunion/_build?definitionId=81063&_a=summary)
and run the build against your PR branch with the default parameters.