Skip to content

Conversation

@consvc
Copy link
Contributor

@consvc consvc commented May 28, 2025

Checklist for Pull Requests

Manifests

  • Have you checked that there aren't other open pull requests for the same manifest update/change?
  • This PR only modifies one (1) manifest
  • Have you validated your manifest locally with winget validate --manifest <path>?
  • Have you tested your manifest locally with winget install --manifest <path>?
  • Does your manifest conform to the 1.10 schema?

Note: <path> is the directory's name containing the manifest you're submitting.


Microsoft Reviewers: Open in CodeFlow

@wingetbot
Copy link
Collaborator

Service Badge  Service Badge  

@wingetbot
Copy link
Collaborator

Validation Pipeline Run WinGetSvc-Validation-61-261303-20250528-1

@microsoft-github-policy-service microsoft-github-policy-service bot added the Moderator-Approved One of the Moderators has reviewed and approved this PR label May 28, 2025
@wingetbot wingetbot added New-Manifest Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. Validation-Executable-Error labels May 28, 2025
@DHowett
Copy link
Member

DHowett commented May 29, 2025

FWIW the "executable error" validation response is because we don't write a version message out to stdout when redirected. We should, but failing to detect that is a winget validation issue rather than a product issue.

@Frulfump
Copy link
Contributor

FWIW the "executable error" validation response is because we don't write a version message out to stdout when redirected. We should, but failing to detect that is a winget validation issue rather than a product issue.

Could you add issues for those to winget and Edit respectively?

@DHowett
Copy link
Member

DHowett commented May 29, 2025

Well, good and bad news. I was slightly wrong about what the issue is, and we won't be fixing it in Edit.

edit does not produce an exit code when it's launched with no arguments. Because it is a text editor. 🙂

WinGet validation appears to require--or desire?--that a console subsystem application do so.

To quote johnmcp:

It is generally not great for command line utilities since most of the time they "fail" without any params. We are capturing the output to help diagnose those "failures". But when you are relying on that console stdout handle... that is certainly unique.

@JohnMcPMS
Copy link
Member

The issue appears to be that Edit does not like it when it receives a redirected output handle. The process exits with a value of 1 and outputs "Error 0x80070006: The handle is invalid." I can repro locally with edit > err.txt. This is understandable given what it is.

@DHowett
Copy link
Member

DHowett commented May 29, 2025

We can probably make the behavior comprehensible for 1.2, but either way it's gonna just keep running rather than exiting. Vim does that, for example - it just writes the output of the editor screen to the redirected file ;P

@denelon denelon added the Needs-Manual-Merge The pull request requires a manual merge from a repository maintainer label May 29, 2025
@denelon
Copy link
Collaborator

denelon commented May 29, 2025

Manually validated.

@denelon denelon merged commit dbd274f into microsoft:master May 29, 2025
1 check passed
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Attention This work item needs to be reviewed by a member of the core team. and removed Needs-Attention This work item needs to be reviewed by a member of the core team. labels May 29, 2025
@JohnMcPMS
Copy link
Member

We can probably make the behavior comprehensible for 1.2, but either way it's gonna just keep running rather than exiting. Vim does that, for example - it just writes the output of the editor screen to the redirected file ;P

I believe that we are happy as long as the process runs for 10 seconds; the goal is that the package is "functional", not missing static dependencies or the wrong architecture, etc.

@stephengillie
Copy link
Collaborator

Automatic Validation ended with:

Executable C:\Users\validator\AppData\Local\Microsoft\WinGet\Links\edit.exe returned exit code: 1

Hex Dec Inverted Dec Symbol Description
00000001 1 -4294967295 ERROR_INVALID_FUNCTION Incorrect function.

(Automated response - build 1058.)

@stephengillie stephengillie added Validation-Completed Validation passed and removed Validation-Executable-Error Needs-Manual-Merge The pull request requires a manual merge from a repository maintainer labels May 29, 2025
@wingetbot
Copy link
Collaborator

Publish pipeline succeeded for this Pull Request. Once you refresh your index, this change should be present.

@DHowett
Copy link
Member

DHowett commented May 29, 2025

@wingetbot installationmetadata exe add edit.exe --version

1 similar comment
@denelon
Copy link
Collaborator

denelon commented May 29, 2025

@wingetbot installationmetadata exe add edit.exe --version

@denelon
Copy link
Collaborator

denelon commented May 29, 2025

@DHowett, only "maintainers" can call wingetbot to do things. It might not work since the PR has already been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. Moderator-Approved One of the Moderators has reviewed and approved this PR New-Manifest Publish-Pipeline-Succeeded Validation-Completed Validation passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants