This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Fix command line is too long issue which breaks test on Windows
ClosedPublic

Authored by dougpuob on Aug 2 2021, 11:26 PM.

Details

Summary

This patch tries to fix command line too long problem on Windows for https://reviews.llvm.org/D86671.

The command line is too long with check_clang_tidy.py program on Windows, because the configuration is long for regression test. Fix this issue by creating a temporary response file then pass it to target program. (error log: http://45.33.8.238/win/43180/step_8.txt)

Diff Detail

Event Timeline

dougpuob created this revision.Aug 2 2021, 11:26 PM
dougpuob requested review of this revision.Aug 2 2021, 11:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2021, 11:26 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
dougpuob edited the summary of this revision. (Show Details)Aug 3 2021, 12:11 AM
dougpuob added a reviewer: thakis.
thakis added a comment.Aug 3 2021, 3:51 AM

Thanks!

Since the config file contents are fixed as far as I can tell, maybe we could instead just add a file with the right contents to clang-tools-extra/test/clang-tidy/checkers/Inputs and refer to that? (Use %S/Inputs/myfile.txt) (Alternatively you could use the split-file utility, but putting the file in Inputs is probably easier if you're new, and it's not any worse :) )

dougpuob updated this revision to Diff 363698.Aug 3 2021, 4:45 AM
  • Moved config content of regression test to clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/hungarian-notation*/.clang-tidy, then refer to those files in tests.

Thanks!

Since the config file contents are fixed as far as I can tell, maybe we could instead just add a file with the right contents to clang-tools-extra/test/clang-tidy/checkers/Inputs and refer to that? (Use %S/Inputs/myfile.txt) (Alternatively you could use the split-file utility, but putting the file in Inputs is probably easier if you're new, and it's not any worse :) )

Hi @thakis:
It's a really good idea, thank you for the suggestion. I just upload the latest diff.

thakis accepted this revision.Aug 3 2021, 4:56 AM

Thanks!

clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/hungarian-notation1/.clang-tidy
4

can we drop all the spaces here? :)

This revision is now accepted and ready to land.Aug 3 2021, 4:56 AM

In general, would it be possible to specify the configuration file as a custom path, and not use the hidden filename .clang-tidy in an otherwise empty directory? I think there's a --config flag on Tidy's interface for giving any file from any path for this. So it could be config1.yaml or something like that instead.

clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/hungarian-notation1/.clang-tidy
2

I do not think this file should have the execute bit set...

4

And maybe even the object brackets.

dougpuob marked 3 inline comments as done.Aug 3 2021, 6:11 AM
dougpuob added inline comments.
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/hungarian-notation1/.clang-tidy
2

I will change it to 664.

4

Sure.

4

OK

dougpuob updated this revision to Diff 363717.Aug 3 2021, 6:12 AM
dougpuob marked 3 inline comments as done.
  • Improved code review suggestions.

Hi @thakis:
I can't access the repo, could you please help me to land?

thakis added a comment.Aug 3 2021, 1:40 PM

The bot is happy again, thanks.

The bot is happy again, thanks.

Nice, thank you for your help :)