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.

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

Add Support for clang-format #763

Closed
Kurt-von-Laven opened this issue Sep 22, 2021 · 20 comments · Fixed by #3089
Closed

Add Support for clang-format #763

Kurt-von-Laven opened this issue Sep 22, 2021 · 20 comments · Fixed by #3089
Labels
enhancement New feature or request

Comments

@Kurt-von-Laven
Copy link
Collaborator

"clang-format is located in clang/tools/clang-format and can be used to format C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# code." ~ https://clang.llvm.org/docs/ClangFormat.html

Covering 6 different languages and 2 different serialization formats, this linter offers a lot of bang for your buck and seems like a good fit for Mega-Linter. It is one of a very small number of linters present in Super-Linter but missing from Mega-Linter.

The only officially supported installation method is building from source in conjunction with all of LLVM, but unofficially it is also available as a Docker image and Node.js package from Unibeautify.

@Kurt-von-Laven Kurt-von-Laven added the enhancement New feature or request label Sep 22, 2021
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Oct 23, 2021
@Kurt-von-Laven
Copy link
Collaborator Author

This still seems worthwhile to me.

@Kurt-von-Laven
Copy link
Collaborator Author

Looks like the bot failed to remove the stale label.

@nvuillam nvuillam removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Oct 24, 2021
@nvuillam
Copy link
Member

Did it manually :)

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Nov 24, 2021
@Kurt-von-Laven Kurt-von-Laven removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Nov 25, 2021
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Dec 26, 2021
@github-actions github-actions bot closed this as completed Jan 9, 2022
@nvuillam nvuillam removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Jan 18, 2022
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Feb 18, 2022
@Kurt-von-Laven Kurt-von-Laven removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Feb 19, 2022
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Mar 23, 2022
@github-actions github-actions bot closed this as completed Apr 7, 2022
@daltonv
Copy link
Contributor

daltonv commented Nov 1, 2023

I'm interested enough in this issue to try adding this myself, but is there a consensus if megalinter should install clang-format the official way (ie building llvm from source), or is the much simpler but unofficial npm package acceptable?

@nvuillam
Copy link
Member

nvuillam commented Nov 1, 2023

@daltonv thanks for your motivation :)

It's ok to use a package manager if it is more performant ... and maintained, so we are sure to get the latest version ^^

The neighbors of super-linter use another docker image, it can be checked too

image

@echoix
Copy link
Collaborator

echoix commented Nov 1, 2023

Great for your interest! It is one of the linters from super-linter that is missing in Megalinter.

If you're talking about the npm package by angular, https://www.npmjs.com/package/clang-format, it is already outdated and unmaintained ;(

Building llvm + clang + tools from source each time might be a bit too long in GitHub actions runners, we already have workflows that take almost an hour.
Other factor to consider: the size, in the final image and during building. Do you know what are the dependencies for clang-format? Does it need the whole llvm and clang toolchain in order to run?
We are already nearing close to the disk size limits, and cloning the llvm monorepo can be several GB. (Of course shallow clones should be used)

If I remember correctly, clang-format isn't stable between versions, and thus projects using clang-format usually pin a version. How do we handle that? Do we only provide the latest release, and projects must use another solution if they need a specific version?

And lastly, we need to make sure that it works with the muslc in Alpine.

@nvuillam
Copy link
Member

nvuillam commented Nov 1, 2023

I see that there is an apk package containing clang-format, maybe it can work ?

https://pkgs.alpinelinux.org/package/edge/main/ppc64le/clang16-extra-tools

image

@echoix
Copy link
Collaborator

echoix commented Nov 1, 2023

image
I did a quick test, the dependencies add 532.3 MB

@nvuillam
Copy link
Member

nvuillam commented Nov 1, 2023

it's big, but as clang-format is in super-linter I think it has to be in MegaLinter
But it should be only in full image, not in cupcake on other flavors
Maybe we could create a "c" flavor ?

@nvuillam nvuillam reopened this Nov 1, 2023
@nvuillam nvuillam removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Nov 1, 2023
@daltonv
Copy link
Contributor

daltonv commented Nov 2, 2023

I would definitely be in favor of a "c" flavor. I primarily work on embedded C projects, so I'll take a look at all the linters and see what else makes sense to be in that flavor.

@nvuillam
Copy link
Member

nvuillam commented Nov 2, 2023

@daltonv to add a new flavor, you can check this PR -> https://github.com/oxsecurity/megalinter/pull/2778/files#diff-98d0f806abc9af24e6a7c545d3d77e8f9ad57643e27211d7a7b896113e420ed2

Don't be frightened by the number of files, they are almost all generated :)

Basically, update flavor_factory.py, update the descriptors to add the new flavor name , and run bash build.sh :)

@echoix
Copy link
Collaborator

echoix commented Nov 2, 2023

A while back in January, I started drafting a list of linters, when I started using cmake-format:

New C/C++/CMake linter ideas

Is your feature request related to a problem? Please describe.
Here is a short list of potential linters to add

Describe the solution you'd like
A clear and concise description of what you want to happen.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

@nvuillam
Copy link
Member

nvuillam commented Nov 2, 2023

@echoix indeed more reliable C/C++ linters in MegaLinter could be good ! :)

@daltonv
Copy link
Contributor

daltonv commented Nov 2, 2023

So clang-format certainly makes the most sense in the C descriptor file, but then we would be adding clang-format to the cupcake, dotnet, and dotnet web flavors. Either that or removing cpplint from those. Neither of those options sound good to me.

Maybe we have a new clang descriptor? We could even later add clang-tidy in there (it comes with the clang-extra-tools pkg, but I'm not an expert in using clang-tidy yet).

@nvuillam
Copy link
Member

nvuillam commented Nov 2, 2023

We could have a new flavor c_cpp :)

daltonv added a commit to daltonv/megalinter that referenced this issue Nov 3, 2023
Adds a new flavor for pure c_cpp developement. This flavor can later
add the clang-format and other linters suggested in
oxsecurity#763
@daltonv daltonv mentioned this issue Nov 3, 2023
4 tasks
daltonv added a commit to daltonv/megalinter that referenced this issue Nov 3, 2023
Adds a new flavor for pure c_cpp developement. This flavor can later
add the clang-format and other linters suggested in
oxsecurity#763
nvuillam added a commit that referenced this issue Nov 4, 2023
* Add new c_cpp flavor descriptive items

Next commit will build the descriptive items for this flavor.

* Add c_cpp flavor

Adds a new flavor for pure c_cpp developement. This flavor can later
add the clang-format and other linters suggested in
#763

---------

Co-authored-by: Nicolas Vuillamy <nicolas.vuillamy@gmail.com>
daltonv added a commit to daltonv/megalinter that referenced this issue Nov 8, 2023
Adds a new flavor for pure c_cpp developement. This flavor can later
add the clang-format and other linters suggested in
oxsecurity#763
daltonv added a commit to daltonv/megalinter that referenced this issue Nov 8, 2023
Direct follow-up of the previous commit. Just adds the generated files
that are a result of adding clang-format.

This closes oxsecurity#763
daltonv added a commit to daltonv/megalinter that referenced this issue Nov 8, 2023
Direct follow-up of the previous commit. Just adds the generated files
that are a result of adding clang-format.

This closes oxsecurity#763
daltonv added a commit to daltonv/megalinter that referenced this issue Nov 8, 2023
Direct follow-up of the previous commit. Just adds the generated files
that are a result of adding clang-format.

This closes oxsecurity#763
daltonv added a commit to daltonv/megalinter that referenced this issue Nov 8, 2023
Direct follow-up of the previous commit. Just adds the generated files
that are a result of adding clang-format.

This closes oxsecurity#763
daltonv added a commit to daltonv/megalinter that referenced this issue Nov 13, 2023
Direct follow-up of the previous commit. Just adds the generated files
that are a result of adding clang-format.

This closes oxsecurity#763
daltonv added a commit to daltonv/megalinter that referenced this issue Nov 14, 2023
Direct follow-up of the previous commit. Just adds the generated files
that are a result of adding clang-format.

This closes oxsecurity#763
daltonv added a commit to daltonv/megalinter that referenced this issue Nov 19, 2023
Direct follow-up of the previous commit. Just adds the generated files
that are a result of adding clang-format.

This closes oxsecurity#763
daltonv added a commit to daltonv/megalinter that referenced this issue Nov 19, 2023
Direct follow-up of the previous commit. Just adds the generated files
that are a result of adding clang-format.

This closes oxsecurity#763
daltonv added a commit to daltonv/megalinter that referenced this issue Nov 20, 2023
Direct follow-up of the previous commit. Just adds the generated files
that are a result of adding clang-format.

This closes oxsecurity#763
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants