Opened 7 years ago

Closed 6 years ago

#6506 closed defect (implemented)

configure should bomb out early if asciidoc is enabled but the program is not present

Reported by: zwol Owned by:
Priority: Low Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Right now, if you don't have asciidoc installed and you forget to give configure the --disable-asciidoc option, you get an error at the very end of the build, which is annoying. It would be better if the configure script gave you the error instead.

Child Tickets

Attachments (2)

0001-Config-check-for-asciidoc-and-generated-manpages.patch (1.4 KB) - added by arlolra 6 years ago.
0002-Fix-out-of-tree-builds.patch (889 bytes) - added by arlolra 6 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 7 years ago by nickm

Milestone: Tor: 0.2.4.x-final

Agreed; a patch would be welcome here.

comment:2 Changed 7 years ago by Sebastian

This is hard. We used to have it that way, but then you needed asciidoc to build from the tarball. We changed it to the current behaviour where building from the tarball works by default unless you change the asciidoc source, and people who build from git need to install asciidoc or use --disable-asciidoc when building. If all the patch did was restore the previous behaviour I think that'd be bad.

comment:3 Changed 7 years ago by nickm

I think that what zwol is suggesting is that the current configure script check should happen earlier. Right now it checks late in the script, so you have to wait for everything else in the script to finish before you find out "oops, didn't say --disable-asciidoc".

comment:4 Changed 7 years ago by Sebastian

I'm not sure I understand. configure itself never says "oops, didn't say --disable-asciidoc", but rather make says that after the tor executable is built. Maybe I'm completely confused?

comment:5 Changed 7 years ago by zwol

Sebastian is correct, it is 'make' that complains, very late in the build.

To deal with the tarball situation, maybe we can arrange for the configure check to notice that generated manpages already exist. I'll see if I can find time to come up with a patch.

comment:6 Changed 7 years ago by Sebastian

That'd be neat. We didn't find a suitable solution.

comment:7 Changed 7 years ago by nickm

Keywords: tor-client added

comment:8 Changed 7 years ago by nickm

Component: Tor ClientTor

comment:9 Changed 6 years ago by nickm

Milestone: Tor: 0.2.4.x-finalTor: 0.2.5.x-final

comment:10 Changed 6 years ago by arlolra

Status: newneeds_review

I attached a patch to check for generated manpages as zwol described.

comment:11 Changed 6 years ago by nickm

Status: needs_reviewneeds_revision

I think this will break out-of-tree builds. (That is, you're supposed to be able to have the Tor source in directory X, and then cd into directory Y and say "X/configure ; make".)

Changed 6 years ago by arlolra

comment:12 Changed 6 years ago by arlolra

Status: needs_revisionneeds_review

You're "absolutely" right. Second patch builds on the first.

comment:13 Changed 6 years ago by nickm

Hm. One last issue. The reason that we didn't do this in the first place was that for most users without asciidoc, it's safe not to have asciidoc, so long as you have a copy of tor.1.in that's at least as new as tor.1.txt. This change is going to make a bunch of users whose compilation previously worked start to fail, and will hit users building from tarballs especially hard.

Can we do anything about that?

comment:14 Changed 6 years ago by arlolra

That's exactly what this patch checks.

If you forget to --disable-asciidoc (or you are a tarball user) it'll check if tor.1 exists. If so, it'll pass. Of course, if that file ends up being too old, it'll bomb out at the end. But chances are, it won't be.

Give it a try!

comment:15 Changed 6 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Oh hey, right you are.

I've added a few double-quotes, and a changes/ file, and merged it. Thanks!

Note: See TracTickets for help on using tickets.