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

New package: Rizin.Cutter in version 2.2.1 #109817

Merged
merged 1 commit into from
Jun 15, 2023

Conversation

assarbad
Copy link
Contributor

@assarbad assarbad commented Jun 14, 2023

  • Have you signed the Contributor License Agreement?
  • 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.4 schema?

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


xref: rizinorg/cutter#3056

Microsoft Reviewers: codeflow:open?pullrequest=https://github.com/microsoft/winget-pkgs/pull/109817&drop=dogfoodAlpha

@wingetbot
Copy link
Collaborator

Service Badge  Service Badge  

@wingetbot
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@assarbad
Copy link
Contributor Author

@microsoft-github-policy-service agree

@stephengillie
Copy link
Collaborator

@wingetbot waivers Add Validation-Executable-Error

@wingetbot
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@microsoft-github-policy-service microsoft-github-policy-service bot removed Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. Validation-Executable-Error labels Jun 15, 2023
@wingetbot wingetbot added Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. Validation-Completed Validation passed labels Jun 15, 2023
@microsoft-github-policy-service
Copy link
Contributor

assarbad,

The check-in policies require a moderator to approve PRs from the community.

Our moderators are community volunteers, please be patient and allow them sufficient time to review your submission.

Template: msftbot/requiresApproval/moderator

@microsoft-github-policy-service microsoft-github-policy-service bot added the Moderator-Approved One of the Moderators has reviewed and approved this PR label Jun 15, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot merged commit e826f52 into microsoft:master Jun 15, 2023
@microsoft-github-policy-service
Copy link
Contributor

Hello assarbad,
Validation has completed.

Template: msftbot/validationCompleted

@Trenly
Copy link
Contributor

Trenly commented Jun 15, 2023

@stephengillie / @denelon - I'm not sure the Validation-Executable-Error should have been waived here.

Although it may install and run fine on Windows 11, on Windows 10 machines it has a dependency on MSVCP140 and VCRUNTIME140_1 and VCRUNTIME140 - that is to say, has a dependency on Microsoft.VCRedist.2015+.x64 I would suggest that manual validation should performed on Win10 where possible, and on Win11 only if the MinOsVersion requires it, since that will help avoid issues where Win11 has dll's included that Win10 does not, will identify if a package has a MinOsVersion that needs to be specified and wasn't, and to align with what a majority of enterprise and consumers are still running as Win10 is still much more prevalent than Win11

image
image

After installing the dependency on Win10, it runs fine -
image

@wingetbot
Copy link
Collaborator

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

@denelon
Copy link
Contributor

denelon commented Jun 15, 2023

@Trenly We validate on Windows 11 by default unless the minimum OS version specifies Windows 10 if I'm not mistaken.

@Trenly
Copy link
Contributor

Trenly commented Jun 15, 2023

@Trenly We validate on Windows 11 by default unless the minimum OS version specifies Windows 10 if I'm not mistaken.

And I'm suggesting it should be the other way around - validation should be run on Windows 10 unless the minimum OS specifies Windows 11

@denelon
Copy link
Contributor

denelon commented Jun 15, 2023

Let me confirm with the team.

@Trenly
Copy link
Contributor

Trenly commented Jun 15, 2023

Confirmed with @denelon the Pipelines run Win10 by default and Win11 only if specified - It's possible that manual validation uses Win11 instead of Win10

@denelon
Copy link
Contributor

denelon commented Jun 15, 2023

Sorry for the delay. I was offline for some mandatory in person training.

I was wrong.

We default to a Windows 10 VM during validation unless the manifest states a requirement for Windows 11.

It appears not all users who were performing manual validation were also defaulting to a Windows 10 VM. This will be communicated to the team today and added to our process documentation.

@Trenly
Copy link
Contributor

Trenly commented Jun 15, 2023

Thanks for the quick follow up!

@assarbad
Copy link
Contributor Author

Could you explain in brief how this affects future contributions? Is there a way to express the dependency on Microsoft.VCRedist.2015+.x64?

Thanks anyway for pointing it out!

@denelon
Copy link
Contributor

denelon commented Jun 26, 2023

We don't yet have support for dependencies as a stable feature. We're still looking at a few challenges related to reboots that can get triggered by dependencies being upgraded. Yes, the dependency can be identified in the manifest, but we're holding off on accepting packages that fail during install if the dependency isn't present.

@assarbad
Copy link
Contributor Author

We don't yet have support for dependencies as a stable feature. We're still looking at a few challenges related to reboots that can get triggered by dependencies being upgraded. Yes, the dependency can be identified in the manifest, but we're holding off on accepting packages that fail during install if the dependency isn't present.

Shame, but thanks so much for the clarification!

@denelon
Copy link
Contributor

denelon commented Jun 30, 2023

I've got it in the 1.6 milestone so we're aiming for September.

@XVilka
Copy link

XVilka commented Aug 14, 2023

@assarbad @Trenly you might be interested in #115801

BTW, if something is needed from our side - please let us know by opening an issue at https://github.com/rizinorg/cutter/issues

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 Publish-Pipeline-Succeeded Validation-Completed Validation passed Waived-Validation-Executable-Error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants