Use curly braces for variable expansion in makefiles.

added component::core tor/tor owner::ln5 priority::low resolution::fixed status::closed tor-relay type::defect labels

Trac:
Status: new to accepted
Owner: N/A to ln5

Trac:
Status: accepted to needs_review

Trac:
Reporter: linus to ln5

Looks good to me. Thanks for finding this! Testing on OS X and a few different linux distributions didn't reveal any problems.

Looks great, except it's missing a changes file. E.g. a changes/build-on-netbsd which says o Minor bugfixes: - Resume building on NetBSD when --disable-asciidoc is chosen during ./configure.

Looks good to me too. The commit message is wonky, though: in Git, the first line of a commit message is supposed to be a short summary of the commit log, so that tools like "git shortlog" will work. Cleaned that up, added a 'changes' file, and merged.

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

Replacing parentheses with curly braces didn't really cut it.

What it takes is to change ':=' to '::='.
GNU Make seems happy enough with this but Automake gets annoyed by it and omits a warning for every occurrence in Makefile.am:

asciidoc_files:: non-POSIX variable name

which is a bit misleading since it's not the variable name it's upset about but rather the '::='.

Appending a patch. Let me know if we want this or if we rather should try to find some other way of doing this.

Trac:
Status: closed to reopened
Resolution: fixed to N/A

Trac:
a

Replace := with ::=.

To be clear, this isn't a gmake-only thing, is it? We try not to require gnu make.

If it does work with gmake and non-gnu makes, then I say "merge".

Trac:
Status: reopened to needs_review

It has nothing to do with GNU Make. One could argue that Automake is wrong here but I haven't looked into it. They seem to warn like this when you do non-portable things if I understand things correctly, which might be a hint that this is not a good idea. I have tested with GNU make and BSD make (netbsd) but only with a recent version of each.

Another option, as discussed with Sebastian on #tor-dev the other day, would be to set the makefile variables using this expansion thing within the 'if USE_ASCIIDOC' clause. A bit cludgy, admittedly.

I've pushed the s/:=/::=/ change to my makefile-var-exp in case you'd like to merge. I hope that branch is in proper shape, please tell if it's not. We don't need another changes file, do we?

Hm. With OSX 10.5 [GNU Make 3.81] and this patch, I get the following warnings:

Makefile:556: warning: overriding commands for target `tor'
Makefile:546: warning: ignoring old commands for target `tor'
Makefile:556: warning: overriding commands for target `tor-gencert'
Makefile:546: warning: ignoring old commands for target `tor-gencert'
Makefile:556: warning: overriding commands for target `tor-resolve'
Makefile:546: warning: ignoring old commands for target `tor-resolve'
Makefile:556: warning: overriding commands for target `torify'
Makefile:546: warning: ignoring old commands for target `torify'
Makefile:566: warning: overriding commands for target `tor'
Makefile:556: warning: ignoring old commands for target `tor'
Makefile:566: warning: overriding commands for target `tor-gencert'
Makefile:556: warning: ignoring old commands for target `tor-gencert'
Makefile:566: warning: overriding commands for target `tor-resolve'
Makefile:556: warning: ignoring old commands for target `tor-resolve'
Makefile:566: warning: overriding commands for target `torify'
Makefile:556: warning: ignoring old commands for target `torify'
Makefile:565: target `tor' given more than once in the same rule.
Makefile:565: target `tor-gencert' given more than once in the same rule.
Makefile:565: target `tor-resolve' given more than once in the same rule.
Makefile:565: target `torify' given more than once in the same rule.

Weird. I get nothing like that with the exact same GNU Make on a system reporting itself as

$ uname -v Darwin Kernel Version 9.8.0: Wed Jul 15 16:55:01 PDT 2009; root:xnu-1228.15.4~1/RELEASE_I386

I'm sure you did the usual autogen.sh and configure dance, right? I'll try to reproduce on OSX 10.5 as soon as I get hold of one of these.

Yup. When I do autogen.sh, I get {{{[48]% sh autogen.sh doc/Makefile.am:22: asciidoc_files:: non-POSIX variable name doc/Makefile.am:24: asciidoc_files:: non-POSIX variable name doc/Makefile.am:26: asciidoc_files:: non-POSIX variable name doc/Makefile.am:31: asciidoc_files:: non-POSIX variable name doc/Makefile.am:35: asciidoc_files:: non-POSIX variable name }}} then I configure, then "cd doc && make" gets me the warnings listed above.

My autoconf is 2.61; my automake is 1.10.

Right, I see it too. Sorry about that. So, this patch is obviously a real sad one. :-(

Let's forget about this and prepare something more cludgy. But also working.

Branch build-wo-asciidoc-on-bsd in my repo reverts the curly braces changes (since they didn't help anyway) and fixes the issue in a less elegant (but more working) way.

It's been tested with and without --disable-asciidoc on Linux and NetBSD, both make and make distcheck.

Please note that make distcheck is broken in master when --disable-asciidoc, see bug #1820 (moved).

Ah, one more use case you need to think about. We want it to be the case that if the distributor has asciidoc they can "make dist", and then the user who gets the tarball can make and install the manpages and documentation without actually having asciidoc themselves. If I'm reading this right, we lose that ability here. True?

Or I could be wrong. Maybe USE_ASCIIDOC is only false when the user has explicitly disabled asciidoc and the scenario I mentioned will still work. Hrmnmnm.

USE_ASCIIDOC should be false only on explicit demand by the user.

I've updated the changes/ file to reflect what's actually done.

I think build-wo-asciidoc-on-bsd is ready for merge.

Trac:
Status: needs_review to assigned

Squashed and merged. Thanks again!

Trac:
Resolution: N/A to fixed
Status: assigned to closed

Trac:
Keywords: N/A deleted, tor-relay added

Trac:
Component: Tor Relay to Tor

closed

moved to tpo/core/tor#1773 (closed)

Use curly braces for variable expansion in makefiles.

Child items 0

Activity