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

Support 1.1.0 manifests #31

Closed
Trenly opened this issue Sep 17, 2021 · 18 comments
Closed

Support 1.1.0 manifests #31

Trenly opened this issue Sep 17, 2021 · 18 comments
Labels
Enhancement New feature or request Schema 1.1.0

Comments

@Trenly
Copy link
Owner

Trenly commented Sep 17, 2021

Description of the new feature/enhancement

Be prepared to switch to Manifest version 1.1.0 once microsoft/issues/27937 is completed

Proposed technical implementation details (optional)

Other Considerations

  • Do we need to support 1.1.0 and 1.0.0 schema manifest creations
@Trenly Trenly added the Enhancement New feature or request label Sep 17, 2021
@Trenly
Copy link
Owner Author

Trenly commented Sep 18, 2021

@OfficialEsco Would you be able to suggest which of the fields we should prompt for?

I'm thinking

  • Release Date
  • Release notes URL
  • ElevationRequirement

@vedantmgoyal9
Copy link

@OfficialEsco Would you be able to suggest which of the fields we should prompt for?

Well, I also have some fields which can be taken into consideration:

  • Channel
  • Dependencies
  • DisplayVersion

@Trenly
Copy link
Owner Author

Trenly commented Sep 18, 2021

@OfficialEsco Would you be able to suggest which of the fields we should prompt for?

Well, I also have some fields which can be taken into consideration:

  • Channel
  • Dependencies
  • DisplayVersion

Dependencies might be a bit difficult due to the structure, but I could see where it would be good to add. I think they exist in the 1.0 schema too

@OfficialEsco
Copy link
Collaborator

Release Date
Release notes URL
ElevationRequirement

Yeah these are good/important

Channel
Dependencies
DisplayVersion

Channel is important, however we need to know if they actually support and how to use it.. This might be a waaaay too advanced for most people.

Dependencies is also complicated and not something you need to add often other than once

I think we might need all of these to be optional

AppsAndFeaturesEntry:
  DisplayName:
  Publisher: 
  DisplayVersion: 
  ProductCode: 

Tbh Schema 1.1.0 seems a lill bit messy

@vedantmgoyal9
Copy link

I think just before testing and submitting the manifest, we can include a prompt something like this "Do you want to fill additional/advanced manifest fields" and then we can prompt for these fields.

@OfficialEsco
Copy link
Collaborator

OfficialEsco commented Nov 17, 2021

I think we can and should be able to move over to manifest schema 1.1.0 fairly quickly

We can add the different variables as needed

edit: First step is probably just replacing 1.0.0 with 1.1.0, the only change in the manifest then is Agreements, ReleaseNotes and ReleaseNotesUrl getting commented out in the DefaultLocale manifest and obv ManifestVersion and the $schema line turning into 1.1.0

edit2: experimental package microsoft#35148

Trenly added a commit that referenced this issue Nov 17, 2021
@Trenly
Copy link
Owner Author

Trenly commented Nov 17, 2021

I think there's a couple ways to approach it; The first is to just add the variables, and that is probably the best next step. It is the approach I want to take.

The second step, then, it is to change how the data gets populated into the manifest. Ideally, we would never be creating new manifests, only modifying them. Yes, I do truly mean this. Even for "new" manifests, we should ideally be starting with a full manifest, where all the fields are simply null. This would make a lot of the logic more direct, since we would never have to worry about fields being missed. I'll document a bit more of this later in #37, but the main idea is that we always either 1) Create a blank manifest of the latest version, copy over all the metadata that we can, and then continue - or 2) Check which fields exist in the current manifest, add the ones which do not exist, then continue.

@vedantmgoyal9

This comment has been minimized.

denelon pushed a commit to microsoft/winget-pkgs that referenced this issue Nov 30, 2021
* Prepare file for 1.1.0 manifests - Trenly#31

* Installer detection improvements (#111)

* Begin update in place

* Condense installer switch logic

* Fix Null values

* Prevent null checking

* Cleanup

* Fix missed variable

* Bump version

* Update function calls

* Fix empty ProductCode

* Confirm Architecture when Automatically Detected

* Remove Debug Line

* Add duplicate url check for Quick update

* Remove line used in debugging

* Remove unneeded comment

* Resolves Trenly#121

Co-authored-by: Vedant <83997633+vedantmgoyal2009@users.noreply.github.com>
Trenly added a commit that referenced this issue Dec 1, 2021
@Trenly
Copy link
Owner Author

Trenly commented Dec 15, 2021

I think that the best way to go about adding support for additional manifest fields will be to rethink the idea of how we are creating the Yaml files in the first place.

Currently, we are asking users to select a program flow, then enter variables which get populated into an untracked object, and then eventually get written into Yaml. This requires careful scrutiny of all the parameters and where they are placed, has a lot of potential for duplicate code, and is overall an inefficient way of coding. As the manifest schema gets more complex, this requires each flow to be updated and validated independently.

I had done some experimentation in a separate branch on using a -InputObject parameter for updating existing manifests. After more consideration, I think that this can be used to optimize the logic flow. Here's what I'm thinking:
image

The main thing to note is that every single property that we would request would get its own function - referred to here as a property function. This would allow for the ability to customize each flow individually and handle the cases on a more dynamic basis, while avoiding duplication of code.

The second thing to note here is the recursive nature of the script. It would only ever go to writing the manifests if it was called using the input object parameter. This means that the script has two main sections -

  1. Primary Flow - Creating or updating a manifest from an input object
  2. Secondary Flow - Creating the object to feed into the primary flow

By doing it this way, the primary flow becomes the single point that needs to be tested. Not to say that the secondary flow doesn't need to be tested, but more that it reduces the logic to a single place where manifest data is ever changed, making debugging easier.

@OfficialEsco @jedieaston I'd love to know your thoughts on this proposal before I start the work


Finally, some examples of how this would simplify the flows -

Example 1 - Auto Update

PS> YamlCreate.ps1 -PackageIdentifier Example.Package -PackageVersion 1.0.0 -AutoUpdate
# becomes
PS> YamlCreate.ps1 -InputObject @{'PackageIdentifier'='Example.Package';'PackageVersion'='1.0.0'} 

Example 2 - Simple Update

Object output from Simple Update Secondary Flow -

{
'PackageIdentifier' : 'Example.Package',
'PackageVersion' : '1.0.0',
'Installers' : [
  {
    'InstallerUrl' : 'https://example.url/1.exe',
    'InstallerSha256' : 'BLAHBLAHBLAH',
    'Architecture' : 'x64'
  },
  {
    'InstallerUrl' : 'https://example.url/1.exe',
    'InstallerSha256' : 'WORDSWORDSWORDS',
    'Architecture' : 'x86'
  }
  ]
}

This data would then be passed back into the YamlCreate script, and the data would be merged with the existing manifest to create the new manifest

@jedieaston
Copy link
Collaborator

I'm running on final exam brain atm, but this looks like a good idea to me. The refactor will be complicated.

@Trenly
Copy link
Owner Author

Trenly commented Dec 15, 2021

I'm running on final exam brain atm, but this looks like a good idea to me. The refactor will be complicated.

It will be very complicated, if this is a route we want to go. I'm thinking that I might start it in a new file until I actually get it working

@SpecterShell
Copy link

The ReleaseNotes field should be cleared during Quick Update.

@OfficialEsco
Copy link
Collaborator

@SpecterShell that will be implemented in YamlCreate 2.1.0, same as #116
@Trenly took upon him self to rewrite/structure YamlCreate as you can see so it will take some time, winget dosn't fully support the 1.1.0 schema yet (wingetcreate reverts to 1.0.0) either so we got some time, currently we manually implement the needed 1.1.0 features where it is needed and stay away from the features that require updated metadata for each version.

Trenly added a commit that referenced this issue Jan 19, 2022
@OfficialEsco
Copy link
Collaborator

@Trenly since i don't understand the script anymore, how "hard" would it clear the ReleaseDate, ReleaseNotes and ReleaseNotesUrl field? Since @SpecterShell love adding them i think this could be prioritized for 2.0.5 since its not merged yet

Later we could look into suggesting ReleaseDate as today (skip prompt for Vedant's automation), + there is currently a "bug" here that makes the date invalid, not sure how it affects Vendant's automation

ReleaseNotesUrl could also be automated IF its a GitHub url, could probably do the same for LicenseUrl.
ReleaseNotes could be scraped, not sure if Vedant have looked into this or he just added patch notes as a test?

@Trenly
Copy link
Owner Author

Trenly commented Jan 21, 2022

@Trenly since i don't understand the script anymore, how "hard" would it clear the ReleaseDate, ReleaseNotes and ReleaseNotesUrl field? Since @SpecterShell love adding them i think this could be prioritized for 2.0.5 since its not merged yet

Later we could look into suggesting ReleaseDate as today (skip prompt for Vedant's automation), + there is currently a "bug" here that makes the date invalid, not sure how it affects Vendant's automation

ReleaseNotesUrl could also be automated IF its a GitHub url, could probably do the same for LicenseUrl. ReleaseNotes could be scraped, not sure if Vedant have looked into this or he just added patch notes as a test?

I don't think it would be hard to clear them; Adding a prompt for them gets a little more involved, and adding a parameter for it would be difficult until the rewrite.

What are all the fields that should be cleared when updating automatically? Is it just the 3 you mentioned?

Trenly added a commit that referenced this issue Jan 21, 2022
@OfficialEsco
Copy link
Collaborator

What are all the fields that should be cleared when updating automatically? Is it just the 3 you mentioned?

Yup that should be enough
https://github.com/microsoft/winget-pkgs/pull/41683/files
https://github.com/microsoft/winget-pkgs/pull/41248/files
https://github.com/microsoft/winget-pkgs/pull/41247/files

Also about our Channel discussion somewhere on GitHub, the REST Schema 1.1.0 does still not support it, but would be smart to have a module ready in the rewrite

@Trenly
Copy link
Owner Author

Trenly commented Jan 22, 2022

What are all the fields that should be cleared when updating automatically? Is it just the 3 you mentioned?

Yup that should be enough https://github.com/microsoft/winget-pkgs/pull/41683/files https://github.com/microsoft/winget-pkgs/pull/41248/files https://github.com/microsoft/winget-pkgs/pull/41247/files

Also about our Channel discussion somewhere on GitHub, the REST Schema 1.1.0 does still not support it, but would be smart to have a module ready in the rewrite

See #137 regarding the fields;

As for channels, I'll include it in the rewrite; Seems like it's a bit far out for right now

@vedantmgoyal9
Copy link

vedantmgoyal9 commented Jan 22, 2022

Later we could look into suggesting ReleaseDate as today (skip prompt for Vedant's automation), + there is currently a "bug" here that makes the date invalid, not sure how it affects Vendant's automation

GitHub's API returns created_at and published_at parameters so I can automate ReleaseDate once I know which format I have to convert it to. (edit: or YamlCreate can convert it automatically😁)

API returns the data in ISO-8601 format:
image

ReleaseNotesUrl could also be automated IF its a GitHub url, could probably do the same for LicenseUrl. ReleaseNotes could be scraped, not sure if Vedant have looked into this or he just added patch notes as a test?

ReleaseNotesUrl is pretty simple. For ReleaseNotes, it is a little tricky since the API returns data in RAW format:

"body": "### Bug Fixes\n\n* use debug for logging pwd ([d75f6e3](https://github.com/JanDeDobbeleer/oh-my-posh/commit/d75f6e30aafbe8818332cc1010d37607355062a8))"

so, I need to write a manual function to scrape the real content and remove all formatting, headings, code, etc...

Edit 2: Some "good" APIs also return ReleaseNotes, ReleaseDate and ReleaseNotesUrl (not sure), so I can take a look at every package manually and make changes in the automation to add them too.

denelon pushed a commit to microsoft/winget-pkgs that referenced this issue Jan 24, 2022
* Prepare file for 1.1.0 manifests - Trenly#31

* Fix Trenly#131

* Minor bug fixes and enhancements (#135)

* Check dependency before settings

* Execute `Get-AppLockerFileInformation` only in Win

* Fix: Use combined hashes (#136)

* Bump to v2.0.5

* Clear volatile fields 1.1.0 (#137)

* Prompt for volatile Release Values

* Add Patterns for release notes

* Clear volatile Locale fields on update

* Clear volatile Locale for additional locales

* Clear ReleaseDate on automatic-type upgrades

Co-authored-by: Vedant <83997633+vedantmgoyal2009@users.noreply.github.com>
Trenly added a commit that referenced this issue Jul 8, 2022
@Trenly Trenly closed this as completed Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Schema 1.1.0
Projects
None yet
Development

No branches or pull requests

5 participants