Skip to content

Conversation

@randy-waterhouse
Copy link
Contributor

Added warnings and verbosity for autoreconf, should be useful for debugging.
AC_CONFIG_SRCDIR argument should actually be a unique file in the src dir, not the dir itself.
(http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Input.html)
Also moved two AC_CONFIG calls to a more readable (usual) location.

@laanwj
Copy link
Member

laanwj commented Aug 26, 2014

Verbose used to be on, and was only recently disabled: 82ccb05. I don't changing things back and forth, so lets leave it like it is. People that need to debug can always enable verbose.

@randy-waterhouse
Copy link
Contributor Author

What's the practice with the warnings=all flag?
There are quite a lot of warnings that needs fixing btw.

@laanwj
Copy link
Member

laanwj commented Aug 26, 2014

Showing all warnings seems like a good precaution, unless they're really silly (I haven't tested).

@randy-waterhouse
Copy link
Contributor Author

Ok updated. No more verbosity. Warnings on, when they are fixed it will quieten back to standard output (not silly at the moment, imo.)

@theuni
Copy link
Member

theuni commented Aug 26, 2014

Mm. I agree with these changes, but they're going to lead to lots of devs scratching their heads and thinking "wtf broke?". See here: https://s3.amazonaws.com/archive.travis-ci.org/jobs/33572253/log.txt and grep for AC_TRY_COMPILE. That's nearly 100 lines of spew that is pretty incomprehensible to anyone but an autotools dev. And every warning but 1 is for deprecated (but working) functions.

I'd say either:

  • use --warnings=all,no-obsolete and push this in as-is, then fix the deprecations and remove no-obsolete as a next step or
  • fix the deprecated uses in the same PR that turns on the warnings

@theuni
Copy link
Member

theuni commented Aug 26, 2014

It seems autoupdate is able to correctly fix the deprecations for me:
autoupdate configure.ac src/m4/bitcoin_find_bdb48.m4 src/m4/bitcoin_qt.m4

If you'll add those changes here, ACK.

@randy-waterhouse
Copy link
Contributor Author

I've add the fixes for the warnings (which I intended to do in a subsequent PR anyway). Turning off warnings for bad code is not good practice, regardless of which devs are scratching their heads or otherwise, imho.

@randy-waterhouse randy-waterhouse force-pushed the config_fixes branch 3 times, most recently from 5d7ce8f to 732e06c Compare August 28, 2014 07:48
@randy-waterhouse randy-waterhouse force-pushed the config_fixes branch 7 times, most recently from e9e3d88 to 5d89bbc Compare September 3, 2014 19:32
@randy-waterhouse randy-waterhouse force-pushed the config_fixes branch 4 times, most recently from 8d7fdf4 to c4661a6 Compare September 11, 2014 10:25
@sipa
Copy link
Member

sipa commented Sep 12, 2014

Does this need to be rebased every day?

@laanwj
Copy link
Member

laanwj commented Sep 13, 2014

@theuni are you OK with this now?

@theuni
Copy link
Member

theuni commented Sep 13, 2014

Yes. It'll cause a new warning about the GZIP var, but that's fine. I think we can get rid of that now.

@randy-waterhouse
Copy link
Contributor Author

@sipa wasn't aware constant rebase would have any negative downstream implications? Seems like a good practice to keep up-to-date?

@sipa
Copy link
Member

sipa commented Sep 13, 2014

The only negative downsides are:

  • Prior code review may be invalidated (which seems not very much of an
    issue here).
  • It causes a mail in my mailbox every time (from pulltester) :)

In general, keeping code in sync with the main tree is a good practice
imho, but only if you're already changing the code, or it did get
unmergable.

@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4765_e2a98d270d511711111a54f350c25733bc15a5b2/ for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@laanwj laanwj merged commit e2a98d2 into bitcoin:master Sep 22, 2014
laanwj added a commit that referenced this pull request Sep 22, 2014
e2a98d2 Update obsolete AC macros. (randy-waterhouse)
e543811 Add warnings for autogen.sh. Fix AC_CONFIG_SRCDIR. (randy-waterhouse)
@randy-waterhouse randy-waterhouse deleted the config_fixes branch September 23, 2014 01:27
@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.

5 participants