Skip to content
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

Workloads: Additional Checks #7679

Merged
merged 4 commits into from
Aug 6, 2021
Merged

Workloads: Additional Checks #7679

merged 4 commits into from
Aug 6, 2021

Conversation

joeloff
Copy link
Member

@joeloff joeloff commented Jul 27, 2021

This PR includes a number of improvements / fixes around workload generation:

  1. The generated manifest MSI for CLI admin installs contains an incorrect provider key (different from the actual provider key). This change ensures we generate the value once and use it throughout for both the actual MSI and the manifest we create.
  2. Added additional checks to catch potential issues that will only be found when VS performs manifest validation, specifically around relative payload paths being limited. The code tries to mimic it as best as possible and changes to the VS validation logic could make this inaccurate.
  3. Short names for manifest installers are now supported.
  4. Generate a warning when a manifest contains a workload that supports a Windows RID, but none of the dependent packs support Windows. While we won't add the dependency in the generated authoring, it does point to a potential authoring error that should be addressed instead of worked around in the tooling

@joeloff joeloff requested review from pjcollins and chcosta July 27, 2021 22:19
@joeloff joeloff changed the title Make dependency provider key consistent for manifest installers Workloads: Additional Checks Jul 28, 2021
@steveisok steveisok requested a review from directhex July 29, 2021 17:45
@pjcollins
Copy link
Member

@joeloff On the ShortNames front, I'm replacing semver+label version information with the 4part VS component version everywhere to save some characters. I think this replacement only happens the .msi file name, and not the generated component ID? This patch is fully untested - https://gist.github.com/pjcollins/03d48224a1588444de3d854c34b49221, but maybe would be useful in cases where we want to trim without modifying the pack name at all?

@joeloff
Copy link
Member Author

joeloff commented Jul 29, 2021

@pjcollins shortnames impact the package IDs (leaf nodes that wrap MSIs) in VS. We don't want to shorten component IDs because those need to line up with workload IDs so that the resolvers and CPS can acquire them through IPA in VS.

@joeloff
Copy link
Member Author

joeloff commented Aug 3, 2021

Are we good to merge or are there any other concerns?

@joeloff joeloff merged commit 742bf02 into dotnet:main Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants