Ensure default values are passed to nested installers within zip #2413
Conversation
ghost
added
the
Issue-Bug
| @@ -183,7 +183,7 @@ namespace AppInstaller::CLI::Workflow | |||
| void CheckForExistingInstaller(Execution::Context& context) | |||
| { | |||
| const auto& installer = context.Get<Execution::Data::Installer>().value(); | |||
| if (installer.InstallerType == InstallerTypeEnum::MSStore) | |||
| if (installer.EffectiveInstallerType() == InstallerTypeEnum::MSStore) | |||
There was a problem hiding this comment.
[This is the correct usage here just calling this out if it isn't already done...]
As MSStore is not an installer type that actually has an installer file (to us), it should not be a valid nested type. Is that already handled elsewhere?
There was a problem hiding this comment.
The only check I had was from manifest validation. I added another check prior to the install flow to verify that if the NestedInstallertype is valid if the BaseInstallertype is an archive. Outputs the appropriate error message if NestedInstallertype is not supported.
| @@ -173,7 +173,7 @@ namespace AppInstaller::CLI::Workflow | |||
| info << Execution::ManifestInfoEmphasis << Resource::String::ShowLabelInstaller << std::endl; | |||
| if (installer) | |||
| { | |||
| info << " "_liv << Execution::ManifestInfoEmphasis << Resource::String::ShowLabelInstallerType << ' ' << Manifest::InstallerTypeToString(installer->InstallerType) << std::endl; | |||
| info << " "_liv << Execution::ManifestInfoEmphasis << Resource::String::ShowLabelInstallerType << ' ' << Manifest::InstallerTypeToString(installer->BaseInstallerType) << std::endl; | |||
There was a problem hiding this comment.
I think that this should be something like:
Effective (Base)
When effective and base are not the same. The important part is the effective type, but also showing the base is useful.
There was a problem hiding this comment.
Changed to suggested.
If effective and base are not the same, will show both "Effective (Base)". Added tests to verify output
| @@ -158,7 +158,7 @@ namespace AppInstaller::Repository::Rest::Schema::V1_1::Json | |||
| } | |||
|
|
|||
| // Populate installer default return codes if not present in ExpectedReturnCodes and InstallerSuccessCodes | |||
| auto defaultReturnCodes = GetDefaultKnownReturnCodes(installer.InstallerType); | |||
| auto defaultReturnCodes = GetDefaultKnownReturnCodes(installer.BaseInstallerType); | |||
There was a problem hiding this comment.
Effective, unless it is not yet know, then some more effort will be required.
There was a problem hiding this comment.
Changed to effective
| @@ -366,7 +366,7 @@ namespace AppInstaller::Repository::Rest::Schema::V1_0::Json | |||
| } | |||
|
|
|||
| // Installer Switches | |||
| installer.Switches = Manifest::GetDefaultKnownSwitches(installer.InstallerType); | |||
| installer.Switches = Manifest::GetDefaultKnownSwitches(installer.BaseInstallerType); | |||
There was a problem hiding this comment.
Effective, unless it is not yet know, then some more effort will be required.
There was a problem hiding this comment.
Changed to effective
ghost
added
the
Needs-Author-Feedback
ghost
removed
the
Needs-Author-Feedback
Fixes #2392
Changes:
Tests:
Microsoft Reviewers: Open in CodeFlow