Code review comment for lp:~niemeyer/juju-core/halve-tools-tarball-size

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Reviewers: mp+126364_code.launchpad.net,

Message:
Please take a look.

Description:
environs: halve tools tarball size

Strip jujud and jujuc and do not add the "juju" command
to the tarball. This drops the tarball size by 50%+.

https://code.launchpad.net/~niemeyer/juju-core/halve-tools-tarball-size/+merge/126364

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6579043/

Affected files:
   A [revision details]
   M environs/tools.go
   M environs/tools_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: environs/tools.go
=== modified file 'environs/tools.go'
--- environs/tools.go 2012-09-21 14:41:58 +0000
+++ environs/tools.go 2012-09-26 02:08:23 +0000
@@ -437,19 +437,28 @@
   }
   defer os.RemoveAll(dir)

- cmd := exec.Command("go", "install", "launchpad.net/juju-core/cmd/...")
- cmd.Env = setenv(os.Environ(), "GOBIN="+dir)
- out, err := cmd.CombinedOutput()
- if err != nil {
- return version.Binary{}, fmt.Errorf("build failed: %v; %s", err, out)
+ cmds := [][]string{
+ {"go", "install", "launchpad.net/juju-core/cmd/jujud"},
+ {"go", "install", "launchpad.net/juju-core/cmd/jujuc"},
+ {"strip", dir + "/jujud"},
+ {"strip", dir + "/jujuc"},
+ }
+ env := setenv(os.Environ(), "GOBIN="+dir)
+ for _, args := range cmds {
+ cmd := exec.Command(args[0], args[1:]...)
+ cmd.Env = env
+ out, err := cmd.CombinedOutput()
+ if err != nil {
+ return version.Binary{}, fmt.Errorf("build failed: %v; %s", err, out)
+ }
   }
   if vers != nil {
    if err := ioutil.WriteFile(filepath.Join(dir, "FORCE-VERSION"),
[]byte((*vers).String()), 0666); err != nil {
     return version.Binary{}, err
    }
   }
- cmd = exec.Command(filepath.Join(dir, "jujud"), "version")
- out, err = cmd.CombinedOutput()
+ cmd := exec.Command(filepath.Join(dir, "jujud"), "version")
+ out, err := cmd.CombinedOutput()
   if err != nil {
    return version.Binary{}, fmt.Errorf("cannot get version
from %q: %v; %s", cmd.Args[0], err, out)
   }

Index: environs/tools_test.go
=== modified file 'environs/tools_test.go'
--- environs/tools_test.go 2012-09-21 11:45:08 +0000
+++ environs/tools_test.go 2012-09-26 02:08:23 +0000
@@ -60,10 +60,12 @@
   cmd []string
   output string
  }{
+ // TODO(niemeyer): Reintroduce this once we start deploying to the public
bucket.
+ //{
+ // []string{"juju", "arble"},
+ // "error: unrecognized command: juju arble\n",
+ //},
   {
- []string{"juju", "arble"},
- "error: unrecognized command: juju arble\n",
- }, {
    []string{"jujud", "arble"},
    "error: unrecognized command: jujud arble\n",
   }, {

« Back to merge proposal