test_bitcoin from pre-built 28.0rc2 tarball is failing for JSON parsing #30938

issue bitcoin-tools openend this issue on September 20, 2024
  1. bitcoin-tools commented at 2:14 pm on September 20, 2024: none

    Is there an existing issue for this?

    • I have searched the existing issues

    Current behaviour

    Tested the 28.0rc2 packages from bitcoincore.org/bin for Linux x86_64, Darwin x86_64, and Darwin arm64.

    Seeing consistent failures in the rc2 test_bitcoin binary running:

    • Docker with base image of Arch (1, 2, 3)
    • Docker with base image of Debian (1, 2, 3), Ubuntu (1, 2, 3), and Kali (1, 2, 3)
    • Docker with base image of openSUSE Leap (1, 2, 3), openSUSE Tumbleweed (1, 2, 3), and SUSE Enterprise (1, 2, 3)

    rc2 test_bitcoin is consistently NOT failing on the following:

    • Ubuntu running on bare metal github CI runner (not in Docker)
    • macOS 13 x86_64 and macOS 14 arm64 on bare metal github CI runner
    • Docker with base image of Alpine, Clear Linux, Fedora, Manjaro, Oracle Linux, RHEL, and Rocky Linux

    Also, test_bitcoin was consistently NOT failing like this on 28.0rc1.

    Expected behaviour

    test_bitcoin from pre-built package should return exit status 0 when run inside a Docker container.

    Steps to reproduce

    To reproduce the test_bitcoin failures on 28.0rc2:

    0git clone https://github.com/bitcoin-tools/nodebuilder
    1cd nodebuilder/docker/
    2docker build --build-arg NODEBUILDER_VERSION=f0194173557c364d929148952c6a28e2a8965f97 -f Dockerfile_archlinux .
    3docker build --build-arg NODEBUILDER_VERSION=f0194173557c364d929148952c6a28e2a8965f97 -f Dockerfile_debian .
    4docker build --build-arg NODEBUILDER_VERSION=f0194173557c364d929148952c6a28e2a8965f97 -f Dockerfile_kali .
    5docker build --build-arg NODEBUILDER_VERSION=f0194173557c364d929148952c6a28e2a8965f97 -f Dockerfile_openususe-leap .
    6docker build --build-arg NODEBUILDER_VERSION=f0194173557c364d929148952c6a28e2a8965f97 -f Dockerfile_opensuse-tumbleweed .
    7docker build --build-arg NODEBUILDER_VERSION=f0194173557c364d929148952c6a28e2a8965f97 -f Dockerfile_sles .
    8docker build --build-arg NODEBUILDER_VERSION=f0194173557c364d929148952c6a28e2a8965f97 .
    

    To reproduce the expected behavior on 28.0rc1:

    0docker build --build-arg NODEBUILDER_VERSION=f453fb64c665beabfce70fe037625215ea0e96bb -f Dockerfile_archlinux .
    1docker build --build-arg NODEBUILDER_VERSION=f453fb64c665beabfce70fe037625215ea0e96bb -f Dockerfile_debian .
    2docker build --build-arg NODEBUILDER_VERSION=f453fb64c665beabfce70fe037625215ea0e96bb -f Dockerfile_kali .
    3docker build --build-arg NODEBUILDER_VERSION=f453fb64c665beabfce70fe037625215ea0e96bb -f Dockerfile_openususe-leap .
    4docker build --build-arg NODEBUILDER_VERSION=f453fb64c665beabfce70fe037625215ea0e96bb -f Dockerfile_opensuse-tumbleweed .
    5docker build --build-arg NODEBUILDER_VERSION=f453fb64c665beabfce70fe037625215ea0e96bb -f Dockerfile_sles .
    6docker build --build-arg NODEBUILDER_VERSION=f453fb64c665beabfce70fe037625215ea0e96bb .
    

    Commit hashes are taken from this PR.

    Relevant log output

     0[2024-09-20T13:36:14]  INFO: Running the unit tests.
     1
     2Running 630 test cases...
     3test/system_tests.cpp(61): error: in "system_tests/run_command": check what.find(tfm::format("RunCommandParseJSON error: process(%s) returned", command)) != std::string::npos has failed
     4test/system_tests.cpp(62): error: in "system_tests/run_command": check what.find(expected) != std::string::npos has failed
     5
     6*** 2 failures are detected in the test module "Bitcoin Core Test Suite"
     7
     8[2024-09-20T13:39:42] ERROR: Unit tests failed.
     9[2024-09-20T13:39:42] ERROR: Unhandled error at line 1.
    10The command '/bin/sh -c /bin/sh -c "$(curl -fsSL ${NODEBUILDER_URL} )"' returned a non-zero code: 1
    

    How did you obtain Bitcoin Core

    Pre-built binaries

    What version of Bitcoin Core are you using?

    28.0rc2

    Operating system and version

    Docker on Ubuntu 24

    Machine specifications

    • Inside a Docker container on GitHub Actions CI
    • Inside docker on my Ubuntu 24 local environment.
  2. bitcoin-tools renamed this:
    `test_bitcoin` from pre-built 28.0rc2 tarball is failing for a JSON parsing error
    `test_bitcoin` from pre-built 28.0rc2 tarball is failing for JSON parsing
    on Sep 20, 2024
  3. maflcko commented at 2:26 pm on September 20, 2024: member
    This can be worked around by installing python3. An alternative would be something like done here: https://github.com/bitcoin/bitcoin/pull/29868/files#r1747435819 (but that diff currently does not compile, as can be seen in the CI output)
  4. maflcko added the label Tests on Sep 20, 2024
  5. maflcko commented at 2:30 pm on September 20, 2024: member
    Another alternative would be to drop 199bb09d88e28d951c5068eb65643390dbedd066 from the 28.x branch. It fixed one bug, but introduced another.
  6. bitcoin-tools commented at 2:38 pm on September 20, 2024: none

    worked around by installing python3

    That explains why it doesn’t run on certain docker base images. Is python3 considered a runtime dependency for test_bitcoin?

  7. fanquake commented at 2:38 pm on September 20, 2024: member
    I think the bug is that commit introduced a (silent) runtime dependency that isn’t actually checked/enforced in anyway.
  8. maflcko commented at 2:40 pm on September 20, 2024: member
    Yeah, python3 is needed for ctest, IIUC. However, test_bitcoin is probably considered stand-alone and shouldn’t depend on anything other than itself.
  9. fanquake commented at 2:40 pm on September 20, 2024: member
    Yea. Even if ctest/development likely requires it, the test_bitcoin release binary we are shipping to end users shouldn’t have a (undocumented) dependency on invoking Python.
  10. maflcko commented at 2:45 pm on September 20, 2024: member

    The cleanest fix would probably be https://github.com/bitcoin/bitcoin/pull/29868/files#diff-4ea52ba9a5642d6946fb0016ca168b6647a8c6403d9e15e2a7197e72022824d4R87 const std::string command{"sh -c 'echo err 1>&2 && false'"};.

    Previously ls was a requirement, so making it sh seems fine as well?

    cc @hebasto

  11. achow101 added this to the milestone 28.0 on Sep 20, 2024
  12. bitcoin-tools commented at 3:47 pm on September 20, 2024: none
    Can it be simplified to const std::string command{"echo err 1>&2 && false"}; or is sh -c '...' necessary to ensure portability?
  13. maflcko commented at 3:53 pm on September 20, 2024: member
    I do not know, but I think that echo and false would fall back to the coreutils one, if the shell built-in isn’t available. The previous requirement ls is also from coreutils, IIUC, so it should be fine as well.
  14. bitcoin-tools commented at 9:02 pm on September 20, 2024: none
    Researching it deeper, it may be better to wrap the command in sh -c '...', since that would run echo and false as shell built-ins, rather than depending on coreutils binaries. But I agree with your reasoning regarding ls.
  15. hebasto commented at 9:25 am on September 23, 2024: member

    I think the bug is that commit introduced a (silent) runtime dependency that isn’t actually checked/enforced in anyway.

    I agree.

    I’d suggest reverting 199bb09d88e28d951c5068eb65643390dbedd066, as workarounds for #30601 and #30608 are available.

    Additionally, I suspect there is a bug in src/util/subprocess.h that needs to be investigated and fixed, along with the test fixes.

  16. achow101 commented at 5:06 pm on September 23, 2024: member

    The cleanest fix would probably be https://github.com/bitcoin/bitcoin/pull/29868/files#diff-4ea52ba9a5642d6946fb0016ca168b6647a8c6403d9e15e2a7197e72022824d4R87 const std::string command{"sh -c 'echo err 1>&2 && false'"};.

    Done in #30952

    I’d suggest reverting 199bb09, as workarounds for #30601 and #30608 are available.

    If @maflcko’s suggestion is good enough, I’d prefer to use that rather than reverting.

  17. fanquake closed this on Sep 24, 2024

  18. fanquake referenced this in commit 393f323bd6 on Sep 24, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-22 06:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me