[Merge] lp:~sergiusens/goget-ubuntu-touch/dual_default into lp:goget-ubuntu-touch

Ricardo Salveti rsalveti at rsalveti.net
Fri Nov 28 05:13:58 UTC 2014


Review: Needs Information

LGTM, minor question, and would be nice for this MR to depend on https://code.launchpad.net/~sergiusens/goget-ubuntu-touch/noverb/+merge/242965 to avoid confusion in LP.

Diff comments:

> === modified file 'ubuntu-device-flash/core.go'
> --- ubuntu-device-flash/core.go	2014-11-25 19:31:44 +0000
> +++ ubuntu-device-flash/core.go	2014-11-26 20:48:13 +0000
> @@ -53,7 +53,7 @@
>  	Output        string `long:"output" short:"o" description:"Name of the image file to create" required:"true"`
>  	Size          int64  `long:"size" short:"s" description:"Size of image file to create in GB (min 6)" default:"20"`
>  	DeveloperMode bool   `long:"developer-mode" description:"Finds the latest public key in your ~/.ssh and sets it up"`
> -	Dual          bool   `long:"dual-images" description:"Sets up two images to upgrade with providing rollback support"`
> +	Single        bool   `long:"single-partition" description:"Sets up a single system partiton"`

Should we say that dual-images is the default?

>  }
>  
>  var coreCmd CoreCmd
> @@ -86,6 +86,12 @@
>  		return err
>  	}
>  
> +	if coreCmd.Single {
> +		fmt.Println("Image will have a single system partition...")
> +	}
> +
> +	fmt.Println("Fetching information from server...")
> +
>  	channels, err := ubuntuimage.NewChannels(globalArgs.Server)
>  	if err != nil {
>  		return err
> @@ -108,6 +114,8 @@
>  	sigFilesChan := make(chan Files, len(sigFiles))
>  	defer close(sigFilesChan)
>  
> +	fmt.Println("Downloading and setting up...")
> +
>  	for _, f := range sigFiles {
>  		go bitDownloader(f, sigFilesChan, globalArgs.Server, cacheDir)
>  	}
> @@ -121,14 +129,16 @@
>  	go func() {
>  		for i := 0; i < len(image.Files); i++ {
>  			f := <-filesChan
> -			fmt.Println("Download finished for", f.FilePath)
> +
> +			printOut("Download finished for", f.FilePath)
> +
>  			filePathChan <- f.FilePath
>  		}
>  		close(filePathChan)
>  	}()
>  
>  	img := diskimage.New(coreCmd.Output, "", coreCmd.Size)
> -	if err := img.Partition(coreCmd.Dual); err != nil {
> +	if err := img.Partition(!coreCmd.Single); err != nil {
>  		return err
>  	}
>  	defer func() {
> @@ -143,7 +153,7 @@
>  		signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM)
>  
>  		for sig := range ch {
> -			fmt.Println("Received", sig, "... ignoring")
> +			printOut("Received", sig, "... ignoring")
>  		}
>  	}()
>  
> @@ -177,7 +187,7 @@
>  }
>  
>  func (coreCmd *CoreCmd) partition(img *diskimage.DiskImage) error {
> -	if err := img.MapPartitions(coreCmd.Dual); err != nil {
> +	if err := img.MapPartitions(!coreCmd.Single); err != nil {
>  		return fmt.Errorf("issue while mapping partitions: %s", err)
>  	}
>  	defer img.UnMapPartitions()
> @@ -186,7 +196,7 @@
>  }
>  
>  func (coreCmd *CoreCmd) setup(img *diskimage.DiskImage, filePathChan <-chan string) error {
> -	if err := img.MapPartitions(coreCmd.Dual); err != nil {
> +	if err := img.MapPartitions(!coreCmd.Single); err != nil {
>  		return err
>  	}
>  	defer img.UnMapPartitions()
> @@ -212,7 +222,7 @@
>  		return err
>  	}
>  
> -	if coreCmd.Dual {
> +	if !coreCmd.Single {
>  		src := fmt.Sprintf("%s/system/.", img.Mountpoint)
>  		dst := fmt.Sprintf("%s/system-2", img.Mountpoint)
>  		cmd := exec.Command("cp", "-r", "--preserve=all", src, dst)
> @@ -268,6 +278,8 @@
>  	}
>  
>  	if coreCmd.DeveloperMode {
> +		fmt.Println("Enabling developer mode...")
> +
>  		authorizedKey, err := getAuthorizedSshKey()
>  		if err != nil {
>  			return fmt.Errorf("failed to obtain a public key for developer mode: %s", err)
> @@ -353,7 +365,7 @@
>  	if out, err := exec.Command("chroot", systemPath, "grub-install", "/root_dev").CombinedOutput(); err != nil {
>  		return fmt.Errorf("unable to install grub: %s", out)
>  	} else {
> -		fmt.Println(string(out))
> +		printOut(string(out))
>  	}
>  
>  	// ensure we run not into recordfail issue
> @@ -376,7 +388,7 @@
>  	if out, err := exec.Command("chroot", systemPath, "update-grub").CombinedOutput(); err != nil {
>  		return fmt.Errorf("unable to update grub: %s", out)
>  	} else {
> -		fmt.Println(string(out))
> +		printOut(string(out))
>  	}
>  
>  	return nil
> 
> === modified file 'ubuntu-device-flash/main.go'
> --- ubuntu-device-flash/main.go	2014-11-04 13:46:00 +0000
> +++ ubuntu-device-flash/main.go	2014-11-26 20:48:13 +0000
> @@ -35,6 +35,7 @@
>  	Server        string `long:"server" description:"Use a different image server" default:"https://system-image.ubuntu.com"`
>  	CleanCache    bool   `long:"clean-cache" description:"Cleans up cache with all downloaded bits"`
>  	TLSSkipVerify bool   `long:"tls-skip-verify" description:"Skip TLS certificate validation"`
> +	Verbose       bool   `long:"verbose" short:"v" description:"More messages will be printed out"`
>  }
>  
>  var globalArgs arguments
> @@ -63,3 +64,9 @@
>  		os.Exit(1)
>  	}
>  }
> +
> +func printOut(args ...interface{}) {
> +	if globalArgs.Verbose {
> +		fmt.Println(args...)
> +	}
> +}
> 


-- 
https://code.launchpad.net/~sergiusens/goget-ubuntu-touch/dual_default/+merge/242976
Your team Ubuntu Phablet Team is subscribed to branch lp:goget-ubuntu-touch.



More information about the Ubuntu-reviews mailing list