Merge into trunk : all-snaps : Code : goget-ubuntu-touch

Status:	Needs review
Proposed branch:	lp://qastaging/~snappy-dev/goget-ubuntu-touch/all-snaps
Merge into:	lp://qastaging/goget-ubuntu-touch
Diff against target:	1882 lines (+1243/-150) (has conflicts) 14 files modified debian/changelog (+15/-0) debian/control (+14/-0) dependencies.tsv (+6/-1) diskimage/bootloader.go (+21/-12) diskimage/common.go (+99/-69) diskimage/common_test.go (+14/-14) diskimage/core_grub.go (+31/-27) diskimage/core_uboot.go (+32/-16) diskimage/partition.go (+1/-0) ubuntu-device-flash/common.go (+3/-3) ubuntu-device-flash/core.go (+12/-5) ubuntu-device-flash/helpers.go (+89/-0) ubuntu-device-flash/helpers_test.go (+135/-0) ubuntu-device-flash/snappy.go (+771/-3) Text conflict in debian/changelog Text conflict in debian/control Text conflict in dependencies.tsv Text conflict in diskimage/bootloader.go Text conflict in diskimage/common.go Text conflict in diskimage/core_grub.go Text conflict in diskimage/core_uboot.go Text conflict in ubuntu-device-flash/core.go Text conflict in ubuntu-device-flash/snappy.go
To merge this branch:	bzr merge lp://qastaging/~snappy-dev/goget-ubuntu-touch/all-snaps
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Sergio Schvezov		2015-10-22	Approve on 2016-03-30
Review via email: mp+275273@code.qastaging.launchpad.net

Revision history for this message

Michael Vogt (mvo) wrote on 2015-10-22:

#

Just added some comments so that we don't forget them :)

Reply

Revision history for this message

Sergio Schvezov (sergiusens) wrote on 2015-10-22:

#

Download full text (7.0 KiB)

On Oct 22, 2015 4:28 AM, "Michael Vogt" <email address hidden> wrote:
>
> Just added some comments so that we don't forget them :)
>
> Diff comments:
>
> > === modified file 'diskimage/common.go'
> > --- diskimage/common.go 2015-09-09 14:36:46 +0000
> > +++ diskimage/common.go 2015-10-22 07:16:55 +0000
> > @@ -190,16 +190,19 @@
> >
> > // BaseImage implements the basic primitives to manage images.
> > type BaseImage struct {
> > - baseMount string
> > - hardware HardwareDescription
> > - location string
> > - oem OemDescription
> > - parts []partition
> > - partCount int
> > - size int64
> > - rootSize int
> > + baseMount string
> > + bindMounts []string
> > + hardware HardwareDescription
> > + location string
> > + oem OemDescription
> > + parts []partition
> > + partCount int
> > + size int64
> > + rootSize int
> > }
> >
> > +var bindMounts = []string{"dev", "sys", "proc", filepath.Join("sys",
"firmware")}
>
> It looks like we are doing this twice (once here, once in snappy).

You will see a bunch of these little things which could go away when
migrating the provisioner into snappy itself.
>
> > +
> > // Mount mounts the image. This also maps the loop device.
> > func (img *BaseImage) Mount() error {
> > if err := img.doMap(); err != nil {
> >
> > === modified file 'ubuntu-device-flash/snappy.go'
> > --- ubuntu-device-flash/snappy.go 2015-09-16 11:13:20 +0000
> > +++ ubuntu-device-flash/snappy.go 2015-10-22 07:16:55 +0000
> > @@ -339,7 +343,21 @@
> > return hw, err
> > }
> >
> > -func (s *Snapper) setup(filePathChan <-chan string, fileCount int)
error {
> > +func (s *Snapper) bindMount(d string) (string, error) {
>
> I gues the name could be better, something like bindMountInWritable or so
>
> > + src := filepath.Join(s.img.Writable(), "system-data", d)
> > + dst := filepath.Join(s.img.System(), d)
> > +
> > + if err := os.MkdirAll(src, 0755); err != nil {
> > + return "", err
> > + }
> > + cmd := exec.Command("mount", "--bind", src, dst)
> > + if o, err := cmd.CombinedOutput(); err != nil {
> > + return "", fmt.Errorf("os snap mount failed with: %s %v
", err, string(o))
> > + }
> > + return dst, nil
> > +}
> > +
> > +func (s *Snapper) setup(systemImageFiles []Files) error {
> > printOut("Mounting...")
> > if err := s.img.Mount(); err != nil {
> > return err
> > @@ -362,14 +379,82 @@
> >
> > systemPath := s.img.System()
> >
> > + // setup a fake system
> > + if s.oem.PartitionLayout() == "minimal" {
> > + if err := os.MkdirAll(systemPath, 0755); err != nil {
> > + return err
> > + }
> > + // mount os snap
> > + cmd := exec.Command("mount", s.OS, systemPath)
> > + if o, err := cmd.CombinedOutput(); err != nil {
> > + return fmt.Errorf("os snap mount failed with: %s
%v ", err, string(o))
> > + }
> > + defer exec.Command("umount", systemPath).Run()
> > +
> > + //...

On Oct 22, 2015 4:28 AM, "Michael Vogt" <michael.vogt@canonical.com> wrote:
>
> Just added some comments so that we don't forget them :)
>
> Diff comments:
>
> > === modified file 'diskimage/common.go'
> > --- diskimage/common.go       2015-09-09 14:36:46 +0000
> > +++ diskimage/common.go       2015-10-22 07:16:55 +0000
> > @@ -190,16 +190,19 @@
> >
> >  // BaseImage implements the basic primitives to manage images.
> >  type BaseImage struct {
> > -     baseMount string
> > -     hardware  HardwareDescription
> > -     location  string
> > -     oem       OemDescription
> > -     parts     []partition
> > -     partCount int
> > -     size      int64
> > -     rootSize  int
> > +     baseMount  string
> > +     bindMounts []string
> > +     hardware   HardwareDescription
> > +     location   string
> > +     oem        OemDescription
> > +     parts      []partition
> > +     partCount  int
> > +     size       int64
> > +     rootSize   int
> >  }
> >
> > +var bindMounts = []string{"dev", "sys", "proc", filepath.Join("sys",
"firmware")}
>
> It looks like we are doing this twice (once here, once in snappy).

You will see a bunch of these little things which could go away when
migrating the provisioner into snappy itself.
>
> > +
> >  // Mount mounts the image. This also maps the loop device.
> >  func (img *BaseImage) Mount() error {
> >       if err := img.doMap(); err != nil {
> >
> > === modified file 'ubuntu-device-flash/snappy.go'
> > --- ubuntu-device-flash/snappy.go     2015-09-16 11:13:20 +0000
> > +++ ubuntu-device-flash/snappy.go     2015-10-22 07:16:55 +0000
> > @@ -339,7 +343,21 @@
> >       return hw, err
> >  }
> >
> > -func (s *Snapper) setup(filePathChan <-chan string, fileCount int)
error {
> > +func (s *Snapper) bindMount(d string) (string, error) {
>
> I gues the name could be better, something like bindMountInWritable or so
>
> > +     src := filepath.Join(s.img.Writable(), "system-data", d)
> > +     dst := filepath.Join(s.img.System(), d)
> > +
> > +     if err := os.MkdirAll(src, 0755); err != nil {
> > +             return "", err
> > +     }
> > +     cmd := exec.Command("mount", "--bind", src, dst)
> > +     if o, err := cmd.CombinedOutput(); err != nil {
> > +             return "", fmt.Errorf("os snap mount failed with: %s %v
", err, string(o))
> > +     }
> > +     return dst, nil
> > +}
> > +
> > +func (s *Snapper) setup(systemImageFiles []Files) error {
> >       printOut("Mounting...")
> >       if err := s.img.Mount(); err != nil {
> >               return err
> > @@ -362,14 +379,82 @@
> >
> >       systemPath := s.img.System()
> >
> > +     // setup a fake system
> > +     if s.oem.PartitionLayout() == "minimal" {
> > +             if err := os.MkdirAll(systemPath, 0755); err != nil {
> > +                     return err
> > +             }
> > +             // mount os snap
> > +             cmd := exec.Command("mount", s.OS, systemPath)
> > +             if o, err := cmd.CombinedOutput(); err != nil {
> > +                     return fmt.Errorf("os snap mount failed with: %s
%v ", err, string(o))
> > +             }
> > +             defer exec.Command("umount", systemPath).Run()
> > +
> > +             // we need to do what "writable-paths" normally does on
> > +             // boot for etc/systemd/system, i.e. copy all the stuff
> > +             // from the os into the writable partition. normally
> > +             // this is the job of the initrd, however it won't touch
> > +             // the dir if there are files in there already. and a
> > +             // kernel/os install will create auto-mount units in there
> > +             src := filepath.Join(systemPath, "etc", "systemd",
"system")
> > +             dst := filepath.Join(s.img.Writable(), "system-data",
"etc", "systemd")
> > +             if err := os.MkdirAll(dst, 0755); err != nil {
> > +                     return err
> > +             }
> > +             cmd = exec.Command("cp", "-a", src, dst)
> > +             if o, err := cmd.CombinedOutput(); err != nil {
> > +                     return fmt.Errorf("copy failed: %s %s", err, o)
> > +             }
> > +
> > +             // bind mount all relevant dirs
> > +             for _, d := range []string{"apps", "oem",
"var/lib/snappy", "var/lib/apps", "etc/systemd/system/", "tmp"} {
> > +                     dst, err := s.bindMount(d)
> > +                     if err != nil {
> > +                             return err
> > +                     }
> > +                     defer exec.Command("umount", dst).Run()
> > +             }
> > +
> > +             // bind mount /boot/efi
> > +             dst = filepath.Join(systemPath, "/boot/efi")
> > +             cmd = exec.Command("mount", "--bind", s.img.Boot(), dst)
> > +             if o, err := cmd.CombinedOutput(); err != nil {
> > +                     return fmt.Errorf("boot bind mount failed with:
%s %v ", err, string(o))
> > +             }
> > +             defer exec.Command("umount", dst).Run()
> > +
> > +             // grub needs this
> > +             grubUbuntu := filepath.Join(s.img.Boot(),
"EFI/ubuntu/grub")
> > +             os.MkdirAll(grubUbuntu, 0755)
> > +
> > +             // and /boot/grub
> > +             src = grubUbuntu
> > +             dst = filepath.Join(systemPath, "/boot/grub")
> > +             cmd = exec.Command("mount", "--bind", src, dst)
> > +             if o, err := cmd.CombinedOutput(); err != nil {
> > +                     return fmt.Errorf("boot/ubuntu bind mount failed
with: %s %v ", err, string(o))
> > +             }
> > +             defer exec.Command("umount", dst).Run()
> > +
> > +             // TERRIBLE but we need a /boot/grub/grub.cfg so that
>
> It looks like this is no longer needed, I removed it and its still fine
it seems.

Right. GenericBootSetup should be taking care of this.

>
> > +             //          the kernel and os snap can be installed
> > +             oemGrubCfg := filepath.Join(s.stagingRootPath, "oem",
"generic-amd64", "current", "grub.cfg")
> > +             cmd = exec.Command("cp", oemGrubCfg, grubUbuntu)
> > +             o, err := cmd.CombinedOutput()
> > +             if err != nil {
> > +                     return fmt.Errorf("failed to copy %s %s", err, o)
> > +             }
> > +     }
> > +
> > +     if err := s.img.SetupBoot(); err != nil {
> > +             return err
> > +     }
> > +
> >       if err := s.install(systemPath); err != nil {
> >               return err
> >       }
> >
> > -     if err := s.img.SetupBoot(); err != nil {
> > -             return err
> > -     }
> > -
> >       for i := range s.customizationFunc {
> >               if err := s.customizationFunc[i](); err != nil {
> >                       return err
>
>
> --
>
https://code.launchpad.net/~snappy-dev/goget-ubuntu-touch/all-snaps/+merge/275273
> Your team Snappy Developers is subscribed to branch
lp:~snappy-dev/goget-ubuntu-touch/all-snaps.
>
> Launchpad-Message-Rationale: Subscriber @snappy-dev
> Launchpad-Message-For: snappy-dev
> Launchpad-Notification-Type: code-review
> Launchpad-Branch: ~snappy-dev/goget-ubuntu-touch/all-snaps
> Launchpad-Project: goget-ubuntu-touch

Reply

Revision history for this message

Sergio Schvezov (sergiusens) wrote on 2016-01-12:

#

Thanks for this, it removes a bunch of legacy. I added a couple of inline comments, nothing major.

MPs like this https://code.launchpad.net/~sil2100/goget-ubuntu-touch/touch-devel-warning/+merge/282045 make me think we need to fork though since this will be expected to go all the way to trusty.

review: Needs Fixing

Reply

Revision history for this message

Sergio Schvezov (sergiusens) on 2016-03-08:

#

Reply

Revision history for this message

Michael Vogt (mvo) wrote on 2016-03-30:

#

This is updated and all comments are addressed now.

Reply

Revision history for this message

Sergio Schvezov (sergiusens) on 2016-03-30:

#

review: Approve

Reply

Revision history for this message

Snappy Tarmac (snappydevtarmac) wrote on 2016-03-30:

#

Download full text (3.6 KiB)

The attempt to merge lp:~snappy-dev/goget-ubuntu-touch/all-snaps into lp:goget-ubuntu-touch failed. Below is the output from the failed tests.

Checking formatting
Installing godeps
Install golint
Obtaining dependencies
update launchpad.net/gocheck failed; trying to fetch newer version
launchpad.net/gocheck now at <email address hidden>
update github.com/mvo5/goconfigparser failed; trying to fetch newer version
github.com/mvo5/goconfigparser now at 26426272dda20cc76aa1fa44286dc743d2972fe8
update github.com/cheggaaa/pb failed; trying to fetch newer version
github.com/cheggaaa/pb now at da1f27ad1d9509b16f65f52fd9d8138b0f2dc7b2
update github.com/gosexy/gettext failed; trying to fetch newer version
github.com/gosexy/gettext now at 98b7b91596d20b96909e6b60d57411547dd9959c
update github.com/jessevdk/go-flags failed; trying to fetch newer version
github.com/jessevdk/go-flags now at 4047bd797dd935ae2b557a79cc43f223066c9659
update github.com/ubuntu-core/snappy failed; trying to fetch newer version
github.com/ubuntu-core/snappy now at a6e683cb4eca7027b331713c2f10e26a31050f20
update github.com/mvo5/uboot-go failed; trying to fetch newer version
github.com/mvo5/uboot-go now at 69978a3e4b05cca9d7cfee489b3453dfed45e72c
update gopkg.in/yaml.v2 failed; trying to fetch newer version
gopkg.in/yaml.v2 now at 7ad95dd0798a40da1ccdff6dff35fd177b5edf40
Building

# we always run in a fresh dir in tarmac
export GOPATH=$(mktemp -d)
trap 'rm -rf "$GOPATH"' EXIT

# this is a hack, but not sure tarmac is golang friendly
mkdir -p $GOPATH/src/launchpad.net/goget-ubuntu-touch
cp -a . $GOPATH/src/launchpad.net/goget-ubuntu-touch
cd $GOPATH/src/launchpad.net/goget-ubuntu-touch

./run-checks
launchpad.net/goget-ubuntu-touch/bootimg
launchpad.net/goget-ubuntu-touch/devices
launchpad.net/goget-ubuntu-touch/bootimg/example/abootimg-extract
launchpad.net/goget-ubuntu-touch/sysutils
launchpad.net/goget-ubuntu-touch/diskimage
github.com/jessevdk/go-flags
github.com/cheggaaa/pb
launchpad.net/goget-ubuntu-touch/ubuntuimage
github.com/ubuntu-core/snappy/arch
github.com/ubuntu-core/snappy/dirs
launchpad.net/goget-ubuntu-touch/ubuntu-device-do
github.com/ubuntu-core/snappy/osutil
github.com/mvo5/goconfigparser
github.com/mvo5/uboot-go/uenv
github.com/ubuntu-core/snappy/logger
github.com/ubuntu-core/snappy/helpers
github.com/ubuntu-core/snappy/partition
github.com/ubuntu-core/snappy/progress
gopkg.in/yaml.v2
github.com/ubuntu-core/snappy/release
github.com/ubuntu-core/snappy/systemd
github.com/gosexy/gettext
github.com/ubuntu-core/snappy/i18n
github.com/ubuntu-core/snappy/oauth
github.com/ubuntu-core/snappy/policy
github.com/ubuntu-core/snappy/timeout
launchpad.net/goget-ubuntu-touch/ubuntu-emulator
github.com/ubuntu-core/snappy/provisioning
github.com/ubuntu-core/snappy/snap
github.com/ubuntu-core/snappy/coreconfig
github.com/ubuntu-core/snappy/snap/remote
github.com/ubuntu-core/snappy/snap/squashfs
github.com/ubuntu-core/snappy/snappy
launchpad.net/goget-ubuntu-touch/ubuntu-device-flash
# launchpad.net/goget-ubuntu-touch/ubuntu-device-flash
ubuntu-device-flash/helpers.go:42: undefined: osutil.FileExists
ubuntu-device-flash/helpers.go:76: undefined: osutil....

The attempt to merge lp:~snappy-dev/goget-ubuntu-touch/all-snaps into lp:goget-ubuntu-touch failed. Below is the output from the failed tests.

Checking formatting
Installing godeps
Install golint
Obtaining dependencies
update launchpad.net/gocheck failed; trying to fetch newer version
launchpad.net/gocheck now at gustavo@niemeyer.net-20140225173054-xu9zlkf9kxhvow02
update github.com/mvo5/goconfigparser failed; trying to fetch newer version
github.com/mvo5/goconfigparser now at 26426272dda20cc76aa1fa44286dc743d2972fe8
update github.com/cheggaaa/pb failed; trying to fetch newer version
github.com/cheggaaa/pb now at da1f27ad1d9509b16f65f52fd9d8138b0f2dc7b2
update github.com/gosexy/gettext failed; trying to fetch newer version
github.com/gosexy/gettext now at 98b7b91596d20b96909e6b60d57411547dd9959c
update github.com/jessevdk/go-flags failed; trying to fetch newer version
github.com/jessevdk/go-flags now at 4047bd797dd935ae2b557a79cc43f223066c9659
update github.com/ubuntu-core/snappy failed; trying to fetch newer version
github.com/ubuntu-core/snappy now at a6e683cb4eca7027b331713c2f10e26a31050f20
update github.com/mvo5/uboot-go failed; trying to fetch newer version
github.com/mvo5/uboot-go now at 69978a3e4b05cca9d7cfee489b3453dfed45e72c
update gopkg.in/yaml.v2 failed; trying to fetch newer version
gopkg.in/yaml.v2 now at 7ad95dd0798a40da1ccdff6dff35fd177b5edf40
Building

# we always run in a fresh dir in tarmac
export GOPATH=$(mktemp -d)
trap 'rm -rf "$GOPATH"' EXIT

# this is a hack, but not sure tarmac is golang friendly
mkdir -p $GOPATH/src/launchpad.net/goget-ubuntu-touch
cp -a . $GOPATH/src/launchpad.net/goget-ubuntu-touch
cd $GOPATH/src/launchpad.net/goget-ubuntu-touch

./run-checks
launchpad.net/goget-ubuntu-touch/bootimg
launchpad.net/goget-ubuntu-touch/devices
launchpad.net/goget-ubuntu-touch/bootimg/example/abootimg-extract
launchpad.net/goget-ubuntu-touch/sysutils
launchpad.net/goget-ubuntu-touch/diskimage
github.com/jessevdk/go-flags
github.com/cheggaaa/pb
launchpad.net/goget-ubuntu-touch/ubuntuimage
github.com/ubuntu-core/snappy/arch
github.com/ubuntu-core/snappy/dirs
launchpad.net/goget-ubuntu-touch/ubuntu-device-do
github.com/ubuntu-core/snappy/osutil
github.com/mvo5/goconfigparser
github.com/mvo5/uboot-go/uenv
github.com/ubuntu-core/snappy/logger
github.com/ubuntu-core/snappy/helpers
github.com/ubuntu-core/snappy/partition
github.com/ubuntu-core/snappy/progress
gopkg.in/yaml.v2
github.com/ubuntu-core/snappy/release
github.com/ubuntu-core/snappy/systemd
github.com/gosexy/gettext
github.com/ubuntu-core/snappy/i18n
github.com/ubuntu-core/snappy/oauth
github.com/ubuntu-core/snappy/policy
github.com/ubuntu-core/snappy/timeout
launchpad.net/goget-ubuntu-touch/ubuntu-emulator
github.com/ubuntu-core/snappy/provisioning
github.com/ubuntu-core/snappy/snap
github.com/ubuntu-core/snappy/coreconfig
github.com/ubuntu-core/snappy/snap/remote
github.com/ubuntu-core/snappy/snap/squashfs
github.com/ubuntu-core/snappy/snappy
launchpad.net/goget-ubuntu-touch/ubuntu-device-flash
# launchpad.net/goget-ubuntu-touch/ubuntu-device-flash
ubuntu-device-flash/helpers.go:42: undefined: osutil.FileExists
ubuntu-device-flash/helpers.go:76: undefined: osutil.FilesAreEqual
ubuntu-device-flash/snappy.go:116: undefined: osutil.FileExists
ubuntu-device-flash/snappy.go:200: too many arguments in call to snappy.Install
ubuntu-device-flash/snappy.go:212: undefined: partition.FindBootloader
ubuntu-device-flash/snappy.go:294: undefined: osutil.FileExists
ubuntu-device-flash/snappy.go:295: undefined: snappy.NewConfiguredUbuntuStoreSnapRepository
ubuntu-device-flash/snappy.go:469: undefined: snappy.NewConfiguredUbuntuStoreSnapRepository

Reply

lp://qastaging/~snappy-dev/goget-ubuntu-touch/all-snaps updated on 2016-05-10

264. By Sergio Schvezov on 2016-04-01: Update to latest snappy code
265. By Michael Vogt on 2016-04-06: ppa uploads
266. By Michael Vogt on 2016-04-09: fix bind mounts
267. By Michael Vogt on 2016-04-15: update to the latest api
268. By Michael Vogt on 2016-04-18: manually enable all snaps on the system
269. By Michael Vogt on 2016-04-19: remove debug code
270. By Michael Vogt on 2016-04-19: update for latest snappy
271. By Michael Vogt on 2016-04-26: add check for having systemctl
272. By Michael Vogt on 2016-04-27: set snappy_{,good}_{kernel,os} to fix LP:#1534137
273. By Michael Vogt on 2016-05-02: fix squashfs format issue
274. By Michael Vogt on 2016-05-04: use gadget snap whitelist in preparation for the rework of the gadget yaml
275. By Michael Vogt on 2016-05-04: merged lp:~pedronis/goget-ubuntu-touch/use-read-info-from-snapfile
276. By Michael Vogt on 2016-05-10: fix for latest api change

goget-ubuntu-touch

Merge lp://qastaging/~snappy-dev/goget-ubuntu-touch/all-snaps into lp://qastaging/goget-ubuntu-touch

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers