cmirror has quit [Remote host closed the connection]
cmirror has joined #bitcoin-core-dev
mcey_ has quit [Remote host closed the connection]
mcey_ has joined #bitcoin-core-dev
<maflcko>
Looks like a rebase on top of cmake *has* to be done before every push to a pull request, otherwise the GHA CI will be failing.
<maflcko>
The reason is that the "test each commit" task is using the cmake invocation, but it is running on the non-rebased commits, which may be lacking CMakeLists.txt
<maflcko>
but doing a rebase should be harmless and may otherwise be useful anyway
<maflcko>
I've removed the "Needs CMake port" label from pull requests that fail CI or otherwise need a rebase. The CI failure should be self-explanatory.
abubakarsadiq has joined #bitcoin-core-dev
zeropoint has quit [Quit: leaving]
dermoth has quit [Read error: Connection reset by peer]
dermoth has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] l0rinc closed pull request #29847: test: add a few more base32/64 calculation corner cases (master...paplorinc/base-32-64-padding-simplification) https://github.com/bitcoin/bitcoin/pull/29847
<bitcoin-git>
[bitcoin] l0rinc opened pull request #30746: Add base32 and base64 roundtrip random padding tests (master...l0rinc/roundtrip-tests-base32-base64) https://github.com/bitcoin/bitcoin/pull/30746
<maflcko>
Looks like "make --keep-going" now aborts early in cmake, whereas previously it really did try to compile everything
<maflcko>
This makes code search and static analysis using a compiler harder
<TheCharlatan>
maflcko I think the recommendation is to use cmake instead of make and passing any arguments you would normally give make after `--` to cmake.
<fanquake>
instagibbs: why did you need to disable asm? Sounds like something that should be fixed (not sure how CMake would have fixed your compiler)
Guyver2 has joined #bitcoin-core-dev
<maflcko>
TheCharlatan: I know. I was trying to say the the "-k" behavior changed. I used "cmake --build ./bld-cmake --parallel $( nproc ) -- --keep-going"
<maflcko>
For example, if libtest_util can not be compiled, cmake aborts early, whereas autotools compiled everything else as well.
<TheCharlatan>
ah, I see, looks like any dependents on the library won't be built. I was previously checking a broken bitcoind.cpp, which would still allow the tests to compile with `--keep-going`
<bitcoin-git>
[bitcoin] maflcko opened pull request #30748: test: Pin and document TEST_DIR_PATH_ELEMENT, SeedRand::FIXED_SEED (master...2408-test) https://github.com/bitcoin/bitcoin/pull/30748
<fanquake>
If anyone happened to "guix pull" yesterday, and your guix is now segfaulting, "guix pull --roll-back" and then a "guix pull" should fix your guix
<fanquake>
my suggestion during rebasing would be to drop any autotools changes, unless you just want to rebase again
<fanquake>
or force everyone to review two sets of the same change
<fjahr>
editing some stuff on the build system too so interested in feedback from competent reviewers ;)
<Sjors[m]11>
What's the ETA for kill autotools?
<achow101>
also, i guess if people don't rebase their stuff soon, those prs are gonna be closed
<Sjors[m]11>
Or does it not matter if you merge something that doesn't work with it?
<cfields>
hi
<fanquake>
what do you mean not work with? Nothing like that will be merged
<fanquake>
autotools is going to be entirely removed very soon
<fanquake>
CI is only using cmake, guix is only using cmake etc
<fanquake>
so anything doing stuff in autotools wont even be tested properly at this point
<fjahr>
I kept the autotools stuff in so far, I felt like for me it would be easier to review both side by side but I am happy to remove it if that is the policy we want to follow generally
<Sjors[m]11>
E.g. #30409 adds a header, so it touches Makefile.am.
<Sjors[m]11>
I can drop that, but then if it's merged before autotools goes away, it won't compile.
<fjahr>
(easier to review as a cmake noob)
<Sjors[m]11>
My other PRs are probably not getting merged before that time anyway, so I made them cmake-only.
<fanquake>
Sjors I don't understand. Just make the change in cmake, and drop the autotools parts
<fanquake>
everything will compile fine, regardless of if autotools exists or not
<fanquake>
and then you don't have to worry about when it's removed
<fanquake>
and your changes will be tested in the CI
Guest40 has quit [Quit: Client closed]
<cfields>
Any reason not to nuke autotools today? To moot this conversation entirely?
<fanquake>
cfields: that would be my preference, so we don't waste time with all this, but cmake is still lacking working features, like coverage, so it's a little bit of a blocker
<cfields>
We discussed exactly this issue for exactly this reason and came to the conclusion that (at the very least) configure.ac+Makefile.* should be removed asap.
<cfields>
Seems we can live without coverage for a few days?
<maflcko>
Can I plz haz 24 more hours?
<fanquake>
I would have preferred to at least have a change ready for review / testing, but it's not clear who is working on it
<maflcko>
fanquake: I think the change is there, but no current acks
<fanquake>
is there a pr in bitcoin/bitcoin?
<maflcko>
I'll try to review it later today or tomorrow
<cfields>
maflcko: for coverage? or something else?
<bitcoin-git>
bitcoin/master 1f4cdb3 Martin Saposnic: Replace custom funding tx creation with MiniWallet.
<bitcoin-git>
bitcoin/master a563f41 Martin Saposnic: Remove second node since only 1 is needed for the test
<bitcoin-git>
bitcoin/master 7349d57 glozow: Merge bitcoin/bitcoin#30701: Use MiniWallet in functional test rpc_signraw...
<bitcoin-git>
[bitcoin] glozow merged pull request #30701: Use MiniWallet in functional test rpc_signrawtransactionwithkey. (master...test-miniwallet-on-rpc-signrawtransactionwithkey) https://github.com/bitcoin/bitcoin/pull/30701
<sdaftuar>
is there anything special we need to do -- such as rerunning cmake -B -- if we add a new functional test?
<sdaftuar>
i ran into an issue with a functional test not appearing in my build directory that I think I eventually fixed by doing that, but i'm not sure if i understand correctly
<sdaftuar>
(this was while i was just rebasing #28676)
<instagibbs>
worked normally for my PR that adds a functional test
<sdaftuar>
i'll do some experiments maybe...
<sdaftuar>
ok if i copy an existing test to a new file (and add it to test_runner.py for good measure), and then run cmake --build build, it does not appear in the build/ directory. but if i run cmake -B and then cmake --build, it shows up...
<sdaftuar>
so maybe the right thing to do is always run cmake -B first when kicking off a build?
<instagibbs>
oh maybe, I'm running a small script that does that
<hebasto>
`cmake -B` creates symlinks for test files in source dir to a build tree
<hebasto>
and tests run from the build directory
<sdaftuar>
hebasto: i see, so that means that if a test is changed, we don't need to rerun cmake -B, but if a test is added then we should?
<hebasto>
correct
<hebasto>
I'd say that something like `rm -rf build && cmake -B build` is the safest way
<sdaftuar>
maflcko: any idea what the equivalent thing to do would be in the cmake build system? (i also don't understand why the build change was needed but it seemed to work)
<sdaftuar>
the commit i linked before seemed to fix the issue for autotools, but now that i'm rebasing i'm trying to figure out how to fix it with cmake
<hebasto>
I'll comment in the PR shortly
<sdaftuar>
however i don't really understand the issue or why the fix works, so if you have a better wya of doing things i'd appreciate it! thanks
jarthur has quit [Quit: jarthur]
Talkless has quit [Remote host closed the connection]
Randolf has quit [Quit: Leaving]
<achow101>
what is the equivalent for `make check`?
<hebasto>
`ctest --test-dir build`
<achow101>
that will also build if not built?
<hebasto>
no, it won't
<achow101>
'make check' was build and test, i'd like to do the same with minimal verbosity
<hebasto>
sdaftuar: your change is correct (I just was waiting finishing my own builds)
<sdaftuar>
ok great, i just decided to guess :) watching the ci now to see if it works (for whatever reason, i can't reproduce the build error i was seeing locally)
<achow101>
how do i make compiler errors and warnings be colorized as they were previously?