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

Microsoft.Git: update to 2.32.0.vfs.0.0 #16867

Merged
3 commits merged into from
Jun 11, 2021
Merged

Microsoft.Git: update to 2.32.0.vfs.0.0 #16867

3 commits merged into from
Jun 11, 2021

Conversation

ldennington
Copy link
Contributor

@ldennington ldennington commented Jun 9, 2021

Microsoft Reviewers: Open in CodeFlow

@wingetbot
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wingetbot wingetbot added the Internal-Error-Dynamic-Scan The test for Dynamic Scanning in the Installation Validation failed. label Jun 9, 2021
@ghost
Copy link

ghost commented Jun 9, 2021

Hello @ldennington,
The pull request encountered an internal error and has been assigned to a developer to investigate.

@ghost ghost added Needs: Attention Retry-1 flag to indicate retried labels Jun 9, 2021
@ghost ghost assigned ChuckFerring-zz Jun 9, 2021
@ChuckFerring-zz ChuckFerring-zz added Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. and removed Internal-Error-Dynamic-Scan The test for Dynamic Scanning in the Installation Validation failed. labels Jun 9, 2021
@ChuckFerring-zz ChuckFerring-zz removed their assignment Jun 9, 2021
@KevinLaMS
Copy link
Contributor

/azp run

@ghost ghost removed Needs: Attention Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. labels Jun 10, 2021
@KevinLaMS KevinLaMS removed the Retry-1 flag to indicate retried label Jun 10, 2021
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wingetbot wingetbot added the Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. label Jun 10, 2021
Copy link
Contributor

@ItzLevvie ItzLevvie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Control Panel displays the version as 2.32.0.0.0 and as Publisher as The Git Development Community:
image

Windows Package Manager requires the entires to match what's shown in Control Panel (for this particular manifest - that will exclude PackageName as updating it will conflict with the Git.Git package) so that other people can upgrade the application without being on a constant upgrade loop.

@denelon denelon added the Needs-Author-Feedback This needs a response from the author. label Jun 10, 2021
@ldennington
Copy link
Contributor Author

The Control Panel displays the version as 2.32.0.0.0 and as Publisher as The Git Development Community:
image

Windows Package Manager requires the entires to match what's shown in Control Panel (for this particular manifest - that will exclude PackageName as updating it will conflict with the Git.Git package) so that other people can upgrade the application without being on a constant upgrade loop.

Sure, I'm fine with making the necessary changes. To be clear - are you just asking for an update of the PackageVersion field to align with what's shown in Control Panel, or are there additional fields that should be updated?

@ghost ghost added Needs: Attention and removed Needs: author feedback Needs-Author-Feedback This needs a response from the author. labels Jun 11, 2021
@ItzLevvie
Copy link
Contributor

I'm asking for an update to the PackageVersion and Publisher fields to match what's shown in Control Panel so others can easily upgrade via winget upgrade.

@ghost ghost removed Needs: Attention Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. labels Jun 11, 2021
@wingetbot
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wingetbot wingetbot added the Manifest-Validation-Error Manifest validation failed label Jun 11, 2021
@ghost
Copy link

ghost commented Jun 11, 2021

Hello @ldennington,
The package manager bot determined that the metadata was not compliant.

Please verify the manifest file is compliant with the package manager 1.0 manifest specification.
Make sure the ID is of the form publisher.appname and that the folder structure is manifests\partition\publisher\appname\version.
Note: The path and "PackageIdentifier" are case sensitive.
Be sure to use a tool like VSCode (https://code.visualstudio.com/) to make sure the manifest YAML syntax is correct.

You could also try our Windows Package Manager Manifest Creator Preview.

For details on the specific error, see the details link below in the build pipeline.

@ghost ghost added Needs: author feedback Needs-Author-Feedback This needs a response from the author. labels Jun 11, 2021
@ghost ghost removed Needs: author feedback Needs-Author-Feedback This needs a response from the author. Manifest-Validation-Error Manifest validation failed labels Jun 11, 2021
@wingetbot
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wingetbot wingetbot added the Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. label Jun 11, 2021
@ghost ghost added Moderator-Approved One of the Moderators has reviewed and approved this PR Validation-Completed Validation passed labels Jun 11, 2021
@ghost
Copy link

ghost commented Jun 11, 2021

Hello @msftbot[bot]!

Because this pull request has the Validation-Completed label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

InstallerType: inno
InstallerSha256: 73fe3e86ae419b7f63f87635850dc5d3c8285379e28b765e67501e502d4588b1
InstallerSwitches:
Custom: /COMPONENTS="AUTOUPDATE"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ldennington We should remove these InstallerSwitches now that we've adjusted the installer defaults.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do - I'm going to do a larger manifest update once mjcheetham/update-winget#124 merges and a new release of the update-winget task is cut. I will be sure to include at that time, and will manually make the update for now.

Copy link
Contributor Author

@ldennington ldennington Jun 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh never mind - thought this was the new PR against one of the new versions. Will not manually update since it already merged 😁.

@derrickstolee
Copy link
Contributor

The Control Panel displays the version as 2.32.0.0.0 and as Publisher as The Git Development Community:
image

Windows Package Manager requires the entires to match what's shown in Control Panel (for this particular manifest - that will exclude PackageName as updating it will conflict with the Git.Git package) so that other people can upgrade the application without being on a constant upgrade loop.

@ItzLevvie: We want to be very careful to differentiate this package from the "standard" one that is Git for Windows. This version includes custom changes created by Microsoft and GitHub. Can we change the publisher to "Microsoft" instead of "The Git Development Community"?

I also notice that the license is listed as "Copyright (C) Microsoft Corporation" but at least the Git code is LGPLv2. Perhaps some other pieces that we ship in the package are not (@dscho might know a few of these components).

I look forward to your advice here.

@dscho
Copy link
Member

dscho commented Jun 16, 2021

the Git code is LGPLv2

It's actually GPLv2. And yes, different components shipped together with Git have different licenses (git.exe does not link to them).

@ItzLevvie
Copy link
Contributor

ItzLevvie commented Jun 17, 2021

cc @derrickstolee: I believe you may have to update the publisher in the application to be Microsoft or Microsoft Corporation or something along those lines rather than changing it in the manifest itself.

This is what the documentation shows Note: With the 1.0 release of the Windows Package Manager, this name affects how packages from a source are mapped to Apps installed in Windows 10 via Add / Remove Programs (ARP). The best practice is to ensure this matches the ARP entry for the package when it has been installed. The impact is associated with winget upgrade and winget list. - from https://github.com/microsoft/winget-pkgs/blob/master/doc/manifest/schema/1.0.0/defaultLocale.md#publisher; which is why I suggested to change it.

The license can be changed to LGPLv2. It was Copyright (C) Microsoft Corporation because there wasn't a Copyright and CopyrightUrl field before the 1.0 release of Windows Package Manager, so it was put in License and LicenseUrl instead.

@derrickstolee
Copy link
Contributor

cc @derrickstolee: I believe you may have to update the publisher in the application to be Microsoft or Microsoft Corporation or something along those lines rather than changing it in the manifest itself.

Ok, so there is something we are doing wrong in our bundling of the product, probably because of how it shares a lot of components with git-for-windows/git. We will look into that part on our side.

@ldennington ldennington deleted the update-1623258808595 branch July 18, 2022 18:07
This pull request was closed.
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 Validation-Completed Validation passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants