when cross-compiling, lzma and zstd will be detected on build system
When cross-compiling, for example for Android, ./configure will detect lzma and zstd setups from the build system, when it should be ignoring them. It should only look in the NDK and --prefix values.
$ sudo apt install liblzma-dev
$ export host_triplet=armv7a-linux-androideabi
$ export EXTERNAL_ROOT=`pwd`/external
$ export platform=21
$ export CC=${host_triplet}${platform}-clang
$ export PATH=$ANDROID_NDK_HOME/toolchains/llvm/prebuilt/linux-x86_64/bin:$PATH
$ ./configure --host="$host_triplet" --enable-android --enable-static-libevent --with-libevent-dir=$EXTERNAL_ROOT --enable-static-openssl --with-openssl-dir=$EXTERNAL_ROOT --prefix=$EXTERNAL_ROOT
[snip]
CC src/lib/evloop/compat_libevent.o
src/lib/compress/compress_lzma.c:24:18: fatal error: lzma.h: No such file or directory
#include <lzma.h>
^
compilation terminated.
The workaround is to add --disable-lzma --disable-zstd.
Activity
changed milestone to %Tor: 0.4.2.x-final
Trac:
Child Ticket(s): #32298 (moved)
That reminds me, I saw an issue when running all these Android builds where
./configure --enable-lzmawould fail to include without stopping with an error when running the build without pkg-config. I don't remember the exact details, unfortuately../configurewould print out an error, but it would continue with the whole build, and the resultingtorbinary did not have lzma.Replying to nickm:
(These are libraries that we detect with pkg-config; an appropriate solution would probably depend on how pkg-config expects people to handle this.)
You can tell pkg-config to look in your cross-compile root using $PKG_CONFIG_PATH, $PKG_CONFIG_LIBDIR, and $PKG_CONFIG_SYSROOT_DIR: https://linux.die.net/man/1/pkg-config
But I'm not even sure how you got to this stage.
lzma is not enabled by default: https://gitweb.torproject.org/tor.git/tree/configure.ac#n1077
So how did it get enabled with that configure command line? Did you edit configure, or change env vars?
Replying to eighthave:
That reminds me, I saw an issue when running all these Android builds where
./configure --enable-lzmawould fail to include without stopping with an error when running the build without pkg-config. I don't remember the exact details, unfortuately../configurewould print out an error, but it would continue with the whole build, and the resultingtorbinary did not have lzma.You reported that issue here: https://trac.torproject.org/projects/tor/ticket/31922#comment:description
It is fixed in 0.4.2 and later, by logging a detailed warning message when --enable-lzma fails: https://gitweb.torproject.org/tor.git/tree/configure.ac#n1096
We couldn't change it to an error, because that would break existing build scripts.
-
On a plain Debian/buster system, running the commands in the issue report should reproduce this. None of the
PKG_CONFIG_*env vars are set. Seems to me that if./configuredepends onpkg-config, it should set those env vars when cross-compiling. Or it would probably be enough to use the value set by--prefix=. That's in effect what we are doing in our complete Android builds:https://gitlab.com/guardianproject/tor-android/blob/c7953925/external/Makefile#L255
It would not be too much work for me to setup a cross-compiling test CI job in gitlab-ci if you would use it. I don't really know how to do that in travis, since you can start with a plain base image, AFAIK.
-
from https://github.com/guardianproject/tor-android
tor-android $ ls -1 external/lib/pkgconfig/ libcrypto.pc libevent_core.pc libevent_extra.pc libevent_openssl.pc libevent.pc libevent_pthreads.pc liblzma.pc libssl.pc libzstd.pc openssl.pc Replying to eighthave:
It would not be too much work for me to setup a cross-compiling test CI job in gitlab-ci if you would use it. I don't really know how to do that in travis, since you can start with a plain base image, AFAIK.
As far as I know, the network team monitors Jenkins, Travis, and Appveyor CI. That's enough for now. We might move to GitLab after the ticket tracker transition. Or a few people might be using GitLab for their own builds.
If you want to contribute GitLab configs, that's fine, but you should also find someone to run and monitor them.
-
PKG_CONFIG_SYSROOT_DIR doesn't seem right because it includes the prefix e.g.
/usr. Here's the snipped from the docs:"Modify -I and -L to use the directories located in target sysroot. this option is usefull when crosscompiling package that use pkg-config to determine CFLAGS anf LDFLAGS. -I and -L are modified to point to the new system root. this means that a -I/usr/include/libfoo will become -I/var/target/usr/include/libfoo with a PKG_CONFIG_SYSROOT_DIR equal to /var/target (same rule apply to -L)"
export PKG_CONFIG_PATH=$EXTERNAL_ROOT/lib/pkgconfighas been working in the tor-android project.About gitlab-ci, I have the
.gitlab-ci.ymlupdates already done. And they don't have to be monitored by the network team to be useful. They'll take affect on anyone who has a tor fork in a gitlab instance. That said, gitlab-ci can be set up like Travis-CI and Appveyor in GitHub pull requests. Replying to eighthave:
PKG_CONFIG_SYSROOT_DIR doesn't seem right because it includes the prefix e.g.
/usr. Here's the snipped from the docs:"Modify -I and -L to use the directories located in target sysroot. this option is usefull when crosscompiling package that use pkg-config to determine CFLAGS anf LDFLAGS. -I and -L are modified to point to the new system root. this means that a -I/usr/include/libfoo will become -I/var/target/usr/include/libfoo with a PKG_CONFIG_SYSROOT_DIR equal to /var/target (same rule apply to -L)"
export PKG_CONFIG_PATH=$EXTERNAL_ROOT/lib/pkgconfighas been working in the tor-android project.Ok, let's work out how to set PKG_CONFIG_PATH in configure?
-
I think it makes sense to require setting
--prefixfor cross-compiling, setPKG_CONFIG_PATH=$prefix/lib/pkgconfigwhenever cross-compiling. I've taken that approach now with tor-android, then each library is installed usingmake install: https://github.com/guardianproject/tor-android/blob/c795392511e5d2a21be14f6096f2c17a6e75f989/external/Makefile#L171I'm guessing the MinGW/Windows builds us a cross-compile too, I'm not aware of Tor's Windows build setup, so I can't speak to that, though I have maintained autotools build systems for Windows before.
Sounds good.
Let's think carefully about how to avoid breaking existing build setups.
Here's one possible design:
- If prefix is set, and PKG_CONFIG_PATH is not set, use prefix to derive a value for PKG_CONFIG_PATH That should minimise breakage.
Normally, we'd add the new path to the front of the list of paths, but that could cause subtle bugs. (Like enabling the wrong lzma.)
I also wonder if Windows and some other cross-compiles want to use PKG_CONFIG_SYSROOT_DIR instead. We'll need to check what Tor Browser is doing.
-
Now that I think about this, I think this could break some setups on UNIX since those might rely on the pkg-config PATH having multiple paths in it, and when cross-compiling, there should only be --prefix. It needs to be set only when cross-compiling. So I think this needs to set based on these conditions
- if --host is set for cross-compiling
- if --prefix is set
- if PKG_CONFIG_PATH is not set
-
PR: https://github.com/torproject/tor/pull/1481 branch:
pkg-config-prefixTrac:
Status: needs_revision to needs_review -
Can you provide an example of where that check is wrong? libevent is a hard requirement for building, and the idea here is that this is testing that the user ran make install into prefix for libevent. In that case, libevent.pc will be present. This error points out incomplete or missing installs, and is only enabled in very specific cases than can be overridden. This check is more isolated than
enable_tool_name_check, and that check is totally broken on Android and iOS #32334 (moved). -
I don't think it makes sense for me to contribute any more patches to the tor build system, my experience and point of view seems to only conflict with the opinions here. It seems that the tor devs have a very different conception of what ./configure should provide that is specific to tor. In my contributions to gnupg, openssl, ffmpeg, puredata, and others, I've built an understanding that ./configure is meant to provide a clean build interface for consumers first and foremost. Providing useful errors is an essential part of that. I've never worked on a build system that put all the possible customization options first, before the simple build.
Trac:
Status: needs_revision to new -
We're going to maintain the Android and iOS build systems in this fork:
Please take anything from there as you see fit, under the license you require.
I made the requested changes, and cherry-picked to 0.4.2. Here are the PRs:
Test branches:
Nick, do you think we should backport this build fix to 0.4.2?
Trac:
Owner: N/A to teor
Type: defect to enhancement
Keywords: Android 042-backport deleted, Android, 042-backport added
Cc: n8fr8, gk, sisbell to n8fr8, gk, sisbell, nickm
Status: new to assigned
Reviewer: teor to nickmReplying to nickm:
Nick, do you think we should backport this build fix to 0.4.2?
Hm. Can we envision this breaking any reasonable build that would previously have succeeded? If not, I think a backport is reasonable.
It only sets PKG_CONFIG_PATH when it's unset. So it does override the default path compiled in to the pkg-config binary, but that path is wrong for cross-compiles. It never overrides user-configured paths.
Trac:
Status: assigned to needs_reviewmentioned in issue #32298 (moved)
mentioned in issue #32298 (moved)
moved to tpo/core/tor#32191 (closed)
mentioned in issue tpo/core/tor#32298 (closed)
