Skip to content

Add CI script to check line ends#1452

Merged
wks merged 12 commits into
mmtk:masterfrom
wks:feature/line-end-check
Feb 27, 2026
Merged

Add CI script to check line ends#1452
wks merged 12 commits into
mmtk:masterfrom
wks:feature/line-end-check

Conversation

@wks

@wks wks commented Feb 25, 2026

Copy link
Copy Markdown
Collaborator

We add CI scripts to ensure that all text files

  • use UNIX line ends (LF only), and
  • have a newline character at the end of the file.

This PR is inspired by the scripts introduced in the OpenJDK binding in mmtk/mmtk-openjdk#105, but also offers the ability to automatically fix line ends for the user:

./.github/scripts/ci-check-lineends.sh -f

This PR also fixes existing text files that have wrong line ends.

@wks

wks commented Feb 26, 2026

Copy link
Copy Markdown
Collaborator Author

The dos2unix --info=e option is only available since version 7.5.2.

The dos2unix --add-eol option is only available since version 7.5.0.

But Ubuntu 22.04 (on which we run mmtk-core CI tests) only provides dos2unix 7.4.2, and Ubuntu 24.04 only provides 7.5.1. This means we won't be able to implement line-end checking/fixing using dos2unix until we switch to the not-yet-released Ubuntu 26.04 image, or install dos2unix manually.

@wks

wks commented Feb 27, 2026

Copy link
Copy Markdown
Collaborator Author

As we can see in #1453, the CI style check will now fail if any text file has wrong line ending style.

We may introduce .gitattributes later (see #1418 (comment)). We can discuss later whether we use that list for determining which file to check, or we maintain .gitattributes and ci-check-lineends.sh separately.

@wks wks requested a review from qinsoon February 27, 2026 04:13

@qinsoon qinsoon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment thread .github/scripts/ci-style.sh Outdated

# --- Check line ends of text files ---

if ! $project_root/.github/scripts/ci-check-lineends.sh -v; then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we want -v. From the log in https://github.com/mmtk/mmtk-core/actions/runs/22472311227/job/65091763326?pr=1453, without -v, we will skip printing "Processing file" lines, and the output will only show the files that violate the check, which is much cleaner and useful.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes. That makes sense. I just tried to mimic the style of our existing bash scripts which use -x. I think it is cleaner to just print files with errors.

@wks wks enabled auto-merge February 27, 2026 05:55
@wks wks added this pull request to the merge queue Feb 27, 2026
Merged via the queue into mmtk:master with commit 084923f Feb 27, 2026
34 checks passed
@wks wks deleted the feature/line-end-check branch February 27, 2026 07:16
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.

2 participants