Merge branch 'master' into cleanup-packaging-variables

Author: GeertJohan

cmd/build.go

@@ -28,17 +28,25 @@ var dotSlash = string([]byte{'.', filepath.Separator})
 
 var (
 	// common build flages (shared with `hover run`)
-	buildTarget          string
-	buildGoFlutterBranch string
-	buildCachePath       string
-	buildOpenGlVersion   string
-	buildEngineVersion   string
-	// TODO(GeertJohan): Separate the init code for the common build flags (run
-	// and build) into a function that's used for both buildCmd and runCmd. Also
-	// rename those to `compileFooBar`.
+	buildOrRunFlutterTarget   string
+	buildOrRunGoFlutterBranch string
+	buildOrRunCachePath       string
+	buildOrRunOpenGlVersion   string
+	buildOrRunEngineVersion   string
+	buildOrRunDocker          bool
+)
+
+func initCompileFlags(cmd *cobra.Command) {
+	cmd.PersistentFlags().StringVarP(&buildOrRunFlutterTarget, "target", "t", config.BuildTargetDefault, "The main entry-point file of the application.")
+	cmd.PersistentFlags().StringVarP(&buildOrRunGoFlutterBranch, "branch", "b", config.BuildBranchDefault, "The 'go-flutter' version to use. (@master or @v0.20.0 for example)")
+	cmd.PersistentFlags().StringVar(&buildOrRunCachePath, "cache-path", enginecache.DefaultCachePath(), "The path that hover uses to cache dependencies such as the Flutter engine .so/.dll")
+	cmd.PersistentFlags().StringVar(&buildOrRunOpenGlVersion, "opengl", config.BuildOpenGlVersionDefault, "The OpenGL version specified here is only relevant for external texture plugin (i.e. video_plugin).\nIf 'none' is provided, texture won't be supported. Note: the Flutter Engine still needs a OpenGL compatible context.")
+	cmd.PersistentFlags().StringVar(&buildOrRunEngineVersion, "engine-version", config.BuildEngineDefault, "The flutter engine version to use.")
+	cmd.PersistentFlags().BoolVar(&buildOrRunDocker, "docker", false, "Execute the go build and packaging in a docker container. The Flutter build is always run locally")
+}
 
+var (
 	// `hover build`-only build flags
-	buildDocker                 bool
 	buildDebug                  bool
 	buildVersionNumber          string
 	buildSkipEngineDownload     bool
@@ -51,14 +59,10 @@ const clangBinName = "o32-clang"
 var engineCachePath string
 
 func init() {
-	buildCmd.PersistentFlags().StringVarP(&buildTarget, "target", "t", config.BuildTargetDefault, "The main entry-point file of the application.")
-	buildCmd.PersistentFlags().StringVarP(&buildGoFlutterBranch, "branch", "b", config.BuildBranchDefault, "The 'go-flutter' version to use. (@master or @v0.20.0 for example)")
-	buildCmd.PersistentFlags().StringVar(&buildEngineVersion, "engine-version", config.BuildEngineDefault, "The flutter engine version to use.")
-	buildCmd.PersistentFlags().StringVar(&buildCachePath, "cache-path", enginecache.DefaultCachePath(), "The path that hover uses to cache dependencies such as the Flutter engine .so/.dll")
-	buildCmd.PersistentFlags().StringVar(&buildOpenGlVersion, "opengl", config.BuildOpenGlVersionDefault, "The OpenGL version specified here is only relevant for external texture plugin (i.e. video_plugin).\nIf 'none' is provided, texture won't be supported. Note: the Flutter Engine still needs a OpenGL compatible context.")
+	initCompileFlags(buildCmd)
+
 	buildCmd.PersistentFlags().StringVar(&buildVersionNumber, "version-number", "", "Override the version number used in build and packaging. You may use it with $(git describe --tags)")
 	buildCmd.PersistentFlags().BoolVar(&buildDebug, "debug", false, "Build a debug version of the app.")
-	buildCmd.PersistentFlags().BoolVar(&buildDocker, "docker", false, "Execute the go build and packaging in a docker container. The Flutter build is always run locally.")
 	buildCmd.PersistentFlags().BoolVar(&buildSkipEngineDownload, "skip-engine-download", false, "Skip donwloading the Flutter Engine and artifacts.")
 	buildCmd.PersistentFlags().BoolVar(&buildSkipFlutterBuildBundle, "skip-flutter-build-bundle", false, "Skip the 'flutter build bundle' step.")
 	buildCmd.AddCommand(buildLinuxCmd)
@@ -181,15 +185,15 @@ var buildWindowsMsiCmd = &cobra.Command{
 func subcommandBuild(targetOS string, packagingTask packaging.Task) {
 	assertHoverInitialized()
 	packagingTask.AssertInitialized()
-	if !buildDocker {
+	if !buildOrRunDocker {
 		packagingTask.AssertSupported()
 	}
 
 	if !buildSkipFlutterBuildBundle {
 		cleanBuildOutputsDir(targetOS)
 		buildFlutterBundle(targetOS)
 	}
-	if buildDocker {
+	if buildOrRunDocker {
 		var buildFlags []string
 		buildFlags = append(buildFlags, commonFlags()...)
 		buildFlags = append(buildFlags, "--skip-flutter-build-bundle")
@@ -211,18 +215,18 @@ func subcommandBuild(targetOS string, packagingTask packaging.Task) {
 // fallback values based on config or defaults for values that have not
 // explicitly been set through flags.
 func initBuildParameters(targetOS string) {
-	if buildGoFlutterBranch == config.BuildBranchDefault && config.GetConfig().Branch != "" {
-		buildGoFlutterBranch = config.GetConfig().Branch
+	if buildOrRunGoFlutterBranch == config.BuildBranchDefault && config.GetConfig().Branch != "" {
+		buildOrRunGoFlutterBranch = config.GetConfig().Branch
 	}
 
-	if buildCachePath == "" {
+	if buildOrRunCachePath == "" {
 		log.Errorf("Missing cache path, cannot continue. Please see previous warning.")
 		os.Exit(1)
 	}
 
-	if buildEngineVersion == config.BuildEngineDefault && config.GetConfig().Engine != "" {
+	if buildOrRunEngineVersion == config.BuildEngineDefault && config.GetConfig().Engine != "" {
 		log.Warnf("changing the engine version can lead to undesirable behavior")
-		buildEngineVersion = config.GetConfig().Engine
+		buildOrRunEngineVersion = config.GetConfig().Engine
 	}
 
 	// TODO: This override doesn't work properly when the config specifies a
@@ -231,32 +235,30 @@ func initBuildParameters(targetOS string) {
 	//
 	// The comment on (*FlagSet).Lookup(..).Changed isn't very clear, but we
 	// could test how that behaves and switch to it.
-	if buildOpenGlVersion == config.BuildOpenGlVersionDefault && config.GetConfig().OpenGL != "" {
-		buildOpenGlVersion = config.GetConfig().OpenGL
+	if buildOrRunOpenGlVersion == config.BuildOpenGlVersionDefault && config.GetConfig().OpenGL != "" {
+		buildOrRunOpenGlVersion = config.GetConfig().OpenGL
 	}
 
 	if buildVersionNumber == "" {
 		buildVersionNumber = pubspec.GetPubSpec().GetVersion()
 	}
 
-	// TODO(GeertJohan): This is complicated to read and can be simplified.
-	if buildSkipEngineDownload {
-		engineCachePath = enginecache.EngineCachePath(targetOS, buildCachePath)
-	} else {
-		engineCachePath = enginecache.ValidateOrUpdateEngineAtPath(targetOS, buildCachePath, buildEngineVersion)
+	engineCachePath = enginecache.EngineCachePath(targetOS, buildOrRunCachePath)
+	if !buildSkipEngineDownload {
+		enginecache.ValidateOrUpdateEngine(targetOS, buildOrRunCachePath, buildOrRunEngineVersion)
 	}
 }
 
 func commonFlags() []string {
 	f := []string{}
-	if buildTarget != config.BuildTargetDefault {
-		f = append(f, "--target", buildTarget)
+	if buildOrRunFlutterTarget != config.BuildTargetDefault {
+		f = append(f, "--target", buildOrRunFlutterTarget)
 	}
-	if buildGoFlutterBranch != config.BuildBranchDefault {
-		f = append(f, "--branch", buildGoFlutterBranch)
+	if buildOrRunGoFlutterBranch != config.BuildBranchDefault {
+		f = append(f, "--branch", buildOrRunGoFlutterBranch)
 	}
-	if buildOpenGlVersion != config.BuildOpenGlVersionDefault {
-		f = append(f, "--opengl", buildOpenGlVersion)
+	if buildOrRunOpenGlVersion != config.BuildOpenGlVersionDefault {
+		f = append(f, "--opengl", buildOrRunOpenGlVersion)
 	}
 	return f
 }
@@ -300,10 +302,10 @@ func cleanBuildOutputsDir(targetOS string) {
 }
 
 func buildFlutterBundle(targetOS string) {
-	if buildTarget == config.BuildTargetDefault && config.GetConfig().Target != "" {
-		buildTarget = config.GetConfig().Target
+	if buildOrRunFlutterTarget == config.BuildTargetDefault && config.GetConfig().Target != "" {
+		buildOrRunFlutterTarget = config.GetConfig().Target
 	}
-	assertTargetFileExists(buildTarget)
+	assertTargetFileExists(buildOrRunFlutterTarget)
 
 	runPluginGet, err := shouldRunPluginGet()
 	if err != nil {
@@ -323,7 +325,7 @@ func buildFlutterBundle(targetOS string) {
 	var flutterBuildBundleArgs = []string{
 		"build", "bundle",
 		"--asset-dir", filepath.Join(build.OutputDirectoryPath(targetOS), "flutter_assets"),
-		"--target", buildTarget,
+		"--target", buildOrRunFlutterTarget,
 	}
 	if buildDebug {
 		flutterBuildBundleArgs = append(flutterBuildBundleArgs, "--track-widget-creation")
@@ -378,7 +380,7 @@ func buildGoBinary(targetOS string, vmArguments []string) {
 		os.Exit(1)
 	}
 
-	if buildGoFlutterBranch == "" {
+	if buildOrRunGoFlutterBranch == "" {
 		currentTag, err := versioncheck.CurrentGoFlutterTag(filepath.Join(wd, build.BuildPath))
 		if err != nil {
 			log.Errorf("%v", err)
@@ -395,7 +397,7 @@ func buildGoBinary(targetOS string, vmArguments []string) {
 			log.Infof("Upgrading 'go-flutter' to the latest release")
 			// no buildBranch provided and currentTag isn't a release,
 			// force update. (same behaviour as previous version of hover).
-			err = upgradeGoFlutter(targetOS, engineCachePath)
+			err = upgradeGoFlutter(targetOS)
 			if err != nil {
 				// the upgrade can fail silently
 				log.Warnf("Upgrade ignored, current 'go-flutter' version: %s", currentTag)
@@ -407,18 +409,18 @@ func buildGoBinary(targetOS string, vmArguments []string) {
 		}
 
 	} else {
-		log.Printf("Downloading 'go-flutter' %s", buildGoFlutterBranch)
+		log.Printf("Downloading 'go-flutter' %s", buildOrRunGoFlutterBranch)
 
 		// when the buildBranch is set, fetch the go-flutter branch version.
-		err = upgradeGoFlutter(targetOS, engineCachePath)
+		err = upgradeGoFlutter(targetOS)
 		if err != nil {
 			os.Exit(1)
 		}
 	}
 
 	versioncheck.CheckForHoverUpdate(hoverVersion())
 
-	if buildOpenGlVersion == "none" {
+	if buildOrRunOpenGlVersion == "none" {
 		log.Warnf("The '--opengl=none' flag makes go-flutter incompatible with texture plugins!")
 	}
 
@@ -451,13 +453,16 @@ func buildGoBinary(targetOS string, vmArguments []string) {
 
 func buildEnv(targetOS string, engineCachePath string) []string {
 	var cgoLdflags string
+	var cgoCflags string
 
 	outputDirPath := filepath.Join("build", "outputs", targetOS)
 
 	switch targetOS {
 	case "darwin":
 		cgoLdflags = fmt.Sprintf("-F%s -Wl,-rpath,@executable_path", engineCachePath)
-		cgoLdflags = fmt.Sprintf("%s -F%s -L%s", cgoLdflags, outputDirPath, outputDirPath)
+		cgoLdflags += fmt.Sprintf(" -F%s -L%s", outputDirPath, outputDirPath)
+		cgoLdflags += " -mmacosx-version-min=10.10"
+		cgoCflags = "-mmacosx-version-min=10.10"
 	case "linux":
 		cgoLdflags = fmt.Sprintf("-L%s -L%s", engineCachePath, outputDirPath)
 	case "windows":
@@ -469,6 +474,7 @@ func buildEnv(targetOS string, engineCachePath string) []string {
 	env := []string{
 		"GO111MODULE=on",
 		"CGO_LDFLAGS=" + cgoLdflags,
+		"CGO_CFLAGS=" + cgoCflags,
 		"GOOS=" + targetOS,
 		"GOARCH=amd64",
 		"CGO_ENABLED=1",
@@ -527,7 +533,7 @@ func buildCommand(targetOS string, vmArguments []string, outputBinaryPath string
 	outputCommand := []string{
 		"go",
 		"build",
-		"-tags=opengl" + buildOpenGlVersion,
+		"-tags=opengl" + buildOrRunOpenGlVersion,
 		"-o", outputBinaryPath,
 		"-v",
 	}

cmd/bumpversion.go

@@ -16,7 +16,7 @@ import (
 )
 
 func init() {
-	upgradeCmd.Flags().StringVarP(&buildCachePath, "cache-path", "", enginecache.DefaultCachePath(), "The path that hover uses to cache dependencies such as the Flutter engine .so/.dll (defaults to the standard user cache directory)")
+	upgradeCmd.Flags().StringVarP(&buildOrRunCachePath, "cache-path", "", enginecache.DefaultCachePath(), "The path that hover uses to cache dependencies such as the Flutter engine .so/.dll (defaults to the standard user cache directory)")
 	upgradeCmd.Flags().MarkHidden("branch")
 	rootCmd.AddCommand(upgradeCmd)
 }
@@ -37,10 +37,12 @@ var upgradeCmd = &cobra.Command{
 }
 
 func upgrade(targetOS string) (err error) {
-	return upgradeGoFlutter(targetOS, enginecache.ValidateOrUpdateEngineAtPath(targetOS, buildCachePath, ""))
+	enginecache.ValidateOrUpdateEngine(targetOS, buildOrRunCachePath, "")
+	return upgradeGoFlutter(targetOS)
 }
 
-func upgradeGoFlutter(targetOS string, engineCachePath string) (err error) {
+func upgradeGoFlutter(targetOS string) (err error) {
+	engineCachePath := enginecache.EngineCachePath(targetOS, buildOrRunCachePath)
 	wd, err := os.Getwd()
 	if err != nil {
 		log.Errorf("Failed to get working dir: %v", err)
@@ -60,11 +62,11 @@ func upgradeGoFlutter(targetOS string, engineCachePath string) (err error) {
 		return
 	}
 
-	if buildGoFlutterBranch == "" {
-		buildGoFlutterBranch = "@latest"
+	if buildOrRunGoFlutterBranch == "" {
+		buildOrRunGoFlutterBranch = "@latest"
 	}
 
-	cmdGoGetU := exec.Command(build.GoBin(), "get", "-u", "github.com/go-flutter-desktop/go-flutter"+buildGoFlutterBranch)
+	cmdGoGetU := exec.Command(build.GoBin(), "get", "-u", "github.com/go-flutter-desktop/go-flutter"+buildOrRunGoFlutterBranch)
 	cmdGoGetU.Dir = filepath.Join(wd, build.BuildPath)
 	cmdGoGetU.Env = append(os.Environ(),
 		"GOPROXY=direct", // github.com/golang/go/issues/32955 (allows '/' in branch name)
@@ -77,7 +79,7 @@ func upgradeGoFlutter(targetOS string, engineCachePath string) (err error) {
 	err = cmdGoGetU.Run()
 	// When cross-compiling the command fails, but that is not an error
 	if err != nil {
-		log.Errorf("Updating go-flutter to %s version failed: %v", buildGoFlutterBranch, err)
+		log.Errorf("Updating go-flutter to %s version failed: %v", buildOrRunGoFlutterBranch, err)
 		return
 	}
 

cmd/docker.go

@@ -20,7 +20,7 @@ func dockerHoverBuild(targetOS string, packagingTask packaging.Task, buildFlags
 	var err error
 	dockerBin := build.DockerBin()
 
-	hoverCacheDir := filepath.Join(buildCachePath, "hover")
+	hoverCacheDir := filepath.Join(buildOrRunCachePath, "hover")
 
 	engineCacheDir := filepath.Join(hoverCacheDir, "engine")
 	err = os.MkdirAll(engineCacheDir, 0755)

cmd/run.go

@@ -15,7 +15,6 @@ import (
 	"github.com/go-flutter-desktop/hover/cmd/packaging"
 	"github.com/go-flutter-desktop/hover/internal/build"
 	"github.com/go-flutter-desktop/hover/internal/config"
-	"github.com/go-flutter-desktop/hover/internal/enginecache"
 	"github.com/go-flutter-desktop/hover/internal/log"
 	"github.com/go-flutter-desktop/hover/internal/pubspec"
 )
@@ -25,24 +24,15 @@ var (
 	runInitialRoute      string
 	runOmitEmbedder      bool
 	runOmitFlutterBundle bool
-	runDocker            bool
 )
 
 func init() {
-	runCmd.Flags().StringVarP(&buildTarget, "target", "t", config.BuildTargetDefault, "The main entry-point file of the application.")
-	runCmd.Flags().StringVarP(&buildGoFlutterBranch, "branch", "b", config.BuildBranchDefault, "The 'go-flutter' version to use. (@master or @v0.20.0 for example)")
-	// TODO: The variable buildCachePath is set twice, once during the setup of
-	// buildCmd, and once during the setup of runCmd. The last of the two will
-	// override the value with it's default, which leads to strange problems.
-	runCmd.Flags().StringVar(&buildCachePath, "cache-path", enginecache.DefaultCachePath(), "The path that hover uses to cache dependencies such as the Flutter engine .so/.dll (defaults to the standard user cache directory)")
-	runCmd.PersistentFlags().StringVar(&buildEngineVersion, "engine-version", "", "The flutter engine version to use.")
-	runCmd.Flags().StringVar(&buildOpenGlVersion, "opengl", config.BuildOpenGlVersionDefault, "The OpenGL version specified here is only relevant for external texture plugin (i.e. video_plugin).\nIf 'none' is provided, texture won't be supported. Note: the Flutter Engine still needs a OpenGL compatible context.")
+	initCompileFlags(runCmd)
 
 	runCmd.Flags().StringVar(&runInitialRoute, "route", "", "Which route to load when running the app.")
 	runCmd.Flags().StringVarP(&runObservatoryPort, "observatory-port", "", "50300", "The observatory port used to connect hover to VM services (hot-reload/debug/..)")
 	runCmd.Flags().BoolVar(&runOmitFlutterBundle, "omit-flutter", false, "Don't (re)compile the current Flutter project, useful when only working with Golang code (plugin)")
 	runCmd.Flags().BoolVar(&runOmitEmbedder, "omit-embedder", false, "Don't (re)compile 'go-flutter' source code, useful when only working with Dart code")
-	runCmd.Flags().BoolVar(&runDocker, "docker", false, "Execute the go build in a docker container. The Flutter build is always run locally")
 	rootCmd.AddCommand(runCmd)
 }
 
@@ -76,7 +66,7 @@ var runCmd = &cobra.Command{
 			log.Infof("Omiting build the embedder")
 		} else {
 			vmArguments := []string{"--observatory-port=" + runObservatoryPort, "--enable-service-port-fallback", "--disable-service-auth-codes"}
-			if runDocker {
+			if buildOrRunDocker {
 				var buildFlags []string
 				buildFlags = append(buildFlags, commonFlags()...)
 				buildFlags = append(buildFlags, []string{
@@ -122,7 +112,7 @@ func runAndAttach(projectName string, targetOS string) {
 			match := regexObservatory.FindStringSubmatch(text)
 			if len(match) == 2 {
 				log.Infof("Connecting hover to '%s' for hot reload", projectName)
-				startHotReloadProcess(cmdFlutterAttach, buildTarget, match[1])
+				startHotReloadProcess(cmdFlutterAttach, buildOrRunFlutterTarget, match[1])
 				break
 			}
 		}

internal/enginecache/cache.go

@@ -199,11 +199,11 @@ func EngineCachePath(targetOS, cachePath string) string {
 	return filepath.Join(cachePath, "hover", "engine", targetOS)
 }
 
-// ValidateOrUpdateEngineAtPath validates the engine we have cached matches the
+// ValidateOrUpdateEngine validates the engine we have cached matches the
 // flutter version, or otherwise downloads a new engine. The engine cache
 // location is set by the the user.
-func ValidateOrUpdateEngineAtPath(targetOS, cachePath, requiredEngineVersion string) (engineCachePath string) {
-	engineCachePath = EngineCachePath(targetOS, cachePath)
+func ValidateOrUpdateEngine(targetOS, cachePath, requiredEngineVersion string) {
+	engineCachePath := EngineCachePath(targetOS, cachePath)
 
 	if strings.Contains(engineCachePath, " ") {
 		log.Errorf("Cannot save the engine to '%s', engine cache is not compatible with path containing spaces.", cachePath)
@@ -227,7 +227,7 @@ func ValidateOrUpdateEngineAtPath(targetOS, cachePath, requiredEngineVersion str
 	if cachedEngineVersion != "" {
 		if cachedEngineVersion == requiredEngineVersion {
 			log.Printf("Using engine from cache")
-			return engineCachePath
+			return
 		}
 
 		// Engine is outdated, we remove the old engine and continue to download
@@ -356,5 +356,5 @@ func ValidateOrUpdateEngineAtPath(targetOS, cachePath, requiredEngineVersion str
 		os.Exit(1)
 	}
 
-	return engineCachePath
+	return
 }