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

[crashpad] Fetch missing dependency for crashpad on linux #30957

Merged
merged 1 commit into from
Apr 20, 2023

Conversation

omartijn
Copy link
Contributor

Note that this does not change the fact that crashpad is marked as unsupported, but it makes the build work now with
--allow-unsupported.

The build seems to work fine if the default compiler is set to clang, it does not build with gcc, which is probably why it's marked as unsupported.

  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@LilyWangLL LilyWangLL added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Apr 19, 2023
@LilyWangLL LilyWangLL changed the title Fetch missing dependency for crashpad on linux [crashpad] Fetch missing dependency for crashpad on linux Apr 19, 2023
LilyWangLL
LilyWangLL previously approved these changes Apr 19, 2023
@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Apr 19, 2023
@BillyONeal
Copy link
Member

Note that this does not change the fact that crashpad is marked as unsupported, but it makes the build work now with --allow-unsupported.

The build seems to work fine if the default compiler is set to clang, it does not build with gcc, which is probably why it's marked as unsupported.

It sounds like it should be removed from the supports expression and added to ci.baseline.txt. (ci.baseline.txt is the 'something about vcpkg's build lab prevents this from working' where supports is intended to be 'something in this prevents the build from ever working for anyone')

@LilyWangLL LilyWangLL removed the info:reviewed Pull Request changes follow basic guidelines label Apr 19, 2023
@LilyWangLL
Copy link
Contributor

LilyWangLL commented Apr 19, 2023

It sounds like it should be removed from the supports expression and added to ci.baseline.txt. (ci.baseline.txt is the 'something about vcpkg's build lab prevents this from working' where supports is intended to be 'something in this prevents the build from ever working for anyone')

Please don't remove "supports": "osx | (windows & !uwp)".
You need add linux to "supports": "osx | (windows & !uwp)", and add crashpad:x64-linux=fail to [VCPKG_ROOT]/scripts/ci.baseline.txt.

@omartijn
Copy link
Contributor Author

Right. I actually realized that since one of the latest versions it actually does build with gcc (which definitely wasn't the case before), so I wanted to see if the CI agreed with me. I hadn't realized that there was a restriction on the windows-support.

I'll let the pipeline run for now, since I'm waiting to see what the linux-build will do on the ci, if that works I'll put the supports line back - but with linux included. Then we might not even need to add it to ci.baseline.txt.

Note that this does not change the fact that crashpad is marked as
unsupported, but it makes the build work now with
`--allow-unsupported`.

The build seems to work fine if the default compiler is set to clang, it
does not build with gcc, which is probably why it's marked as
unsupported.
@omartijn
Copy link
Contributor Author

Figured it out: It does still need clang, but it no longer uses the system default compiler. So it'll work, even when the default compiler is gcc, as long as clang is installed and in the PATH. So I've added it to the ci.baseline.txt and marked it as supported on linux.

@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Apr 20, 2023
@BillyONeal BillyONeal merged commit 38b5cc2 into microsoft:master Apr 20, 2023
@BillyONeal
Copy link
Member

Thanks!

@omartijn omartijn deleted the crashpad-linux branch April 21, 2023 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants