util: warn about ignored recursive -includeconf calls #13197
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I (personally) prefer if PRs are more or less independent. |
|
utACK 6f10309. BTW couldn't |
|
concept ACK (and quick skim utACK). Will review & test after #12755 is merged. |
|
#12755 is merged, please rebase |
Since -includeconf cannot be used recursively, the user would not see feedback that an -includeconf in an -includeconf'd file was silently ignored.
6f10309 to
2352aa9
Compare
|
Rebased. |
|
@promag Is it better locking the whole time? |
I don't know what the potential races here are, but If everything else is the same, keeping the lock closely around the section where it is needed is preferable. Also: calls such as |
|
@laanwj I mean, |
As far as I know we don't really make any assumptions in that regard right now, but in sane API design I think that makes sense. |
|
I can try moving the lock to the top of the method and remove it elsewhere. I am concerned it may cause deadlocks though. |
|
ba5b9fc |
|
@kallewoof I think it's great to change that but in a different PR. Commit 2352aa9 is enough here. Sorry for asking here! |
ba5b9fc to
2352aa9
Compare
|
Yeah doesn't seem as simple as I hoped. Dropping ba5b9fc. |
|
utACK 2352aa9 |
laanwj
added a commit
that referenced
this pull request
May 14, 2018
2352aa9 test: Ensure that recursive -includeconf produces appropriate warnings (Karl-Johan Alm) c5bcc7d util: warn about recursive -includeconf arguments in configuration files (Karl-Johan Alm) Pull request description: This is a follow-up PR to #10267, and addresses #10267 (comment). ~~I am adding extra work for @jnewbery in #12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.~~ Tree-SHA512: 87f0c32436b70424e33616ffb88d7cb699f90d6a583a10237e224b28fc936d6a9df95536c8c52ee8546b3942da92b2a357e61bf87e00d1462bc10d46d3bee352
jasonbcox
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Sep 27, 2019
Summary: 2352aa9 test: Ensure that recursive -includeconf produces appropriate warnings (Karl-Johan Alm) c5bcc7d util: warn about recursive -includeconf arguments in configuration files (Karl-Johan Alm) Pull request description: This is a follow-up PR to #10267, and addresses bitcoin/bitcoin#10267 (comment). ~~I am adding extra work for @jnewbery in #12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.~~ Tree-SHA512: 87f0c32436b70424e33616ffb88d7cb699f90d6a583a10237e224b28fc936d6a9df95536c8c52ee8546b3942da92b2a357e61bf87e00d1462bc10d46d3bee352 Backport of Core PR13197 bitcoin/bitcoin#13197 Test Plan: make check test_runner.py Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: deadalnix, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D4118
UdjinM6
pushed a commit
to UdjinM6/dash
that referenced
this pull request
Jun 18, 2021
…calls 2352aa9 test: Ensure that recursive -includeconf produces appropriate warnings (Karl-Johan Alm) c5bcc7d util: warn about recursive -includeconf arguments in configuration files (Karl-Johan Alm) Pull request description: This is a follow-up PR to bitcoin#10267, and addresses bitcoin#10267 (comment). ~~I am adding extra work for @jnewbery in bitcoin#12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.~~ Tree-SHA512: 87f0c32436b70424e33616ffb88d7cb699f90d6a583a10237e224b28fc936d6a9df95536c8c52ee8546b3942da92b2a357e61bf87e00d1462bc10d46d3bee352
UdjinM6
pushed a commit
to UdjinM6/dash
that referenced
this pull request
Jun 24, 2021
…calls 2352aa9 test: Ensure that recursive -includeconf produces appropriate warnings (Karl-Johan Alm) c5bcc7d util: warn about recursive -includeconf arguments in configuration files (Karl-Johan Alm) Pull request description: This is a follow-up PR to bitcoin#10267, and addresses bitcoin#10267 (comment). ~~I am adding extra work for @jnewbery in bitcoin#12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.~~ Tree-SHA512: 87f0c32436b70424e33616ffb88d7cb699f90d6a583a10237e224b28fc936d6a9df95536c8c52ee8546b3942da92b2a357e61bf87e00d1462bc10d46d3bee352
UdjinM6
pushed a commit
to UdjinM6/dash
that referenced
this pull request
Jun 26, 2021
…calls 2352aa9 test: Ensure that recursive -includeconf produces appropriate warnings (Karl-Johan Alm) c5bcc7d util: warn about recursive -includeconf arguments in configuration files (Karl-Johan Alm) Pull request description: This is a follow-up PR to bitcoin#10267, and addresses bitcoin#10267 (comment). ~~I am adding extra work for @jnewbery in bitcoin#12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.~~ Tree-SHA512: 87f0c32436b70424e33616ffb88d7cb699f90d6a583a10237e224b28fc936d6a9df95536c8c52ee8546b3942da92b2a357e61bf87e00d1462bc10d46d3bee352
UdjinM6
pushed a commit
to UdjinM6/dash
that referenced
this pull request
Jun 26, 2021
…calls 2352aa9 test: Ensure that recursive -includeconf produces appropriate warnings (Karl-Johan Alm) c5bcc7d util: warn about recursive -includeconf arguments in configuration files (Karl-Johan Alm) Pull request description: This is a follow-up PR to bitcoin#10267, and addresses bitcoin#10267 (comment). ~~I am adding extra work for @jnewbery in bitcoin#12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.~~ Tree-SHA512: 87f0c32436b70424e33616ffb88d7cb699f90d6a583a10237e224b28fc936d6a9df95536c8c52ee8546b3942da92b2a357e61bf87e00d1462bc10d46d3bee352
UdjinM6
pushed a commit
to UdjinM6/dash
that referenced
this pull request
Jun 28, 2021
…calls 2352aa9 test: Ensure that recursive -includeconf produces appropriate warnings (Karl-Johan Alm) c5bcc7d util: warn about recursive -includeconf arguments in configuration files (Karl-Johan Alm) Pull request description: This is a follow-up PR to bitcoin#10267, and addresses bitcoin#10267 (comment). ~~I am adding extra work for @jnewbery in bitcoin#12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.~~ Tree-SHA512: 87f0c32436b70424e33616ffb88d7cb699f90d6a583a10237e224b28fc936d6a9df95536c8c52ee8546b3942da92b2a357e61bf87e00d1462bc10d46d3bee352
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a follow-up PR to #10267, and addresses #10267 (comment).
I am adding extra work for @jnewbery in #12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.