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

revert: fix: missing changes from bitcoin#19267 - run multiprocess on CI #6195

Closed

Conversation

PastaPastaPasta
Copy link
Member

Issue being fixed or feature implemented

See #6192 for a discussion around this: for now simply make CI happy again, by not running for multiprocessor (since this is new, and unstable / dev feature, and not something shipped).

What was done?

Revert commit which reintroduces multiprocessor CI while we investigate proper fix

How Has This Been Tested?

CI

Breaking Changes

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@PastaPastaPasta PastaPastaPasta added this to the 22 milestone Aug 11, 2024
@PastaPastaPasta PastaPastaPasta changed the title Revert: fix: missing changes from bitcoin#19267 - run multiprocess on CI revert: fix: missing changes from bitcoin#19267 - run multiprocess on CI Aug 11, 2024
@PastaPastaPasta
Copy link
Member Author

Since this simply reverts a prior commit, and CI on this branch is happy, I'm going to merge this in

PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Aug 11, 2024
… run multiprocess on CI

d5a944d Revert: fix: missing changes from bitcoin#19267 - run multiprocess on CI (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  See dashpay#6192 for a discussion around this: for now simply make CI happy again, by not running for multiprocessor (since this is new, and unstable / dev feature, and not something shipped).

  ## What was done?
  Revert commit which reintroduces multiprocessor CI while we investigate proper fix

  ## How Has This Been Tested?
  CI

  ## Breaking Changes

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

Top commit has no ACKs.

Tree-SHA512: d878b02efc72019e09b09e2482e63dca276824b66272135e00ff340ba60b5b58e5f5ae3feff3b894f08958ca8fc6ce71f91c926f6aaf29a459d6230f129316f6
@knst
Copy link
Collaborator

knst commented Aug 12, 2024

https://gitlab.com/dashpay/dash/-/jobs/7554559145

CI failed with tsan, seems as real crash. restarted just in case

PastaPastaPasta added a commit that referenced this pull request Aug 12, 2024
…#22937 and ipc/process

540f687 fix: lock `::cs_main` before accessing `ChainstateManager::m_best_header` (Kittywhiskers Van Gogh)
aafded6 fix: compilation error due to rebase error between bitcoin#22937 and ipc/process (Kittywhiskers Van Gogh)

Pull request description:

  ## Issue being fixed or feature implemented
  CI failure for multiprocess

  ## What was done?
  It resolve compilation failure on develop (apply changes from 22937 to ipc/process)

  See alternate solutions:
   - #6192
   - #6195

  ## How Has This Been Tested?
  Run build locally:
  ```
  make MULTIPROCESS=1 -j10
  ./configure --prefix=$(pwd)/depends/x86_64-pc-linux-gnu --enable-suppress-external-warnings --enable-werror    --enable-debug --enable-crash-hooks  --enable-maintainer-mode --enable-stacktraces  --enable-multi-process
  ```

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK 540f687

Tree-SHA512: 75b675e93763e8e53086a10b93516b8ec94418902f9be2de738517176cc835c0dad25c286426089a5327a9c13d1787b89debda2c6025cb598489204d7d5af2cf
@knst
Copy link
Collaborator

knst commented Aug 14, 2024

It succeed now: https://gitlab.com/dashpay/dash/-/pipelines/1412133645

Super-seeded by #6199

@knst knst closed this Aug 14, 2024
@UdjinM6 UdjinM6 removed this from the 22 milestone Aug 26, 2024
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.

3 participants