Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge bitcoin#14209, #13878, #14192, #17086: logging and filesystem updates #4199

Merged
merged 4 commits into from
Jun 25, 2021

Conversation

kwvg
Copy link
Collaborator

@kwvg kwvg commented Jun 16, 2021

Disclosure:

Includes patches 6f5f2e6, 49e69be and backport bitcoin#17086 (144bf39) by UdjinM6

@PastaPastaPasta
Copy link
Member

Build fails. Please fix

@kwvg
Copy link
Collaborator Author

kwvg commented Jun 22, 2021

Build fail due to test failure, not reproducible on macOS

@PastaPastaPasta
Copy link
Member

I am unable to reproduce these build failures locally. Build works and tests pass fully

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK, however we need to figure out why CI isn't happy... Unit tests are happy locally. @UdjinM6 hopefully you have an idea?

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@UdjinM6 UdjinM6 added this to the 17.1 milestone Jun 23, 2021
kwvg and others added 3 commits June 24, 2021 23:43
d48f664 tests: Fix fs_tests for unknown locales (Daki Carnhof)

Pull request description:

  Fix by removing "L" as suggested by meeDamian in
  bitcoin#14948 (comment)

  ```
  # all in .../bitcoin/src/test
  $ uname -m
  x86_64
  $ export LC_ALL=randomnonexistentlocale
  $ ./test_bitcoin
  Running 369 test cases...
  unknown location(0): fatal error: in "fs_tests/fsbridge_fstream": boost::system::system_error: boost::filesystem::path codecvt to string: error
  test/fs_tests.cpp(13): last checkpoint: "fsbridge_fstream" test entry

  *** 1 failure is detected in the test module "Bitcoin Core Test Suite"
  ```

  After the patch is applied, the same test under the same conditions runs fine.

  ```
  $ export LC_ALL=randomnonexistentlocale
  $ ./test_bitcoin
  Running 369 test cases...

  *** No errors detected
  ```

  Co-Authored-By: bugs@meedamian.com

ACKs for top commit:
  laanwj:
    ACK d48f664

Tree-SHA512: a9910252b8ce6a05cab5530874549c2999ca2c28e835fc18aa8e5468fb417bd7d245864ec71d9233dd53e02940a9f0691b247430257f27eb0d7c20745d1c846d
@kwvg kwvg force-pushed the fstreamWrapper branch from a017f92 to 6a11e06 Compare June 24, 2021 18:13
@kwvg kwvg changed the title merge bitcoin#14209, #13878, #14192: logging and filesystem updates merge bitcoin#14209, #13878, #14192, #17086: logging and filesystem updates Jun 24, 2021
@kwvg
Copy link
Collaborator Author

kwvg commented Jun 24, 2021

Title has been updated to include Udjin's backport, thanks for figuring it out!

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, utACK (assuming CI is going to be happy)

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@PastaPastaPasta PastaPastaPasta merged commit e887596 into dashpay:develop Jun 25, 2021
@kwvg kwvg deleted the fstreamWrapper branch July 18, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants