Skip to content

Commit

Permalink
refactor(make): better checks for tool existence and paths (ooni#327)
Browse files Browse the repository at this point in the history 
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 ooni/probe#1466.
  • Loading branch information
@bassosimone
bassosimone authored May 5, 2021
1 parent b2ab8d7 commit 4e1e165
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 64 deletions.
16 changes: 0 additions & 16 deletions MOBILE/go
View file

This file was deleted.

24 changes: 0 additions & 24 deletions MOBILE/gomobile
View file

This file was deleted.

50 changes: 26 additions & 24 deletions make
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)

Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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():
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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")
Expand All @@ -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)


Expand Down Expand Up @@ -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():
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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")
Expand All @@ -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)


Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)


Expand Down Expand Up @@ -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)


Expand Down

0 comments on commit 4e1e165

Please sign in to comment.