[Merge] lp:~jani/goget-ubuntu-touch/local-tarball into lp:goget-ubuntu-touch

Sergio Schvezov sergio.schvezov at canonical.com
Mon Jun 16 19:26:24 UTC 2014


Review: Needs Information

I guess the next step would be to have a make target to create the tarball from the android tree :-)

Diff comments:

> === modified file 'ubuntu-device-flash/args.go'
> --- ubuntu-device-flash/args.go	2014-05-15 13:21:24 +0000
> +++ ubuntu-device-flash/args.go	2014-06-02 13:22:31 +0000
> @@ -31,6 +31,7 @@
>  	Wipe          bool   `long:"wipe" description:"Clear all data after flashing"`
>  	Channel       string `long:"channel" description:"Specify an alternate channel"`
>  	Device        string `long:"device" description:"Specify the device to flash"`
> +	LocalTarball  string `long:"local-tarball" description:"Specify a local device tarball to override the one from the server (using official Ubuntu images with custom device tarballs)"`

this should probably called device-tarball

>  	Serial        string `long:"serial" description:"Serial of the device to operate"`
>  	Server        string `long:"server" description:"Use a different image server"`
>  	CleanCache    bool   `long:"clean-cache" description:"Cleans up cache with all downloaded bits"`
> 
> === modified file 'ubuntu-device-flash/main.go'
> --- ubuntu-device-flash/main.go	2014-06-02 13:22:31 +0000
> +++ ubuntu-device-flash/main.go	2014-06-02 13:22:31 +0000
> @@ -31,6 +31,7 @@
>  	"os/exec"
>  	"path/filepath"
>  	"strings"
> +	"syscall"
>  
>  	"launchpad.net/goget-ubuntu-touch/devices"
>  	"launchpad.net/goget-ubuntu-touch/ubuntuimage"
> @@ -55,10 +56,26 @@
>  		if err != nil {
>  			log.Fatal(err)
>  		}
> -		if fi.Mode()&0100 == 0 {
> +		if fi.Mode()&0100 == 0 || !fi.Mode().IsRegular() {
>  			log.Fatalf("The script %s passed via --run-script is not executable", script)
>  		}
>  	}
> +
> +	tarballPath := args.LocalTarball
> +	if tarballPath != "" {
> +		if p, err := filepath.Abs(tarballPath); err != nil {
> +			log.Fatal("Local tarball not found", err)
> +		} else {
> +			tarballPath = p
> +		}
> +		fi, err := os.Lstat(tarballPath)
> +		if err != nil {
> +			log.Fatal(err)
> +		}
> +		if !fi.Mode().IsRegular() {
> +			log.Fatalf("The file %s passed via --local-tarball is not a regular file\n", tarballPath)
> +		}
> +	}
>  	channels, err := ubuntuimage.NewChannels(args.Server)
>  	if err != nil {
>  		log.Fatal(err)
> @@ -131,7 +148,11 @@
>  	files := make(chan Files, totalFiles)
>  	done := make(chan bool, totalFiles)
>  	for _, file := range image.Files {
> -		go bitDownloader(file, files, args.Server, cacheDir)
> +		if tarballPath != "" && strings.HasPrefix(file.Path, "/pool/device") {
> +			useLocalTarball(tarballPath, file, files, args.Server, cacheDir)
> +		} else {
> +			go bitDownloader(file, files, args.Server, cacheDir)
> +		}
>  	}
>  	for _, file := range signFiles {
>  		go bitDownloader(file, files, args.Server, cacheDir)
> @@ -217,8 +238,30 @@
>  	}
>  }
>  
> +// ensureExists touches a file. It can be used to create a dummy .asc file if none exists
> +func ensureExists(path string) {
> +	_, err := os.OpenFile(path, syscall.O_WRONLY|syscall.O_CREAT, 0666)

Why don't you do an lstat here too and create it if it doesn't exist?

I think you are missing a defer [file].Close() regardless after the err check

I'm going to start using go-xdg for most of these standard path management, you might want to take a peek and see if you want to use it (or comment on why I shouldn't migrate if you want)

> +	if err != nil {
> +		log.Fatal("Cannot touch %s : %s", path, err)
> +	}
> +}
> +
>  type Files struct{ FilePath, SigPath string }
>  
> +// useLocalTarball symlinks a user provided local tarball into the cacheDir instead of downloading one
> +func useLocalTarball(tarballPath string, file ubuntuimage.File, files chan<- Files, server, downloadDir string) {
> +	err := file.MakeRelativeToServer(server)
> +	if err != nil {
> +		log.Fatal(err)
> +	}
> +	filePath := filepath.Join(downloadDir, file.Path)
> +	os.MkdirAll(filepath.Dir(filePath), 0700)
> +	os.Symlink(tarballPath, filePath)
> +	sigPath := filePath + ".asc"
> +	ensureExists(sigPath)
> +	files <- Files{FilePath: filePath, SigPath: sigPath}
> +}
> +
>  // bitDownloader downloads
>  func bitDownloader(file ubuntuimage.File, files chan<- Files, server, downloadDir string) {
>  	err := file.MakeRelativeToServer(server)
> 


-- 
https://code.launchpad.net/~jani/goget-ubuntu-touch/local-tarball/+merge/221733
Your team Ubuntu Phablet Team is subscribed to branch lp:goget-ubuntu-touch.



More information about the Ubuntu-reviews mailing list