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

Clarify what it means to be "unsupported" by a supports expression. #879

Merged
merged 4 commits into from
Feb 7, 2023

Conversation

BillyONeal
Copy link
Member

@BillyONeal BillyONeal commented Feb 1, 2023

@JavierMatosD Hopefully this makes it clearer for the customer you were working with.

Also fixes a few places where we were printing a spec but should have only printed a package name.

Copy link
Contributor

@JavierMatosD JavierMatosD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@dg0yt
Copy link
Contributor

dg0yt commented Feb 1, 2023

What I find confusing is how the message prints the requested platform first, i.e.:

atlmfc[core]:x64-linux is only supported on 'windows'

... this reads like "The linux package is only supported on windows".
Not sure how to improve this concisely, maybe

atlmfc[core] is unsupported on x64-linux but supported on 'windows'.
...

This area also needs improvements when a dependency is unsupported. ATM it doesn't tell the requested feature which asks for unsupported dependency. This is difficult to resolve for users. Example:

$ ./vcpkg install gdal[*]:x64-linux
Computing installation plan...
atlmfc[core]:x64-linux is only supported on 'windows'

@BillyONeal
Copy link
Member Author

What I find confusing is how the message prints the requested platform first, i.e.:

That's fair, I'll take a look at this again

This area also needs improvements when a dependency is unsupported.

I agree.

"UnsupportedShortOptions": "short options are not supported: '{value}'",
"_UnsupportedShortOptions.comment": "'{value}' is the short option given",
"UnsupportedSupportsExpression": "{spec} is only supported on '{supports_expression}'. This usually means that there are known build failures, or runtime problems, when building other platforms. To ignore this and attempt to build {spec} anyway, rerun vcpkg with `--allow-unsupported`.",
"_UnsupportedSupportsExpression.comment": "An example of {spec} is zlib:x64-windows. An example of {supports_expression} is windows & !static.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"_UnsupportedSupportsExpression.comment": "An example of {spec} is zlib:x64-windows. An example of {supports_expression} is windows & !static.",
"_UnsupportedSupportsExpression.comment": "An example of {spec} is zlib:x64-windows. An example of {supports_expression} is 'windows & !static'.",

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (and all other examples) are generated code.

"UnsupportedSupportsExpression": "{spec} is only supported on '{supports_expression}'. This usually means that there are known build failures, or runtime problems, when building other platforms. To ignore this and attempt to build {spec} anyway, rerun vcpkg with `--allow-unsupported`.",
"_UnsupportedSupportsExpression.comment": "An example of {spec} is zlib:x64-windows. An example of {supports_expression} is windows & !static.",
"UnsupportedSupportsExpressionWarning": "{spec} is only supported on '{supports_expression}'. This usually means that there are known build failures, or runtime problems, when building other platforms. Proceeding anyway due to `--allow-unsupported`.",
"_UnsupportedSupportsExpressionWarning.comment": "An example of {spec} is zlib:x64-windows. An example of {supports_expression} is windows & !static.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"_UnsupportedSupportsExpressionWarning.comment": "An example of {spec} is zlib:x64-windows. An example of {supports_expression} is windows & !static.",
"_UnsupportedSupportsExpressionWarning.comment": "An example of {spec} is zlib:x64-windows. An example of {supports_expression} is 'windows & !static'.",

"UnsupportedShortOptions": "short options are not supported: '{value}'",
"_UnsupportedShortOptions.comment": "'{value}' is the short option given",
"UnsupportedSupportsExpression": "{spec} is only supported on '{supports_expression}'. This usually means that there are known build failures, or runtime problems, when building other platforms. To ignore this and attempt to build {spec} anyway, rerun vcpkg with `--allow-unsupported`.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"UnsupportedSupportsExpression": "{spec} is only supported on '{supports_expression}'. This usually means that there are known build failures, or runtime problems, when building other platforms. To ignore this and attempt to build {spec} anyway, rerun vcpkg with `--allow-unsupported`.",
"UnsupportedSupportsExpression": "{package_name} is only supported on '{supports_expression}'. This usually means that there are known build failures, or runtime problems, when building other platforms. To ignore this and attempt to build {package_name} anyway, rerun vcpkg with `--allow-unsupported`.",

{spec} implies triplet information (see comment). I think {package_name} is the closest we have without defining a new placeholder.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are printing triplet information here; that's @dg0yt's comment :)

Copy link
Contributor

@dg0yt dg0yt Feb 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Package, features and triplets done right...
See the gdal example I added to #879 (comment).
What I want is app.

$ ./vcpkg install gdal[*]:x64-linux
Computing installation plan...
gdal[aws-ec2-windows]:x64-linux cannot be installed:
atlmfc[core] is only supported on 'windows'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that would be an improvement but that's more than the 10 minute "this should explain what we mean" fix I was going for here.

@@ -2213,9 +2223,10 @@ namespace vcpkg
"",
"To remove dependencies in manifest mode, edit your manifest (vcpkg.json) and run 'install'.");
DECLARE_MESSAGE(RemovePackageConflict,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@BillyONeal BillyONeal merged commit 62228f4 into microsoft:main Feb 7, 2023
@BillyONeal BillyONeal deleted the clarify-supported branch February 7, 2023 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants