-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: CMake update, part 1 #2338
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
20ec16c
feat: drop CMake 3.6 and below, modernize CMake
henryiii c5b2dae
fix: address review points from @YannickJadoul
henryiii d0e66a3
fix: typo for warning on C++17, support Win + 2.7
henryiii c2ef5b4
update: address review points
henryiii 1987d89
format: add cmake-format to pre-commit
henryiii f11e5bc
format: apply cmake-format
henryiii File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| parse: | ||
| additional_commands: | ||
| pybind11_add_module: | ||
| flags: | ||
| - THIN_LTO | ||
| - MODULE | ||
| - SHARED | ||
| - NO_EXTRAS | ||
| - EXCLUDE_FROM_ALL | ||
| - SYSTEM | ||
|
|
||
| format: | ||
| line_width: 99 | ||
| tab_size: 2 | ||
|
|
||
| # If an argument group contains more than this many sub-groups | ||
| # (parg or kwarg groups) then force it to a vertical layout. | ||
| max_subgroups_hwrap: 2 | ||
|
|
||
| # If a positional argument group contains more than this many | ||
| # arguments, then force it to a vertical layout. | ||
| max_pargs_hwrap: 6 | ||
|
|
||
| # If a cmdline positional group consumes more than this many | ||
| # lines without nesting, then invalidate the layout (and nest) | ||
| max_rows_cmdline: 2 | ||
| separate_ctrl_name_with_space: false | ||
| separate_fn_name_with_space: false | ||
| dangle_parens: false | ||
|
|
||
| # If the trailing parenthesis must be 'dangled' on its on | ||
| # 'line, then align it to this reference: `prefix`: the start' | ||
| # 'of the statement, `prefix-indent`: the start of the' | ||
| # 'statement, plus one indentation level, `child`: align to' | ||
| # the column of the arguments | ||
| dangle_align: prefix | ||
| # If the statement spelling length (including space and | ||
| # parenthesis) is smaller than this amount, then force reject | ||
| # nested layouts. | ||
| min_prefix_chars: 4 | ||
|
|
||
| # If the statement spelling length (including space and | ||
| # parenthesis) is larger than the tab width by more than this | ||
| # amount, then force reject un-nested layouts. | ||
| max_prefix_chars: 10 | ||
|
|
||
| # If a candidate layout is wrapped horizontally but it exceeds | ||
| # this many lines, then reject the layout. | ||
| max_lines_hwrap: 2 | ||
|
|
||
| line_ending: unix | ||
|
|
||
| # Format command names consistently as 'lower' or 'upper' case | ||
| command_case: canonical | ||
|
|
||
| # Format keywords consistently as 'lower' or 'upper' case | ||
| # unchanged is valid too | ||
| keyword_case: 'upper' | ||
|
|
||
| # A list of command names which should always be wrapped | ||
| always_wrap: [] | ||
|
|
||
| # If true, the argument lists which are known to be sortable | ||
| # will be sorted lexicographically | ||
| enable_sort: true | ||
|
|
||
| # If true, the parsers may infer whether or not an argument | ||
| # list is sortable (without annotation). | ||
| autosort: false | ||
|
|
||
| # Causes a few issues - can be solved later, possibly. | ||
| markup: | ||
| enable_markup: false | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,33 +34,31 @@ jobs: | |
|
|
||
| - name: Make build directories | ||
| run: | | ||
| mkdir build2.8 | ||
| mkdir build3.7 | ||
| mkdir build3.11 | ||
| mkdir build3.18 | ||
|
|
||
| - name: Setup CMake 2.8 | ||
| - name: Setup CMake 3.7 | ||
| uses: jwlawson/[email protected] | ||
| with: | ||
| cmake-version: 2.8 | ||
| cmake-version: 3.7 | ||
|
|
||
| - name: Configure 2.8 | ||
| working-directory: build2.8 | ||
| - name: Configure 3.7 | ||
| working-directory: build3.7 | ||
| run: > | ||
| cmake --version && | ||
| cmake .. | ||
| -DPYBIND11_WERROR=ON | ||
| -DDOWNLOAD_CATCH=ON | ||
| -DPYTHON_EXECUTABLE=$(python -c "import sys; print(sys.executable)") | ||
|
|
||
| - name: Setup CMake 3.7 | ||
| - name: Setup CMake 3.11 | ||
| uses: jwlawson/[email protected] | ||
| with: | ||
| cmake-version: 3.7 | ||
| cmake-version: 3.11 | ||
|
|
||
| - name: Configure 3.7 | ||
| working-directory: build3.7 | ||
| - name: Configure 3.11 | ||
| working-directory: build3.11 | ||
| run: > | ||
| cmake --version && | ||
| cmake .. | ||
| -DPYBIND11_WERROR=ON | ||
| -DDOWNLOAD_CATCH=ON | ||
|
|
@@ -74,7 +72,6 @@ jobs: | |
| - name: Configure 3.18 | ||
| working-directory: build3.18 | ||
| run: > | ||
| cmake --version && | ||
| cmake .. | ||
| -DPYBIND11_WERROR=ON | ||
| -DDOWNLOAD_CATCH=ON | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First time I heard about .cmake-format, that's neat! One related question: we're beginning to "clog up" the root directory with many dot-files related to CI infrastructure. Can we move them to '.github' as well?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting them in
.githubis probably a bad idea, as the files in there have special meanings for GitHub - like.github/CONTRIBUTING.md, issue templates, pull request templates, actions, workflows, etc. But this one could go anywhere, such as intools. The problem is that if you try to runcmake-formatfrom the command line, it will pick this up only if it's in the main folder. However, I would expect this and all the other formatters to only be run bypre-commit, so that's not a big deal. The.pre-commit-config.yamlfile needs to be in the main folder. I rather think the.clang-formatfile will need to be in the main folder (IIRC,-style=fileis the parameter you run, which doesn't include a path).These are all just dot files, so they only make the main GitHub page longer (which is still a little irritating, but a common issue, especially for Python).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(My general rule is that I always use the default name/location if something expects it, so tools "just work" if someone tries to run them; but if there's not special meaning to a name/path, I put it in a folder to keep the page clean)