build: Remove Autotools-based build system

hebasto commented at 8:05 pm on August 15, 2024: member

This PR deletes the Autotools-based build system.

The MSVC build system is deleted in #30731.

hebasto added the label Build system on Aug 15, 2024

DrahtBot commented at 8:05 pm on August 15, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	Sjors
Stale ACK	vasild, maflcko

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#30635 (rpc: add optional blockhash to waitfornewblock by Sjors)
#30570 ([tests] New fuzz target wallet_rpc by kevkevinpal)
#30510 (multiprocess: Add IPC wrapper for Mining interface by ryanofsky)
#30509 (multiprocess: Add -ipcbind option to bitcoin-node by ryanofsky)
#30437 (multiprocess: add bitcoin-mine test program by ryanofsky)
#30239 (Ephemeral Dust by instagibbs)
#30110 (refactor: TxDownloadManager + fuzzing by glozow)
#30079 (Fee Estimation: Ignore all transactions that are CPFP’d by ismaelsadeeq)
#30075 (test, subprocess: Improve coverage report correctness by hebasto)
#29936 (fuzz: wallet: add target for CreateTransaction by brunoerg)
#29532 (Refactor BnB tests by murchandamus)
#29450 (build: replace custom MAC_OSX macro with existing __APPLE__ by l0rinc)
#29432 (Stratum v2 Template Provider (take 3) by Sjors)
#29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)
#29409 (multiprocess: Add capnp wrapper for Chain interface by ryanofsky)
#29158 (PoC: fuzz chainstate and block managers by darosior)
#29039 (versionbits refactoring by ajtowns)
#28939 (memusage: let PoolResource keep track of all allocated/deallocated memory by martinus)
#28676 ([WIP] Cluster mempool implementation by sdaftuar)
#28574 (wallet: optimize migration process, batch db transactions by furszy)
#28031 (Package Relay 1/3: Introduce TxDownloadManager and improve orphan-handling by glozow)
#27837 (net: introduce block tracker to retry to download blocks after failure by furszy)
#27331 (refactor: extract CCheckQueue’s data handling into a separate container “Bag” by martinus)
#26966 (index: initial sync speedup, parallelize process by furszy)
#26693 (build: special instruction check script by willcl-ark)
#26593 (tracing: Only prepare tracepoint arguments when actually tracing by 0xB10C)
#26334 (Add Signet launch shortcut for Windows by Sjors)
#26022 (Add util::ResultPtr class by ryanofsky)
#25665 (refactor: Add util::Result failure values, multiple error and warning messages by ryanofsky)
#24539 (Add a “tx output spender” index by sstone)
#24230 (indexes: Stop using node internal types and locking cs_main, improve sync logic by ryanofsky)
#21283 (Implement BIP 370 PSBTv2 by achow101)

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.

hebasto force-pushed on Aug 15, 2024

DrahtBot added the label CI failed on Aug 15, 2024

DrahtBot commented at 8:15 pm on August 15, 2024: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/28830258425

Make sure to run all tests locally, according to the documentation.

The failure may happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

DrahtBot removed the label CI failed on Aug 15, 2024

Sjors commented at 2:23 pm on August 19, 2024: member

Concept ACK on dropping Autotools quickly after merging cmake. Maintaining both sounds like a nightmare. Mainly for build system folks, but it’s also more work for people making pull requests that add/remove files. Best just get it over with.

I tested rebasing my Stratum v2 related pull requests (#29346, #30315, #29432 and several on my own fork). It was pretty straight-forward, see https://github.com/Sjors/bitcoin/pull/58.

I haven’t tried rebasing the IPC (multiprocess) variant of my PR. Testing @ryanofsky’s #30437 on top of this PR should probably reveal any relevant issues.

in src/Makefile.am:380 in de2dda9e6a outdated

374-  zmq/zmqutil.h
375-
376-
377-obj/build.h: FORCE
378-	@$(MKDIR_P) $(builddir)/obj
379-	$(AM_V_GEN) $(top_srcdir)/share/genbuild.sh "$(abs_top_builddir)/src/obj/build.h" \

maflcko commented at 7:18 am on August 21, 2024:

style nit: If you are removing the only invocation of this script, wouldn’t it be better to remove the other mentions in the docs as well?

0cmake/script/GenerateBuildInfo.cmake:# This script is a multiplatform port of the share/genbuild.sh shell script.
1
2src/clientversion.cpp:// The <obj/build.h>, which is generated by the build environment (share/genbuild.sh),

maflcko commented at 12:04 pm on August 28, 2024:

Did you see this comment?

maflcko commented at 12:07 pm on August 28, 2024:

Same for all other stuff that is no longer needed, like bear:

0ci/test/03_test_script.sh:  # accepted in src/.bear-tidy-config
1doc/developer-notes.md:make clean && bear --config src/.bear-tidy-config -- make -j $(nproc)

…

maflcko commented at 10:30 am on August 29, 2024:

Looks like the bear part was partly fixed in master.

hebasto commented at 11:41 am on August 29, 2024:

style nit: If you are removing the only invocation of this script, wouldn’t it be better to remove the other mentions in the docs as well?
0cmake/script/GenerateBuildInfo.cmake:# This script is a multiplatform port of the share/genbuild.sh shell script.
1
2src/clientversion.cpp:// The <obj/build.h>, which is generated by the build environment (share/genbuild.sh),

Thanks! Removed.

TheCharlatan commented at 8:01 pm on August 30, 2024:

Is the script left in share/ on purpose?

hebasto commented at 8:32 pm on August 30, 2024:

Thanks! Deleted.

maflcko commented at 7:19 am on August 21, 2024: member

left a nit

maflcko added this to the milestone 29.0 on Aug 21, 2024

maflcko commented at 7:22 am on August 21, 2024: member

It aims reveal conflicts with other PRs.

For more details, please refer to today’s IRC meeting discussion.

It would be good to mention any important key points inline, otherwise it will be harder to understand whether this is the pull request that will be merged as a follow-up, and whether it should be reviewed, or if there is another place where the review should happen.

hebasto commented at 9:26 am on August 22, 2024: member

It aims reveal conflicts with other PRs. For more details, please refer to today’s IRC meeting discussion.

It would be good to mention any important key points inline, otherwise it will be harder to understand whether this is the pull request that will be merged as a follow-up, and whether it should be reviewed, or if there is another place where the review should happen.

The PR description has been updated with mentioning that this PR is not intended to be reviewed/merged in its current state. The branch will be updated once #30454 is merged.

At this moment, it is only about potential conflicts with other PRs.

DrahtBot added the label Needs rebase on Aug 26, 2024

hebasto force-pushed on Aug 28, 2024

hebasto renamed this:
~~CMake replaces Autotools~~
build: Remove Autotools-based build system
on Aug 28, 2024

hebasto marked this as ready for review on Aug 28, 2024

hebasto commented at 11:18 am on August 28, 2024: member

Since #30454 has been merged, this PR is rebased and its description updated.

Ready for review now :)

Additional context to consider:

DrahtBot removed the label Needs rebase on Aug 28, 2024

maflcko changes_requested

maflcko commented at 12:09 pm on August 28, 2024: member

I think this should properly remove all no-longer needed stuff.

Also, I think this should wait a few days, to allow everyone some time to upgrade and test parity, etc. See #30454 (comment)

temporary Approach NACK for that reason, from me at least

vasild approved

vasild commented at 12:10 pm on August 28, 2024: contributor

ACK debeb98fe3822962768127e5b5d312926f123211

I already reviewed this at https://github.com/hebasto/bitcoin/pull/166. Only minor changes since that + doc adjustment.

DrahtBot requested review from Sjors on Aug 28, 2024

maflcko added the label DrahtBot Guix build requested on Aug 28, 2024

vasild commented at 12:15 pm on August 28, 2024: contributor

from #30454 (comment):

I am not going to block the autotools removal

then, here:

NACK

Eh?

maflcko commented at 12:26 pm on August 28, 2024: member

Eh?

Maintainers are free to ignore a review, but as per my previous comment, I think rushing this in without further and proper review will just lead to more follow-up pulls dealing with the stuff that was forgotten to be removed. Also, it is going to make parity testing harder while doing cmake follow-ups.

I just don’t see why this needs to be rushed, when there is no downside or even risk in waiting a few days at a minimum.

If I am missing a downside, it would be good to mention it.

DrahtBot requested review from maflcko on Aug 28, 2024

DrahtBot commented at 5:16 am on August 29, 2024: contributor

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

File	commit 2eb358b25f628a88e3dd48811c0750a71befbead(master)	commit 811f90be8f6a26efb2800a02828298069f495a76(master and this pull)
SHA256SUMS.part	`e8f6ecf13fdf7355...`	`6f16034e19cfdcda...`
*-aarch64-linux-gnu-debug.tar.gz	`e9c1d6379f443848...`	`d02b9a8b8ad232f6...`
*-aarch64-linux-gnu.tar.gz	`c68e67f5c95dab79...`	`cc0236620f4fffa7...`
*-arm-linux-gnueabihf-debug.tar.gz	`5b1019a15913afe2...`	`048aba2e61ff0e3b...`
*-arm-linux-gnueabihf.tar.gz	`92223a91c8f4dae0...`	`a62cf9453946a591...`
*-arm64-apple-darwin-unsigned.tar.gz	`c21004df569f3204...`	`a89e09fd3c5f1e03...`
*-arm64-apple-darwin-unsigned.zip	`1d40c90e757a376c...`	`a3a728ea242b7855...`
*-arm64-apple-darwin.tar.gz	`35176f48396fa660...`	`c853840e0d5c8524...`
*-powerpc64-linux-gnu-debug.tar.gz	`f2756aaf959fb5f3...`	`d91a17ec6980f020...`
*-powerpc64-linux-gnu.tar.gz	`6be4e7945f4cdd72...`	`af14c1b4034d4711...`
*-riscv64-linux-gnu-debug.tar.gz	`87f6d9e58755fc6f...`	`5c78879ee856233a...`
*-riscv64-linux-gnu.tar.gz	`caff6e776229fc89...`	`3fe95380cb228870...`
*-x86_64-apple-darwin-unsigned.tar.gz	`3f16e1495c269ad5...`	`a95422823f39f8f4...`
*-x86_64-apple-darwin-unsigned.zip	`ee3e35bad157338c...`	`5c79ab8e84ddf996...`
*-x86_64-apple-darwin.tar.gz	`102ebcb5f4385ca7...`	`c09c02712db7d535...`
*-x86_64-linux-gnu-debug.tar.gz	`e1e0230561496988...`	`d10bb6c6927858f0...`
*-x86_64-linux-gnu.tar.gz	`eea856d372c7ba09...`	`6d308ef653493957...`
*.tar.gz	`a8774ab55c5bb782...`	`2696cd37e9bef28d...`
guix_build.log	`5fb05c37c38011d5...`	`fb51299eae807b9b...`
guix_build.log.diff		`6a4bf3e8dbe6d325...`

DrahtBot removed the label DrahtBot Guix build requested on Aug 29, 2024

maflcko commented at 10:39 am on August 29, 2024: member

In any case, it would be good to clarify whether review comments such as #30664 (review) are expected to be addressed in this pull or in follow-ups. Otherwise, it is just unclear how reviewers should review this pull request.

bitcoin deleted a comment on Aug 29, 2024

doc: Drop mentions of `share/genbuild.sh` d209e4f156

doc: Adjust `doc/design/libraries.md` e268b48419

hebasto force-pushed on Aug 29, 2024

hebasto commented at 11:46 am on August 29, 2024: member

In any case, it would be good to clarify whether review comments such as #30664 (comment) are expected to be addressed in this pull or in follow-ups. Otherwise, it is just unclear how reviewers should review this pull request.

All comments have been addressed unless I’ve overlooked something.

in ci/test/03_test_script.sh:166 in 688221d15d outdated

161@@ -162,8 +162,6 @@ if [ "${RUN_TIDY}" = "true" ]; then
162     echo "^^^ ⚠️ Failure generated from clang-tidy"
163     false
164   fi
165-  # Filter out files by regex here, because regex may not be
166-  # accepted in src/.bear-tidy-config

fanquake commented at 11:47 am on August 29, 2024:

So is the filtering actually needed anymore, if it was only needed for bear? It’d be good to clarify if it is, or at the same time this comment is dropped, the redundant code should also be dropped?

hebasto commented at 11:51 am on August 29, 2024:

CMake filters out targets where the EXPORT_COMPILE_COMMANDS property is set to OFF.

fanquake commented at 11:54 am on August 29, 2024:

I’m not sure if I understand this answer, but, if the filtering is still needed, shouldn’t the comment be updated instead to mention is is required for CMake?

maflcko commented at 12:12 pm on August 29, 2024:

Yes, the filtering is still needed. If the filtering is moved to the cmake side, I’d say a follow-up would be better.

fanquake commented at 12:21 pm on August 29, 2024:

Ok, so I guess this comment should instead be changed to mention that it is still required by CMake. (Otherwise there is no context for why the filtering is happening or needed).

hebasto commented at 4:14 pm on August 29, 2024:

Yes, the filtering is still needed. If the filtering is moved to the cmake side, I’d say a follow-up would be better.

I’ve just finished my tests. Apparently, filtering is no longer required. ~I’ll submit a PR shortly and we can observe the IWYU output.~

UPD. By “is no longer required”, I mean there are no IWYU violations in those files.

hebasto commented at 10:04 pm on August 29, 2024:

Ok, so I guess this comment should instead be changed to mention that it is still required by CMake. (Otherwise there is no context for why the filtering is happening or needed).

The comment has been adjusted.

fanquake referenced this in commit ffdc3d6060 on Aug 29, 2024

maflcko commented at 4:38 pm on August 29, 2024: member

Could rebase once more to make review easier?

hebasto force-pushed on Aug 29, 2024

hebasto commented at 10:04 pm on August 29, 2024: member

Could rebase once more to make review easier?

Rebased.

DrahtBot added the label CI failed on Aug 30, 2024

vasild approved

vasild commented at 9:42 am on August 30, 2024: contributor

ACK 1954151c98c0ee4a9de3b10c450da235474d24a5

maflcko commented at 11:15 am on August 30, 2024: member

There is a lot more docs to update still. After looking for a few seconds:

 0.editorconfig:[configure.ac]
 1src/util/fs_helpers.cpp:// for posix_fallocate, in configure.ac we check if it is present after this
 2doc/developer-notes.md:Run configure with `--enable-debug` to add additional compiler flags that
 3doc/developer-notes.md:configure option adds `-DDEBUG_LOCKORDER` to the compiler flags. This inserts
 4doc/developer-notes.md:The `--enable-debug` configure option adds `-DDEBUG_LOCKCONTENTION` to the
 5doc/developer-notes.md:configure with `-DDEBUG_LOCKCONTENTION` added to your CPPFLAGS,
 6doc/developer-notes.md:`--with-sanitizers` configure flag, which should be a comma separated list of
 7doc/developer-notes.md:./configure --with-sanitizers=address,undefined
 8doc/developer-notes.md:./configure --with-sanitizers=thread
 9doc/developer-notes.md:undefined sanitizer. If you are missing required libraries, the configure script
10doc/developer-notes.md:with `--with-sanitizers=address,thread` will fail in the configure script as
11doc/multiprocess.md:On unix systems, the `--enable-multiprocess` build option can be passed to `./configure` to build new `bitcoin-node`, `bitcoin-wallet`, and `bitcoin-gui` executables alongside existing `bitcoind` and `bitcoin-qt` executables.
12doc/multiprocess.md:CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure
13doc/multiprocess.md:The configure script will pick up settings and library locations from the depends directory, so there is no need to pass `--enable-multiprocess` as a separate flag when using the depends system (it's controlled by the `MULTIPROCESS=1` option).
14doc/multiprocess.md:Alternately, you can install [Cap'n Proto](https://capnproto.org/) and [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) packages on your system, and just run `./configure --enable-multiprocess` without using the depends system. The configure script will be able to locate the installed packages via [pkg-config](https://www.freedesktop.org/wiki/Software/pkg-config/). See [Installation](https://github.com/chaincodelabs/libmultiprocess/blob/master/doc/install.md) section of the libmultiprocess readme for install steps. See [build-unix.md](build-unix.md) and [build-osx.md](build-osx.md) for information about installing dependencies in general.
15doc/zmq.md:    $ ./configure --disable-zmq (other options)
16share/qt/extract_strings_qt.py:    print('Please install package "gettext" and re-run \'./configure\'.',file=sys.stderr)
17src/qt/README.md:2. Follow the compile instructions for your system, run `./configure` with the `--enable-debug` flag
18test/functional/test_runner.py:        print("Rerun ./configure with --with-daemon and then make")
19test/get_previous_releases.py:            './configure {}'.format(config_flags),

Again, it would be good to clarify whether reviewers should actually review this pull request or just blindly ACK it.

maflcko commented at 11:18 am on August 30, 2024: member

I am not really looking forward to 35 two-line doc fixup pulls over the next couple of months.

It would be good to complete the process fully and quickly with (let’s say) less than 3 pull requests.

DrahtBot removed the label CI failed on Aug 30, 2024

hebasto commented at 12:10 pm on August 30, 2024: member

… or just blindly ACK it.

No-one should blindly ACK.

maflcko commented at 12:42 pm on August 30, 2024: member

To rephrase my question: Do you want reviewers to point out stuff that was forgotten to be removed/adjusted, or not? If yes, do you plan to address it, or not?

hebasto commented at 1:54 pm on August 30, 2024: member

@maflcko

My sincerest apologies for the inconvenience caused by the CMake transition process. My initial plan was to submit one more PR to fix all documentation once this PR is merged.

in .gitignore:138 in abd2c90263 outdated

134-linux-build
135-win32-build
136-test/config.ini
137-test/cache/*
138-test/.mypy_cache/
139-test/lint/test_runner/target/

maflcko commented at 2:51 pm on August 30, 2024:

abd2c902632ecfe4aa5ec886e58d2672de11ee84: Looks wrong.

hebasto commented at 3:34 pm on August 30, 2024:

abd2c90: Looks wrong.

Why? These artifacts are to be created in a build directory. The source directory remains untouched.

maflcko commented at 3:36 pm on August 30, 2024:

The lint runner isn’t integrated into cmake yet

hebasto commented at 3:46 pm on August 30, 2024:

Thanks! Fixed.

in depends/.gitignore:14 in abd2c90263 outdated

 8-arm*
 9-aarch64*
10-powerpc*
11-riscv32*
12-riscv64*
13-s390x*

maflcko commented at 3:14 pm on August 30, 2024:

https://github.com/bitcoin/bitcoin/commit/abd2c902632ecfe4aa5ec886e58d2672de11ee84: Why delete this file?

fanquake commented at 3:39 pm on August 30, 2024:

Yea this is wrong.

hebasto commented at 3:45 pm on August 30, 2024:

Thanks! Fixed.

maflcko added the label DrahtBot Guix build requested on Aug 30, 2024

maflcko commented at 3:26 pm on August 30, 2024: member

Ok, sounds like a plan! (No need to apologize) I can do the follow-up update also, but earliest in the second half of next week. The remaining missing removals/updates (on top of the one in my previous comment) would be:

0test/lint/lint-spelling.py:FILES_ARGS = ['git', 'ls-files', '--', ":(exclude)build-aux/m4/", ":(exclude)contrib/seeds/*.txt", ":(exclude)depends/", ":(exclude)doc/release-notes/", ":(exclude)src/qt/locale/", ":(exclude)src/qt/*.qrc", ":(exclude)contrib/guix/patches"]
1
2doc/build-unix.md:pass `--with-incompatible-bdb` to configure. Otherwise, you can build Berkeley DB [yourself](#berkeley-db).

Possibly drop libtool from:

0.github/workflows/ci.yml:          brew install --quiet automake libtool pkg-config gnu-getopt ccache boost libevent miniupnpc libnatpmp zeromq qt@5 qrencode
1ci/test/00_setup_env.sh:export CI_BASE_PACKAGES=${CI_BASE_PACKAGES:-build-essential libtool autotools-dev automake pkg-config curl ca-certificates ccache python3 rsync git procps bison e2fsprogs cmake}
2ci/test/00_setup_env_i686_centos.sh:export CI_BASE_PACKAGES="gcc-c++ glibc-devel.x86_64 libstdc++-devel.x86_64 glibc-devel.i686 libstdc++-devel.i686 ccache libtool make git python3 python3-pip which patch lbzip2 xz procps-ng dash rsync coreutils bison util-linux e2fsprogs cmake"
3contrib/devtools/check-deps.sh:# Output makefile targets, converting library .a paths to libtool .la targets
4depends/README.md:    apt install automake bison cmake curl libtool make patch pkg-config python3 xz-utils
5depends/packages.md:In general, the output of a depends package should not contain any libtool
6doc/build-unix.md:    sudo dnf install gcc-c++ libtool make autoconf automake python3

maflcko commented at 3:26 pm on August 30, 2024: member

review ACK 1954151c98c0ee4a9de3b10c450da235474d24a5 🎾

Signature:

0untrusted 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}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK 1954151c98c0ee4a9de3b10c450da235474d24a5 🎾
3Kqi8XTd1pLYujuYSDQVRtxI8OH+AtHXDCqSbA+vpV79rlO/FYrHG3y5idrP0qDVFT/A38/VueOT8880DQXZLDQ==

hebasto force-pushed on Aug 30, 2024

in .gitignore:90 in fae230fe68 outdated

85-*.raw.h
86-
87-# Only ignore unexpected patches
88-*.patch
89-!contrib/guix/patches/*.patch
90-!depends/patches/**/*.patch

fanquake commented at 3:52 pm on August 30, 2024:

Is removing lines like these related to removing autotools?

hebasto commented at 3:58 pm on August 30, 2024:

Sure. Dropped from the diff.

in .gitignore:72 in fae230fe68 outdated

67-.*.swp
68-*~
69-*.bak
70-*.rej
71-*.orig
72-*.pyc

fanquake commented at 3:53 pm on August 30, 2024:

Also not sure? If I do cmake -B build && cmake --build build -j12 && ctest --test-dir build with this branch, now I have:

0# git status
1On branch 30664
2Untracked files:
3  (use "git add <file>..." to include in what will be committed)
4	share/rpcauth/__pycache__/

Which is share/rpcauth/__pycache__//rpcauth.cpython-312.pyc.

hebasto commented at 4:00 pm on August 30, 2024:

Right. The master branch is not ready for that yet.

Fixed.

hebasto force-pushed on Aug 30, 2024

build: Remove Autotools-based build system d71ac76842

ci, doc: Drop reference to `src/.bear-tidy-config` faa382ae76

in .gitignore:6 in 6811a2b492 outdated

2@@ -3,154 +3,21 @@
3 !/build-aux
4 !/build_msvc
5 
6-*.tar.gz

TheCharlatan commented at 8:23 pm on August 30, 2024:

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?

hebasto commented at 8:35 pm on August 30, 2024:

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.

TheCharlatan commented at 8:47 pm on August 30, 2024:

When I review a backport and come back again, this is how it looks like:

0> git status
1On branch master
2Untracked files:
3  (use "git add <file>..." to include in what will be committed)
4	Makefile
5	Makefile.in
6	aclocal.m4
7	autom4te.cache/
8... another 420 lines

This makes using git status and a bunch of other git commands very annoying.

maflcko commented at 6:55 am on September 2, 2024:

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?

hebasto force-pushed on Aug 30, 2024

DrahtBot commented at 7:27 am on August 31, 2024: contributor

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

File	commit 96b0a8f858ab24f3672360b8c830553b963de726(master)	commit 0f8c6a90776ef10d36ecdb5c0cb3613514ec6aa9(master and this pull)
SHA256SUMS.part	`27365d9d56b05cce...`	`784c3e2b804e07e7...`
*-aarch64-linux-gnu-debug.tar.gz	`04a5bb232114f782...`	`c2133e8ddf8e0ff5...`
*-aarch64-linux-gnu.tar.gz	`bd5d23a64ab7d779...`	`c05c3d1da815228e...`
*-arm-linux-gnueabihf-debug.tar.gz	`0021ca776f7dcc8b...`	`8b1b558d4827116f...`
*-arm-linux-gnueabihf.tar.gz	`2340d1119a1ec632...`	`1cb0c6b13bd34139...`
*-arm64-apple-darwin-unsigned.tar.gz	`ab00b80f00cd09b8...`	`40140f7e82682c13...`
*-arm64-apple-darwin-unsigned.zip	`cb84320f60e26fed...`	`44aa0706622b8040...`
*-arm64-apple-darwin.tar.gz	`a1a2786cfcecd350...`	`36894d0906c06caf...`
*-powerpc64-linux-gnu-debug.tar.gz	`9544adef5060dfab...`	`28eea1c218761abc...`
*-powerpc64-linux-gnu.tar.gz	`faf390f38f51cc68...`	`3d3412d5ac36b43f...`
*-riscv64-linux-gnu-debug.tar.gz	`67b921646742efc8...`	`fd9f661bbc433058...`
*-riscv64-linux-gnu.tar.gz	`c4759b761c171f92...`	`46330a9dca896a01...`
*-x86_64-apple-darwin-unsigned.tar.gz	`459ba0d98d89d678...`	`f9a861066bb1d832...`
*-x86_64-apple-darwin-unsigned.zip	`d19b0e188ac78eb6...`	`1fb2374c3fcbe325...`
*-x86_64-apple-darwin.tar.gz	`7b3f41e90375e361...`	`e74a163674a27fe1...`
*-x86_64-linux-gnu-debug.tar.gz	`a1c953af5210e0d6...`	`545a5a9e20fdd2cd...`
*-x86_64-linux-gnu.tar.gz	`5d74858c83c440f2...`	`d179bb44acebc7cf...`
*.tar.gz	`b3128d336e5c9680...`	`fd4ba27bf0577127...`
guix_build.log	`3e54d141b4a84788...`	`042744d2a0c05f33...`
guix_build.log.diff		`70d0ef7493c2b7a0...`

DrahtBot removed the label DrahtBot Guix build requested on Aug 31, 2024

in .gitignore:67 in faa382ae76

62-src/qt/bitcoin-qt.includes
63-
64-.deps
65-.dirstamp
66-.libs
67-.*.swp

maflcko commented at 7:02 am on September 2, 2024:

Why remove this? Seem unrelated to autotools.

in .gitignore:70 in faa382ae76

65-.dirstamp
66-.libs
67-.*.swp
68-*~
69-*.bak
70-*.rej

maflcko commented at 7:03 am on September 2, 2024:

Same, etc

maflcko approved

maflcko commented at 7:06 am on September 2, 2024: member

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 faa382ae7642da0e436ea2c7f7eac67386280a7e 🍦

Signature:

0untrusted 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}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: re-ACK faa382ae7642da0e436ea2c7f7eac67386280a7e 🍦
3Pa+7MqVHY/VDhJZ9Bdbr49ElHmQ1m0FcmiE8YrnUkMTICFE3Iy6QvS8NHDfZWYXhVTSI8h8SiPcuVU/h+1urAQ==

in .gitignore:147 in faa382ae76

144-/doc/doxygen/
145-
146-contrib/devtools/split-debug.sh
147-
148-# Output from running db4 installation
149-db4/

maflcko commented at 7:12 am on September 2, 2024:

Note: This is a follow-up to the install_db4.sh removal, but seems fine to do here.

TheCharlatan approved

TheCharlatan commented at 7:18 am on September 2, 2024: contributor

ACK faa382ae7642da0e436ea2c7f7eac67386280a7e

fanquake approved

fanquake commented at 10:39 am on September 2, 2024: member

ACK faa382ae7642da0e436ea2c7f7eac67386280a7e

The gitignore still seem inconsistent,

Especially given the comments in #30731 (review). Seems those should have been removed for the same reasoning as #30664 (review). Changes here seem completely arbitrary.

fanquake merged this on Sep 2, 2024

fanquake closed this on Sep 2, 2024

maflcko commented at 10:49 am on September 2, 2024: member

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.

hebasto deleted the branch on Sep 2, 2024

build: Remove Autotools-based build system #30664

Code Coverage

Reviews

Conflicts

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]