Building Environment: Set ARFLAGS to cr #10766
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
Contributor
|
Concept ACK Nit: I don't think that |
Member
|
Concept ACK. Though, please don't hard-code it to "cr" in case the change causes trouble for some toolchain. Let's use AC_ARG_VAR instead, so that it may be overridden if necessary: AC_ARG_VAR(ARFLAGS, [flags passed to the archiver])
if test "x${ARFLAGS+set}" != "xset"; then
ARFLAGS="cr"
fi |
Contributor
Author
|
I agree / have created a hopefully improved pull request: 10782 |
Member
|
Thanks. utACK after squash. |
Contributor
|
utACK bb4bb93 |
Contributor
Author
|
I've squashed to commits / Hope the user is now recognized. |
TheBlueMatt
reviewed
Jul 14, 2017
Contributor
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Concept ACK, though I tested and overriding doesnt work.
configure.ac
Outdated
|
|
||
| dnl Unless the user specified ARFLAGS, force it to be cr | ||
| AC_ARG_VAR(ARFLAGS, [Flags for the archiver, defaults to <cr> if not set]) | ||
| if test "x${ARFLAGS}" != "xset"; then |
Contributor
There was a problem hiding this comment.
Do you need the x${ARFLAGS+set} syntax that we use elsewhere?
| dnl Unless the user specified ARFLAGS, force it to be cr | ||
| AC_ARG_VAR(ARFLAGS, [Flags for the archiver, defaults to <cr> if not set]) | ||
| if test "x${ARFLAGS}" != "xset"; then | ||
| ARFLAGS="cr" |
Contributor
There was a problem hiding this comment.
Maybe add D to the default here, so that we're consistent even on platforms with a strange ar build?
Contributor
Author
|
overriding didn't work.... because I goofed up when I manually tried to merge two branches: I had With the fix, I've also added the |
Contributor
Author
|
Apparently, the |
Contributor
|
Heh, ok, just leave it as cr I suppose. |
ReneNyffenegger
force-pushed
the
ARFLAGS
branch
2 times, most recently
from
824ae38 to
152d488
Compare
Contributor
Author
|
I have removed the D flag again. Now, I have also commit 816aa59. I don't know why this is, if I should remove it, and how I could remove it. |
Contributor
|
@ReneNyffenegger Please do: git rebase -i HEAD~2
# remove the line with "Fix multi_rpc test …"
git push -f |
Contributor
Author
|
done rebase. |
Contributor
|
utACK 517bfb174a5affc426d8eeb008cb42c49b8f59a9 |
The user can set ARFLAGS in the ./configure step with ./configure ARFLAGS=... If he chooses not to do so, ARFLAGS will be set to cr.
Contributor
|
ACK 912da1d. We'll need to do something about similar warnings in leveldb and secp256k1, but those will need to be submitted separately upstream. |
Member
|
utACK 912da1d |
Contributor
|
utACK 912da1d |
sipa
added a commit
that referenced
this pull request
Jul 16, 2017
912da1d Use AC_ARG_VAR to set ARFLAGS. (René Nyffenegger) Pull request description: Override the default of ARFLAGS of `cru` to `cr`. When building, ar produces a warning for each archive, for example ``` AR libbitcoin_server.a /usr/bin/ar: `u' modifier ignored since `D' is the default (see `U') ``` Since `u` is the default anyway, it cannot hurt to remove it. Tree-SHA512: 7466764f847b70f0f67db25dac87a7794477abf1997cb946682f394fe80ae86ac3ed52cbadb35f0c18a87467755bde5a5158430444cd26fb60fa363cc7bd486d
zkbot
added a commit
to zcash/zcash
that referenced
this pull request
Dec 1, 2017
Build system improvements Includes commits cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6978 - Only the first commit (second is for QT) - bitcoin/bitcoin#7059 - bitcoin/bitcoin#7603 - Only the first commit (the rest are for QT) - bitcoin/bitcoin#7954 - bitcoin/bitcoin#8314 - Only the second commit (first is for QT) - bitcoin/bitcoin#8504 - Only the first commit (second was undoing something we didn't have) - bitcoin/bitcoin#8520 - bitcoin/bitcoin#8563 - bitcoin/bitcoin#8249 - bitcoin/bitcoin#9156 - bitcoin/bitcoin#9831 - bitcoin/bitcoin#9789 - bitcoin/bitcoin#10766 Part of #2074.
zkbot
added a commit
to zcash/zcash
that referenced
this pull request
Dec 15, 2017
Build system improvements Includes commits cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6978 - Only the first commit (second is for QT) - bitcoin/bitcoin#7059 - bitcoin/bitcoin#7603 - Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT) - bitcoin/bitcoin#7954 - bitcoin/bitcoin#8314 - Only the second commit (first is for QT) - bitcoin/bitcoin#8504 - Only the first commit (second was undoing something we didn't have) - bitcoin/bitcoin#8520 - bitcoin/bitcoin#8563 - bitcoin/bitcoin#8249 - bitcoin/bitcoin#9156 - bitcoin/bitcoin#9831 - bitcoin/bitcoin#9789 - bitcoin/bitcoin#10766 Part of #2074.
kotodev
pushed a commit
to koto-dev/koto.old
that referenced
this pull request
Jan 25, 2018
Build system improvements Includes commits cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6978 - Only the first commit (second is for QT) - bitcoin/bitcoin#7059 - bitcoin/bitcoin#7603 - Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT) - bitcoin/bitcoin#7954 - bitcoin/bitcoin#8314 - Only the second commit (first is for QT) - bitcoin/bitcoin#8504 - Only the first commit (second was undoing something we didn't have) - bitcoin/bitcoin#8520 - bitcoin/bitcoin#8563 - bitcoin/bitcoin#8249 - bitcoin/bitcoin#9156 - bitcoin/bitcoin#9831 - bitcoin/bitcoin#9789 - bitcoin/bitcoin#10766 Part of #2074.
renium9
added a commit
to koto-dev/koto.old
that referenced
this pull request
Feb 6, 2018
Build system improvements Includes commits cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6978 - Only the first commit (second is for QT) - bitcoin/bitcoin#7059 - bitcoin/bitcoin#7603 - Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT) - bitcoin/bitcoin#7954 - bitcoin/bitcoin#8314 - Only the second commit (first is for QT) - bitcoin/bitcoin#8504 - Only the first commit (second was undoing something we didn't have) - bitcoin/bitcoin#8520 - bitcoin/bitcoin#8563 - bitcoin/bitcoin#8249 - bitcoin/bitcoin#9156 - bitcoin/bitcoin#9831 - bitcoin/bitcoin#9789 - bitcoin/bitcoin#10766 Part of #2074. # Conflicts: # configure.ac # src/Makefile.am # src/Makefile.gtest.include # src/Makefile.test.include # zcutil/build.sh
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jul 18, 2019
912da1d Use AC_ARG_VAR to set ARFLAGS. (René Nyffenegger) Pull request description: Override the default of ARFLAGS of `cru` to `cr`. When building, ar produces a warning for each archive, for example ``` AR libbitcoin_server.a /usr/bin/ar: `u' modifier ignored since `D' is the default (see `U') ``` Since `u` is the default anyway, it cannot hurt to remove it. Tree-SHA512: 7466764f847b70f0f67db25dac87a7794477abf1997cb946682f394fe80ae86ac3ed52cbadb35f0c18a87467755bde5a5158430444cd26fb60fa363cc7bd486d
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
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.
Override the default of ARFLAGS of
crutocr.When building, ar produces a warning for each archive, for example
Since
uis the default anyway, it cannot hurt to remove it.