diff --git a/internal/cmd/buildtool/android_test.go b/internal/cmd/buildtool/android_test.go index 7daa564dfb..30169927bf 100644 --- a/internal/cmd/buildtool/android_test.go +++ b/internal/cmd/buildtool/android_test.go @@ -754,12 +754,6 @@ func TestAndroidBuildCdepsOpenSSL(t *testing.T) { "DESTDIR=" + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm", "install_dev", }, - }, { - Env: []string{}, - Argv: []string{ - "rm", "-rf", - faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm/lib/pkgconfig", - }, }, { Env: []string{}, Argv: []string{ @@ -815,12 +809,6 @@ func TestAndroidBuildCdepsOpenSSL(t *testing.T) { "DESTDIR=" + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm64", "install_dev", }, - }, { - Env: []string{}, - Argv: []string{ - "rm", "-rf", - faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm64/lib/pkgconfig", - }, }, { Env: []string{}, Argv: []string{ @@ -876,12 +864,6 @@ func TestAndroidBuildCdepsOpenSSL(t *testing.T) { "DESTDIR=" + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/386", "install_dev", }, - }, { - Env: []string{}, - Argv: []string{ - "rm", "-rf", - faketopdir + "/internal/cmd/buildtool/internal/libtor/android/386/lib/pkgconfig", - }, }, { Env: []string{}, Argv: []string{ @@ -937,12 +919,6 @@ func TestAndroidBuildCdepsOpenSSL(t *testing.T) { "DESTDIR=" + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/amd64", "install_dev", }, - }, { - Env: []string{}, - Argv: []string{ - "rm", "-rf", - faketopdir + "/internal/cmd/buildtool/internal/libtor/android/amd64/lib/pkgconfig", - }, }}, }} @@ -1045,6 +1021,7 @@ func TestAndroidBuildCdepsLibevent(t *testing.T) { "CXXFLAGS=-fdata-sections -ffunction-sections -fstack-protector-strong -funwind-tables -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -fpic -mthumb -Oz -DANDROID", "-I"+faketopdir+"/internal/cmd/buildtool/internal/libtor/android/arm/include", ), + "PKG_CONFIG_PATH=" + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm/lib/pkgconfig", }, Argv: []string{ "./configure", @@ -1077,8 +1054,36 @@ func TestAndroidBuildCdepsLibevent(t *testing.T) { Env: []string{}, Argv: []string{ "rm", - "-rf", - faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm/lib/pkgconfig", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm/lib/pkgconfig/libevent.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm/lib/pkgconfig/libevent_core.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm/lib/pkgconfig/libevent_extra.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm/lib/pkgconfig/libevent_openssl.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm/lib/pkgconfig/libevent_pthreads.pc", }, }, { Env: []string{}, @@ -1195,6 +1200,7 @@ func TestAndroidBuildCdepsLibevent(t *testing.T) { "CXXFLAGS=-fdata-sections -ffunction-sections -fstack-protector-strong -funwind-tables -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -fpic -O2 -DANDROID", "-I"+faketopdir+"/internal/cmd/buildtool/internal/libtor/android/arm64/include", ), + "PKG_CONFIG_PATH=" + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm64/lib/pkgconfig", }, Argv: []string{ "./configure", @@ -1227,8 +1233,36 @@ func TestAndroidBuildCdepsLibevent(t *testing.T) { Env: []string{}, Argv: []string{ "rm", - "-rf", - faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm64/lib/pkgconfig", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm64/lib/pkgconfig/libevent.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm64/lib/pkgconfig/libevent_core.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm64/lib/pkgconfig/libevent_extra.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm64/lib/pkgconfig/libevent_openssl.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/arm64/lib/pkgconfig/libevent_pthreads.pc", }, }, { Env: []string{}, @@ -1345,6 +1379,7 @@ func TestAndroidBuildCdepsLibevent(t *testing.T) { "CXXFLAGS=-fdata-sections -ffunction-sections -fstack-protector-strong -funwind-tables -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -fPIC -O2 -DANDROID -mstackrealign", "-I"+faketopdir+"/internal/cmd/buildtool/internal/libtor/android/386/include", ), + "PKG_CONFIG_PATH=" + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/386/lib/pkgconfig", }, Argv: []string{ "./configure", @@ -1377,8 +1412,36 @@ func TestAndroidBuildCdepsLibevent(t *testing.T) { Env: []string{}, Argv: []string{ "rm", - "-rf", - faketopdir + "/internal/cmd/buildtool/internal/libtor/android/386/lib/pkgconfig", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/386/lib/pkgconfig/libevent.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/386/lib/pkgconfig/libevent_core.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/386/lib/pkgconfig/libevent_extra.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/386/lib/pkgconfig/libevent_openssl.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/386/lib/pkgconfig/libevent_pthreads.pc", }, }, { Env: []string{}, @@ -1495,6 +1558,7 @@ func TestAndroidBuildCdepsLibevent(t *testing.T) { "CXXFLAGS=-fdata-sections -ffunction-sections -fstack-protector-strong -funwind-tables -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -fPIC -O2 -DANDROID", "-I"+faketopdir+"/internal/cmd/buildtool/internal/libtor/android/amd64/include", ), + "PKG_CONFIG_PATH=" + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/amd64/lib/pkgconfig", }, Argv: []string{ "./configure", @@ -1527,8 +1591,36 @@ func TestAndroidBuildCdepsLibevent(t *testing.T) { Env: []string{}, Argv: []string{ "rm", - "-rf", - faketopdir + "/internal/cmd/buildtool/internal/libtor/android/amd64/lib/pkgconfig", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/amd64/lib/pkgconfig/libevent.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/amd64/lib/pkgconfig/libevent_core.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/amd64/lib/pkgconfig/libevent_extra.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/amd64/lib/pkgconfig/libevent_openssl.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/internal/cmd/buildtool/internal/libtor/android/amd64/lib/pkgconfig/libevent_pthreads.pc", }, }, { Env: []string{}, diff --git a/internal/cmd/buildtool/cdepslibevent.go b/internal/cmd/buildtool/cdepslibevent.go index f6b8504079..925fb4d4e6 100644 --- a/internal/cmd/buildtool/cdepslibevent.go +++ b/internal/cmd/buildtool/cdepslibevent.go @@ -49,8 +49,8 @@ func cdepsLibeventBuildMain(globalEnv *cBuildEnv, deps buildtoolmodel.Dependenci } envp := cBuildExportAutotools(cBuildMerge(globalEnv, localEnv)) - // On iOS, we need PKG_CONFIG_PATH to convince libevent to use the OpenSSL we built but] - // always letting libevent's configure be pkgconfig aware would probably be fine + // On iOS, we need PKG_CONFIG_PATH to convince libevent to use the OpenSSL we built and + // always letting libevent's configure use pkgconfig is actually fine. envp.Append("PKG_CONFIG_PATH", filepath.Join(globalEnv.DESTDIR, "lib", "pkgconfig")) argv := runtimex.Try1(shellx.NewArgv("./configure")) @@ -63,7 +63,17 @@ func cdepsLibeventBuildMain(globalEnv *cBuildEnv, deps buildtoolmodel.Dependenci must.Run(log.Log, "make", "V=1", "-j", strconv.Itoa(runtime.NumCPU())) must.Run(log.Log, "make", "DESTDIR="+globalEnv.DESTDIR, "install") must.Run(log.Log, "rm", "-rf", filepath.Join(globalEnv.DESTDIR, "bin")) - must.Run(log.Log, "rm", "-rf", filepath.Join(globalEnv.DESTDIR, "lib", "pkgconfig")) + + // We used to delete the whole pkgconfig directory but libevent's build needs OpenSSL's + // pkgconfig, so removing the whole directory means rebuilding libevent requires building + // OpenSSL because its pkgconfig is also removed. We discovered this need when working + // on the https://github.com/ooni/probe-cli/pull/1366 MVP. To keep the build idempotent, + // let's be more gentle and just remove libevent's pkgconfig files instead. + must.Run(log.Log, "rm", "-f", filepath.Join(globalEnv.DESTDIR, "lib", "pkgconfig", "libevent.pc")) + must.Run(log.Log, "rm", "-f", filepath.Join(globalEnv.DESTDIR, "lib", "pkgconfig", "libevent_core.pc")) + must.Run(log.Log, "rm", "-f", filepath.Join(globalEnv.DESTDIR, "lib", "pkgconfig", "libevent_extra.pc")) + must.Run(log.Log, "rm", "-f", filepath.Join(globalEnv.DESTDIR, "lib", "pkgconfig", "libevent_openssl.pc")) + must.Run(log.Log, "rm", "-f", filepath.Join(globalEnv.DESTDIR, "lib", "pkgconfig", "libevent_pthreads.pc")) // we just need libevent.a must.Run(log.Log, "rm", "-rf", filepath.Join(globalEnv.DESTDIR, "lib", "libevent.la")) diff --git a/internal/cmd/buildtool/cdepsopenssl.go b/internal/cmd/buildtool/cdepsopenssl.go index 9ad149860e..fba0eb6535 100644 --- a/internal/cmd/buildtool/cdepsopenssl.go +++ b/internal/cmd/buildtool/cdepsopenssl.go @@ -83,6 +83,7 @@ func cdepsOpenSSLBuildMain(globalEnv *cBuildEnv, deps buildtoolmodel.Dependencie must.Run(log.Log, "make", "DESTDIR="+globalEnv.DESTDIR, "install_dev") - // FIXME: we need to explain this change + // We used to delete the pkgconfig but it turns out this is important for libevent iOS builds, which + // means now we need to keep it. See https://github.com/ooni/probe-cli/pull/1369 for details. //must.Run(log.Log, "rm", "-rf", filepath.Join(globalEnv.DESTDIR, "lib", "pkgconfig")) } diff --git a/internal/cmd/buildtool/linuxcdeps_test.go b/internal/cmd/buildtool/linuxcdeps_test.go index 85ca172934..40d4b5817e 100644 --- a/internal/cmd/buildtool/linuxcdeps_test.go +++ b/internal/cmd/buildtool/linuxcdeps_test.go @@ -136,12 +136,6 @@ func TestLinuxCdepsBuildMain(t *testing.T) { "DESTDIR=" + faketopdir + "/" + sysDepDestDir, "install_dev", }, - }, { - Env: []string{}, - Argv: []string{ - "rm", "-rf", - faketopdir + "/" + sysDepDestDir + "/lib/pkgconfig", - }, }}, }, { name: "we can build libevent", @@ -195,6 +189,7 @@ func TestLinuxCdepsBuildMain(t *testing.T) { faketopdir, sysDepDestDir, ), + "PKG_CONFIG_PATH=" + faketopdir + "/" + sysDepDestDir + "/lib/pkgconfig", }, Argv: []string{ "./configure", @@ -226,8 +221,36 @@ func TestLinuxCdepsBuildMain(t *testing.T) { Env: []string{}, Argv: []string{ "rm", - "-rf", - faketopdir + "/" + sysDepDestDir + "/lib/pkgconfig", + "-f", + faketopdir + "/" + sysDepDestDir + "/lib/pkgconfig/libevent.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/" + sysDepDestDir + "/lib/pkgconfig/libevent_core.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/" + sysDepDestDir + "/lib/pkgconfig/libevent_extra.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/" + sysDepDestDir + "/lib/pkgconfig/libevent_openssl.pc", + }, + }, { + Env: []string{}, + Argv: []string{ + "rm", + "-f", + faketopdir + "/" + sysDepDestDir + "/lib/pkgconfig/libevent_pthreads.pc", }, }, { Env: []string{},