Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mdanish-kh committed Jul 4, 2024
1 parent cc68297 commit 4ac025c
Show file tree
Hide file tree
Showing 7 changed files with 11 additions and 31 deletions.
5 changes: 1 addition & 4 deletions doc/update.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,30 +39,27 @@ e.g.,
The following additional arguments can be provided with the installer URL(s):

#### Format

`'<InstallerUrl>|<Argument1>|<Argument2>...'`

#### Override Architecture
`'<InstallerUrl>|<InstallerArchitecture>'`

Winget-Create will attempt to determine the architecture of the installer package by performing a regex string match to identify the possible architecture in the installer url. If no match is found, Winget-Create will resort to obtaining the architecture from the downloaded installer. If Winget-Create fails to detect the architecture from the binary or the detected architecture does not match an architecture in the existing manifest, Winget-Create will fail to generate the manifest. In this case, you can explicitly provide the intended architecture and override the detected architecture using the following format:

`'<InstallerUrl>|<InstallerArchitecture>'`

#### Override Scope
`'<InstallerUrl>|<InstallerScope>'`

In case there are multiple installers with the same architecture, it may mean the same installer is available for multiple scopes. In this case, you can explicitly provide the installer scope in the update command using the following following argument format:

`'<InstallerUrl>|<InstallerScope>'`

#### Display Version
`'<InstallerUrl>|<DisplayVersion>'`

In some cases, the publisher of the package may use a different marketing version than the actual version written to Apps & Features. In this case, the manifest will contain `DisplayVersion` field. You can update the `DisplayVersion` field using the `--display-version` CLI arg if all installers use the same display version. If the display version differs for each installer, you can use following argument format:

`'<InstallerUrl>|<DisplayVersion>'`


## Usage Examples

Search for an existing manifest and update the version:
Expand Down
14 changes: 3 additions & 11 deletions src/WingetCreateCLI/Commands/UpdateCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -825,18 +825,10 @@ private List<InstallerMetadata> ParseInstallerUrlsForArguments(List<string> inst
}

// If value is not a convertible enum, it is assumed to be a display version.
else if (!string.IsNullOrEmpty(argumentString))
else if (!string.IsNullOrEmpty(argumentString) && !displayVersionPresent)
{
if (displayVersionPresent)
{
Logger.ErrorLocalized(nameof(Resources.MultipleDisplayVersion_Error));
return null;
}
else
{
displayVersionPresent = true;
installerMetadata.DisplayVersion = argumentString;
}
displayVersionPresent = true;
installerMetadata.DisplayVersion = argumentString;
}
else
{
Expand Down
9 changes: 0 additions & 9 deletions src/WingetCreateCLI/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions src/WingetCreateCLI/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1363,9 +1363,6 @@
<data name="DisplayVersion_HelpText" xml:space="preserve">
<value>Version to be used when updating the display version field. Version provided in the installer URL arguments will take precendence over this value.</value>
</data>
<data name="MultipleDisplayVersion_Error" xml:space="preserve">
<value>Multiple display versions detected. Only one version can be specified in the installer URL.</value>
</data>
<data name="UsingDisplayVersion_Message" xml:space="preserve">
<value>Using display version '{0}' for {1}</value>
<comment>{0} - will be replaced with the display version provided in the installer URL
Expand Down
3 changes: 2 additions & 1 deletion src/WingetCreateCore/Common/PackageParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,8 @@ private static void UpdateInstallerMetadata(Installer existingInstaller, Install

if (existingInstaller.AppsAndFeaturesEntries != null && newInstaller.AppsAndFeaturesEntries != null)
{
// New installer will always have a single entry in AppsAndFeaturesEntries
// When --display-version is provided, AppsAndFeaturesEntries for the new installer will not be null
// and will contain a single entry.
string newDisplayVersion = newInstaller.AppsAndFeaturesEntries.FirstOrDefault().DisplayVersion;

// Set DisplayVersion for each new installer if it exists in the corresponding existing installer.
Expand Down
2 changes: 1 addition & 1 deletion src/WingetCreateCore/Models/InstallerMetadata.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public class InstallerMetadata
public Scope? OverrideScope { get; set; }

/// <summary>
/// Gets or sets the display version specified as a CLI arg or an installer argument.
/// Gets or sets the display version specified as a CLI arg or an installer url argument.
/// </summary>
public string DisplayVersion { get; set; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,12 +390,14 @@ public async Task UpdateFailsOverrideWithMultipleScopes()
public async Task UpdateFailsWithMultipleDisplayVersions()
{
string installerUrl = $"https://fakedomain.com/{TestConstants.TestExeInstaller}";
string displayVersion1 = "3.4";
string displayVersion2 = "1.2";
(UpdateCommand badCommand, var manifests) =
GetUpdateCommandAndManifestData("TestPublisher.ScopeOverride", "1.2.3.4", this.tempPath, new[] { $"{installerUrl}|3.4|1.2" });
GetUpdateCommandAndManifestData("TestPublisher.ScopeOverride", "1.2.3.4", this.tempPath, new[] { $"{installerUrl}|{displayVersion1}|{displayVersion2}" });
var failedUpdateManifests = await RunUpdateCommand(badCommand, manifests);
ClassicAssert.IsNull(failedUpdateManifests, "Command should have failed due to multiple display versions specified for a single installer.");
string result = this.sw.ToString();
Assert.That(result, Does.Contain(Resources.MultipleDisplayVersion_Error), "Failed to show multiple display version error.");
Assert.That(result, Does.Contain(string.Format(Resources.UnableToParseArgument_Error, displayVersion2)), "Failed to show parsing error due to multiple string overrides.");
}

/// <summary>
Expand Down

0 comments on commit 4ac025c

Please sign in to comment.