💬 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)

💬 maflcko commented on pull request "lint: Check for release note snippets in the wrong folder":
(https://github.com/bitcoin/bitcoin/pull/30812#discussion_r1745224882)
> Maybe, in that case we can use shell wildcard patterns with `ls-files`:

I think you meant "git pathspec" instead of "shell wildcard patterns", but I like the suggestion, because it let's `git` do everything without requiring any additional code or external dependency.

💬 maflcko commented on pull request "lint: Check for release note snippets in the wrong folder":
(https://github.com/bitcoin/bitcoin/pull/30812#discussion_r1745225587)
Added *two* linebreaks

💬 maflcko commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#discussion_r1745232776)
Ah, thanks for testing. I guess this is a possible cmake regression, but I haven't ever used different dirs with autotools, so I can't say for sure.

Seems fine to fix here, or in a follow-up, as this pull request is an improvement already and this bug already exists in current `master`.

👍 theStack approved a pull request: "refactor: TxDownloadManager + fuzzing"
(https://github.com/bitcoin/bitcoin/pull/30110#pullrequestreview-2278788723)
Light review ACK 1d4e33e7f88162cf6fcd6aee0b63973015853591

Reviewed the refactoring changes and the unit tests thouroughly, but didn't look ran or look in-depth at the fuzz tests.

💬 theStack commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1742884245)
in commit c40cb8565952f62732fa64875c99ef9082b9a6f4: now that the if-body above only reads and writes from local variables (`vInv` and `tx_invs`), acquiring the `m_tx_download_mutex` lock before the for-loop (line 5140 at this commit) is not needed anymore