Bitcoin Core Github
42 subscribers
125K links
Download Telegram
✳️ MarcoFalke pushed commits to a branch: bitcoin/bitcoin:master
(https://github.com/bitcoin/bitcoin/compare/a6316590d5bc...141115a06040)
Merge bitcoin/bitcoin#27033: ci: Cache stuff in volumes, not host folders

fa8e92c022057adcb8b98647bde626ed9c054df2 doc: Update ci docs (721217.xyz)
5fffff54e9fcf154c722dc421025a567fa0c5c97 ci: Cache stuff in volumes, not host folders (MarcoFalke)

Pull request description:

Storing cached stuff in host system folders may lead to unexpected issues when the ci-built stuff is used for a non-ci build or a ci task leaks into another ci task.

ACKs for top commit:
john-moffett:
ACK fa8e92c022057adcb8b98647bde626ed9c054df2

Tree-SHA512: 8b0c9019452fbe507a272c1037c3dce3c178c21f85ab1096ed3372ad9d4b3c7aa27d89e5bf80c9a6260ea652e0268be0cbe61d6a4fcb3add569fa38076d32287
🚀 MarcoFalke merged a pull request: "ci: Cache stuff in volumes, not host folders"
(https://github.com/bitcoin/bitcoin/pull/27033)
💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104354933)
Thanks, I adapted these changes with @pinheadmz name change suggested below
💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104355153)
Agreed, and renamed.
💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104355315)
agreed
💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104356262)
Now returns a file path
💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#issuecomment-1427800810)
Thanks @stickies-v & @pinheadmz , I addressed your comments in the new push.
💬 stickies-v commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104364182)
nit
```suggestion
const auto path{GetDataDir(net_specific)};
```
💬 stickies-v commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104368899)
```suggestion
args.EnsureDataDir(/*net_specific=*/false);
```
💬 stickies-v commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104369081)
```suggestion
gArgs.EnsureDataDir(/*net_specific=*/false);
```
💬 stickies-v commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104365397)
sorry, forgot to mark this as `const` in my diff
```suggestion
const auto conf_path{GetConfigFilePath()};
```
💬 hebasto commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#issuecomment-1427815930)
Concept ACK.
💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104375173)
sure, also agree
👍 stickies-v approved a pull request: "refactor, wallet: Avoid variable shadowing"
(https://github.com/bitcoin/bitcoin/pull/27087)
📝 MarcoFalke opened a pull request: "ci: Use dedicated root path for ci "
(https://github.com/bitcoin/bitcoin/pull/27090)
Now that stuff (like depends) is no longer propagated back to the host
folder after commit 141115a0604001b8cce848804798dc174770a994, commit
85892f77c98c7a08834a06d52af3eb474275afd8 can be reverted and
`LOCAL_USER` removed.

So do that. Also for clarity, in the same commit, use a dedicated path
for the ci root folder `CI_ROOT_DIR`.
💬 dergoegge commented on pull request "Add simulation-based `CCoinsViewCache` fuzzer":
(https://github.com/bitcoin/bitcoin/pull/27011#issuecomment-1427833942)
Code review ACK 561848aaf2d67791e92754f3d11813bc53959a8f
💬 hebasto commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104384205)
1. Should we care about possible exceptions here?
2. What is the point to run this code when `net_specific == false`?
💬 hebasto commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104386865)
Maybe introduce two functions: one for `net_specific == true` and the other for `net_specific == true`?

It will make code cleaner on the caller sites.
💬 MarcoFalke commented on pull request "Minor edits - punctuation, spelling to make the contributing instructions more readable for all.":
(https://github.com/bitcoin/bitcoin/pull/27082#issuecomment-1427850629)
Closing for now due to controversy.

In the future, please make sure to read the contribution guideline and make sure the tests pass (including the lint CI task).
MarcoFalke closed a pull request: "Minor edits - punctuation, spelling to make the contributing instructions more readable for all."
(https://github.com/bitcoin/bitcoin/pull/27082)