move-only: Extract common/args from util/system #27419

TheCharlatan · 2023-04-04T13:12:04Z

This pull request is part of the libbitcoinkernel project #24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is part of a series of patches splitting up the util/system files. Its preceding pull request is #27254.

The pull request contains an extraction of ArgsManager related functions from util/system into their own common/ file.

The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See doc/design/libraries.md for more information on this rationale.

DrahtBot · 2023-04-04T13:12:07Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	MarcoFalke, ryanofsky, hebasto
Concept ACK	fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#27491 (refactor: Move chain constants to the util library by TheCharlatan)
#27409 (Make GUI and CLI tools use the same datadir by ryanofsky)
#27385 (net, refactor: extract Network and BIP155Network logic to node/network by jonatack)
#27302 (init: Error if ignored bitcoin.conf file is found by ryanofsky)
#27125 (refactor, kernel: Decouple ArgsManager from blockstorage by TheCharlatan)
#27064 (system: use %LOCALAPPDATA% as default datadir on windows by pinheadmz)
#26951 (wallet: improve rescan performance 640% by pstratem)
#26762 (refactor: Make CCheckQueue RAII-styled by hebasto)
#25908 (p2p: remove adjusted time by fanquake)
#24479 (Bugfix: util: Correctly handle Number value types in GetArg/GetBoolArg by luke-jr)
#24313 (Improve display address handling for external signer by Sjors)
#24230 (indexes: Stop using node internal types and locking cs_main, improve sync logic by ryanofsky)
#16545 (refactor: Implement missing error checking for ArgsManager flags by ryanofsky)
#10443 (Add fee_est tool for debugging fee estimation code by ryanofsky)
#10102 (Multiprocess bitcoin by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

src/test/util_tests.cpp

TheCharlatan · 2023-04-04T20:43:48Z

Updated 93c56f4 -> c02e451 (splitSystemArgs_0 -> splitSystemArgs_1, compare):

Addressed @MarcoFalke's comment for cleaning up a needless rename.

hebasto · 2023-04-05T06:51:54Z

Concept ACK.

hebasto

Approach ACK c02e451.

Suggesting to pass the new common/args.cpp to IWYU:

diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh
index c2cf2a8720..0cef4683ac 100755
--- a/ci/test/06_script_b.sh
+++ b/ci/test/06_script_b.sh
@@ -42,6 +42,7 @@ if [ "${RUN_TIDY}" = "true" ]; then
   ( CI_EXEC run-clang-tidy-15 -quiet "${MAKEJOBS}" ) | grep -C5 "error"
   export P_CI_DIR="${BASE_BUILD_DIR}/bitcoin-$HOST/"
   CI_EXEC "python3 ${DIR_IWYU}/include-what-you-use/iwyu_tool.py"\
+          " src/common/args.cpp"\
           " src/common/init.cpp"\
           " src/common/url.cpp"\
           " src/compat"\
diff --git a/src/common/args.cpp b/src/common/args.cpp
index 42ea171a45..2aa378016b 100644
--- a/src/common/args.cpp
+++ b/src/common/args.cpp
@@ -15,7 +15,6 @@
 #include <util/settings.h>
 #include <util/strencodings.h>
 #include <util/string.h>
-#include <util/time.h>
 
 #ifndef WIN32
 #include <algorithm>
@@ -28,17 +27,16 @@
 
 #include <univalue.h>
 
-#include <cstdint>
 #include <cstdlib>
 #include <cstring>
 #include <filesystem>
 #include <fstream>
 #include <iostream>
 #include <map>
-#include <memory>
 #include <optional>
 #include <stdexcept>
 #include <string>
+#include <string_view>
 #include <utility>
 
 const char * const BITCOIN_CONF_FILENAME = "bitcoin.conf";
diff --git a/src/common/args.h b/src/common/args.h
index 638bb7034e..35115e2dc4 100644
--- a/src/common/args.h
+++ b/src/common/args.h
@@ -1,10 +1,6 @@
 #ifndef BITCOIN_COMMON_ARGS_H
 #define BITCOIN_COMMON_ARGS_H
 
-#if defined(HAVE_CONFIG_H)
-#include <config/bitcoin-config.h>
-#endif
-
 #include <compat/compat.h>
 #include <sync.h>
 #include <util/fs.h>

src/common/args.h

ryanofsky

Code review ACK c02e451

Just an option to consider, but I think could be good to move GetConfigFile, GetConfigOptions, ArgsManager::ReadConfigStream, and ArgsManager::ReadConfigFiles functions to src/common/config.cpp instead of src/common/args.cpp, since in longer run, it would be nice if it would be nice if there were a src/common/settings.cpp file to parse the settings file, src/common/config.cpp to parse the config file, and src/common/args.cpp that would parse arguments and depend on the settings and config modules. If we move the 4 config functions above now to src/common/config.cpp, it would avoid having to move them again later.

src/util/system.h

fanquake · 2023-04-05T19:19:00Z

Concept ACK

TheCharlatan · 2023-04-05T21:01:08Z

RE #27419 (review)

Just an option to consider, but I think could be good to move GetConfigFile, GetConfigOptions, ArgsManager::ReadConfigStream, and ArgsManager::ReadConfigFiles functions to src/common/config.cpp instead of src/common/args.cpp, since in longer run, it would be nice if it would be nice if there were a src/common/settings.cpp file to parse the settings file, src/common/config.cpp to parse the config file, and src/common/args.cpp that would parse arguments and depend on the settings and config modules. If we move the 4 config functions above now to src/common/config.cpp, it would avoid having to move them again later.

I thought about doing this split and was a bit hesitant to include it here, because it is less of a pure move. Is 1c9adff something along the lines of what you meant? Couldn't think if a less intrusive way to implement this besides declaring a friend class for now.

TheCharlatan · 2023-04-05T21:11:13Z

Updated c02e451 -> de2546e (splitSystemArgs_1 -> splitSystemArgs_2, compare):

Addressed @hebasto's comment adding a missing copyright header
Addressed @hebasto's comment creating a commit adding args.cpp to the iwyu list and tweaking some of the included headers
Addressed @ryanofsky 's comment correcting a docstring

ryanofsky

Code review ACK de2546e. Just comment & iwyu change since last review.

re: #27419 (comment)

Sorry for the ambiguity, I was suggesting keeping it a pure move and just literally moving those functions to a src/common/config.cpp file without changing them so they don't have to move again later. If you're breaking up a big class in c++ you don't have to move the methods and refactor the class at the same time, you can just move the methods.

It is a judgement call whether you think a having single class split into 2 cpp files is worse than moving the same code twice. I think moving the same code twice is worse, obviously.

ryanofsky · 2023-04-12T15:10:42Z

I think this PR could be labeled move-only instead of refactoring, because no code is changing other than #includes.

maflcko

review ACK ec51c25 🦅

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK ec51c252de42e11906f114ac05e476fd88ca2176  🦅
0o6nucYAS/HVSSMijvjryysBuyCio5TbnDcNBbv5vgNSeWWPEXYDIv5BUC7sRVuZCfXkAEtNXghoTUB0D3ZDBg==

maflcko · 2023-04-19T08:04:57Z

src/common/args.cpp

+#include <shlobj.h>   /* for CSIDL_APPDATA */
+#endif
+
+#include <univalue.h>


Any reason for a separate section for this header?

maflcko · 2023-04-19T08:15:15Z

src/node/context.h

@@ -7,6 +7,8 @@

 #include <kernel/context.h>

+#include <util/system.h>


Can you explain this change?

maflcko · 2023-04-19T08:24:38Z

src/util/system.h

@@ -4,8 +4,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.

 /**
- * Server/client environment: argument handling, config file parsing,
- * thread wrappers, startup time
+ * Server/client environment, startup time


nit: The comment is incomplete (missing AnyPtr stuff etc), and I don't think it is a good use of time to make it complete. Also, the file has less than 10 functions and we don't put those comments in other files, so might as well remove it.

This is an extraction of ArgsManager related functions from util/system into their own common file. Config file related functions are moved to common/config.cpp. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See doc/design/libraries.md for more information on this rationale.

TheCharlatan · 2023-04-19T10:04:06Z

Thank you for the reviews, decided to address the left over nits here:

Updated ec51c25 -> be55f54 (splitSystemArgs_3 -> splitSystemArgs_4, compare):

Addressed @ryanofsky's comment by cleaning up conditional includes.
Addressed @ryanofsky's comment by running clang-format-diff on said lines.
Addressed @MarcoFalke's comment by including src/util/config.cpp in the iwyu list.
Addressed @MarcoFalke's comment by improving include grouping
Addressed @MarcoFalke's comment by removing the include in src/node/context.h and adding it in the actual source file that relied on it.
Addressed @MarcoFalke's comment by removing the comment on the top of src/util/system.h.

DrahtBot · 2023-04-19T14:19:42Z

Guix builds

File	commit `2fa7344` (master)	commit `e5d840f` (master and this pull)
SHA256SUMS.part	`f39c4aab7a75ae88...`	`8db0e7efbcc324e2...`
*-aarch64-linux-gnu-debug.tar.gz	`6a8a9df7860d92b3...`	`65a0d4d8731dc21e...`
*-aarch64-linux-gnu.tar.gz	`ef4badcddf1fe9b9...`	`174e22e15c5d8c1b...`
*-arm-linux-gnueabihf-debug.tar.gz	`1f082d00c7cbf8c5...`	`c71b67d19511ca3c...`
*-arm-linux-gnueabihf.tar.gz	`a0a8a339ada2a975...`	`35b10d8c61286b31...`
*-powerpc64-linux-gnu-debug.tar.gz	`7d41b00495ccc998...`	`3d12bbf9277da578...`
*-powerpc64-linux-gnu.tar.gz	`67bbed17bc022b6d...`	`c5374770cbbd1c3e...`
*-powerpc64le-linux-gnu-debug.tar.gz	`7577e6eb6051d000...`	`9b60c7a535c7e98c...`
*-powerpc64le-linux-gnu.tar.gz	`ad4d0975d92dae94...`	`617e3b2e98c704ca...`
*-riscv64-linux-gnu-debug.tar.gz	`b063704406c70468...`	`94030fb3791f034e...`
*-riscv64-linux-gnu.tar.gz	`83d5ec066fc6b49a...`	`1668c688c50b80b3...`
*-x86_64-linux-gnu-debug.tar.gz	`201a187a4e15341b...`	`21558c25323b630e...`
*-x86_64-linux-gnu.tar.gz	`d0d4c3fab8f71c33...`	`660f27dcc8ff2d1d...`
*.tar.gz	`5ce615208ca33fa3...`	`156682e1e3f80778...`
guix_build.log	`1190b264b335453a...`	`cff9cfc1d2f4e039...`
guix_build.log.diff		`5f90a092717fef72...`

maflcko · 2023-04-19T14:41:19Z

re-ACK be55f54 🚲

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6  🚲
m22irb0PI71mDxsZbJ0k+W9xNfoLWj7bnpCW6Vbsib7wG5l6Im6jM1S1/Vu5XpYquBRBeE5T9aJH0baY8ydGCQ==

maflcko · 2023-04-19T14:46:16Z

unrelated: red CI can be ignored

ryanofsky

Code review ACK be55f54. Just small cleanups since the last review.

Confirmed move-only except for making InterpretValue function nonstatic, and moving some symbols from util to common namespace.

ryanofsky · 2023-04-19T15:36:53Z

src/common/args.h

+KeyInfo InterpretKey(std::string key);
+
+std::optional<util::SettingsValue> InterpretValue(const KeyInfo& key, const std::string* value,
+                                                         unsigned int flags, std::string& error);


In commit "move-only: Extract common/args and common/config.cpp from util/system" (be55f54)

Not important, but there is still some extra whitespace on this line

maflcko · 2023-04-21T08:44:20Z

Is this rfm with two reviews?

hebasto

ACK be55f54, I have reviewed the code and it looks OK, I agree it can be merged.

hebasto · 2023-04-21T10:02:40Z

Hoping this moveonly change could be reviewed and merged quickly since it conflicts with various PR's I have open

So am I (meaning cmake staging branch) :D

…ibrary, interface_ui from validation. 7d3b350 refactor: Move system from util to common library (TheCharlatan) 7eee356 refactor: Split util::AnyPtr into its own file (TheCharlatan) 44de325 refactor: Split util::insert into its own file (TheCharlatan) 9ec5da3 refactor: Move ScheduleBatchPriority to its own file (TheCharlatan) f871c69 kernel: Add warning method to notifications (TheCharlatan) 4452707 kernel: Add progress method to notifications (TheCharlatan) 84d7145 kernel: Add headerTip method to notifications (TheCharlatan) 447761c kernel: Add notification interface (TheCharlatan) Pull request description: This pull request is part of the `libbitcoinkernel` project bitcoin/bitcoin#27587 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". --- It removes the kernel library's dependency on `util/system` and `interface_ui`. `util/system` contains networking and shell-related code that should not be part of the kernel library. The following pull requests prepared `util/system` for this final step: bitcoin/bitcoin#27419 bitcoin/bitcoin#27254 bitcoin/bitcoin#27238. `interface_ui` defines functions for a more general node interface and has a dependency on `boost/signals2`. After applying the patches from this pull request, the kernel's reliance on boost is down to `boost::multiindex`. The approach implemented here introduces some indirection, which makes the code a bit harder to read. Any suggestions for improving or reworking this pull request to make it more concise, or even reworking it into a more proper interface, are appreciated. ACKs for top commit: MarcoFalke: re-ACK 7d3b350 (no change) 🎋 stickies-v: Code Review ACK 7d3b350 hebasto: re-ACK 7d3b350, only last two commits dropped since my [recent](bitcoin/bitcoin#27636 (review)) review. Tree-SHA512: c8cfc698dc9d78e20191c444708f2d957501229abe95e5806106d1126fb9c5fbcee686fb55645658c0107ce71f10646f37a2fdf7fde16bbf22cbf1ac885dd08d

The following changes considered: - bitcoin/bitcoin#27419 - bitcoin/bitcoin#27491 - bitcoin/bitcoin#27576 - bitcoin/bitcoin#27636

Pull request description: Sync with the main repo up to the latest bitcoin/bitcoin@ab42b2e, which includes the recent changes in the CI. There is no downloadable artifacts support for now. It will be done in a separated PR(s). Additionally: - The code was adjusted to reflect changes from [PR27419](bitcoin/bitcoin#27419), [PR27491](bitcoin/bitcoin#27491), [PR27576](bitcoin/bitcoin#27576) and [PR27636](bitcoin/bitcoin#27636). - Fixed `modernize-use-default-member-init` clang-tidy warnings. - The ARM task has been temporarily disabled until the issue with the depends cache is resolved. Guix builds: ``` e92b8c4c3298165edb1a0e85ee516d52c81af1269405dcbc6520e63069de2363 guix-build-b3261144c892/output/aarch64-linux-gnu/SHA256SUMS.part 939c6c002490d5649bdbfabacd20cd2270b41b20b7b3a254c9fcd5780209900d guix-build-b3261144c892/output/aarch64-linux-gnu/bitcoin-b3261144c892-aarch64-linux-gnu-debug.tar.gz b3c1383fb394997378997bdd2933965cf4ecc694143b4703108ff6ecb946696c guix-build-b3261144c892/output/aarch64-linux-gnu/bitcoin-b3261144c892-aarch64-linux-gnu.tar.gz f43fedf3af666d35e83b84e63cfe19f315f74f01296982f47d8c159385c3b03c guix-build-b3261144c892/output/arm-linux-gnueabihf/SHA256SUMS.part 73b89b0487e8eee474a6c9c96ae0e7ad635cccc332fc062eb5d4ff5555356c3e guix-build-b3261144c892/output/arm-linux-gnueabihf/bitcoin-b3261144c892-arm-linux-gnueabihf-debug.tar.gz b4518dd9396f316de8d7de5181b8b5d1083e0afa9081625c37117472d2559380 guix-build-b3261144c892/output/arm-linux-gnueabihf/bitcoin-b3261144c892-arm-linux-gnueabihf.tar.gz 0213e754408e2a032cef61a946354656f5b5f755f85aeac1ce4b37f1d22528e6 guix-build-b3261144c892/output/arm64-apple-darwin/SHA256SUMS.part 11bc1be1f53dad337565f3c556dd69abc2d702a31e661359daad6ff89225c794 guix-build-b3261144c892/output/arm64-apple-darwin/bitcoin-b3261144c892-arm64-apple-darwin-unsigned.dmg 558d8e805420c7a348759df6f559ca349953646aa28840efafe5a3d245ea917f guix-build-b3261144c892/output/arm64-apple-darwin/bitcoin-b3261144c892-arm64-apple-darwin-unsigned.tar.gz e679ce3f1c80aff11a5eab8890efbd0d396a851875fbd6f93f32eef5cdf06813 guix-build-b3261144c892/output/arm64-apple-darwin/bitcoin-b3261144c892-arm64-apple-darwin.tar.gz 0cb346390dc6620593b1af5b6669ddc3c1a8d2219a51b1697747c5ab24069c27 guix-build-b3261144c892/output/dist-archive/bitcoin-b3261144c892.tar.gz ac8bd2d58d9d0ebe2da1c8efa2d57bd97c3ef2b2590c758edbc4919808c528c5 guix-build-b3261144c892/output/powerpc64-linux-gnu/SHA256SUMS.part cdf8252fa8aca6da61ff6926de5c7e2e6560ab046049c84c26ba44823f83236a guix-build-b3261144c892/output/powerpc64-linux-gnu/bitcoin-b3261144c892-powerpc64-linux-gnu-debug.tar.gz 3b8b5f53d365b5bf962ecd7def9f06b6f13af0e5c9ef69c6d028f1ed772459be guix-build-b3261144c892/output/powerpc64-linux-gnu/bitcoin-b3261144c892-powerpc64-linux-gnu.tar.gz b44e688d233dcb46a7d6d0b1d97979335d3cc559d16190cc5cd647add79298d2 guix-build-b3261144c892/output/powerpc64le-linux-gnu/SHA256SUMS.part ae5c19afefd523cdc171a3f9aa9f707870fd99749c01c01166086619dfd95ece guix-build-b3261144c892/output/powerpc64le-linux-gnu/bitcoin-b3261144c892-powerpc64le-linux-gnu-debug.tar.gz bb581b1444fa1686f8889248af13d1859f2915091cd640bc522185d5ad83e13d guix-build-b3261144c892/output/powerpc64le-linux-gnu/bitcoin-b3261144c892-powerpc64le-linux-gnu.tar.gz bdca0a3c19b5a9a5c72b2b43b07050678d960009d3fa80cf7e0689d508346974 guix-build-b3261144c892/output/riscv64-linux-gnu/SHA256SUMS.part b0b9c91abe2ad0b5ab3b0bfd10c90133d8d75b50aef0a6a98ac2c2ae4219eaa8 guix-build-b3261144c892/output/riscv64-linux-gnu/bitcoin-b3261144c892-riscv64-linux-gnu-debug.tar.gz fcce0ea00f1d9df136dd677cbc468183faa92bd4bfcd4a77cd1c70f1b894b5f0 guix-build-b3261144c892/output/riscv64-linux-gnu/bitcoin-b3261144c892-riscv64-linux-gnu.tar.gz 7be84969950bb9570522be5a37551c01698cd3fb65eca3988fc9bd6867460552 guix-build-b3261144c892/output/x86_64-apple-darwin/SHA256SUMS.part 25203f50aa6a344ad1c6c4a44a48082440bb0af9bf38f0d60506569f216d1672 guix-build-b3261144c892/output/x86_64-apple-darwin/bitcoin-b3261144c892-x86_64-apple-darwin-unsigned.dmg 16c5baaf6d00ed43b0611c86c2d4555d500b3896daa1daac6a567bc2611c39f6 guix-build-b3261144c892/output/x86_64-apple-darwin/bitcoin-b3261144c892-x86_64-apple-darwin-unsigned.tar.gz 86662f39c29b013b576e6555ecb6cbbc98eaa08532a541e22a7ed6b1baf87209 guix-build-b3261144c892/output/x86_64-apple-darwin/bitcoin-b3261144c892-x86_64-apple-darwin.tar.gz fbbc0ad2376431fdc5b214fd63f24a6da907d87f6f11e0833def50c0d45772cd guix-build-b3261144c892/output/x86_64-linux-gnu/SHA256SUMS.part cba8d700f746a6063809570e45d6dc3d5e60ad5f1a28e0f41f8beed8b546a7b1 guix-build-b3261144c892/output/x86_64-linux-gnu/bitcoin-b3261144c892-x86_64-linux-gnu-debug.tar.gz 0a32985a1e26e13ce883a85e4a92cc68bf51ce096f2f6d74ea499a9fa662d7d0 guix-build-b3261144c892/output/x86_64-linux-gnu/bitcoin-b3261144c892-x86_64-linux-gnu.tar.gz 0bd4cc64cd6ad733cdef87cd74d5034e79dd250b72795cebf9c2c63500509457 guix-build-b3261144c892/output/x86_64-w64-mingw32/SHA256SUMS.part 6ed8f2e6c6cf1992d156672707cd2c254754051f88223dd052a9cd9078d84789 guix-build-b3261144c892/output/x86_64-w64-mingw32/bitcoin-b3261144c892-win64-debug.zip 1ea6d7660652e20b2b1529e406be1f606745d35f6a179b006335a19a19aa9a5b guix-build-b3261144c892/output/x86_64-w64-mingw32/bitcoin-b3261144c892-win64-setup-unsigned.exe 41b0f8cbac614e8c555921de60b25a73a75e6bed025de98ca40d3db48c5db6b1 guix-build-b3261144c892/output/x86_64-w64-mingw32/bitcoin-b3261144c892-win64-unsigned.tar.gz 5c68d711782e76f9e4be93b5468c505f022b72ca299532b200e58fe1e51343b1 guix-build-b3261144c892/output/x86_64-w64-mingw32/bitcoin-b3261144c892-win64.zip ``` Top commit has no ACKs. Tree-SHA512: dd18cfb2cfd6fd45b35bef8a0397bccc0752ce946b304bae986006ff09a9a183d6222b0f607e4dd3373992814ae0e61d5ba63cb54fef9a288152edef3d7ea81d

DrahtBot added the Refactoring label Apr 4, 2023

TheCharlatan marked this pull request as ready for review April 4, 2023 13:12

maflcko reviewed Apr 4, 2023

View reviewed changes

src/test/util_tests.cpp Outdated Show resolved Hide resolved

TheCharlatan force-pushed the splitSystemArgs branch from 93c56f4 to c02e451 Compare April 4, 2023 20:43

hebasto reviewed Apr 5, 2023

View reviewed changes

src/common/args.h Show resolved Hide resolved

ryanofsky approved these changes Apr 5, 2023

View reviewed changes

src/util/system.h Show resolved Hide resolved

TheCharlatan force-pushed the splitSystemArgs branch from c02e451 to de2546e Compare April 5, 2023 21:10

ryanofsky approved these changes Apr 6, 2023

View reviewed changes

bitcoin deleted a comment from Nimano1981 Apr 18, 2023

maflcko added the DrahtBot Guix build requested label Apr 19, 2023

maflcko approved these changes Apr 19, 2023

View reviewed changes

TheCharlatan force-pushed the splitSystemArgs branch from ec51c25 to be55f54 Compare April 19, 2023 10:03

TheCharlatan changed the title ~~refactor: Extract common/args from util/system~~ move-only: Extract common/args from util/system Apr 19, 2023

DrahtBot removed the DrahtBot Guix build requested label Apr 19, 2023

DrahtBot requested a review from ryanofsky April 19, 2023 14:41

ryanofsky approved these changes Apr 19, 2023

View reviewed changes

DrahtBot mentioned this pull request Apr 19, 2023

refactor: Move chain constants to the util library #27491

Merged

hebasto approved these changes Apr 21, 2023

View reviewed changes

fanquake merged commit 669af32 into bitcoin:master Apr 21, 2023

hebasto mentioned this pull request Apr 21, 2023

Rebased cmake-staging branch (post PR#13) hebasto/bitcoin#14

Closed

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 21, 2023

Merge bitcoin#27419: move-only: Extract common/args from util/system

6c03826

This was referenced May 12, 2023

Chainstate rm kernel UI interface 0 TheCharlatan/bitcoin#12

Closed

kernel: Remove util/system from kernel library, interface_ui from validation. #27636

Merged

hebasto added a commit to hebasto/gui-qml that referenced this pull request Aug 29, 2023

Adjust code according to changes in the main repository

592da16

The following changes considered: - bitcoin/bitcoin#27419 - bitcoin/bitcoin#27491 - bitcoin/bitcoin#27576 - bitcoin/bitcoin#27636

hebasto added a commit to hebasto/gui-qml that referenced this pull request Aug 29, 2023

Adjust code according to changes in the main repository

c77e8b5

The following changes considered: - bitcoin/bitcoin#27419 - bitcoin/bitcoin#27491 - bitcoin/bitcoin#27576 - bitcoin/bitcoin#27636

hebasto added a commit to hebasto/gui-qml that referenced this pull request Aug 29, 2023

Adjust code according to changes in the main repository

8b02563

The following changes considered: - bitcoin/bitcoin#27419 - bitcoin/bitcoin#27491 - bitcoin/bitcoin#27576 - bitcoin/bitcoin#27636

hebasto mentioned this pull request Aug 30, 2023

Sync with the main repo bitcoin-core/gui-qml#359

Merged

bitcoin locked and limited conversation to collaborators Apr 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

move-only: Extract common/args from util/system #27419

move-only: Extract common/args from util/system #27419

TheCharlatan commented Apr 4, 2023

DrahtBot commented Apr 4, 2023 •

edited

Loading

TheCharlatan commented Apr 4, 2023 •

edited

Loading

hebasto commented Apr 5, 2023

hebasto left a comment

ryanofsky left a comment

fanquake commented Apr 5, 2023

TheCharlatan commented Apr 5, 2023

TheCharlatan commented Apr 5, 2023

ryanofsky left a comment

ryanofsky commented Apr 12, 2023

maflcko left a comment

maflcko Apr 19, 2023

maflcko Apr 19, 2023

maflcko Apr 19, 2023

TheCharlatan commented Apr 19, 2023

DrahtBot commented Apr 19, 2023

maflcko commented Apr 19, 2023

maflcko commented Apr 19, 2023

ryanofsky left a comment

ryanofsky Apr 19, 2023

maflcko commented Apr 21, 2023

hebasto left a comment

hebasto commented Apr 21, 2023

		@@ -7,6 +7,8 @@

		#include <kernel/context.h>

		#include <util/system.h>

move-only: Extract common/args from util/system #27419

move-only: Extract common/args from util/system #27419

Conversation

TheCharlatan commented Apr 4, 2023

DrahtBot commented Apr 4, 2023 • edited Loading

Reviews

Conflicts

TheCharlatan commented Apr 4, 2023 • edited Loading

hebasto commented Apr 5, 2023

hebasto left a comment

Choose a reason for hiding this comment

ryanofsky left a comment

Choose a reason for hiding this comment

fanquake commented Apr 5, 2023

TheCharlatan commented Apr 5, 2023

TheCharlatan commented Apr 5, 2023

ryanofsky left a comment

Choose a reason for hiding this comment

ryanofsky commented Apr 12, 2023

maflcko left a comment

Choose a reason for hiding this comment

maflcko Apr 19, 2023

Choose a reason for hiding this comment

maflcko Apr 19, 2023

Choose a reason for hiding this comment

maflcko Apr 19, 2023

Choose a reason for hiding this comment

TheCharlatan commented Apr 19, 2023

DrahtBot commented Apr 19, 2023

Guix builds

maflcko commented Apr 19, 2023

maflcko commented Apr 19, 2023

ryanofsky left a comment

Choose a reason for hiding this comment

ryanofsky Apr 19, 2023

Choose a reason for hiding this comment

maflcko commented Apr 21, 2023

hebasto left a comment

Choose a reason for hiding this comment

hebasto commented Apr 21, 2023

DrahtBot commented Apr 4, 2023 •

edited

Loading

TheCharlatan commented Apr 4, 2023 •

edited

Loading