Add script to build Debian packages

Add script to build Debian packages

The script create_debian_packages.py will build source and binary
Debian packages from the generated tarball.

R=kustermann@google.com, ricow@google.com
BUG=

Committed: https://code.google.com/p/dart/source/detail?r=32550

[kustermann, 2014-02-10 18:04:48]

lgtm with comments

https://codereview.chromium.org/159003002/diff/1/tools/bots/src-tarball.py
File tools/bots/src-tarball.py (right):

https://codereview.chromium.org/159003002/diff/1/tools/bots/src-tarball.py#ne...
tools/bots/src-tarball.py:3: # Copyright (c) 2014, the Dart project authors. 
Please see the AUTHORS file
I can't remember reviewing this file. Why was it named "src-tarball.py" ?

Please don't use dashes in python module names, but use underscores instead! 
(e.g. you cannot say "import src-tarball", but you can say "import
src_tarball").

https://codereview.chromium.org/159003002/diff/1/tools/bots/src-tarball.py#ne...
tools/bots/src-tarball.py:41: bot.RunProcess(args)
Our buildbots are not debian based, which means we create *.deb files for
ubuntu. We cannot use that on "normal" GCE VMs (glibc version issue ...).

I think we should create *.deb files for debian -- they may even install/run on
ubuntu (but not vice versa).

Did anyone look into debian chroot()'s ?

https://codereview.chromium.org/159003002/diff/1/tools/create_debian_packages.py
File tools/create_debian_packages.py (right):

https://codereview.chromium.org/159003002/diff/1/tools/create_debian_packages...
tools/create_debian_packages.py:27: pipe = subprocess.Popen(cmd,
It's not really a "pipe", maybe "process" would fit better.

https://codereview.chromium.org/159003002/diff/1/tools/create_debian_packages...
tools/create_debian_packages.py:33: (' '.join(cmd), str(stderr), str(stdout)))
Are these str() conversions needed? Isn't it already a string?

https://codereview.chromium.org/159003002/diff/1/tools/create_debian_packages...
tools/create_debian_packages.py:49: tar.extractall(path=builddir)
I'd actually prefer if you used a temporary directory to do this work and only
put the final *.deb/*.tar.gz files into the build directory.

https://codereview.chromium.org/159003002/diff/1/tools/create_debian_packages...
tools/create_debian_packages.py:53: RunBuildPackage(['-S', '-us', '-uc'],
join(DART_DIR, builddir, tarroot));
I think you're constructing the wrong directory here:

  37 builddir = join(DART_DIR, utils.GetBuildDir(HOST_OS, HOST_OS)
     ...
  53 RunBuildPackage([...], join(DART_DIR, builddir, tarroot))

Isn't that pre-pending DART_DIR twice?

[Søren Gjesse, 2014-02-11 10:21:57]

PTAL

https://codereview.chromium.org/159003002/diff/1/tools/bots/src-tarball.py
File tools/bots/src-tarball.py (right):

https://codereview.chromium.org/159003002/diff/1/tools/bots/src-tarball.py#ne...
tools/bots/src-tarball.py:3: # Copyright (c) 2014, the Dart project authors. 
Please see the AUTHORS file
On 2014/02/10 18:04:48, kustermann wrote:
> I can't remember reviewing this file. Why was it named "src-tarball.py" ?
> 
> Please don't use dashes in python module names, but use underscores instead! 
> (e.g. you cannot say "import src-tarball", but you can say "import
> src_tarball").
> 

https://codereview.chromium.org/157223004/

https://codereview.chromium.org/159003002/diff/1/tools/bots/src-tarball.py#ne...
tools/bots/src-tarball.py:41: bot.RunProcess(args)
On 2014/02/10 18:04:48, kustermann wrote:
> Our buildbots are not debian based, which means we create *.deb files for
> ubuntu. We cannot use that on "normal" GCE VMs (glibc version issue ...).
> 
> I think we should create *.deb files for debian -- they may even install/run
on
> ubuntu (but not vice versa).
> 
> Did anyone look into debian chroot()'s ?

I agree me should build these for Debian and not for whatever we are running on
the bots. This is a first step to get this running.

I looked into pbuilder and we might be able to use that for building for a
different distro.

https://codereview.chromium.org/159003002/diff/1/tools/create_debian_packages.py
File tools/create_debian_packages.py (right):

https://codereview.chromium.org/159003002/diff/1/tools/create_debian_packages...
tools/create_debian_packages.py:27: pipe = subprocess.Popen(cmd,
On 2014/02/10 18:04:48, kustermann wrote:
> It's not really a "pipe", maybe "process" would fit better.

Done.

https://codereview.chromium.org/159003002/diff/1/tools/create_debian_packages...
tools/create_debian_packages.py:33: (' '.join(cmd), str(stderr), str(stdout)))
On 2014/02/10 18:04:48, kustermann wrote:
> Are these str() conversions needed? Isn't it already a string?

Removed (I copied the str() part from someplace else).

https://codereview.chromium.org/159003002/diff/1/tools/create_debian_packages...
tools/create_debian_packages.py:49: tar.extractall(path=builddir)
On 2014/02/10 18:04:48, kustermann wrote:
> I'd actually prefer if you used a temporary directory to do this work and only
> put the final *.deb/*.tar.gz files into the build directory.

OK, changed

https://codereview.chromium.org/159003002/diff/1/tools/create_debian_packages...
tools/create_debian_packages.py:53: RunBuildPackage(['-S', '-us', '-uc'],
join(DART_DIR, builddir, tarroot));
On 2014/02/10 18:04:48, kustermann wrote:
> I think you're constructing the wrong directory here:
> 
>   37 builddir = join(DART_DIR, utils.GetBuildDir(HOST_OS, HOST_OS)
>      ...
>   53 RunBuildPackage([...], join(DART_DIR, builddir, tarroot))
> 
> Isn't that pre-pending DART_DIR twice?

Absolutely - fixed. As os.path.join "restarts" when meeting an absolute path it
still worked.

[kustermann, 2014-02-11 10:33:02]

LGTM

https://codereview.chromium.org/159003002/diff/70001/tools/create_debian_pack...
File tools/create_debian_packages.py (right):

https://codereview.chromium.org/159003002/diff/70001/tools/create_debian_pack...
tools/create_debian_packages.py:48: copyfile(tarball, origtarball)
When you extract the tarball, you specify the destination directory anyway.
Couldn't you just do

  with tarfile.open(tarball) as tar:
    tar.extractall(path=temp_dir)

And remove lines 47/48?

https://codereview.chromium.org/159003002/diff/70001/tools/create_debian_pack...
tools/create_debian_packages.py:76: join('%s-1_amd64.deb' % debbase),
These join() calls do nothing, so you can remove them. The string expansions are
enough.

[Søren Gjesse, 2014-02-11 11:08:33]

Committed patchset #3 manually as r32550 (presubmit successful).

[Søren Gjesse, 2014-02-12 08:29:18]

https://codereview.chromium.org/159003002/diff/70001/tools/create_debian_pack...
File tools/create_debian_packages.py (right):

https://codereview.chromium.org/159003002/diff/70001/tools/create_debian_pack...
tools/create_debian_packages.py:48: copyfile(tarball, origtarball)
On 2014/02/11 10:33:02, kustermann wrote:
> When you extract the tarball, you specify the destination directory anyway.
> Couldn't you just do
> 
>   with tarfile.open(tarball) as tar:
>     tar.extractall(path=temp_dir)
> 
> And remove lines 47/48?

The tar-file with the "orig" name needs to be in the parent directory of where
it is extracted for the Debian package builder to run.

https://codereview.chromium.org/159003002/diff/70001/tools/create_debian_pack...
tools/create_debian_packages.py:76: join('%s-1_amd64.deb' % debbase),
On 2014/02/11 10:33:02, kustermann wrote:
> These join() calls do nothing, so you can remove them. The string expansions
are
> enough.

Done.