Skip to content

fix(scripts/dag_traversal.py): kill timed-out builds on Windows - #44403

Open
zhikaip wants to merge 1 commit into
leanprover-community:masterfrom
zhikaip:windows_kill_build
Open

zhikaip wants to merge 1 commit into
leanprover-community:masterfrom
zhikaip:windows_kill_build

Conversation

@zhikaip

@zhikaip zhikaip commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

os.killpg does not exist on Windows, so the first per-module timeout in rm_set_option.py raised AttributeError and left the build running. Add a _kill_tree helper that falls back to taskkill /F /T.

Found and tested by running rm_set_option.py on Physlib (leanprover-community/physlib#1705), where the run then completed.

Disclaimer: I encountered this while running the script on Physlib using a windows PC, and Claude suggested this fix for me which I think may be useful to upstream.


`os.killpg` does not exist on Windows, so the first per-module timeout in
`rm_set_option.py` raised `AttributeError` and left the build running. Add a
`_kill_tree` helper that falls back to `taskkill /F /T`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the new-contributor This PR was made by a contributor with at most 5 merged PRs. Welcome to the community! label Oct 1, 2026
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

Welcome new contributor!

Thank you for contributing to Mathlib! If you haven't done so already, please review our contribution guidelines, as well as the style guide and naming conventions. In particular, we kindly remind contributors that we have guidelines regarding the use of AI when making pull requests.

We use a review queue to manage reviews. If your PR does not appear there, it is probably because it is not successfully building (i.e., it doesn't have a green checkmark), has the awaiting-author tag, or another reason described in the Lifecycle of a PR. The review dashboard has a dedicated webpage which shows whether your PR is on the review queue, and (if not), why.

If you haven't already done so, please come to Zulip and join the Lean community.
Thank you again for joining our community.

@zhikaip
zhikaip requested a review from bryangingechen October 1, 2026 21:33
@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

PR summary 9c320c48ed

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff (regex)

+ _kill_tree(proc:

You can run this locally as follows
## from your `mathlib4` directory:
git clone https://github.com/leanprover-community/mathlib-ci.git ../mathlib-ci

## summary with just the declaration names:
../mathlib-ci/scripts/pr_summary/declarations_diff.sh <optional_commit>

## more verbose report:
../mathlib-ci/scripts/pr_summary/declarations_diff.sh long <optional_commit>

The doc-module for scripts/pr_summary/declarations_diff.sh in the mathlib-ci repository contains some details about this script.

Declarations diff (Lean)

✅ Lean-aware diff — post-build, computed from the Lean environment (commit 9c320c4).

  • +0 new declarations
  • −0 removed declarations

No declaration differences.


No changes to strong technical debt.
No changes to weak technical debt.

Current commit 9c320c48ed
Reference commit 850d7abd39

This script lives in the mathlib-ci repository. To run it locally, from your mathlib4 directory:

git clone https://github.com/leanprover-community/mathlib-ci.git ../mathlib-ci
../mathlib-ci/scripts/reporting/technical-debt-metrics.py pr_summary
  • The relative value is the weighted sum of the differences with weight given by the inverse of the current value of the statistic.
  • The absolute value is the relative value divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).

@github-actions github-actions Bot added the CI Modifies the continuous integration setup or other automation label Oct 1, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Modifies the continuous integration setup or other automation new-contributor This PR was made by a contributor with at most 5 merged PRs. Welcome to the community!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant