Skip to content

Commit

Permalink
Make exe missing silent switch a warning and add tool info (microsoft#93
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@yao-msft
yao-msft authored Apr 30, 2020
1 parent 3929ecd commit 69dfb82
Show file tree
Hide file tree
Showing 11 changed files with 196 additions and 89 deletions.
4 changes: 2 additions & 2 deletions src/AppInstallerCLICore/Command.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ namespace AppInstaller::CLI
void Command::OutputIntroHeader(Execution::Reporter& reporter) const
{
reporter.Info() <<
"AppInstaller Command Line v" << Runtime::GetClientVersion() << std::endl <<
"Copyright (c) Microsoft Corporation" << std::endl;
"Windows Package Manager v" << Runtime::GetClientVersion() << std::endl <<
"Copyright (c) Microsoft Corporation. All rights reserved." << std::endl;
}

void Command::OutputHelp(Execution::Reporter& reporter, const CommandException* exception) const
Expand Down
14 changes: 13 additions & 1 deletion src/AppInstallerCLICore/Commands/RootCommand.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ namespace AppInstaller::CLI
return
{
Argument{ "version", 'v', Execution::Args::Type::ListVersions, LOCME("Display the version of the tool"), ArgumentType::Flag, Visibility::Help },
Argument{ "info", APPINSTALLER_CLI_ARGUMENT_NO_SHORT_VER, Execution::Args::Type::Info, LOCME("Display general info of the tool"), ArgumentType::Flag, Visibility::Help },
};
}

Expand All @@ -40,7 +41,18 @@ namespace AppInstaller::CLI

void RootCommand::ExecuteInternal(Execution::Context& context) const
{
if (context.Args.Contains(Execution::Args::Type::ListVersions))
if (context.Args.Contains(Execution::Args::Type::Info))
{
OutputIntroHeader(context.Reporter);

context.Reporter.Info() << std::endl <<
"Links:" << std::endl <<
" Privacy Statement: https://aka.ms/winget-privacy" << std::endl <<
" License agreement: https://aka.ms/winget-license" << std::endl <<
" 3rd Party Notices: https://aka.ms/winget-3rdPartyNotice" << std::endl <<
" Homepage: https://aka.ms/winget" << std::endl;
}
else if (context.Args.Contains(Execution::Args::Type::ListVersions))
{
context.Reporter.Info() << 'v' << Runtime::GetClientVersion() << std::endl;
}
Expand Down
14 changes: 11 additions & 3 deletions src/AppInstallerCLICore/Commands/ValidateCommand.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,21 @@ namespace AppInstaller::CLI

try
{
(void)Manifest::Manifest::CreateFromPath(inputFile, true);
(void)Manifest::Manifest::CreateFromPath(inputFile, true, true);
context.Reporter.Info() << "Manifest validation succeeded." << std::endl;
}
catch (const Manifest::ManifestException& e)
{
context.Reporter.Warn() << "Manifest validation failed." << std::endl;
context.Reporter.Warn() << e.GetManifestErrorMessage() << std::endl;
if (e.IsWarningOnly())
{
context.Reporter.Warn() << "Manifest validation succeeded with warnings." << std::endl;
}
else
{
context.Reporter.Error() << "Manifest validation failed." << std::endl;
}

context.Reporter.Info() << e.GetManifestErrorMessage() << std::endl;
}
};
}
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/ExecutionArgs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ namespace AppInstaller::CLI::Execution
PlainStyle, // Makes progress display as plain
RainbowStyle, // Makes progress display as a rainbow
Help, // Show command usage
Info, // Show general info about WinGet
};

bool Contains(Type arg) const { return (m_parsedArgs.count(arg) != 0); }
Expand Down
130 changes: 80 additions & 50 deletions src/AppInstallerCLITests/YamlManifest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,80 +122,110 @@ TEST_CASE("ReadGoodManifestWithSpaces", "[ManifestValidation]")
REQUIRE(manifest.FileExtensions == MultiValue{ "appx", "appxbundle", "msix", "msixbundle" });
}

void TestManifest(const std::filesystem::path& manifestPath, const std::string& expectedError = {})
struct ManifestExceptionMatcher : public Catch::MatcherBase<ManifestException>
{
if (expectedError.empty())
ManifestExceptionMatcher(std::string expectedMessage, bool expectedWarningOnly) :
m_expectedMessage(expectedMessage), m_expectedWarningOnly(expectedWarningOnly) {}

// Performs the test for this matcher
bool match(ManifestException const& e) const override
{
return e.GetManifestErrorMessage().find(m_expectedMessage) != std::string::npos &&
e.IsWarningOnly() == m_expectedWarningOnly;
}

virtual std::string describe() const override {
std::ostringstream ss;
ss << std::boolalpha << "Expected exception message: " << m_expectedMessage << "Expected IsWarningOnly: " << m_expectedWarningOnly;
return ss.str();
}

private:
std::string m_expectedMessage;
bool m_expectedWarningOnly;
};

void TestManifest(const std::filesystem::path& manifestPath, const std::string& expectedMessage = {}, bool expectedWarningOnly = false)
{
if (expectedMessage.empty())
{
CHECK_NOTHROW(Manifest::CreateFromPath(TestDataFile(manifestPath), true));
CHECK_NOTHROW(Manifest::CreateFromPath(TestDataFile(manifestPath), true, true));
}
else
{
CHECK_THROWS_WITH(Manifest::CreateFromPath(TestDataFile(manifestPath), true), Catch::Contains(expectedError));
CHECK_THROWS_MATCHES(Manifest::CreateFromPath(TestDataFile(manifestPath), true, true), ManifestException, ManifestExceptionMatcher(expectedMessage, expectedWarningOnly));
}
}

struct ManifestTestCase
{
std::string TestFile;
std::string ExpectedMessage = {};
bool IsWarningOnly = false;
};

TEST_CASE("ReadGoodManifests", "[ManifestValidation]")
{
std::string TestCases[] =
ManifestTestCase TestCases[] =
{
"Manifest-Good-InstallerTypeExeRoot-Silent.yaml",
"Manifest-Good-InstallerTypeExeRoot-SilentRoot.yaml",
"Manifest-Good-InstallerTypeExe-Silent.yaml",
"Manifest-Good-InstallerTypeExe-SilentRoot.yaml",
"Manifest-Good-Installeruniqueness-DefaultLang.yaml",
"Manifest-Good-Installeruniqueness-DiffLangs.yaml",
"Manifest-Good-InstallerUniqueness-DiffScope.yaml",
"Manifest-Good-Minimum.yaml",
"Manifest-Good-Minimum-InstallerType.yaml",
"Manifest-Good-Switches.yaml",
{ "Manifest-Good-InstallerTypeExeRoot-Silent.yaml" },
{ "Manifest-Good-InstallerTypeExeRoot-SilentRoot.yaml" },
{ "Manifest-Good-InstallerTypeExe-Silent.yaml" },
{ "Manifest-Good-InstallerTypeExe-SilentRoot.yaml" },
{ "Manifest-Good-Installeruniqueness-DefaultLang.yaml" },
{ "Manifest-Good-Installeruniqueness-DiffLangs.yaml" },
{ "Manifest-Good-InstallerUniqueness-DiffScope.yaml" },
{ "Manifest-Good-Minimum.yaml" },
{ "Manifest-Good-Minimum-InstallerType.yaml" },
{ "Manifest-Good-Switches.yaml" },
};

for (auto const& testCase : TestCases)
{
TestManifest(testCase);
TestManifest(testCase.TestFile);
}
}

TEST_CASE("ReadBadManifests", "[ManifestValidation]")
{
std::pair<std::string, std::string> TestCases[] =
ManifestTestCase TestCases[] =
{
{ "Manifest-Bad-ArchInvalid.yaml", "Manifest: Invalid field value. Field: Arch" },
{ "Manifest-Bad-ArchMissing.yaml", "Manifest: Required field missing. Field: Arch" },
{ "Manifest-Bad-Channel-NotSupported.yaml", "Manifest: Field is not supported. Field: Channel" },
{ "Manifest-Bad-DifferentCase-camelCase.yaml", "Manifest: All field names should be PascalCased. Field: installerType" },
{ "Manifest-Bad-DifferentCase-lower.yaml", "Manifest: All field names should be PascalCased. Field: installertype" },
{ "Manifest-Bad-DifferentCase-UPPER.yaml", "Manifest: All field names should be PascalCased. Field: INSTALLERTYPE" },
{ "Manifest-Bad-DuplicateKey.yaml", "Manifest: Duplicate field found in the manifest." },
{ "Manifest-Bad-DuplicateKey-DifferentCase.yaml", "Manifest: Duplicate field found in the manifest." },
{ "Manifest-Bad-DuplicateKey-DifferentCase-lower.yaml", "Manifest: Duplicate field found in the manifest." },
{ "Manifest-Bad-IdInvalid.yaml", "Manifest: Invalid field value. Field: Id" },
{ "Manifest-Bad-IdMissing.yaml", "Manifest: Required field missing. Field: Id" },
{ "Manifest-Bad-InstallersMissing.yaml", "Manifest: Required field missing. Field: Installers" },
{ "Manifest-Bad-InstallerTypeExe-NoSilent.yaml", "Manifest: Silent switches are required for InstallerType exe." },
{ "Manifest-Bad-InstallerTypeExe-NoSilentRoot.yaml", "Manifest: Silent switches are required for InstallerType exe." },
{ "Manifest-Bad-InstallerTypeExeRoot-NoSilent.yaml", "Manifest: Silent switches are required for InstallerType exe." },
{ "Manifest-Bad-InstallerTypeExeRoot-NoSilentRoot.yaml", "Manifest: Silent switches are required for InstallerType exe." },
{ "Manifest-Bad-InstallerTypeInvalid.yaml", "Manifest: Invalid field value. Field: InstallerType" },
{ "Manifest-Bad-InstallerTypeMissing.yaml", "Manifest: Invalid field value. Field: InstallerType" },
{ "Manifest-Bad-InstallerUniqueness.yaml", "Manifest: Duplicate installer entry found." },
{ "Manifest-Bad-InstallerUniqueness-DefaultScope.yaml", "Manifest: Duplicate installer entry found." },
{ "Manifest-Bad-InstallerUniqueness-DefaultValues.yaml", "Manifest: Duplicate installer entry found." },
{ "Manifest-Bad-InstallerUniqueness-SameLang.yaml", "Manifest: Duplicate installer entry found." },
{ "Manifest-Bad-NameMissing.yaml", "Manifest: Required field missing. Field: Name" },
{ "Manifest-Bad-PublisherMissing.yaml", "Manifest: Required field missing. Field: Publisher" },
{ "Manifest-Bad-Sha256Invalid.yaml", "Manifest: Invalid field value. Field: Sha256" },
{ "Manifest-Bad-Sha256Missing.yaml", "Manifest: Required field missing. Field: Sha256" },
{ "Manifest-Bad-SwitchInvalid.yaml", "Manifest: Unknown field. Field: NotASwitch" },
{ "Manifest-Bad-UnknownProperty.yaml", "Manifest: Unknown field. Field: Fake" },
{ "Manifest-Bad-UrlInvalid.yaml", "Manifest: Invalid field value. Field: Url" },
{ "Manifest-Bad-UrlMissing.yaml", "Manifest: Required field missing. Field: Url" },
{ "Manifest-Bad-VersionInvalid.yaml", "Manifest: Invalid field value. Field: Version" },
{ "Manifest-Bad-VersionMissing.yaml", "Manifest: Required field missing. Field: Version" },
{ "Manifest-Bad-ArchInvalid.yaml", "Invalid field value. Field: Arch" },
{ "Manifest-Bad-ArchMissing.yaml", "Required field missing. Field: Arch" },
{ "Manifest-Bad-Channel-NotSupported.yaml", "Field is not supported. Field: Channel" },
{ "Manifest-Bad-DifferentCase-camelCase.yaml", "All field names should be PascalCased. Field: installerType" },
{ "Manifest-Bad-DifferentCase-lower.yaml", "All field names should be PascalCased. Field: installertype" },
{ "Manifest-Bad-DifferentCase-UPPER.yaml", "All field names should be PascalCased. Field: INSTALLERTYPE" },
{ "Manifest-Bad-DuplicateKey.yaml", "Duplicate field found in the manifest." },
{ "Manifest-Bad-DuplicateKey-DifferentCase.yaml", "Duplicate field found in the manifest." },
{ "Manifest-Bad-DuplicateKey-DifferentCase-lower.yaml", "Duplicate field found in the manifest." },
{ "Manifest-Bad-IdInvalid.yaml", "Invalid field value. Field: Id" },
{ "Manifest-Bad-IdMissing.yaml", "Required field missing. Field: Id" },
{ "Manifest-Bad-InstallersMissing.yaml", "Required field missing. Field: Installers" },
{ "Manifest-Bad-InstallerTypeExe-NoSilent.yaml", "Silent and SilentWithProgress switches are not specified for InstallerType exe.", true },
{ "Manifest-Bad-InstallerTypeExe-NoSilentRoot.yaml", "Silent and SilentWithProgress switches are not specified for InstallerType exe.", true },
{ "Manifest-Bad-InstallerTypeExeRoot-NoSilent.yaml", "Silent and SilentWithProgress switches are not specified for InstallerType exe.", true },
{ "Manifest-Bad-InstallerTypeExeRoot-NoSilentRoot.yaml", "Silent and SilentWithProgress switches are not specified for InstallerType exe.", true },
{ "Manifest-Bad-InstallerTypeInvalid.yaml", "Invalid field value. Field: InstallerType" },
{ "Manifest-Bad-InstallerTypeMissing.yaml", "Invalid field value. Field: InstallerType" },
{ "Manifest-Bad-InstallerUniqueness.yaml", "Duplicate installer entry found." },
{ "Manifest-Bad-InstallerUniqueness-DefaultScope.yaml", "Duplicate installer entry found." },
{ "Manifest-Bad-InstallerUniqueness-DefaultValues.yaml", "Duplicate installer entry found." },
{ "Manifest-Bad-InstallerUniqueness-SameLang.yaml", "Duplicate installer entry found." },
{ "Manifest-Bad-NameMissing.yaml", "Required field missing. Field: Name" },
{ "Manifest-Bad-PublisherMissing.yaml", "Required field missing. Field: Publisher" },
{ "Manifest-Bad-Sha256Invalid.yaml", "Invalid field value. Field: Sha256" },
{ "Manifest-Bad-Sha256Missing.yaml", "Required field missing. Field: Sha256" },
{ "Manifest-Bad-SwitchInvalid.yaml", "Unknown field. Field: NotASwitch" },
{ "Manifest-Bad-UnknownProperty.yaml", "Unknown field. Field: Fake" },
{ "Manifest-Bad-UrlInvalid.yaml", "Invalid field value. Field: Url" },
{ "Manifest-Bad-UrlMissing.yaml", "Required field missing. Field: Url" },
{ "Manifest-Bad-VersionInvalid.yaml", "Invalid field value. Field: Version" },
{ "Manifest-Bad-VersionMissing.yaml", "Required field missing. Field: Version" },
};

for (auto const& testCase : TestCases)
{
TestManifest(testCase.first, testCase.second);
TestManifest(testCase.TestFile, testCase.ExpectedMessage, testCase.IsWarningOnly);
}
}
20 changes: 14 additions & 6 deletions src/AppInstallerRepositoryCore/Manifest/Manifest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ namespace AppInstaller::Manifest
return resultErrors;
}

Manifest Manifest::CreateFromPath(const std::filesystem::path& inputFile, bool fullValidation)
Manifest Manifest::CreateFromPath(const std::filesystem::path& inputFile, bool fullValidation, bool throwOnWarning)
{
Manifest manifest;
std::vector<ValidationError> errors;
Expand All @@ -170,19 +170,23 @@ namespace AppInstaller::Manifest
}
catch (const std::exception& e)
{
AICLI_LOG(YAML, Error, << "Failed to create manifest from file: " << inputFile.u8string());
THROW_EXCEPTION_MSG(ManifestException(), e.what());
}

if (!errors.empty())
{
THROW_EXCEPTION(ManifestException(std::move(errors)));
ManifestException ex{ std::move(errors) };

if (throwOnWarning || !ex.IsWarningOnly())
{
THROW_EXCEPTION(ex);
}
}

return manifest;
}

Manifest Manifest::Create(const std::string& input, bool fullValidation)
Manifest Manifest::Create(const std::string& input, bool fullValidation, bool throwOnWarning)
{
Manifest manifest;
std::vector<ValidationError> errors;
Expand All @@ -194,13 +198,17 @@ namespace AppInstaller::Manifest
}
catch (const std::exception& e)
{
AICLI_LOG(YAML, Error, << "Failed to create manifest: " << input);
THROW_EXCEPTION_MSG(ManifestException(), e.what());
}

if (!errors.empty())
{
THROW_EXCEPTION(ManifestException(std::move(errors)));
ManifestException ex{ std::move(errors) };

if (throwOnWarning || !ex.IsWarningOnly())
{
THROW_EXCEPTION(ex);
}
}

return manifest;
Expand Down
9 changes: 5 additions & 4 deletions src/AppInstallerRepositoryCore/Manifest/Manifest.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,11 @@ namespace AppInstaller::Manifest

std::vector<ValidationError> PopulateManifestFields(const YAML::Node& rootNode, bool fullValidation);

// fullValidation Bool to set if manifest creation should perform extra validation that client does not need.
// e.g. Channel should be null. Client code does not need this check to work properly.
static Manifest CreateFromPath(const std::filesystem::path& inputFile, bool fullValidation = false);
// fullValidation: Bool to set if manifest creation should perform extra validation that client does not need.
// e.g. Channel should be null. Client code does not need this check to work properly.
// throwOnWarning: Bool to indicate if an exception should be thrown with only warnings detected in the manifest.
static Manifest CreateFromPath(const std::filesystem::path& inputFile, bool fullValidation = false, bool throwOnWarning = false);

static Manifest Create(const std::string& input, bool fullValidation = false);
static Manifest Create(const std::string& input, bool fullValidation = false, bool throwOnWarning = false);
};
}
2 changes: 1 addition & 1 deletion src/AppInstallerRepositoryCore/Manifest/ManifestInstaller.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ namespace AppInstaller::Manifest
(Switches.find(InstallerSwitchType::SilentWithProgress) == Switches.end() ||
Switches.find(InstallerSwitchType::Silent) == Switches.end()))
{
resultErrors.emplace_back(ManifestError::ExeInstallerMissingSilentSwitches);
resultErrors.emplace_back(ManifestError::ExeInstallerMissingSilentSwitches, ValidationError::Level::Warning);
}

// Check empty string before calling IsValidUrl to avoid duplicate error reporting.
Expand Down
Loading

0 comments on commit 69dfb82

Please sign in to comment.