Skip to content

prefer bin as directory name to hold binaries#47

Open
zancas wants to merge 1 commit intozcash:mainfrom
zingolabs:bin_include
Open

prefer bin as directory name to hold binaries#47
zancas wants to merge 1 commit intozcash:mainfrom
zingolabs:bin_include

Conversation

@zancas
Copy link
Contributor

@zancas zancas commented Mar 6, 2026 •
edited
Loading

Fixes: #68

Local sanity check run after the name change:

getrawtransaction_sidechain.py:
Pass: True, Duration: 0 s

feature_nu6.py:
Pass: True, Duration: 1 s
......
indexer.py:
Pass: True, Duration: 3 s
..
fix_block_commitments.py:
Pass: True, Duration: 5 s
.........................
feature_backup_non_finalized_state.py:
Pass: True, Duration: 19 s
.......................................
addnode.py:
Pass: True, Duration: 39 s
TEST                                  | PASSED | DURATION

addnode.py                            | True   | 39 s
feature_backup_non_finalized_state.py | True   | 19 s
feature_nu6.py                        | True   | 1 s
fix_block_commitments.py              | True   | 5 s
getrawtransaction_sidechain.py        | True   | 0 s
indexer.py                            | True   | 3 s
ALL                                   | True   | 67 s (accumulated)
Runtime: 39 s

@zancas zancas changed the title include bin prefer bin as directory name to hold binaries Mar 6, 2026
nuttycom
nuttycom requested changes Mar 6, 2026
View reviewed changes
Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

Please grep the codebase for all remaining instances of "src" and inspect them individually to ensure you catch everything.

@zancas zancas marked this pull request as draft March 10, 2026 22:20
@zancas zancas marked this pull request as draft March 10, 2026 22:20
@zancas zancas marked this pull request as ready for review March 10, 2026 22:22
@zancas
Copy link
Contributor Author

zancas commented Mar 10, 2026

I have not confirmed that CI runs, which is necessary to validate the numerous changes inside .github.

@zancas zancas requested a review from nuttycom March 12, 2026 22:42
@zancas
Copy link
Contributor Author

zancas commented Mar 19, 2026

Squashed to a single commit.

nuttycom
nuttycom approved these changes Mar 19, 2026
View reviewed changes
Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

utACK. I have not verified that all src instances are replaced, but this looks fine to run tests against.

@zancas
Copy link
Contributor Author

zancas commented Mar 19, 2026

Can we get a sanity check CI run against main?

nuttycom
nuttycom requested changes Mar 19, 2026
View reviewed changes
Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

NACK; shard 9 is failing (shard 9 is expected to pass): https://github.com/zcash/integration-tests/actions/runs/23312471056/job/67816462978?pr=47

@zancas
Copy link
Contributor Author

zancas commented Mar 20, 2026

I think that test is failing because zebrad updated to a new behavior.

Some recent version of zebrad extended the functionality of getblocksubsidy to handle pre-Canopy founders rewards.

I don't think this PR is the place to fix that.

@str4d str4d mentioned this pull request Mar 20, 2026
Draft
@str4d
Copy link
Collaborator

str4d commented Mar 20, 2026

I think that test is failing because zebrad updated to a new behavior.

Some recent version of zebrad extended the functionality of getblocksubsidy to handle pre-Canopy founders rewards.

I don't think this PR is the place to fix that.

I agree. In the normal workflow, this would have caused a build failure in the Zebra repo, and they would have seen the failure and come over here to fix it. But these integration tests are not yet running in their repo, so they didn't see their breakage.

@zancas
Copy link
Contributor Author

zancas commented Mar 20, 2026

So the question then becomes... what to do with this PR? Should it be set to draft until the CI issue is fixed?

@zancas
Copy link
Contributor Author

zancas commented Mar 20, 2026

The failure of the wallet.py test is separately addressed by: #78.
The failure of nuparams.py and feature_nu6.py is separately addressed by: #77

@zancas zancas marked this pull request as draft March 20, 2026 22:25
@zancas @claude
"src" is incorrect for a dir that contains bins
67beffe 
This commit is the squash of name changes that
convert the "src" stub-directoy for binary
placement to "bin", which is .gitincluded and
whose contents are ignored.  The reference is
updated across the codebase. i.e. src --> bin
including in the readme.

include bin

systematic src-->bin replacement, note zcash-cli refs are likely obsolete

ignore "bin" contents it is a stub directory
for-binaries

Co-Authored-By: Claude <noreply@anthropic.com>
@zancas zancas marked this pull request as ready for review March 20, 2026 22:39
@zancas
Copy link
Contributor Author

zancas commented Mar 21, 2026

The wallet.py failure is explained (superficially) by a version skew, my local zallet was not built against the latest CI-used version 6da10751.

wallet.py no longer fails in my local tests of this branch.

@zancas zancas requested a review from nuttycom March 23, 2026 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nuttycom nuttycom Awaiting requested review from nuttycom

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

historically test binaries have been stashed in "src"

3 participants

@zancas @str4d @nuttycom