Re: [Tails-dev] Persistence: display nicer paths

2013-12-25 Thread intrigeri
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

2013-12-22 Thread Andres Gomez Ramirez
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

2013-12-19 Thread intrigeri
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

2013-12-18 Thread Andres Gomez Ramirez
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

2013-12-15 Thread intrigeri
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

2013-11-22 Thread intrigeri
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