-
Notifications
You must be signed in to change notification settings - Fork 37.7k
build: Remove Autotools-based build system #30664
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
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
6e768be
to
9c4cf4f
Compare
🚧 At least one of the CI tasks failed. HintsMake sure to run all tests locally, according to the documentation. The failure may happen due to a number of reasons, for example:
Leave a comment here, if you need help tracking down a confusing failure. |
@@ -3,154 +3,21 @@ | |||
!/build-aux | |||
!/build_msvc | |||
|
|||
*.tar.gz |
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.
I'm still not sure how I feel about removing all of these. We are going to switch a lot between the versions with and without cmake over the next two years. Is it really desirable to have all these files appear in the untracked list?
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.
What do you suggest?
I'm still not sure how I feel about removing all of these. We are going to switch a lot between the versions with and without cmake over the next two years. Is it really desirable to have all these files appear in the untracked list?
From my personal experience during recent months, it isn't a problem at all.
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.
When I review a backport and come back again, this is how it looks like:
> git status
On branch master
Untracked files:
(use "git add <file>..." to include in what will be committed)
Makefile
Makefile.in
aclocal.m4
autom4te.cache/
... another 420 lines
This makes using git status
and a bunch of other git commands very annoying.
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.
I think it is generally unsupported to switch to old branches back-and-forth and expect the gitignore to function. Does make clean && make distclean
before switching branches help? Alternatively, do out-of-tree autotools builds help to avoid the untracked files?
6811a2b
to
faa382a
Compare
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.
Only change since last review is addressing feedback (removing a script and gitignore changes).
The gitignore still seem inconsistent, but I don't care much either way personally.
re-ACK faa382a 🍦
Show signature
Signature:
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK faa382ae7642da0e436ea2c7f7eac67386280a7e 🍦
Pa+7MqVHY/VDhJZ9Bdbr49ElHmQ1m0FcmiE8YrnUkMTICFE3Iy6QvS8NHDfZWYXhVTSI8h8SiPcuVU/h+1urAQ==
.deps | ||
.dirstamp | ||
.libs | ||
.*.swp |
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.
Why remove this? Seem unrelated to autotools.
.*.swp | ||
*~ | ||
*.bak | ||
*.rej |
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.
Same, etc
contrib/devtools/split-debug.sh | ||
|
||
# Output from running db4 installation | ||
db4/ |
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.
Note: This is a follow-up to the install_db4.sh
removal, but seems fine to do here.
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.
ACK faa382a
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.
ACK faa382a
The gitignore still seem inconsistent,
Especially given the comments in #30731 (comment). Seems those should have been removed for the same reasoning as #30664 (comment). Changes here seem completely arbitrary.
My latest review comments are on lines that are unrelated to switching branches. It is about temporary files in the source dir. Such files were created before cmake and will be created after cmake, because developers editing the source files will create them. Personally I don't use gitignore, so I don't care, but it would be good to explain why the source-editing temporary-files changes are made and how they relate to autotools/cmake at all. Otherwise, there will be more follow-ups and confusion in the future. |
After the recent full removal of Autotools (PR #30664), these macros are not needed anymore in the .cpp files according to the TODO in qt's CMakeLists.txt. Tested building on OpenBSD 7.5, where the XCB plugin was still imported according to the debug log.
…IN` macro calls 7346b01 qt, build: remove unneeded `Q_IMPORT_PLUGIN` macro calls (Sebastian Falbesoner) Pull request description: After the recent full removal of Autotools (PR [#30664](#30664)), these macros are not needed anymore in the .cpp files according to the TODO in qt's CMakeLists.txt. Tested building on OpenBSD 7.5, where the XCB plugin was still imported according to the debug log: ``` 2024-09-02T21:13:27Z Bitcoin Core version v28.99.0-7346b0109208 (release build) 2024-09-02T21:13:27Z Qt 5.15.12 (dynamic), plugin=xcb 2024-09-02T21:13:27Z No static plugins. 2024-09-02T21:13:27Z Style: fusion / QFusionStyle 2024-09-02T21:13:27Z System: OpenBSD 7.5, x86_64-little_endian-lp64 ``` ACKs for top commit: hebasto: ACK 7346b01. Tree-SHA512: ffa033fc6e0412a99d2c167044cc7af89512a731172d6911db71434f5353e811802c268d853a76d3732e9da954444cf6c39a852aeb25938c435826e117a16fca
Updates the .editorconfig file, first introduced in 2021 (see PR bitcoin#21123, commit 7a135d5) w.r.t. following changes: - consider Rust .rs files (relevant since bitcoin#29076, commit bbbbdb0) - reflect build system change to CMake (bitcoin#30454, bitcoin#30664) - add setting for bare Makefile still used for depends builds Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim).
Updates the .editorconfig file, first introduced in 2021 (see PR bitcoin#21123, commit 7a135d5) w.r.t. following changes: - consider Rust .rs files (relevant since bitcoin#28076, commit bbbbdb0) - reflect build system change to CMake (bitcoin#30454, bitcoin#30664) - add setting for the bare Makefile still used for depends builds Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim).
Updates the .editorconfig file, first introduced in 2021 (see PR bitcoin#21123, commit 7a135d5) w.r.t. following changes: - consider Rust .rs files (relevant since bitcoin#28076, commit bbbbdb0) - reflect build system change to CMake (bitcoin#30454, bitcoin#30664) - add setting for the bare Makefile still used for depends builds Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim).
9556061 code style: update .editorconfig file (Sebastian Falbesoner) Pull request description: Updates the .editorconfig file, first introduced in 2021 (see PR #21123, commit 7a135d5) w.r.t. following changes: - consider Rust .rs files (relevant since #28076, commit bbbbdb0) - reflect build system change to CMake (#30454, #30664) - add setting for bare Makefile still used for depends builds Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim). The PR is made under the assumption that the file is still considered useful, especially for new contributors. If people feel like that's not the case anymore, the alternative is to delete it, obviously. Top commit has no ACKs. Tree-SHA512: 8406b1caf31e310f7e17c607d97beac583481e71b4425e0be2bbd8207096aa374a70151b58aae5fdb648ef5ff5c7e1d0a2949e6de3355bdd2009d8353ee24af0
Updates the .editorconfig file, first introduced in 2021 (see PR bitcoin#21123, commit 7a135d5) w.r.t. following changes: - consider Rust .rs files (relevant since bitcoin#28076, commit bbbbdb0) - reflect build system change to CMake (bitcoin#30454, bitcoin#30664) - add setting for the bare Makefile still used for depends builds Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim).
Updates the .editorconfig file, first introduced in 2021 (see PR bitcoin#21123, commit 7a135d5) w.r.t. following changes: - consider Rust .rs files (relevant since bitcoin#28076, commit bbbbdb0) - reflect build system change to CMake (bitcoin#30454, bitcoin#30664) - add setting for the bare Makefile still used for depends builds Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim).
Updates the .editorconfig file, first introduced in 2021 (see PR bitcoin#21123, commit 7a135d5) w.r.t. following changes: - consider Rust .rs files (relevant since bitcoin#28076, commit bbbbdb0) - reflect build system change to CMake (bitcoin#30454, bitcoin#30664) - add setting for the bare Makefile still used for depends builds Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim).
7025942 build: drop superfluous `HAVE_BUILD_INFO` define (Sebastian Falbesoner) 0dd6625 build: drop obj/ subdir for generated build.h, rename to bitcoin-build-info.h (Sebastian Falbesoner) Pull request description: As indicated by the TODO, the obj subdirectory is not needed anymore now for the generated build.h header, since autotools are gone and we don't have in-source builds anymore (see #30454, #30664). In the second commit the superflous `HAVE_BUILD_INFO` macro is dropped, as suggested in #30856 (review). ACKs for top commit: theuni: utACK 7025942 Tree-SHA512: 0a3b2cbbcf638344ceb74e5ba5a0fe2b1718427b23a18c8890258db36ce7177006a146178ed88d9c5ae956a5426f3844e86c1f4cca7c40946359742bffda983b
This PR deletes the Autotools-based build system.
The MSVC build system is deleted in #30731.