The Wayback Machine - https://web.archive.org/web/20200930185350/https://github.com/microsoft/winget-cli/pull/296
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

Fix #137: Add SuccessExitCode to ManifestInstaller #296

Open
wants to merge 1 commit into
base: master
from

Conversation

@chausner
Copy link
Contributor

@chausner chausner commented May 22, 2020

This PR suggests a fix for #137 by adding an optional SuccessExitCode attribute to the Installers entries in the manifest. If present, it overrides the default value of zero.

For now, this only allows a single value to be defined as success status code. I am not sure if there are cases where we would want multiple exit codes to count as success.

Right now, SuccessExitCode is not printed by the "show" command but we could add it if desired.

Microsoft Reviewers: Open in CodeFlow
@chausner chausner requested a review from microsoft/windows-package-manager-admins as a code owner May 22, 2020
@JohnMcPMS
Copy link
Member

@JohnMcPMS JohnMcPMS commented May 22, 2020

We have a manifest versioning strategy that isn't really mature in the codebase yet, but we want to ensure we build that muscle/work out the kinks. So any new additions to the manifest would need increment the manifest version and be interpreted appropriately.

In addition, I think we probably do want to support multiple exit codes for success. While the uses are probably low, it would be better to support it from the beginning than to have a versioning problem later on. But this kind of thing is best worked out in a spec (which I think we need to document the process better). See https://github.com/microsoft/winget-cli/projects/1

@chausner
Copy link
Contributor Author

@chausner chausner commented May 22, 2020

Thank you for your feedback. I'd be happy to make changes once the spec is finalized and the details have been clarified.

@benferse
Copy link
Member

@benferse benferse commented May 23, 2020

@JohnMcPMS The canonical example that comes to mind are the "pseudo-success" codes that are returned for installs that require a reboot to complete:

  • ERROR_SUCCESS_REBOOT_REQUIRED
  • ERROR_SUCCESS_REBOOT_INITIATED
  • ERROR_INSTALL_SUSPEND
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.