💬 fjahr commented on pull request "test: Use std::span and std::string_view for raw data":
(https://github.com/bitcoin/bitcoin/pull/30796#issuecomment-2331002451)
re-ACK https://github.com/bitcoin/bitcoin/commit/faecca9a85c1c1917d024f55cca34d19cc94f3b9

Only changes since last review were usage of `std::to_integer` and some shuffling of newline characters.

👍 TheCharlatan approved a pull request: "depends: build libevent with `-D_GNU_SOURCE`"
(https://github.com/bitcoin/bitcoin/pull/30743#pullrequestreview-2282333673)
ACK 556775408797d8e27154c3edaf139820b0979cce

The reproducibility issues seems to be unrelated to this change, so I don't think this should be blocked by it.

💬 somethingbeta commented on issue "Test Framework - test_framework.test_node.FailedToStartError: No RPC credentials":
(https://github.com/bitcoin/bitcoin/issues/30818#issuecomment-2331029828)
> > Error: Specified data directory "/tmp/bitcoin_func_test_u9zpt_2g/node0" does not exist.
>
> This seems to be the first error, so it would be good to debug this first. Can you check whether `/usr/src/bitcoin/test/cache/node0` exists and whether the following is successful:
>
> > 2024-09-04T18:07:10.780000Z TestFramework (DEBUG): Copy cache directory /usr/src/bitcoin/test/cache/node0 to node 0
>
> Given that you claim that `bitcoin.conf` exists (presumably in this dir), the error mess
...

💬 TheCharlatan commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1745115109)
I think we are trying to consistently use `cmake --build` over `make `.

👍 TheCharlatan approved a pull request: "contrib: fix check-deps.sh to check for weak symbols"
(https://github.com/bitcoin/bitcoin/pull/30415#pullrequestreview-2282355222)
ACK 3e4312eef78f233eb7ae1d7d85e497de34144f2e

💬 somethingbeta commented on issue "Test Framework - test_framework.test_node.FailedToStartError: No RPC credentials":
(https://github.com/bitcoin/bitcoin/issues/30818#issuecomment-2331040665)
> Running the Windows tests on Wine in Linux is obviously better than nothing, but it would be better to run them directly on Windows. However, I am not sure what the easiest way to do this would be. For now, editing the config ini to adjust the paths is required.

When you say edit the config.ini paths, what would i be editing here please?

💬 fanquake commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1745128233)
> Does export LC_ALL=C.UTF-8 make any difference?

Yes, this seems to fix the failures when running directly under wine. Can followup with an issues/docs.

💬 maflcko commented on issue "Test Framework - test_framework.test_node.FailedToStartError: No RPC credentials":
(https://github.com/bitcoin/bitcoin/issues/30818#issuecomment-2331047225)
For WSL, can you share exact steps to reproduce, please? I guess they are https://github.com/bitcoin/bitcoin/blob/28.x/doc/build-windows.md ?



> > Running the Windows tests on Wine in Linux is obviously better than nothing, but it would be better to run them directly on Windows. However, I am not sure what the easiest way to do this would be. For now, editing the config ini to adjust the paths is required.
>
> When you say edit the config.ini paths, what would i be editing here please?
...

💬 maflcko commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1745136573)
Longer term, I wonder if it makes sense for the build system (and guix) to produce a "test kit" that contains the test config ini file required to run the tests (unit and functional ones) as well as other required stuff, so that on native Windows, one can simply move the exe files to a folder inside the "test kit" and then run the kit to execute all tests.

🚀 fanquake merged a pull request: "depends: build libevent with `-D_GNU_SOURCE`"
(https://github.com/bitcoin/bitcoin/pull/30743)

💬 fanquake commented on pull request "depends: build libevent with `-D_GNU_SOURCE`":
(https://github.com/bitcoin/bitcoin/pull/30743#issuecomment-2331063559)
Backported to 28.x in #30762.

👋 fanquake's pull request is ready for review: "[28.x] rc backports"
(https://github.com/bitcoin/bitcoin/pull/30762)

💬 somethingbeta commented on issue "Test Framework - test_framework.test_node.FailedToStartError: No RPC credentials":
(https://github.com/bitcoin/bitcoin/issues/30818#issuecomment-2331066512)
> For WSL, can you share exact steps to reproduce, please? I guess they are https://github.com/bitcoin/bitcoin/blob/28.x/doc/build-windows.md ?
>
> > > Running the Windows tests on Wine in Linux is obviously better than nothing, but it would be better to run them directly on Windows. However, I am not sure what the easiest way to do this would be. For now, editing the config ini to adjust the paths is required.
> >
> >
> > When you say edit the config.ini paths, what would i be editing h
...

💬 maflcko commented on issue "Test Framework - test_framework.test_node.FailedToStartError: No RPC credentials":
(https://github.com/bitcoin/bitcoin/issues/30818#issuecomment-2331088089)
Ok, thanks for confirming.

I don't have WSL, so I can't test this. However running the functional tests inside WSL was never supported, IIRC.

Even if you work around your initial issue, the test framework and individual tests have to modified for the Wine environment.

So my recommendation would be to run the tests on Windows directly. However that isn't possible out-of-the-box either and requires modifying the config ini file.

💬 maflcko commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2331096969)
Upgrading my review with a logsourcelocations test on Linux (compiled and guix) binaries. I did *not* test macOS/Windows (compiled and guix) binaries.

However, ccache does *not* hit across two different build dirs, compiling the same commit.

💬 maflcko commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#discussion_r1745166246)
Can you explain this. How would this be tested?

I tried with `cmake -B ./bld-1-cmake` and `cmake -B ./bld-2-cmake` and it did not work.

💬 somethingbeta commented on issue "Test Framework - test_framework.test_node.FailedToStartError: No RPC credentials":
(https://github.com/bitcoin/bitcoin/issues/30818#issuecomment-2331109182)
> Ok, thanks for confirming.
>
> I don't have WSL, so I can't test this. However running the functional tests inside WSL was never supported, IIRC.
>
> Even if you work around your initial issue, the test framework and individual tests have to modified for the Wine environment.
>
> So my recommendation would be to run the tests on Windows directly. However that isn't possible out-of-the-box either and requires modifying the config ini file.

Ok good to know re support for running the
...

💬 fanquake commented on issue "build: reproducibility issue with macOS Guix builds":
(https://github.com/bitcoin/bitcoin/issues/30815#issuecomment-2331110721)
> This issue boils down to two following calls:

At a minimum I think we should start explicitly specifying [`NO_SOURCE_PERMISSIONS`](https://cmake.org/cmake/help/latest/command/configure_file.html#options) or [`USE_SOURCE_PERMISSIONS`](https://cmake.org/cmake/help/latest/command/configure_file.html#options) (the default behaviour) everywhere that we call [`configure_file()`](https://cmake.org/cmake/help/latest/command/configure_file.html), to make our intent explicit in all cases.

💬 fanquake commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#discussion_r1745194733)
I also tried testing. It doesn't seem to work, because the `-I` argument passed to the compiler contains the the full `path/to/the/src` directory, which means the compiler invocations differ, and ccache wont hit?

🚀 fanquake merged a pull request: "build: Fix / improve coverage scripts"
(https://github.com/bitcoin/bitcoin/pull/30772)