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

Update clang-format/clang-tidy tools #59545

Closed
BruceForstall opened this issue Sep 23, 2021 · 8 comments
Closed

Update clang-format/clang-tidy tools #59545

BruceForstall opened this issue Sep 23, 2021 · 8 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@BruceForstall
Copy link
Member

BruceForstall commented Sep 23, 2021

Currently, we use cached 3.8 versions, stored in the https://clrjit.blob.core.windows.net/clang-tools Azure Storage location. These haven't been updated for 5+ years. Update them to something current, which will help enable broader CLR use enabled by #59374.

These are used by the jit-format tool from https://github.com/dotnet/jitutils, both manually and in the "Formatting" AzDO CI pipelines.

cc @jkoritzinsky

category:eng-sys
theme:testing
skill-level:beginner
cost:medium
impact:small

@BruceForstall BruceForstall added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 23, 2021
@BruceForstall BruceForstall added this to the 7.0.0 milestone Sep 23, 2021
@ghost
Copy link

ghost commented Sep 23, 2021

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

Currently, we use cached 3.8 versions, stored in the https://clrjit.blob.core.windows.net/clang-tools Azure Storage location. These haven't been updated for 5+ years. Update them to something current, which will help enable broader CLR use enabled by #59374.

These are used by the jit-format tool from https://github.com/dotnet/jitutils, both manually and in the "Formatting" AzDO CI pipelines.

cc @jkoritzinsky

Author: BruceForstall
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: 7.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Sep 23, 2021
@BruceForstall BruceForstall removed the untriaged New issue has not been triaged by the area owner label Sep 23, 2021
@jkoritzinsky
Copy link
Member

We should make sure to name the new files with their version so as to not break the formatting pipelines on servicing branches.

@BruceForstall
Copy link
Member Author

We should make sure to name the new files with their version so as to not break the formatting pipelines on servicing branches.

Make sense, although we typically turn off formatting jobs in servicing pipelines... if we remember, or it becomes an issue.

@BruceForstall
Copy link
Member Author

@jkoritzinsky Have you thought any more about updating clang-format/clang-tidy from 3.8 to something newer?

@jkoritzinsky
Copy link
Member

Yes I'd love to update the clang tools to the same version as the rest of the clang/llvm suite we use (16.x).

The biggest problem is that we'd need to stage the transition somewhat as (due to new rules, various changes, better parsing support), it's impossible to replicate the existing formatting exactly with newer versions of the tools. So when we transition, we'd need to take a big reformatting commit.

Also, we'd need to update the jit-format tool to have a way to select the version of the tools to use (for downstream and for while we transition to the new version).

I'd recommend that we create a docker image with the new version of the tools and provide an option for the jit tooling to use the "live" tools. That will make it easier to upgrade them in the future and keep them in sync.

@BruceForstall
Copy link
Member Author

Also, we'd need to update the jit-format tool to have a way to select the version of the tools to use (for downstream and for while we transition to the new version).

What do you mean by "downstream"? I don't think we should ever worry about re-formatting non-main branches.

I'd recommend that we create a docker image with the new version of the tools and provide an option for the jit tooling to use the "live" tools. That will make it easier to upgrade them in the future and keep them in sync.

I don't want to depend on Docker. I don't see the benefit, and I see lots of negatives.

One thing I don't understand: you introduced eng\formatting that downloads the JIT cached 3.8 formatting tools: #59374. Who uses this tooling? In my dev inner loop I use jitutils 'bootstrap.sh/cmd' to download the tools. I don't want to need to worry about updating any code base except the JIT when I add new cached clang-format/clang-tidy tools.

@jkoritzinsky
Copy link
Member

I introduced that so developers in the repo could set up a pre-commit hook that uses the downloaded tools to reformat before committing (and as a result not have to worry about the format job failing CI). I never got around to polishing up that flow though.

@BruceForstall
Copy link
Member Author

Fixed by #100498

@github-actions github-actions bot locked and limited conversation to collaborators May 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

No branches or pull requests

3 participants