Skip to content

Commit 3d2ce2b

Browse files
authored
chore(buildtool): add ios-build unit tests (#1371)
We needed to move the runtimex.Assert for darwin, otherwise we cannot run unit tests on linux 😅. While there, be consistent and make sure we avoid panicking on Windows if one runs unit tests there. Part of ooni/probe#2564
1 parent 63edcbe commit 3d2ce2b

File tree

4 files changed

+946
-20
lines changed

4 files changed

+946
-20
lines changed

internal/cmd/buildtool/android.go

+18-14
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ func androidSubcommand() *cobra.Command {
3131
Use: "gomobile",
3232
Short: "Builds oonimkall for android using gomobile",
3333
Run: func(cmd *cobra.Command, args []string) {
34+
// Implementation note: perform the check here such that we can
35+
// run unit test for the building code from any system
36+
runtimex.Assert(
37+
runtime.GOOS == "darwin" || runtime.GOOS == "linux",
38+
"this command requires darwin or linux",
39+
)
3440
androidBuildGomobile(&buildDeps{})
3541
},
3642
})
@@ -39,6 +45,12 @@ func androidSubcommand() *cobra.Command {
3945
Use: "cli",
4046
Short: "Builds ooniprobe and miniooni for usage within termux",
4147
Run: func(cmd *cobra.Command, args []string) {
48+
// Implementation note: perform the check here such that we can
49+
// run unit test for the building code from any system
50+
runtimex.Assert(
51+
runtime.GOOS == "darwin" || runtime.GOOS == "linux",
52+
"this command requires darwin or linux",
53+
)
4254
androidBuildCLIAll(&buildDeps{})
4355
},
4456
})
@@ -48,6 +60,12 @@ func androidSubcommand() *cobra.Command {
4860
Short: "Cross compiles C dependencies for Android",
4961
Run: func(cmd *cobra.Command, args []string) {
5062
for _, arg := range args {
63+
// Implementation note: perform the check here such that we can
64+
// run unit test for the building code from any system
65+
runtimex.Assert(
66+
runtime.GOOS == "darwin" || runtime.GOOS == "linux",
67+
"this command requires darwin or linux",
68+
)
5169
androidCdepsBuildMain(arg, &buildDeps{})
5270
}
5371
},
@@ -59,11 +77,6 @@ func androidSubcommand() *cobra.Command {
5977

6078
// androidBuildGomobile invokes the gomobile build.
6179
func androidBuildGomobile(deps buildtoolmodel.Dependencies) {
62-
runtimex.Assert(
63-
runtime.GOOS == "darwin" || runtime.GOOS == "linux",
64-
"this command requires darwin or linux",
65-
)
66-
6780
deps.PsiphonMaybeCopyConfigFiles()
6881
deps.GolangCheck()
6982

@@ -132,11 +145,6 @@ func androidNDKCheck(androidHome string) string {
132145

133146
// androidBuildCLIAll builds all products in CLI mode for Android
134147
func androidBuildCLIAll(deps buildtoolmodel.Dependencies) {
135-
runtimex.Assert(
136-
runtime.GOOS == "darwin" || runtime.GOOS == "linux",
137-
"this command requires darwin or linux",
138-
)
139-
140148
deps.PsiphonMaybeCopyConfigFiles()
141149
deps.GolangCheck()
142150

@@ -379,10 +387,6 @@ func androidNDKBinPath(ndkDir string) string {
379387

380388
// androidCdepsBuildMain builds C dependencies for android.
381389
func androidCdepsBuildMain(name string, deps buildtoolmodel.Dependencies) {
382-
runtimex.Assert(
383-
runtime.GOOS == "darwin" || runtime.GOOS == "linux",
384-
"this command requires darwin or linux",
385-
)
386390
androidHome := deps.AndroidSDKCheck()
387391
ndkDir := deps.AndroidNDKCheck(androidHome)
388392
archs := []string{"arm", "arm64", "386", "amd64"}

internal/cmd/buildtool/internal/buildtooltest/buildtooltest.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func CheckSingleCommand(cmd *execabs.Cmd, tee ExecExpectations) error {
8585
return err
8686
}
8787
if err := CompareEnv(tee.Env, shellxtesting.CmdEnvironMinusOsEnviron(cmd)); err != nil {
88-
return err
88+
return fmt.Errorf("in %v: %w", tee.Argv, err)
8989
}
9090
return nil
9191
}
@@ -253,12 +253,12 @@ func (*DependenciesCallCounter) XCRun(args ...string) string {
253253
case "-sdk":
254254
runtimex.Assert(len(args) == 3, "expected three arguments")
255255
runtimex.Assert(args[2] == "--show-sdk-path", "the third argument must be --show-sdk-path")
256-
return filepath.Join("Developer", "SDKs", args[1])
256+
return string(filepath.Separator) + filepath.Join("Developer", "SDKs", args[1])
257257

258258
case "-find":
259259
runtimex.Assert(len(args) == 4, "expected four arguments")
260260
runtimex.Assert(args[1] == "-sdk", "the second argument must be -sdk")
261-
return filepath.Join("Developer", "SDKs", args[2], "bin", args[3])
261+
return string(filepath.Separator) + filepath.Join("Developer", "SDKs", args[2], "bin", args[3])
262262

263263
default:
264264
panic(errors.New("the first argument must be -sdk or -find"))

internal/cmd/buildtool/ios.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ func iosSubcommand() *cobra.Command {
3737
Use: "cdeps [zlib|openssl|libevent|tor...]",
3838
Short: "Cross compiles C dependencies for iOS",
3939
Run: func(cmd *cobra.Command, args []string) {
40+
// Implementation note: perform the check here such that we can
41+
// run unit test for the building code from any system
42+
runtimex.Assert(runtime.GOOS == "darwin", "this command requires darwin")
4043
for _, arg := range args {
4144
iosCdepsBuildMain(arg, &buildDeps{})
4245
}
@@ -66,8 +69,6 @@ func iosBuildGomobile(deps buildtoolmodel.Dependencies) {
6669

6770
// iosCdepsBuildMain builds C dependencies for ios.
6871
func iosCdepsBuildMain(name string, deps buildtoolmodel.Dependencies) {
69-
runtimex.Assert(runtime.GOOS == "darwin", "this command requires darwin")
70-
7172
// The ooni/probe-ios app explicitly only targets amd64 and arm64. It also targets
7273
// as the minimum version iOS 12, while one cannot target a version of iOS > 10 when
7374
// building for 32-bit targets. Hence, using only 64 bit archs here is fine.
@@ -150,9 +151,9 @@ func iosNewCBuildEnv(deps buildtoolmodel.Dependencies, ooniArch string) *cBuildE
150151
CFLAGS: []string{
151152
"-isysroot", isysroot,
152153
minVersionFlag + iosMinVersion, // tricky: they must be concatenated
153-
"-O2",
154154
"-arch", appleArch,
155155
"-fembed-bitcode",
156+
"-O2",
156157
},
157158
CONFIGURE_HOST: "", // later
158159
DESTDIR: destdir,

0 commit comments

Comments
 (0)