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

Nachricht löschen

Nachricht beantworten
Autor: anonym
Datum:  
To: The Tails public development discussion list
Betreff: Re: [Tails-dev] [review'n'merge:1.1] feature/vagrant-wheezy-basebox (#7133, #6736)
11/07/14 02:10, intrigeri wrote:
> 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.


Sorry, didn't think about that. OTOH it didn't feel like it would make
sense to rename the old directory to wheezy in a separate commit... or
do you think it's worth that, for future reference?

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


That's what the Veewee maintainer thinks, apparently. I just copied in
the Wheezy 7.5 definition.

>> +# 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.


Fixed the comment.

>> +gpg -a --export C7988EA7A358D82E | sudo apt-key add -
>
> Just curious: is armored output really needed?


No, so it's just unnecessary. Removed.

> 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.


I don't know why, but it didn't work for me to run it form the vagrant
sub-directory of our sources before, but now it does. Adapted.

>> +define defined in `vagrant/Vagrantfile`.
>
> One too much word?


Fixed!

>> +Do not forget to update the SHA256 sum in
>> +`vagrant/lib/tails_build_settings.rb`.
>
> Isn't that what a sed command above does?


Forgot to remove it, now fixed.

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


I did try `gem --user` earlier, which said there was no such option.
Apparently it is `gem install --user`, duh! Adapted.

>> -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)


Ok, added some explanations, some revived, some new.

Cheers!