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

refactor: Make all #includes relative to project root (laanwj, MeshCollider, ryanofsky) #11651

Merged
merged 8 commits into from Nov 16, 2017
Merged

refactor: Make all #includes relative to project root (laanwj, MeshCollider, ryanofsky) #11651

merged 8 commits into from Nov 16, 2017

Conversation

meshcollider
Copy link
Contributor

@meshcollider meshcollider commented Nov 10, 2017

Rebase of #11053

Previously started by @laanwj, ACK'ed by promag, ryanofsky, jonasschnelli, and concept-ACK'ed by practicalswift and jnewbery. Thus should be almost RTM :)

@meshcollider
Copy link
Contributor Author

@ryanofsky you might want to check this over :)

Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

utACK 5d0eae757e30605ec3d26bc515856b1d20a13cc2

@laanwj
Copy link
Member

laanwj commented Nov 10, 2017

So we're doing this?
Okay!
utACK 5d0eae7

@sipa
Copy link
Member

sipa commented Nov 10, 2017

Pieter was here.

@practicalswift
Copy link
Contributor

utACK 5d0eae757e30605ec3d26bc515856b1d20a13cc2

Nice!

@maflcko maflcko changed the title refactor: Make all #includes relative to project root (rebased) refactor: Make all #includes relative to project root (laanwj, MeshCollider, ryanofsky) Nov 10, 2017
@jnewbery
Copy link
Contributor

Thanks for taking and rebasing this. Big concept ACK. Will review later.

@laanwj
Copy link
Member

laanwj commented Nov 15, 2017

Needs rebase, after that please let me know and we should just merge it, has had lots of review and is a pain to keep up to date.

Copy link
Member

@promag promag left a comment

Choose a reason for hiding this comment

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

rere-utACK.

@@ -461,6 +461,14 @@ namespace {

- *Rationale*: Avoids confusion about the namespace context

- Prefer `#include <primitives/transaction.h>` bracket syntax instead of
`#include "primitives/transactions.h" quote syntax when possible.
Copy link
Member

Choose a reason for hiding this comment

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

Missing `.

Copy link
Member

Choose a reason for hiding this comment

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

BTW, unrelated but the Rationale above doesn't render well. And some are missing a period.

meshcollider and others added 8 commits November 16, 2017 08:23
-BEGIN VERIFY SCRIPT-
for f in \
  src/*.cpp \
  src/*.h \
  src/bench/*.cpp \
  src/bench/*.h \
  src/compat/*.cpp \
  src/compat/*.h \
  src/consensus/*.cpp \
  src/consensus/*.h \
  src/crypto/*.cpp \
  src/crypto/*.h \
  src/crypto/ctaes/*.h \
  src/policy/*.cpp \
  src/policy/*.h \
  src/primitives/*.cpp \
  src/primitives/*.h \
  src/qt/*.cpp \
  src/qt/*.h \
  src/qt/test/*.cpp \
  src/qt/test/*.h \
  src/rpc/*.cpp \
  src/rpc/*.h \
  src/script/*.cpp \
  src/script/*.h \
  src/support/*.cpp \
  src/support/*.h \
  src/support/allocators/*.h \
  src/test/*.cpp \
  src/test/*.h \
  src/wallet/*.cpp \
  src/wallet/*.h \
  src/wallet/test/*.cpp \
  src/wallet/test/*.h \
  src/zmq/*.cpp \
  src/zmq/*.h
do
  base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-
Remove -I from build system for everything but the project root,
and built-in dependencies.
This makes all include paths in the GUI absolute.

Many changes are involved as every single source file in
src/qt/ assumes to be able to use relative includes.
@meshcollider
Copy link
Contributor Author

meshcollider commented Nov 15, 2017

Rebased and added missing tick pointed out by @promag, will just wait for travis to verify the scripted-diff again

@jnewbery
Copy link
Contributor

Tested ACK 7b91b5f.

I noticed that there were a few .rc and .mm files still using the "" style includes. Should those also be changed to <> style? I think the change would be like this: jnewbery@1a506c7 but I don't have windows or mac machines to test the build on.

@theuni
Copy link
Member

theuni commented Nov 15, 2017

indifferent utACK 7b91b5f.

@laanwj laanwj merged commit 7b91b5f into bitcoin:master Nov 16, 2017
laanwj added a commit that referenced this pull request Nov 16, 2017
…laanwj, MeshCollider, ryanofsky)

7b91b5f Remove trailing whitespace causing travis failure (MeshCollider)
434f5a2 Recommend #include<> syntax in developer notes (Russell Yanofsky)
96b9281 refactor: Include obj/build.h instead of build.h (Wladimir J. van der Laan)
138016b test: refactor: Use absolute include paths for test data files (Wladimir J. van der Laan)
e7b3163 qt: refactor: Changes to make include paths absolute (Wladimir J. van der Laan)
0c71521 build: Remove -I for everything but project root (Wladimir J. van der Laan)
5b56ec9 qt: refactor: Use absolute include paths in .ui files (Wladimir J. van der Laan)
1a44534 scripted-diff: Replace #include "" with #include <> (ryanofsky) (MeshCollider)

Pull request description:

  Rebase of #11053

  Previously started by @laanwj, ACK'ed by promag, ryanofsky, jonasschnelli, and concept-ACK'ed by practicalswift and jnewbery. Thus should be almost RTM :)

Tree-SHA512: d8d25248309deb06a54686c4a6bafd290ba69dcd0df391a50d1caed2c22ff2659be442459bdd9d1fc3b6a1360ba0804a907b1402d206df3e1cb6e8924e3c7f3e
@meshcollider meshcollider deleted the 201711_absolute_includes branch November 16, 2017 08:34
@laanwj
Copy link
Member

laanwj commented Nov 16, 2017

I noticed that there were a few .rc and .mm files still using the "" style includes.

If they are correctly relative (so to include from the directory the including file is in, or even a directory relative to its directory) it's fine to keep them. "" style includes are a problem when they're used when <> is meant, which was common in this project before this change.

maflcko pushed a commit that referenced this pull request Dec 20, 2017
aac6b3f Update files.md for new wallets/ subdirectory (MeshCollider)
b673429 Cleanups for walletdir PR (MeshCollider)

Pull request description:

  This addresses the remaining nits from #11466

  - Updates `doc/files.md` with respect to the new default wallet directory
  - Fixes @promag and @laanwj's error message nit, and Jonas' release notes nit
  - ~Addresses @laanwj's net-specific wallet subdirectory concern in the case that a walletdir is specified~
  - Changes the #includes from "" to <> style after #11651

Tree-SHA512: b86bf5fdc4de54c1b0f65b60a83af3cf82b35d216ce9c0de724803bfba6934796238b6c412659dcc29ae2e3e856d4eb97ae777c80f36f4089d8acecfddefe9aa
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Feb 13, 2020
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 13, 2020
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Feb 13, 2020
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 13, 2020
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Feb 29, 2020
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 29, 2020
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Feb 29, 2020
aac6b3f Update files.md for new wallets/ subdirectory (MeshCollider)
b673429 Cleanups for walletdir PR (MeshCollider)

Pull request description:

  This addresses the remaining nits from bitcoin#11466

  - Updates `doc/files.md` with respect to the new default wallet directory
  - Fixes @promag and @laanwj's error message nit, and Jonas' release notes nit
  - ~Addresses @laanwj's net-specific wallet subdirectory concern in the case that a walletdir is specified~
  - Changes the #includes from "" to <> style after bitcoin#11651

Tree-SHA512: b86bf5fdc4de54c1b0f65b60a83af3cf82b35d216ce9c0de724803bfba6934796238b6c412659dcc29ae2e3e856d4eb97ae777c80f36f4089d8acecfddefe9aa
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 4, 2020
aac6b3f Update files.md for new wallets/ subdirectory (MeshCollider)
b673429 Cleanups for walletdir PR (MeshCollider)

Pull request description:

  This addresses the remaining nits from bitcoin#11466

  - Updates `doc/files.md` with respect to the new default wallet directory
  - Fixes @promag and @laanwj's error message nit, and Jonas' release notes nit
  - ~Addresses @laanwj's net-specific wallet subdirectory concern in the case that a walletdir is specified~
  - Changes the #includes from "" to <> style after bitcoin#11651

Tree-SHA512: b86bf5fdc4de54c1b0f65b60a83af3cf82b35d216ce9c0de724803bfba6934796238b6c412659dcc29ae2e3e856d4eb97ae777c80f36f4089d8acecfddefe9aa
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Mar 5, 2020
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 5, 2020
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 5, 2020
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 11, 2020
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 13, 2020
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 14, 2020
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 15, 2020
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 15, 2020
UdjinM6 added a commit to dashpay/dash that referenced this pull request Mar 19, 2020
* scripted-diff: Replace #include "" with #include <> (ryanofsky)

-BEGIN VERIFY SCRIPT-
for f in \
  src/*.cpp \
  src/*.h \
  src/bench/*.cpp \
  src/bench/*.h \
  src/compat/*.cpp \
  src/compat/*.h \
  src/consensus/*.cpp \
  src/consensus/*.h \
  src/crypto/*.cpp \
  src/crypto/*.h \
  src/crypto/ctaes/*.h \
  src/policy/*.cpp \
  src/policy/*.h \
  src/primitives/*.cpp \
  src/primitives/*.h \
  src/qt/*.cpp \
  src/qt/*.h \
  src/qt/test/*.cpp \
  src/qt/test/*.h \
  src/rpc/*.cpp \
  src/rpc/*.h \
  src/script/*.cpp \
  src/script/*.h \
  src/support/*.cpp \
  src/support/*.h \
  src/support/allocators/*.h \
  src/test/*.cpp \
  src/test/*.h \
  src/wallet/*.cpp \
  src/wallet/*.h \
  src/wallet/test/*.cpp \
  src/wallet/test/*.h \
  src/zmq/*.cpp \
  src/zmq/*.h
do
  base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-

Signed-off-by: Pasta <pasta@dashboost.org>

* scripted-diff: Replace #include "" with #include <> (Dash Specific)

-BEGIN VERIFY SCRIPT-
for f in \
  src/bls/*.cpp \
  src/bls/*.h \
  src/evo/*.cpp \
  src/evo/*.h \
  src/governance/*.cpp \
  src/governance/*.h \
  src/llmq/*.cpp \
  src/llmq/*.h \
  src/masternode/*.cpp \
  src/masternode/*.h \
  src/privatesend/*.cpp \
  src/privatesend/*.h
do
  base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-

Signed-off-by: Pasta <pasta@dashboost.org>

* build: Remove -I for everything but project root

Remove -I from build system for everything but the project root,
and built-in dependencies.

Signed-off-by: Pasta <pasta@dashboost.org>

# Conflicts:
#	src/Makefile.test.include

* qt: refactor: Use absolute include paths in .ui files

* qt: refactor: Changes to make include paths absolute

This makes all include paths in the GUI absolute.

Many changes are involved as every single source file in
src/qt/ assumes to be able to use relative includes.

Signed-off-by: Pasta <pasta@dashboost.org>

# Conflicts:
#	src/qt/dash.cpp
#	src/qt/optionsmodel.cpp
#	src/qt/test/rpcnestedtests.cpp

* test: refactor: Use absolute include paths for test data files

* Recommend #include<> syntax in developer notes

* refactor: Include obj/build.h instead of build.h

* END BACKPORT bitcoin#11651 Remove trailing whitespace causing travis failure

* fix backport 11651

Signed-off-by: Pasta <pasta@dashboost.org>

* More of 11651

* fix blockchain.cpp

Signed-off-by: pasta <pasta@dashboost.org>

* Add missing "qt/" in includes

* Add missing "test/" in includes

* Fix trailing whitespaces

Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: MeshCollider <dobsonsa68@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 26, 2021
* scripted-diff: Replace #include "" with #include <> (ryanofsky)

-BEGIN VERIFY SCRIPT-
for f in \
  src/*.cpp \
  src/*.h \
  src/bench/*.cpp \
  src/bench/*.h \
  src/compat/*.cpp \
  src/compat/*.h \
  src/consensus/*.cpp \
  src/consensus/*.h \
  src/crypto/*.cpp \
  src/crypto/*.h \
  src/crypto/ctaes/*.h \
  src/policy/*.cpp \
  src/policy/*.h \
  src/primitives/*.cpp \
  src/primitives/*.h \
  src/qt/*.cpp \
  src/qt/*.h \
  src/qt/test/*.cpp \
  src/qt/test/*.h \
  src/rpc/*.cpp \
  src/rpc/*.h \
  src/script/*.cpp \
  src/script/*.h \
  src/support/*.cpp \
  src/support/*.h \
  src/support/allocators/*.h \
  src/test/*.cpp \
  src/test/*.h \
  src/wallet/*.cpp \
  src/wallet/*.h \
  src/wallet/test/*.cpp \
  src/wallet/test/*.h \
  src/zmq/*.cpp \
  src/zmq/*.h
do
  base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-

Signed-off-by: Pasta <pasta@dashboost.org>

* scripted-diff: Replace #include "" with #include <> (Dash Specific)

-BEGIN VERIFY SCRIPT-
for f in \
  src/bls/*.cpp \
  src/bls/*.h \
  src/evo/*.cpp \
  src/evo/*.h \
  src/governance/*.cpp \
  src/governance/*.h \
  src/llmq/*.cpp \
  src/llmq/*.h \
  src/masternode/*.cpp \
  src/masternode/*.h \
  src/privatesend/*.cpp \
  src/privatesend/*.h
do
  base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-

Signed-off-by: Pasta <pasta@dashboost.org>

* build: Remove -I for everything but project root

Remove -I from build system for everything but the project root,
and built-in dependencies.

Signed-off-by: Pasta <pasta@dashboost.org>

* qt: refactor: Use absolute include paths in .ui files

* qt: refactor: Changes to make include paths absolute

This makes all include paths in the GUI absolute.

Many changes are involved as every single source file in
src/qt/ assumes to be able to use relative includes.

Signed-off-by: Pasta <pasta@dashboost.org>

* test: refactor: Use absolute include paths for test data files

* Recommend #include<> syntax in developer notes

* refactor: Include obj/build.h instead of build.h

* END BACKPORT bitcoin#11651 Remove trailing whitespace causing travis failure

* fix backport 11651

Signed-off-by: Pasta <pasta@dashboost.org>

* More of 11651

* fix blockchain.cpp

Signed-off-by: pasta <pasta@dashboost.org>

* Add missing "qt/" in includes

* Add missing "test/" in includes

* Fix trailing whitespaces

Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: MeshCollider <dobsonsa68@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 30, 2021
aac6b3f Update files.md for new wallets/ subdirectory (MeshCollider)
b673429 Cleanups for walletdir PR (MeshCollider)

Pull request description:

  This addresses the remaining nits from bitcoin#11466

  - Updates `doc/files.md` with respect to the new default wallet directory
  - Fixes @promag and @laanwj's error message nit, and Jonas' release notes nit
  - ~Addresses @laanwj's net-specific wallet subdirectory concern in the case that a walletdir is specified~
  - Changes the #includes from "" to <> style after bitcoin#11651

Tree-SHA512: b86bf5fdc4de54c1b0f65b60a83af3cf82b35d216ce9c0de724803bfba6934796238b6c412659dcc29ae2e3e856d4eb97ae777c80f36f4089d8acecfddefe9aa
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants