[Move Only] Move wallet related sources to src/wallet/ #5745

1. 
   jonasschnelli commented at 8:16 pm on February 3, 2015: contributor

   This looks after a heavy changeset, but it’s not. This will only move wallet related files to /src/wallet/. The PR includes no code changes.

   Could once be renamed from /src/wallet to /src/legacywallet.

2. 
   TheBlueMatt commented at 8:42 pm on February 3, 2015: member

   You forgot to git add the new directory, it looks like.
3. jonasschnelli force-pushed on Feb 3, 2015
4. 
   jonasschnelli commented at 8:44 pm on February 3, 2015: contributor

   @TheBlueMatt Bah. Now added. Thanks.
5. jonasschnelli force-pushed on Feb 4, 2015
6. jonasschnelli force-pushed on Feb 4, 2015
7. 
   laanwj commented at 9:25 am on February 4, 2015: member

   ACK (but needs rebase)
8. jonasschnelli force-pushed on Feb 4, 2015
9. jonasschnelli force-pushed on Feb 4, 2015
10. 
    jonasschnelli commented at 12:08 pm on February 4, 2015: contributor

    Rebased.
11. jonasschnelli referenced this in commit 6828277076 on Feb 4, 2015
12. jonasschnelli referenced this in commit 7bd633c39a on Feb 4, 2015
13. laanwj added the label Improvement on Feb 5, 2015
14. laanwj added the label Wallet on Feb 5, 2015
15. 
    theuni commented at 3:52 am on February 5, 2015: member

    Since there’s a reasonably clean separation of the wallet files, I’d prefer to build the objects in the subdir, same as qt and tests:

     0diff --git a/src/Makefile.am b/src/Makefile.am
     1index 88e7af2..1ff3180 100644
     2--- a/src/Makefile.am
     3+++ b/src/Makefile.am
     4@@ -23,7 +23,7 @@ BITCOIN_INCLUDES=-I$(builddir) -I$(builddir)/obj $(BOOST_CPPFLAGS) $(LEVELDB_CPP
     5 BITCOIN_INCLUDES += -I$(srcdir)/secp256k1/include
     6
     7 LIBBITCOIN_SERVER=libbitcoin_server.a
     8-LIBBITCOIN_WALLET=libbitcoin_wallet.a
     9+LIBBITCOIN_WALLET=wallet/libbitcoin_wallet.a
    10 LIBBITCOIN_COMMON=libbitcoin_common.a
    11 LIBBITCOIN_CLI=libbitcoin_cli.a
    12 LIBBITCOIN_UTIL=libbitcoin_util.a
    13@@ -46,7 +46,7 @@ EXTRA_LIBRARIES = \
    14   libbitcoin_cli.a
    15 if ENABLE_WALLET
    16 BITCOIN_INCLUDES += $(BDB_CPPFLAGS)
    17-EXTRA_LIBRARIES += libbitcoin_wallet.a
    18+EXTRA_LIBRARIES += $(LIBBITCOIN_WALLET)
    19 endif
    20
    21 if BUILD_BITCOIN_LIBS
    22@@ -191,8 +191,8 @@ libbitcoin_server_a_SOURCES = \
    23
    24 # wallet: shared between bitcoind and bitcoin-qt, but only linked
    25 # when wallet enabled
    26-libbitcoin_wallet_a_CPPFLAGS = $(BITCOIN_INCLUDES)
    27-libbitcoin_wallet_a_SOURCES = \
    28+wallet_libbitcoin_wallet_a_CPPFLAGS = $(BITCOIN_INCLUDES)
    29+wallet_libbitcoin_wallet_a_SOURCES = \
    30   wallet/db.cpp \
    31   crypter.cpp \
    32   wallet/rpcdump.cpp \
    33@@ -310,7 +310,7 @@ bitcoind_LDADD = \
    34   $(LIBSECP256K1)
    35
    36 if ENABLE_WALLET
    37-bitcoind_LDADD += libbitcoin_wallet.a
    38+bitcoind_LDADD += $(LIBBITCOIN_WALLET)
    39 endif
    40
    41 bitcoind_LDADD += $(BOOST_LIBS) $(BDB_LIBS) $(SSL_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS)
    42diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include
    43index 31fe3a9..7b48b41 100644
    44--- a/src/Makefile.qt.include
    45+++ b/src/Makefile.qt.include
    46@@ -370,7 +370,7 @@ QT_QM=$(QT_TS:.ts=.qm)
    47
    48 SECONDARY: $(QT_QM)
    49
    50-qt/bitcoinstrings.cpp: $(libbitcoin_server_a_SOURCES) $(libbitcoin_wallet_a_SOURCES)
    51+qt/bitcoinstrings.cpp: $(libbitcoin_server_a_SOURCES) $(wallet_libbitcoin_wallet_a_SOURCES) [@test](/bitcoin-bitcoin/contributor/test/) -n $(XGETTEXT) || echo "xgettext is required for updating translations"
    52    $(AM_V_GEN) cd $(srcdir); XGETTEXT=$(XGETTEXT) ../share/qt/extract_strings_qt.py $^
    

    That’d be my preference since imo it’s cleaner, but I’m not strongly opposed to leaving it as-is.

    ACK either way.

16. 
    jonasschnelli commented at 7:56 am on February 5, 2015: contributor

    @theuni Yes. I think this should be the long term goal. Let me play with it. What about the wallet tests under src/wallet/test? I assume we keep these in the src/Makefile.test.include for now? The problem is, that currently other tests relay on a wallet. I try to slowly also decouple things there.
17. 
    theuni commented at 3:20 pm on February 5, 2015: member

    @jonasschnelli sure, no problem if things slowly move that way, no need to try to do it all at once. Yes, tests are fine as you handled them imo. Eventually we can create a Makefile.wallet.include once it’s more decoupled.
18. 
    jtimon commented at 7:38 pm on February 6, 2015: contributor

    ut ACK
19. jonasschnelli force-pushed on Mar 5, 2015
20. 
    jonasschnelli commented at 8:18 am on March 5, 2015: contributor

    Rebased.

    What if we merge this (has ACK from @theuni and @laanwj)? This requires rebase often when getting out of date. The makefile changes (https://github.com/bitcoin/bitcoin/pull/5745#issuecomment-72989968) could be done in a upcoming PR.

21. 
    paveljanik commented at 8:26 am on March 5, 2015: contributor

    utACK
22. 
    fanquake commented at 8:38 am on March 5, 2015: member

    utACK
23. [Move Only] Move wallet related things to src/wallet/

    could once be renamed from /src/wallet to /src/legacywallet.

    50c72f23ad
24. jonasschnelli force-pushed on Mar 12, 2015
25. 
    jonasschnelli commented at 1:15 pm on March 12, 2015: contributor

    Rebased and ready for merge. :)
26. laanwj merged this on Mar 20, 2015
27. laanwj closed this on Mar 20, 2015

    ---

28. laanwj referenced this in commit 05f17d4eaa on Mar 20, 2015
29. MarcoFalke locked this on Sep 8, 2021

Contributors
jonasschnelli TheBlueMatt laanwj theuni jtimon paveljanik fanquake

Labels
Refactoring Wallet