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

Remove unnecessary patch in bdb.mk #3952

Merged
merged 1 commit into from Aug 10, 2020
Merged

Remove unnecessary patch in bdb.mk #3952

merged 1 commit into from Aug 10, 2020

Conversation

LongShao007
Copy link
Contributor

there is no "__atomic_compare_exchange" function in "src/dbinc/atomic.h".

@daira
Copy link
Contributor

daira commented Apr 21, 2019

This is confusing because our CI system tests building BDB, which should be deterministic. Can you post your complete build log please?

@daira
Copy link
Contributor

daira commented Apr 21, 2019

Oh, you mean that particular sed patch is no longer necessary? We'll look into that.

@daira daira added A-build Area: Build system A-dependencies Area: Dependencies M-user-support User support issue labels Apr 21, 2019
@ioptio ioptio added this to Needs Prioritization in Arborist Team via automation Apr 22, 2019
@ioptio ioptio added this to Blocked in User Support Apr 22, 2019
@mms710 mms710 moved this from Needs Prioritization to Bugs/Security Issues/TechDebt Backlog in Arborist Team Apr 25, 2019
@mms710 mms710 moved this from Bugs/Security Issues/TechDebt Backlog to Arborist Product Priorities in Arborist Team Apr 25, 2019
@mms710 mms710 moved this from Arborist Product Priorities to PRs That Need Review + Their Associated Issues in Arborist Team Apr 25, 2019
@Eirik0
Copy link
Contributor

Eirik0 commented Apr 30, 2019

Related: 1f623c6

Compare the lines from upstream:

sed -i.old 's/__atomic_compare_exchange/__atomic_compare_exchange_db/' dbinc/atomic.h && \
sed -i.old 's/atomic_init/atomic_init_db/' dbinc/atomic.h mp/mp_region.c mp/mp_mvcc.c mp/mp_fget.c mutex/mut_method.c mutex/mut_tas.c

to those in zcash:

sed -i.old 's/__atomic_compare_exchange\\(/__atomic_compare_exchange_db(/' src/dbinc/atomic.h && \
sed -i.old 's/atomic_init/atomic_init_db/' src/dbinc/atomic.h src/mp/mp_region.c src/mp/mp_mvcc.c src/mp/mp_fget.c src/mutex/mut_method.c src/mutex/mut_tas.c

In zcash the directories start with src/ but in the first line there is also an extra \\( and (. I am wondering if that is the issue.

@mms710 mms710 moved this from PRs That Need Review + Their Associated Issues to Protocol Backlog in Arborist Team Apr 30, 2019
@mms710 mms710 moved this from Protocol Backlog to Blocked in Arborist Team Apr 30, 2019
@mms710
Copy link

mms710 commented Apr 30, 2019

Blocked pending comment from @Eirik0 on a different approach we could take.

@Eirik0
Copy link
Contributor

Eirik0 commented Apr 30, 2019

@LongShao007 could you try changing the line from

sed -i.old 's/__atomic_compare_exchange\\(/__atomic_compare_exchange_db(/' src/dbinc/atomic.h && \

to

sed -i.old 's/__atomic_compare_exchange/__atomic_compare_exchange_db/' src/dbinc/atomic.h && \

rather than removing it and see if that works?

@Eirik0 Eirik0 self-requested a review May 1, 2019 01:18
@daira daira changed the title fix bug of bdb.mk Remove unnecessary patch in bdb.mk Aug 10, 2020
Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

utACK (with @str4d and @therealyingtong ). Eirik's comment was incorrect; there is no remaining instance of __atomic_compare_exchange that needs to be patched. (It was removed between the version of bdb used by Bitcoin Core, and version 6.2.3.)

@daira
Copy link
Contributor

daira commented Aug 10, 2020

@zkbot r+

@zkbot
Copy link
Contributor

zkbot commented Aug 10, 2020

📌 Commit 9c96aff has been approved by daira

@zkbot
Copy link
Contributor

zkbot commented Aug 10, 2020

⌛ Testing commit 9c96aff with merge 9487115...

@daira daira removed this from Blocked in User Support Aug 10, 2020
@daira daira removed this from Blocked in Arborist Team Aug 10, 2020
@daira daira added this to the Core Sprint 2020-33 milestone Aug 10, 2020
@zkbot
Copy link
Contributor

zkbot commented Aug 10, 2020

☀️ Test successful - pr-merge
Approved by: daira
Pushing 9487115 to master...

@zkbot zkbot merged commit 9487115 into zcash:master Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build Area: Build system A-dependencies Area: Dependencies M-user-support User support issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants