microsoft / microsoft/winget-cli-restsource

Get-WinGetManifest logic is broken around how it performs path detection

Open
#180 1 comment 0 reactions 1 assignee View on GitHub

@RDMacLachlan is already working on this.

Since Oct 5, 2022.

Issue-Bug
Dominant language
C#
Stars
317
Forks
79
PR merge metrics
No merged PRs in 30d

Description

Brief description of your issue

When processing files in a directory, $PathChildItemsJSON and/or $PathChildItemsYAML may be null. This is not accounted for in the subsequent if/else statements that perform validation. If one or the other is null, this will fail internally because of null references. Strict mode is not enabled for these scripts, so the failure cause is not obvious to the caller:

VERBOSE: Path pointed to a directory, found 0 JSON files, and 0 YAML files.
Get-WinGetManifest: D:\b\winget-cli-restsource-ameltzer\bin\Library\Add-WinGetManifest.ps1:109:32
Line |
 109 |          $ApplicationManifest = Get-WinGetManifest -Path $Path
     |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Directory does not contain any combination of JSON and YAML files.

VERBOSE: Returning (0) manifests based on search.
Exception: D:\b\winget-cli-restsource-ameltzer\bin\Library\Add-WinGetManifest.ps1:111:13
Line |
 111 |              throw "Failed to retrieve a proper manifest. Verify and t …
     |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Failed to retrieve a proper manifest. Verify and try again.

This gives the appearance of there being no YAML or JSON files, but there's actually 4 yaml files in my directory. Running again with Set-StrictMode -Version Latest shows detailed failure information:

VERBOSE: Retrieving a copy of the app Manifest file for submission to WinGet source.
PropertyNotFoundException: D:\b\winget-cli-restsource-ameltzer\bin\Library\Get-WinGetManifest.ps1:162:77      
Line |
 162 |  … ath pointed to a directory, found $($PathChildItemsJSON.count) JSON f …
     |                                        ~~~~~~~~~~~~~~~~~~~~~~~~~
     | The property 'count' cannot be found on this object. Verify that the property exists.

VERBOSE: Path pointed to a directory, found  JSON files, and 4 YAML files.
PropertyNotFoundException: D:\b\winget-cli-restsource-ameltzer\bin\Library\Get-WinGetManifest.ps1:166:24      
Line |
 166 |  …          if($PathChildItemsJSON.count -eq 0 -and $PathChildItemsYAML. …
     |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | The property 'count' cannot be found on this object. Verify that the property exists.

InvalidOperation: D:\b\winget-cli-restsource-ameltzer\bin\Library\Get-WinGetManifest.ps1:264:25
Line |
 264 |                  switch ($ManifestFileType) {
     |                          ~~~~~~~~~~~~~~~~~
     | The variable '$ManifestFileType' cannot be retrieved because it has not been set.

This can be fixed by doing this:

                    $PathChildItemsJSON = Get-ChildItem -Path $Path -Filter "*.json" -ErrorAction SilentlyContinue
                    if (!$PathChildItemsJSON) {
                        $PathChildItemsJSON = @()
                    }
                    $PathChildItemsYAML = Get-ChildItem -Path $Path -Filter "*.yaml" -ErrorAction SilentlyContinue
                    if (!$PathChildItemsYAML) {
                        $PathChildItemsYAML = @()
                    }

Unfortunately, while fixing this works around the null reference issues, it then runs into #179 :(

VERBOSE: Retrieving a copy of the app Manifest file for submission to WinGet source.
VERBOSE: Path pointed to a directory, found 0 JSON files, and 4 YAML files.
VERBOSE: Single YAML has been found in the specified directory.
Get-WinGetManifest: D:\b\winget-cli-restsource-ameltzer\bin\Library\Add-WinGetManifest.ps1:109:32
Line |
 109 |          $ApplicationManifest = Get-WinGetManifest -Path $Path
     |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Unable to process YAML files. Re-import the module to reload the required dependencies.

VERBOSE: Returning (0) manifests based on search.
Exception: D:\b\winget-cli-restsource-ameltzer\bin\Library\Add-WinGetManifest.ps1:111:13
Line |
 111 |              throw "Failed to retrieve a proper manifest. Verify and t …
     |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Failed to retrieve a proper manifest. Verify and try again.
Steps to reproduce

Run Add-WinGetManifest and point to a path with .yaml documents.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.