tools: update clang-format to v19

[RedYetiDev]

The previous attempt to update clang-format used a WASM compiled library, which unfortunately had it's issues, and was reverted.

This attempt uses the direct binaries instead of the WASM compiled library, so it can be ensured that they are identical to what is expected.

The source code for the updated clang-format library is https://github.com/RedYetiDev/node-core-clang-format. The binaries are generated from the same repository to ensure security and capability.

[RedYetiDev]

The issue with the WASM was that it wasn't writing the files correctly. I've confirmed that is not this case with this setup.

CC @nodejs/cpp-reviewers

[anonrig]

There is no way to validate the correctness, capability and security of a binary. If we are going to do this, it should be maintained by the Node.js organization, or it should be a package thats well known to the ecosystem.

[targos]

FWIW I still have an experimental repo that builds clang-format on GitHub actions in my computer (I deleted it from GitHub when the wasm-based tool landed), if you want to start from something.

[RedYetiDev]

FWIW I still have an experimental repo that builds clang-format on GitHub actions in my computer (I deleted it from GitHub when the wasm-based tool landed), if you want to start from something.

Interesting. I was using @muttleyxd's built binaries, but I'd be happy to take a look at your builds.

[targos]

https://github.com/targos/clang-format
With test build: https://github.com/targos/clang-format/actions/runs/11124135962

[RedYetiDev]

Thanks! I'll incorporate that workflow

[RedYetiDev]

Here is the workflow I have setup:
https://github.com/RedYetiDev/node-core-clang-format/blob/main/.github/workflows/build.yml.

[RedYetiDev]

@anonrig the binaries are now built from a workflow (https://github.com/RedYetiDev/node-core-clang-format/blob/main/.github/workflows/build.yml) directly from llvm, so it can be assured that they are safe and correct.

it should be maintained by the Node.js organization

I'm happy to transfer the repo into the org if needed.

[anonrig]

cc @nodejs/security-wg

[targos]

So you took my idea and work to your own repository and are now suggesting to transfer that to Node.js. Not nice...

[RedYetiDev]

I didn't mean for it to come off like that, however I don't believe this is taking your idea or work.

While you created a workflow similar, and it was a starting point (for checkout and artifact uploading), I drew inspiration from various sources, such as https://github.com/angular/clang-format/blob/master/build.sh (for MacOS + Linux Building) and https://github.com/muttleyxd/clang-tools-static-binaries/blob/master/.github/workflows/clang-tools-amd64.yml (for Windows Building) when drawing up this method of creating binaries. I wouldn't say I "took [your] idea and work".

Even so, the original "idea" still comes from Angular's clang-format, and the "work" is my own, but as with many OSS projects, it has inspiration from various sources.

OSS is a collaborative ecosystem. Bits and peices from all over help to contribute to the bigger picture.

[RedYetiDev]

@anonrig are you still blocking? Thank you!

[anonrig]

@anonrig are you still blocking? Thank you!

@RedYetiDev I tried to answer it before, but I'll repeat myself once again. Yes, there is no way to validate the correctness of the distributed binaries. Asking the same question over and over again does not change the fact that this creates a significant security concern.

[RedYetiDev]

Apologies, I understand now.

[RedYetiDev]

The way I see it is there is a few things that can be done:

1. Do nothing, leave our clang-format outdated
2. Use a prebuilt binary (Whether it be from the package I created, or another package, such as https://github.com/ssciwr/clang-format-wheel or something)
3. Use the WASM version, which didn't work last time, but has been patched since then.

[lumirlumir]

Hello @RedYetiDev,

I recently came across this by chance and wanted to reach out.

I've developed a package succeding angular/clang-format called lumirlumir/clang-format-node, which has grown to over 10,000 users per week.

The entire build process for this package is open and transparent, with all actions and code available for review. It also includes testing across various platforms to ensure maximum stability.

If Node.js would like to manage this package officially, I can give Node.js member or collaborator access to ensure security and ongoing maintenance.

Please feel free to take a look at the package. It would be a great honor If I can collaborate with Node.js on its maintenance.

[RedYetiDev]

Funny thing, I just noticed your package yesterday, and I was planning to switch this to using it later today. If @anonrig is okay with it, seeing as it seems fit the "package thats well known to the ecosystem" claim.

I can't speak on behalf of the organization, but I don't Node.js will need special privileges to that repository

[lumirlumir]

@RedYetiDev Great thing. If you need any help from me, feel free to mention me. I’m always on GitHub.

[mhdawson]

After some discussion in the private section in the TSC meeting today, I'm going to block until we have a discussion in the security wg on a policy/direction/requirements in terms of pulling in new tools into the build process.

[mhdawson]

Also tagged for discussion in the next security wg meeting.

[RedYetiDev]

Thanks! Just a note for the security team: Dependabot PRs, while they are created from internal branches, run pull_request checks without the same additional access. They run in a "dependabot" context.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.91%. Comparing base (58a8eb4) to head (5484ac9).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55201      +/-   ##
==========================================
- Coverage   88.40%   87.91%   -0.50%     
==========================================
  Files         654      654              
  Lines      187815   187815              
  Branches    36136    35829     -307     
==========================================
- Hits       166045   165113     -932     
- Misses      15001    15892     +891     
- Partials     6769     6810      +41     

see 92 files with indirect coverage changes

[lumirlumir]

Hello @RedYetiDev,

First of all, I apologize if my question is somewhat unrelated to the current PR.

I have a question: would it be possible to transfer the clang-format-node package I created to the Node.js group, so it could be managed collaboratively by the Node.js community?

I read through the contribution guidelines, and it seems this might be possible with the approval of collaborators, so I wanted to reach out and ask. (If posting this question in the discussion section for the Node.js group would be more appropriate, I’d be happy to move it there.)

As I continue developing this package, I believe transferring it to the Node.js group could improve security, especially with regards to binary files, and help ensure stable and robust maintenance. For these reasons, I wanted to explore the idea of working together on it.

Similar to how Google’s Angular team manages their own clang-format package, I feel that having the Node.js group maintain clang-format-node would allow users to trust and adopt it more confidently.

Additionally, since I’m still developing my skills and recognize that my name alone may not yet inspire confidence for a broad user base, I think it would be beneficial to manage the package within a larger community.

Thank you for considering this, and please let me know if there’s a more appropriate way to move forward!

[RedYetiDev]

Hi! I'm not a core collaborator, so I'm not the person to ask. The security working group will meet sometime in the future, and I'm sure they'll take this into account. (Although someone can always correct me)

[lumirlumir]

@RedYetiDev Thank you for your response!

I’ll wait until a decision is made.