Skip to content

chore: add fix_includes.py for include cleanup workflow#25

Open
lucasfang wants to merge 2 commits into
apache:mainfrom
lucasfang:fix
Open

chore: add fix_includes.py for include cleanup workflow#25
lucasfang wants to merge 2 commits into
apache:mainfrom
lucasfang:fix

Conversation

@lucasfang
Copy link
Copy Markdown
Contributor

Purpose

Linked issue: close #TODO

Introduce build_support/fix_includes.py to support include cleanup and ordering workflow in build tooling.
This change adds one utility script and does not modify runtime code paths.

Tests

  1. python3 -m py_compile build_support/fix_includes.py (pass)
  2. Verified migrated file line count: 2498
  3. Confirmed file is tracked as a single new addition in this change set

API and Format

No impact on public API under include/ and no storage/protocol format change.

Documentation

No end-user feature introduced. This is a build/developer tooling migration.

Generative AI tooling

Generated-by: GitHub Copilot (GPT-5.3-Codex)

Copilot AI review requested due to automatic review settings May 27, 2026 07:45
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

@leaves12138 leaves12138 left a comment

Choose a reason for hiding this comment

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

Thanks for adding the include-cleanup helper. I found one workflow blocker compared with the upstream IWYU script behavior.

else:
ProcessIWYUOutput(sys.stdin, files_to_modify, flags, cwd=os.getcwd())

return 0
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This currently always returns 0, even in --dry_run mode when files would be modified. The upstream fix_includes.py returns min(process_ret, 100) for dry runs, and the help text above also documents that behavior. With this change, CI/pre-commit workflows cannot use fix_includes.py --dry_run to fail when includes need cleanup, so include drift would be silently accepted. Please keep the upstream dry-run exit-code behavior (or update the workflow/docs if a different contract is intended).

Copy link
Copy Markdown

@leaves12138 leaves12138 left a comment

Choose a reason for hiding this comment

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

Acknowledged that this file is intended to remain a verbatim IWYU copy. Please disregard my previous dry-run exit-code comment as a blocker; preserving the upstream script unchanged is reasonable. I have no further blocking comments.

Copy link
Copy Markdown

@leaves12138 leaves12138 left a comment

Choose a reason for hiding this comment

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

Re-reviewed the latest update. The LICENSE now lists build_support/fix_includes.py under the include-what-you-use / UIUC-NCSA notice, and preserving the imported script unchanged is fine. I have no further comments.

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.

3 participants