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

Jump to bottom

[gtk3] Turn hang into timeout #32327

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

[gtk3] Turn hang into timeout #32327

wants to merge 2 commits into from
+20 −1

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Jul 1, 2023 •
edited
Loading

Mitigation for #29018.
From proper failures, we will be able to get CI logs for port gtk3.
CC @BillyONeal.

  • 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.
dg0yt
dg0yt commented Jul 2, 2023
View reviewed changes
ports/gtk3/portfile.cmake

# Temporary mitigation for build being stuck for unclear reasons,
# https://github.com/microsoft/vcpkg/issues/29018
if(EXISTS "${SCRIPTS}/cmake/vcpkg_install_meson.cmake")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Avoiding a hard dependency on this filepath.
ports/gtk3/portfile.cmake Outdated
if(EXISTS "${SCRIPTS}/cmake/vcpkg_install_meson.cmake")
file(READ "${SCRIPTS}/cmake/vcpkg_install_meson.cmake" install_meson)
string(REPLACE [[COMMAND "${NINJA}"]] [[
TIMEOUT 3600
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change to 5 if you want to verify the effect.
@LilyWangLL LilyWangLL added the category:port-bug The issue is with a library, which is something the port should already support label Jul 3, 2023
LilyWangLL
LilyWangLL previously approved these changes Jul 3, 2023
View reviewed changes
@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Jul 3, 2023
@vicroms vicroms added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Jul 5, 2023
@vicroms vicroms marked this pull request as draft July 6, 2023 20:52
@vicroms
Copy link
Member

vicroms commented Jul 6, 2023

@ras0219-msft @markle11m @dan-shaw @JavierMatosD

We discussed this PR and feel that this is better implemented as a general build timeout in the vcpkg executable. Any build that is taking longer than the timeout causes vcpkg to stop so that CI has time to upload logs. The timeout should be a user setting and we can pick a value that works for our CI (the team thinks 5 hours should be OK).

@ras0219-msft also points out that the current implementation of the timeout is not good since it couples the implementation of vcpkg_install_meson() and the gtk3 port.
@dg0yt
Copy link
Contributor Author

dg0yt commented Jul 7, 2023

When will it come?
This PR is a specific immediate mitigation for a real problem. And it should simply degrade gracefully for most modifications of vcpkg_install_meson.
@BillyONeal BillyONeal removed the info:reviewed Pull Request changes follow basic guidelines label Jul 17, 2023
@dg0yt
Copy link
Contributor Author

dg0yt commented Jul 26, 2023

(the team thinks 5 hours should be OK).

Now using 5 hours.

it couples the implementation of vcpkg_install_meson() and the gtk3 port.

Already done in a way which should degrade gracefully.
@dg0yt dg0yt mentioned this pull request Jul 27, 2023
Open
@dg0yt dg0yt mentioned this pull request Aug 22, 2023
Draft
2 tasks
@dg0yt dg0yt mentioned this pull request Sep 6, 2023
Merged
@dg0yt dg0yt mentioned this pull request Jun 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.
4 participants
@dg0yt @vicroms @LilyWangLL @BillyONeal