Re: [Tails-dev] Persistence: display nicer paths
Andres Gomez Ramirez wrote (22 Dec 2013 12:40:59 GMT) : > Done, I attached two new patches to "Persistence: display nicer paths". Merged, pushed some minor fixes on top (please do have a look), released as Tails-perl5lib 0.6-1 and persistence-setup 1.0.4-1, will be in Tails 0.23. Thanks! ___ tails-dev mailing list tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev
Re: [Tails-dev] Persistence: display nicer paths
Done, I attached two new patches to "Persistence: display nicer paths". Cheers, Andres From: tails-dev-boun...@boum.org [tails-dev-boun...@boum.org] on behalf of intrigeri [intrig...@boum.org] Sent: 19 December 2013 12:38 To: The Tails public development discussion list Subject: Re: [Tails-dev] Persistence: display nicer paths Hi Andres, Andres Gomez Ramirez wrote (18 Dec 2013 22:20:48 GMT) : > Attached the second version, [...] Merged, great work! Added a few fixes on top (53cd9d1, 4f3f54f), that you surely want to look at and avoid the need thereof next time (that is: first, configure Git to use color in diffs, so that trailing whitespace jumps at your eye; second, run the test suite to make sure your changes don't break it :) Released as tails-persistence-setup 1.0.2, uploaded to devel and experimental, will be part of Tails 0.23. Congrats! I know you already committed to another task (some liveusb-creator bug IIRC), but as a follow-up on this one (#5311), you might want to take care of other places where the Persistent Volume Assistant displays ugly paths, e.g. in "Tails is running from non-USB / non-SDIO device %s." and other checks. It is certainly not critical, but I assume it should be pretty easy to wrap ->boot_device with something that returns DeviceFilePresentation in: my $message = $self->encoding->decode(sprintf( gettext($check->{message}), $self->boot_device)); Time to add a boot_device_file lazy-built attribute to Tails::RunningSystem in our perl5lib, perhaps? Cheers, -- intrigeri | GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc | OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc ___ tails-dev mailing list tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev From b1a669a8404d2a9dccad4de84d7711291afcf7ab Mon Sep 17 00:00:00 2001 From: kurono Date: Sun, 22 Dec 2013 13:30:22 +0100 Subject: [PATCH] persistence: display nicer paths - extend --- lib/Tails/RunningSystem.pm | 10 ++ 1 file changed, 10 insertions(+) diff --git a/lib/Tails/RunningSystem.pm b/lib/Tails/RunningSystem.pm index e7a5c93..4d6258c 100644 --- a/lib/Tails/RunningSystem.pm +++ b/lib/Tails/RunningSystem.pm @@ -63,6 +63,10 @@ has 'boot_device' => lazy_build rw Str, documentation => q{The UDI of the physical block device where Tails is installed, e.g. /org/freedesktop/UDisks/devices/sdb.}; +has 'boot_device_file' => +lazy_build rw Str, +documentation => q{The rute of the file where Tails is installed, e.g. /dev/sdb.}; + has 'system_partition' => lazy_build rw Str, documentation => q{The UDI of the partition where Tails is installed, e.g. /org/freedesktop/UDisks/devices/sdb1.}; @@ -145,6 +149,12 @@ sub _build_boot_device { return $device; } +sub _build_boot_device_file { +my $self = shift; + +$self->get_device_property($self->boot_device, 'DeviceFilePresentation'); +} + sub _build_system_partition { my $self = shift; -- 1.7.9.5 From 71feda6a517573d261bfcd9b219c53de7f2febfa Mon Sep 17 00:00:00 2001 From: kurono Date: Sun, 22 Dec 2013 13:35:49 +0100 Subject: [PATCH] persistence: display nicer paths - extend --- lib/Tails/Persistence/Setup.pm | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Tails/Persistence/Setup.pm b/lib/Tails/Persistence/Setup.pm index bf8b394..cefb080 100644 --- a/lib/Tails/Persistence/Setup.pm +++ b/lib/Tails/Persistence/Setup.pm @@ -81,7 +81,7 @@ has 'running_system' => lazy_build ro 'Tails::RunningSystem', metaclass => 'NoGetopt', handles => [ -qw{boot_device boot_device_model boot_device_vendor boot_device_size +qw{boot_device boot_device_model boot_device_vendor boot_device_size boot_device_file started_from_device_installed_with_tails_installer} ]; @@ -439,7 +439,7 @@ sub check_sanity { if (! $res) { my $message = $self->encoding->decode(sprintf( gettext($check->{message}), -$self->boot_device)); +$self->boot_device_file)); if ($self->force && exists($check->{can_be_forced}) && $check->{can_be_forced}) { warn "$message", "... but --force is enabled, ignoring results of this sanity check."; @@ -461,7 +461,7 @@ sub check_sanity { sub run { my $self = shift; -$self->debug(sprintf("Working on device %s", $self->boot_device)); +$self->debug(sprintf("Working on device %s", $self->boot_device_file)); # Force initialization in the correct order if ($Moose::VE
Re: [Tails-dev] Persistence: display nicer paths
Hi Andres, Andres Gomez Ramirez wrote (18 Dec 2013 22:20:48 GMT) : > Attached the second version, [...] Merged, great work! Added a few fixes on top (53cd9d1, 4f3f54f), that you surely want to look at and avoid the need thereof next time (that is: first, configure Git to use color in diffs, so that trailing whitespace jumps at your eye; second, run the test suite to make sure your changes don't break it :) Released as tails-persistence-setup 1.0.2, uploaded to devel and experimental, will be part of Tails 0.23. Congrats! I know you already committed to another task (some liveusb-creator bug IIRC), but as a follow-up on this one (#5311), you might want to take care of other places where the Persistent Volume Assistant displays ugly paths, e.g. in "Tails is running from non-USB / non-SDIO device %s." and other checks. It is certainly not critical, but I assume it should be pretty easy to wrap ->boot_device with something that returns DeviceFilePresentation in: my $message = $self->encoding->decode(sprintf( gettext($check->{message}), $self->boot_device)); Time to add a boot_device_file lazy-built attribute to Tails::RunningSystem in our perl5lib, perhaps? Cheers, -- intrigeri | GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc | OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc ___ tails-dev mailing list tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev
Re: [Tails-dev] Persistence: display nicer paths
Hi, Attached the second version, my comments: >> +sub _build_partition_file { >> +my $self = shift; >> + >> +my $device_file = $self->get_device_property($self->device, >> 'DeviceFile'); >> +my $partition_number = >> $self->get_device_property($self->persistence_partition, >> 'PartitionNumber'); >> + >> +$device_file.$partition_number; > This naming scheme is not correct for all kinds of supported devices, > e.g. SD cards plugged into a reader wired via SDIO. See commit > 83637f4e for details. Doesn't $self->persistence_partition have > a DeviceFile property that we could use instead of manually appending > the partition number this way? If it has, let's simply use it. Else, > using the info from commit 83637f4e should be good enough. Ok in this version of tails-persistence was possible to use simply DeviceFilePresentation :) Cheers, Andres From 66e863907f7ecb4e02823d3731cc6bd53eb8c7ed Mon Sep 17 00:00:00 2001 From: kurono Date: Wed, 18 Dec 2013 23:10:04 +0100 Subject: [PATCH] persistence: display nicer paths --- lib/Tails/Persistence/Setup.pm |9 + lib/Tails/Persistence/Step/Configure.pm |4 ++-- lib/Tails/Persistence/Step/Delete.pm|5 +++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/Tails/Persistence/Setup.pm b/lib/Tails/Persistence/Setup.pm index 431e71a..1944737 100644 --- a/lib/Tails/Persistence/Setup.pm +++ b/lib/Tails/Persistence/Setup.pm @@ -102,6 +102,7 @@ has 'main_window' => has "$_" => lazy_build ro Str for (qw{override_liveos_mountpoint override_boot_device override_system_partition}); +has 'persistence_partition_device_file'=> lazy_build ro Str, metaclass => 'NoGetopt'; has 'persistence_partition_size' => lazy_build ro Int, metaclass => 'NoGetopt'; has 'persistence_is_enabled' => lazy_build ro Bool, metaclass => 'NoGetopt'; has 'persistence_is_read_write' => lazy_build ro Bool, metaclass => 'NoGetopt'; @@ -270,6 +271,12 @@ sub _build_size_of_free_space { ); } +sub _build_persistence_partition_device_file { +my $self = shift; + +$self->get_device_property($self->persistence_partition, 'DeviceFilePresentation'); +} + sub _build_persistence_partition_size { my $self = shift; @@ -695,6 +702,7 @@ sub step_object_from_name { $self->delete_persistence_partition({ @_ }) }, persistence_partition => $self->persistence_partition, +persistence_partition_device_file => $self->persistence_partition_device_file, persistence_partition_size => $self->persistence_partition_size, ); } @@ -705,6 +713,7 @@ sub step_object_from_name { }, configuration => $self->configuration, persistence_partition => $self->persistence_partition, +persistence_partition_device_file => $self->persistence_partition_device_file, persistence_partition_size => $self->persistence_partition_size, ); } diff --git a/lib/Tails/Persistence/Step/Configure.pm b/lib/Tails/Persistence/Step/Configure.pm index 9f3d4a0..e2c33cf 100644 --- a/lib/Tails/Persistence/Step/Configure.pm +++ b/lib/Tails/Persistence/Step/Configure.pm @@ -31,7 +31,7 @@ textdomain("tails-persistence-setup"); has 'configuration' => required ro 'Tails::Persistence::Configuration'; -has 'persistence_partition' => required ro Str; +has 'persistence_partition_device_file' => required ro Str; has 'persistence_partition_size' => required ro Int; has 'list_box' => lazy_build ro 'Gtk2::VBox'; @@ -66,7 +66,7 @@ sub BUILD { $self->description->set_markup($self->encoding->decode(sprintf( # TRANSLATORS: partition, size, device vendor, device model gettext(q{The selected files will be stored in the encrypted partition %s (%s), on the %s %s device.}), -$self->persistence_partition, +$self->persistence_partition_device_file, format_bytes($self->persistence_partition_size, mode => "iec"), $self->device_vendor, $self->device_model diff --git a/lib/Tails/Persistence/Step/Delete.pm b/lib/Tails/Persistence/Step/Delete.pm index 747dadd..0b5857a 100644 --- a/lib/Tails/Persistence/Step/Delete.pm +++ b/lib/Tails/Persistence/Step/Delete.pm @@ -25,10 +25,11 @@ textdomain("tails-persistence-setup"); =cut -has 'persistence_partition' => required ro Str; +has 'persistence_partition_device_file' => required ro Str; has 'persistence_partition_size' => required ro Int; has 'warning_icon' => lazy_build rw 'Gtk2::Image'; + =head1 CONSTRUCTORS =cut @@ -45,7 +46,7 @@ sub BUILD { # TRANSLATORS: partition, size, device vendor, device model $self->description->set_markup($self->encoding->decode(sprintf( gettext(q{The persistent volume %s (%s), on the %s %s device, will be deleted.}), -$self->persistence_partition, +$self->persistence_partition
Re: [Tails-dev] Persistence: display nicer paths
Hi, intrigeri wrote (22 Nov 2013 21:21:44 GMT) : > Andres Gomez Ramirez wrote (21 Nov 2013 16:03:58 GMT) : >> Ok no problem, attached is the patch for "Persistence: display nicer paths" >> feature -> https://labs.riseup.net/code/issues/5311. > Cool, thanks. I'm very happy to see someone else than me starting to > touch our Perl code in non-trivial ways :) > Here's an initial review. > First, it seems to me that this patch is based on an older, > pre-split-to-tails-perl5lib, version of tails-persistence-setup. > First thing is then to rebase it on top of the current master branch. > Sorry for the bad timing :/ >> +has 'partition_file' => >> +lazy_build rw Str, >> +documentation => q{The persistent partition file, e.g. /dev/sdb1.}; > This attribute's name seems unclear to me. First, it should be > persistence_partition_* for consistency with other similar attributes. > For the rest of the name, I have no particularly awesome proposal, > perhaps simply persistence_partition_device_file, to match > udisks' wording? > Also, this attribute should have the NoGetopt metaclass, since it is > not suitable to be set on the command-line. >> +sub _build_partition_file { >> +my $self = shift; >> + >> +my $device_file = $self->get_device_property($self->device, >> 'DeviceFile'); >> +my $partition_number = >> $self->get_device_property($self->persistence_partition, >> 'PartitionNumber'); >> + >> +$device_file.$partition_number; > This naming scheme is not correct for all kinds of supported devices, > e.g. SD cards plugged into a reader wired via SDIO. See commit > 83637f4e for details. Doesn't $self->persistence_partition have > a DeviceFile property that we could use instead of manually appending > the partition number this way? If it has, let's simply use it. Else, > using the info from commit 83637f4e should be good enough. > Also, DeviceFilePresentation might be nicer in some cases that we > might want to support in the future, so I would pick that one > personally instead of DeviceFile, if it is supported by > Squeeze's udisks. >> has 'persistence_partition_size' => required ro Int; >> - >> +has 'partition_file' => required ro Str; > The two empty lines between attributes and constructors was there on > purpose. I'm not pretending the whole codebase is consistent, but I'm > trying to have two empty lines before each =head1. Refraining from > doing unrelated whitespace changes in a patch is generally a good > thing, anyway. > I can't wait to see the second iteration! :) Ping? (No big emergency, as this is material for 0.23, that should freeze in February.) Cheers, -- intrigeri | GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc | OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc ___ tails-dev mailing list tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev
Re: [Tails-dev] Persistence: display nicer paths
Hi, Andres Gomez Ramirez wrote (21 Nov 2013 16:03:58 GMT) : > Ok no problem, attached is the patch for "Persistence: display nicer paths" > feature -> https://labs.riseup.net/code/issues/5311. Cool, thanks. I'm very happy to see someone else than me starting to touch our Perl code in non-trivial ways :) Here's an initial review. First, it seems to me that this patch is based on an older, pre-split-to-tails-perl5lib, version of tails-persistence-setup. First thing is then to rebase it on top of the current master branch. Sorry for the bad timing :/ > +has 'partition_file' => > +lazy_build rw Str, > +documentation => q{The persistent partition file, e.g. /dev/sdb1.}; This attribute's name seems unclear to me. First, it should be persistence_partition_* for consistency with other similar attributes. For the rest of the name, I have no particularly awesome proposal, perhaps simply persistence_partition_device_file, to match udisks' wording? Also, this attribute should have the NoGetopt metaclass, since it is not suitable to be set on the command-line. > +sub _build_partition_file { > +my $self = shift; > + > +my $device_file = $self->get_device_property($self->device, > 'DeviceFile'); > +my $partition_number = > $self->get_device_property($self->persistence_partition, > 'PartitionNumber'); > + > +$device_file.$partition_number; This naming scheme is not correct for all kinds of supported devices, e.g. SD cards plugged into a reader wired via SDIO. See commit 83637f4e for details. Doesn't $self->persistence_partition have a DeviceFile property that we could use instead of manually appending the partition number this way? If it has, let's simply use it. Else, using the info from commit 83637f4e should be good enough. Also, DeviceFilePresentation might be nicer in some cases that we might want to support in the future, so I would pick that one personally instead of DeviceFile, if it is supported by Squeeze's udisks. > has 'persistence_partition_size' => required ro Int; > - > +has 'partition_file' => required ro Str; The two empty lines between attributes and constructors was there on purpose. I'm not pretending the whole codebase is consistent, but I'm trying to have two empty lines before each =head1. Refraining from doing unrelated whitespace changes in a patch is generally a good thing, anyway. I can't wait to see the second iteration! :) Cheers, -- intrigeri | GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc | OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc ___ tails-dev mailing list tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev