Re: [Tails-dev] [review'n'merge:1.1] feature/vagrant-wheezy-…

Nachricht löschen

Nachricht beantworten
Autor: intrigeri
Datum:  
To: The Tails public development discussion list
Betreff: Re: [Tails-dev] [review'n'merge:1.1] feature/vagrant-wheezy-basebox (#7133, #6736)
Hi,

anonym wrote (10 Jul 2014 23:05:49 GMT) :
> [...] so IMHO only a code review is necessary, which I've assigned
> to intrigeri.


Commit fe9da04 is a pain to review, as it mixes renaming files, with
changing their content. So, I'm manually diff'ing the old files with
the new ones.

> - :sudo_cmd => "echo '%p'|sudo -S sh '%f'",
> + :sudo_cmd => "echo '%p'|sudo -S bash '%f'",


Why?

> +# Use http.debian.net to select closest mirror
> +d-i mirror/country string manual
> +d-i mirror/http/hostname string http.us.debian.org


... feels wrong.

> +gpg -a --export C7988EA7A358D82E | sudo apt-key add -


Just curious: is armored output really needed?

The rest is about the documentation:

> -    # gem install --no-ri --no-rdoc veewee
> [...]
> +    sudo gem install --no-ri --no-rdoc veewee


I suspect this can create problems when run with a strict umask.

I think it should be minimally documented why one still needs a clone
on veewee's Git, after installing the Gem.

> +define defined in `vagrant/Vagrantfile`.


One too much word?

> +Do not forget to update the SHA256 sum in
> +`vagrant/lib/tails_build_settings.rb`.


Isn't that what a sed command above does?

Does #5498 still apply? The notes about it were removed, but we still
instruct to install the gem as root, so I'm wondering.

> -After issuing that command, Veewee will download the boot ISO image,
> -drive the debian-installer using [preeseding] and run `postinstall.sh` that
> -will take care of seting up the environment expected by Vagrant.
> -
> -[preseeding]: https://www.debian.org/releases/stable/i386/apbs02.html
> -
> -In order to support local HTTP proxy, the `preseed.cfg` is generated from an
> -ERB template during the Rake task `create_preseed_cfg`.
> -
> -Once the initial setup is done, it is worthwhile to see if the *basebox*
> -fits Vagrant requirements. Veewee ships with an automated test suite:
> -
> -    $ vagrant basebox validate squeeze


It looks like perhaps some functionality was dropped (which is fine),
but perhaps other bits are still relevant? (at least the part that
explains what Veewee does, and points to the preseeding doc)

Cheers!
--
intrigeri