Hi, This is an answer to blockers only. I belive they are all fixed.
On Tue, 30 Apr 2013 10:05:23 +0200 intrigeri <[email protected]> wrote: > [blocker]: verify this is needed; if yes, add a comment explaining why > (and possibly implement it in a better way); if not, remove it. > > > apt_get_env['LANG'] = "C" > > Why? > For logging in english, comment added in commit d5d8dd1. > [blocker]: verify this is needed; if yes, add a comment explaining > why; if not, remove it. > > > syslog.syslog("WARNING: [...]") > > Is the "WARNING" prefix the way one sets the importance of a given > syslog message, with this syslog library? Else, if it's merely > a ad-hoc way to pack this information with the actual message, then > I suggest using whatever facility (no pun intended) this library > provides to set the syslog message importance in a more structured > way. Sorry, I was not in the mood of learning Python's syslog library, > so I did not do it myself. > Fixed in commit f690185. > [blocker] > > > syslog.syslog("WARNING: installation of %s failed" % " > > ".join(packages)) > > In this case, as a user, I'd like to have access to the APT > stdout/stderr/returncode. Stdout/stderr was already logged. Returncode added in commit 89a7ed1. > [blocker] > > > if os.path.isfile(ACTIVATION_FILE): > > return True > > else: > > return False > > Just curious, as a non-Python-programmer: wouldn't `return > os.path.isfile(ACTIVATION_FILE)' work just as fine, and be nicer? Fixed in commit be19887. > [blocker] > > > + apt_get_returncode = _launch_apt_get(["apt-get", > > + "--quiet", > > + "update"]) > > Adding `--yes' here too would be more consistent and future-proof, > wouldn't it? Done in commit 80cd937. > If we go this way, perhaps the args we always want to > pass to apt-get (`--yes' and `--quiet') could be stored in a variable > on top of the script, and transparently added by _launch_apt_get? > I prefer to do that later (it will change more code and might require more time/testing). > [blocker] > > Also, what does happen in the following user story: > > 1. I boot Tails on January 1st > 2. I setup persistence for APT lists and packages, and I reboot. > 3. I activate persistence, I install a few packages, and I list them > in live-additional-software.conf. Shutdown. > 4. I boot Tails on February 1st > 5. I activate persistence > => The APT indices are expired and not valid anymore, so > `install_additional_packages' fails, right? > What's the APT indices expiration time exactly? > Each APT list contains a "Valid-Until:" field. > At least understanding how Tails behaves in this case is a [blocker], > as is evaluating how good/bad this is. But probably we can ship the > whole thing as is right now, and keep further improvements in this > area for later. > Testing shows that when lists are expired, the initial install fails and I get the following output: Reading package lists... Building dependency tree... Reading state information... The following extra packages will be installed: vim-gui-common Suggested packages: cscope vim-doc The following NEW packages will be installed: vim-gnome vim-gui-common 0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded. Need to get 0 B/1166 kB of archives. After this operation, 2454 kB of additional disk space will be used. WARNING: The following packages cannot be authenticated! vim-gui-common vim-gnome E: There are problems and -y was used without --force-yes WARNING: installation of vim-gnome failed Then, if there is network, the installation is successful after the update. This doesn't seem me that bad, even though I think that explaining to the user what's happening in that case would be a nice bonus. Cheers _______________________________________________ tails-dev mailing list [email protected] https://mailman.boum.org/listinfo/tails-dev
