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.

Sign up for GitHub

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

Make exe missing silent switch a warning and add tool info #93

Merged
merged 4 commits into from
Apr 30, 2020
Merged

Make exe missing silent switch a warning and add tool info #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ee7987d
exe silent switch
yao-msft Apr 29, 2020
d4974ce
Add tool info
yao-msft Apr 29, 2020
beec8ee
Merge remote-tracking branch 'upstream/master' into skipexesilentswitch
yao-msft Apr 29, 2020
75ce91e
PR feedback
yao-msft Apr 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/AppInstallerCLICore/Command.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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