From 5738c07aff1d73c4a6bf55843867251d14a84261 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 5 May 2021 11:33:51 +0200 Subject: [PATCH] refactor(make): better checks for tool existence and paths (#327) After all the refactoring done so far, we can run checks directly inside of `make`, because we have auto-cleanup, temporary environments and we don't need wrapper scripts anymore. Part of https://github.com/ooni/probe/issues/1466. --- MOBILE/go | 16 ---------------- MOBILE/gomobile | 24 ------------------------ make | 50 +++++++++++++++++++++++++------------------------ 3 files changed, 26 insertions(+), 64 deletions(-) delete mode 100755 MOBILE/go delete mode 100755 MOBILE/gomobile diff --git a/MOBILE/go b/MOBILE/go deleted file mode 100755 index e4e05c1..0000000 --- a/MOBILE/go +++ /dev/null @@ -1,16 +0,0 @@ -#!/bin/sh - -# print the fullpath of the go we're using so that it's part -# of the build logs and we can easily increase our confidence -# that we are using the right go binary. - -echo -n "$0: checking for go... " -go=`command -v go` -if [ -z $go ]; then - echo "not found" - exit 1 -fi -echo "$go" - -set -x -$go "$@" diff --git a/MOBILE/gomobile b/MOBILE/gomobile deleted file mode 100755 index 05c3c8a..0000000 --- a/MOBILE/gomobile +++ /dev/null @@ -1,24 +0,0 @@ -#!/bin/sh - -# print the fullpath of the go/gomobile we're using so that it's part -# of the build logs and we can easily increase our confidence -# that we are using the right go/gomobile binary. - -echo -n "$0: checking for go... " -go=`command -v go` -if [ -z $go ]; then - echo "not found" - exit 1 -fi -echo "$go" - -echo -n "$0: checking for gomobile... " -gomobile=`command -v gomobile` -if [ -z $go ]; then - echo "not found" - exit 1 -fi -echo "$gomobile" - -set -x -$gomobile "$@" diff --git a/make b/make index d82a725..b9ff4a1 100755 --- a/make +++ b/make @@ -329,9 +329,13 @@ class CommandExecutor: def require(self, *executable: str) -> None: """require implements Engine.require.""" - self._dry_runner.require(*executable) - # Nothing else to do, because executable is fully - # implemented by CommandDryRunner. + for exc in executable: + self._dry_runner.require(exc) + fullpath = shutil.which(exc) + if not fullpath: + log("checking for {}... not found".format(exc)) + sys.exit(1) + log("checking for {}... {}".format(exc, fullpath)) def run( self, @@ -345,13 +349,13 @@ class CommandExecutor: def setenv(self, key: str, value: str) -> Optional[str]: """setenv implements Engine.setenv.""" - # Nothing else to do, because executable is fully + # Nothing else to do, because setenv is fully # implemented by CommandDryRunner. return self._dry_runner.setenv(key, value) def unsetenv(self, key: str) -> None: """unsetenv implements Engine.unsetenv.""" - # Nothing else to do, because executable is fully + # Nothing else to do, because unsetenv is fully # implemented by CommandDryRunner. self._dry_runner.unsetenv(key) @@ -362,9 +366,6 @@ class CommandDryRunner: # Implementation note: here we try to log valid bash snippets # such that is really obvious what we are doing. - def __init__(self): - self.__cmdcache: Dict[str, str] = {} - def backticks( self, output_variable: str, @@ -396,17 +397,9 @@ class CommandDryRunner: def require(self, *executable: str) -> None: """require implements Engine.require.""" - # implemented here because we want to see the results - # of checks when we're doing a dry run for exc in executable: - if exc in self.__cmdcache: - continue # do not print checks more than once - fullpath = shutil.which(exc) - if not fullpath: - log("./make: checking for {}... not found".format(exc)) - sys.exit(1) - log("./make: checking for {}... {}".format(exc, fullpath)) - self.__cmdcache[exc] = fullpath + log(f"./make: echo -n 'checking for {exc}... '") + log("./make: command -v %s || { echo 'not found'; exit 1 }" % exc) def run( self, @@ -710,7 +703,7 @@ class OONIMKAllAAR: # TODO(bassosimone): find a way to run this command without # adding extra dependencies to go.mod and go.sum. cmdline: List[str] = [] - cmdline.append(os.path.join(".", "MOBILE", "go")) + cmdline.append("go") cmdline.append("get") cmdline.append("-u") if options.verbose(): @@ -720,6 +713,7 @@ class OONIMKAllAAR: cmdline.append("golang.org/x/mobile/cmd/gomobile@latest") with Environ(engine, "GOPATH", gopath()): with AugmentedPath(engine, oonigo.binpath()): + engine.require("go") engine.run(cmdline) def _gomobile_init( @@ -729,12 +723,13 @@ class OONIMKAllAAR: android: SDKAndroid, ) -> None: cmdline: List[str] = [] - cmdline.append(os.path.join(".", "MOBILE", "gomobile")) + cmdline.append("gomobile") cmdline.append("init") with Environ(engine, "ANDROID_HOME", android.home()): with Environ(engine, "ANDROID_NDK_HOME", android.ndk_home()): with AugmentedPath(engine, oonigo.binpath()): with AugmentedPath(engine, os.path.join(gopath(), "bin")): + engine.require("gomobile", "go") engine.run(cmdline) def _gomobile_bind( @@ -745,7 +740,7 @@ class OONIMKAllAAR: android: SDKAndroid, ) -> None: cmdline: List[str] = [] - cmdline.append(os.path.join(".", "MOBILE", "gomobile")) + cmdline.append("gomobile") cmdline.append("bind") if options.verbose(): cmdline.append("-v") @@ -765,6 +760,7 @@ class OONIMKAllAAR: with Environ(engine, "ANDROID_NDK_HOME", android.ndk_home()): with AugmentedPath(engine, oonigo.binpath()): with AugmentedPath(engine, os.path.join(gopath(), "bin")): + engine.require("gomobile", "go") engine.run(cmdline) @@ -881,7 +877,7 @@ class OONIMKAllFramework: # TODO(bassosimone): find a way to run this command without # adding extra dependencies to go.mod and go.sum. cmdline: List[str] = [] - cmdline.append(os.path.join(".", "MOBILE", "go")) + cmdline.append("go") cmdline.append("get") cmdline.append("-u") if options.verbose(): @@ -891,6 +887,7 @@ class OONIMKAllFramework: cmdline.append("golang.org/x/mobile/cmd/gomobile@latest") with AugmentedPath(engine, gogo.binpath()): with Environ(engine, "GOPATH", gopath()): + engine.require("go") engine.run(cmdline) def _gomobile_init( @@ -899,10 +896,11 @@ class OONIMKAllFramework: gogo: SDKGolangGo, ) -> None: cmdline: List[str] = [] - cmdline.append(os.path.join(".", "MOBILE", "gomobile")) + cmdline.append("gomobile") cmdline.append("init") with AugmentedPath(engine, os.path.join(gopath(), "bin")): with AugmentedPath(engine, gogo.binpath()): + engine.require("gomobile", "go") engine.run(cmdline) def _gomobile_bind( @@ -912,7 +910,7 @@ class OONIMKAllFramework: gogo: SDKGolangGo, ) -> None: cmdline: List[str] = [] - cmdline.append(os.path.join(".", "MOBILE", "gomobile")) + cmdline.append("gomobile") cmdline.append("bind") if options.verbose(): cmdline.append("-v") @@ -930,6 +928,7 @@ class OONIMKAllFramework: cmdline.append("./pkg/oonimkall") with AugmentedPath(engine, os.path.join(gopath(), "bin")): with AugmentedPath(engine, gogo.binpath()): + engine.require("gomobile", "go") engine.run(cmdline) @@ -980,6 +979,7 @@ class OONIMKAllPodspec: if os.path.isfile(self.__name) and not options.dry_run(): log("./make: {}: already built".format(self.__name)) return + engine.require("cat", "sed") output = engine.backticks("RELEASE", ["git", "describe", "--tags"]) release = output.decode("utf-8").strip() version = datetime.datetime.now().strftime("%Y.%m.%d-%H%M%S") @@ -1041,6 +1041,7 @@ class MiniOONIDarwinOrWindows: with Environ(engine, "GOARCH", self.__arch): with Environ(engine, "CGO_ENABLED", "0"): with AugmentedPath(engine, gogo.binpath()): + engine.require("go") engine.run(cmdline) @@ -1089,6 +1090,7 @@ class MiniOONILinux: with Environ(engine, "GOARCH", self.__arch): with Environ(engine, "CGO_ENABLED", "0"): with AugmentedPath(engine, gogo.binpath()): + engine.require("go") engine.run(cmdline)