[Patch v2 1/4] nmbug: use dump --format=batch-tag

2013-02-21 Thread Tomi Ollila 
On Thu, Feb 21 2013, David Bremner  wrote:

> Tomi Ollila  writes:
>
>>
>> This patch series looks good to me (as far as I can understand, I did
>> not find the "silly bug" in your previous patch...)
>>
>
> Compare :
>
> +print $notmuch $A_action.$TAGPREFIX.$pair->{tag}, " -- ",
> +  'id:'.$pair->{id};
>
> vs.
>
> +print $notmuch $D_action.$TAGPREFIX.$pair->{tag},
> +  'id:'.$pair->{id};

Ah, ok :)

> I obviously did not test that version very well.

IIRC that has happened to some other people, too ;(

>> Instead of mentioning that "calls are _not_ passed to shell" here,
>> that could be briefly mentioned just before system() calls in the
>> script -- and that definitely should not be 'At the moment' feature.
>
> Sure, the "At the moment" is meant to modify "relies". In the next
> patch, we stop relying on this feature of Perl.

Well, one have to expect the execution to happen one way or another
(and arrange quoting accordingly); for example:

$ echo; perl -e 'exec q/echo "foobar"/'

foobar

vs.

$ echo; perl -e 'exec qw/echo "foobar"/'

"foobar"

first gave exec one arg, 'echo "foobar"' and due to "":s perl used sh
to execute it, second gave exec list [ 'echo', '"foobar"' ] and therefore
echo got foobar with quotes as a command line argument.

Anyone interested for more information, see 'perldoc -f exec'.


>> the system() function in perl(1) never pass execution through the
>> shell in case the args are list more than one arg:
>
> right. that's what I meant by "is a documented feature of perl's system
> function"
>
> So I think we mean to say the same thing here; but I could I add a
> comment in this patch (to delete it in the next). Or maybe reword that
> commit message somehow.

Yes, the message is the same, just that there should not be need for that..

> d

Tomi

[Patch v2 1/4] nmbug: use dump --format=batch-tag

2013-02-21 Thread Tomi Ollila 
On Thu, Feb 21 2013, david at tethera.net wrote:

> From: David Bremner 
>
> This should make nmbug tolerate tags with whitespace and other special
> characters it.  At the moment this relies on _not_ passing calls to
> notmuch tag through the shell, which is a documented feature of perl's
> system function.

This patch series looks good to me (as far as I can understand, I did
not find the "silly bug" in your previous patch...)

Instead of mentioning that "calls are _not_ passed to shell" here,
that could be briefly mentioned just before system() calls in the
script -- and that definitely should not be 'At the moment' feature.

the system() function in perl(1) never pass execution through the
shell in case the args are list more than one arg:

1 $ perl -e 'system( qw/echo |cat; echo second line ?/ )'
|cat; echo second line ?

2 $ perl -le 'system( qw/echo| cat;/ ); print "system: ",$!'
system: No such file or directory

3 $ perl -le 'system( qw/echo|/ ); print "system: ",$!'
sh: -c: line 1: syntax error: unexpected end of file
system:

execve calls to the above could have been:

1: execve( "/bin/echo" [ "echo" "|cat;" "echo" "second" "line" "?" ] )

2: no execve: perl(1) did not find 'echo|' in the PATH

3: execve( "/bin/sh" [ "sh" "-c" "echo|" ] )

Tomi

> ---
>  devel/nmbug/nmbug |   27 ---
>  1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/devel/nmbug/nmbug b/devel/nmbug/nmbug
> index fe103b3..befc3d9 100755
> --- a/devel/nmbug/nmbug
> +++ b/devel/nmbug/nmbug
> @@ -39,6 +39,11 @@ my %command = (
>status => \_status,
>);
>  
> +# Convert prefix into form suitable for literal matching against
> +# notmuch dump --format=batch-tag output.
> +my $ENCPREFIX = encode_for_fs ($TAGPREFIX);
> +$ENCPREFIX =~ s/:/%3a/g;
> +
>  my $subcommand = shift || usage ();
>  
>  if (!exists $command{$subcommand}) {
> @@ -203,9 +208,9 @@ sub index_tags {
>  
>my $index = $NMBGIT.'/nmbug.index';
>  
> -  my $query = join ' ', map ("tag:$_", get_tags ($TAGPREFIX));
> +  my $query = join ' ', map ("tag:\"$_\"", get_tags ($TAGPREFIX));
>  
> -  my $fh = spawn ('-|', qw/notmuch dump --/, $query)
> +  my $fh = spawn ('-|', qw/notmuch dump --format=batch-tag --/, $query)
>  or die "notmuch dump: $!";
>  
>git ('read-tree', '--empty');
> @@ -214,22 +219,30 @@ sub index_tags {
>  or die 'git update-index';
>  
>while (<$fh>) {
> -m/ ( [^ ]* ) \s+ \( ([^\)]* ) \) /x || die 'syntax error in dump';
> -my ($id,$rest) = ($1,$2);
>  
> -#strip prefixes before writing
> -my @tags = grep { s/^$TAGPREFIX//; } split (' ', $rest);
> +chomp();
> +my ($rest,$id) = split(/ -- id:/);
> +
> +if ($id =~ s/^"(.*)"\s*$/$1/) {
> +  # xapian quoted string, dequote.
> +  $id =~ s/""/"/g;
> +}
> +
> +#strip prefixes from tags before writing
> +my @tags = grep { s/^[+]$ENCPREFIX//; } split (' ', $rest);
>  index_tags_for_msg ($git,$id, 'A', @tags);
>}
>unless (close $git) {
>  die "'git update-index --index-info' exited with nonzero value\n";
>}
>unless (close $fh) {
> -die "'notmuch dump -- $query' exited with nonzero value\n";
> +die "'notmuch dump --format=batch-tag -- $query' exited with nonzero 
> value\n";
>}
>return $index;
>  }
>  
> +# update the git index to either create or delete an empty file.
> +# Neither argument should be encoded/escaped.
>  sub index_tags_for_msg {
>my $fh = shift;
>my $msgid = shift;
> -- 
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[Patch v2 1/4] nmbug: use dump --format=batch-tag

2013-02-21 Thread David Bremner 
Tomi Ollila  writes:

>
> This patch series looks good to me (as far as I can understand, I did
> not find the "silly bug" in your previous patch...)
>

Compare :

+print $notmuch $A_action.$TAGPREFIX.$pair->{tag}, " -- ",
+  'id:'.$pair->{id};

vs.

+print $notmuch $D_action.$TAGPREFIX.$pair->{tag},
+  'id:'.$pair->{id};

I obviously did not test that version very well.

> Instead of mentioning that "calls are _not_ passed to shell" here,
> that could be briefly mentioned just before system() calls in the
> script -- and that definitely should not be 'At the moment' feature.

Sure, the "At the moment" is meant to modify "relies". In the next
patch, we stop relying on this feature of Perl.

>
> the system() function in perl(1) never pass execution through the
> shell in case the args are list more than one arg:
>

right. that's what I meant by "is a documented feature of perl's system
function"

So I think we mean to say the same thing here; but I could I add a
comment in this patch (to delete it in the next). Or maybe reword that
commit message somehow.

d

Re: [Patch v2 1/4] nmbug: use dump --format=batch-tag

2013-02-21 Thread Tomi Ollila 
On Thu, Feb 21 2013, da...@tethera.net wrote:

 From: David Bremner brem...@debian.org

 This should make nmbug tolerate tags with whitespace and other special
 characters it.  At the moment this relies on _not_ passing calls to
 notmuch tag through the shell, which is a documented feature of perl's
 system function.

This patch series looks good to me (as far as I can understand, I did
not find the silly bug in your previous patch...)

Instead of mentioning that calls are _not_ passed to shell here,
that could be briefly mentioned just before system() calls in the
script -- and that definitely should not be 'At the moment' feature.

the system() function in perl(1) never pass execution through the
shell in case the args are list more than one arg:

1 $ perl -e 'system( qw/echo |cat; echo second line ?/ )'
|cat; echo second line ?

2 $ perl -le 'system( qw/echo| cat;/ ); print system: ,$!'
system: No such file or directory

3 $ perl -le 'system( qw/echo|/ ); print system: ,$!'
sh: -c: line 1: syntax error: unexpected end of file
system:

execve calls to the above could have been:

1: execve( /bin/echo [ echo |cat; echo second line ? ] )

2: no execve: perl(1) did not find 'echo|' in the PATH

3: execve( /bin/sh [ sh -c echo| ] )

Tomi

 ---
  devel/nmbug/nmbug |   27 ---
  1 file changed, 20 insertions(+), 7 deletions(-)

 diff --git a/devel/nmbug/nmbug b/devel/nmbug/nmbug
 index fe103b3..befc3d9 100755
 --- a/devel/nmbug/nmbug
 +++ b/devel/nmbug/nmbug
 @@ -39,6 +39,11 @@ my %command = (
status = \do_status,
);
  
 +# Convert prefix into form suitable for literal matching against
 +# notmuch dump --format=batch-tag output.
 +my $ENCPREFIX = encode_for_fs ($TAGPREFIX);
 +$ENCPREFIX =~ s/:/%3a/g;
 +
  my $subcommand = shift || usage ();
  
  if (!exists $command{$subcommand}) {
 @@ -203,9 +208,9 @@ sub index_tags {
  
my $index = $NMBGIT.'/nmbug.index';
  
 -  my $query = join ' ', map (tag:$_, get_tags ($TAGPREFIX));
 +  my $query = join ' ', map (tag:\$_\, get_tags ($TAGPREFIX));
  
 -  my $fh = spawn ('-|', qw/notmuch dump --/, $query)
 +  my $fh = spawn ('-|', qw/notmuch dump --format=batch-tag --/, $query)
  or die notmuch dump: $!;
  
git ('read-tree', '--empty');
 @@ -214,22 +219,30 @@ sub index_tags {
  or die 'git update-index';
  
while ($fh) {
 -m/ ( [^ ]* ) \s+ \( ([^\)]* ) \) /x || die 'syntax error in dump';
 -my ($id,$rest) = ($1,$2);
  
 -#strip prefixes before writing
 -my @tags = grep { s/^$TAGPREFIX//; } split (' ', $rest);
 +chomp();
 +my ($rest,$id) = split(/ -- id:/);
 +
 +if ($id =~ s/^(.*)\s*$/$1/) {
 +  # xapian quoted string, dequote.
 +  $id =~ s///g;
 +}
 +
 +#strip prefixes from tags before writing
 +my @tags = grep { s/^[+]$ENCPREFIX//; } split (' ', $rest);
  index_tags_for_msg ($git,$id, 'A', @tags);
}
unless (close $git) {
  die 'git update-index --index-info' exited with nonzero value\n;
}
unless (close $fh) {
 -die 'notmuch dump -- $query' exited with nonzero value\n;
 +die 'notmuch dump --format=batch-tag -- $query' exited with nonzero 
 value\n;
}
return $index;
  }
  
 +# update the git index to either create or delete an empty file.
 +# Neither argument should be encoded/escaped.
  sub index_tags_for_msg {
my $fh = shift;
my $msgid = shift;
 -- 
 1.7.10.4

 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [Patch v2 1/4] nmbug: use dump --format=batch-tag

2013-02-21 Thread David Bremner 
Tomi Ollila tomi.oll...@iki.fi writes:


 This patch series looks good to me (as far as I can understand, I did
 not find the silly bug in your previous patch...)


Compare :

+print $notmuch $A_action.$TAGPREFIX.$pair-{tag},  -- ,
+  'id:'.$pair-{id};

vs.

+print $notmuch $D_action.$TAGPREFIX.$pair-{tag},
+  'id:'.$pair-{id};

I obviously did not test that version very well.

 Instead of mentioning that calls are _not_ passed to shell here,
 that could be briefly mentioned just before system() calls in the
 script -- and that definitely should not be 'At the moment' feature.

Sure, the At the moment is meant to modify relies. In the next
patch, we stop relying on this feature of Perl.


 the system() function in perl(1) never pass execution through the
 shell in case the args are list more than one arg:


right. that's what I meant by is a documented feature of perl's system
function

So I think we mean to say the same thing here; but I could I add a
comment in this patch (to delete it in the next). Or maybe reword that
commit message somehow.

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [Patch v2 1/4] nmbug: use dump --format=batch-tag

2013-02-21 Thread Tomi Ollila 
On Thu, Feb 21 2013, David Bremner da...@tethera.net wrote:

 Tomi Ollila tomi.oll...@iki.fi writes:


 This patch series looks good to me (as far as I can understand, I did
 not find the silly bug in your previous patch...)


 Compare :

 +print $notmuch $A_action.$TAGPREFIX.$pair-{tag},  -- ,
 +  'id:'.$pair-{id};

 vs.

 +print $notmuch $D_action.$TAGPREFIX.$pair-{tag},
 +  'id:'.$pair-{id};

Ah, ok :)

 I obviously did not test that version very well.

IIRC that has happened to some other people, too ;(

 Instead of mentioning that calls are _not_ passed to shell here,
 that could be briefly mentioned just before system() calls in the
 script -- and that definitely should not be 'At the moment' feature.

 Sure, the At the moment is meant to modify relies. In the next
 patch, we stop relying on this feature of Perl.

Well, one have to expect the execution to happen one way or another
(and arrange quoting accordingly); for example:

$ echo; perl -e 'exec q/echo foobar/'

foobar

vs.

$ echo; perl -e 'exec qw/echo foobar/'

foobar

first gave exec one arg, 'echo foobar' and due to :s perl used sh
to execute it, second gave exec list [ 'echo', 'foobar' ] and therefore
echo got foobar with quotes as a command line argument.

Anyone interested for more information, see 'perldoc -f exec'.


 the system() function in perl(1) never pass execution through the
 shell in case the args are list more than one arg:

 right. that's what I meant by is a documented feature of perl's system
 function

 So I think we mean to say the same thing here; but I could I add a
 comment in this patch (to delete it in the next). Or maybe reword that
 commit message somehow.

Yes, the message is the same, just that there should not be need for that..

 d

Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[Patch v2 1/4] nmbug: use dump --format=batch-tag

2013-02-20 Thread da...@tethera.net 
From: David Bremner 

This should make nmbug tolerate tags with whitespace and other special
characters it.  At the moment this relies on _not_ passing calls to
notmuch tag through the shell, which is a documented feature of perl's
system function.
---
 devel/nmbug/nmbug |   27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/devel/nmbug/nmbug b/devel/nmbug/nmbug
index fe103b3..befc3d9 100755
--- a/devel/nmbug/nmbug
+++ b/devel/nmbug/nmbug
@@ -39,6 +39,11 @@ my %command = (
 status => \_status,
 );

+# Convert prefix into form suitable for literal matching against
+# notmuch dump --format=batch-tag output.
+my $ENCPREFIX = encode_for_fs ($TAGPREFIX);
+$ENCPREFIX =~ s/:/%3a/g;
+
 my $subcommand = shift || usage ();

 if (!exists $command{$subcommand}) {
@@ -203,9 +208,9 @@ sub index_tags {

   my $index = $NMBGIT.'/nmbug.index';

-  my $query = join ' ', map ("tag:$_", get_tags ($TAGPREFIX));
+  my $query = join ' ', map ("tag:\"$_\"", get_tags ($TAGPREFIX));

-  my $fh = spawn ('-|', qw/notmuch dump --/, $query)
+  my $fh = spawn ('-|', qw/notmuch dump --format=batch-tag --/, $query)
 or die "notmuch dump: $!";

   git ('read-tree', '--empty');
@@ -214,22 +219,30 @@ sub index_tags {
 or die 'git update-index';

   while (<$fh>) {
-m/ ( [^ ]* ) \s+ \( ([^\)]* ) \) /x || die 'syntax error in dump';
-my ($id,$rest) = ($1,$2);

-#strip prefixes before writing
-my @tags = grep { s/^$TAGPREFIX//; } split (' ', $rest);
+chomp();
+my ($rest,$id) = split(/ -- id:/);
+
+if ($id =~ s/^"(.*)"\s*$/$1/) {
+  # xapian quoted string, dequote.
+  $id =~ s/""/"/g;
+}
+
+#strip prefixes from tags before writing
+my @tags = grep { s/^[+]$ENCPREFIX//; } split (' ', $rest);
 index_tags_for_msg ($git,$id, 'A', @tags);
   }
   unless (close $git) {
 die "'git update-index --index-info' exited with nonzero value\n";
   }
   unless (close $fh) {
-die "'notmuch dump -- $query' exited with nonzero value\n";
+die "'notmuch dump --format=batch-tag -- $query' exited with nonzero 
value\n";
   }
   return $index;
 }

+# update the git index to either create or delete an empty file.
+# Neither argument should be encoded/escaped.
 sub index_tags_for_msg {
   my $fh = shift;
   my $msgid = shift;
-- 
1.7.10.4

[Patch v2 1/4] nmbug: use dump --format=batch-tag

2013-02-20 Thread david 
From: David Bremner brem...@debian.org

This should make nmbug tolerate tags with whitespace and other special
characters it.  At the moment this relies on _not_ passing calls to
notmuch tag through the shell, which is a documented feature of perl's
system function.
---
 devel/nmbug/nmbug |   27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/devel/nmbug/nmbug b/devel/nmbug/nmbug
index fe103b3..befc3d9 100755
--- a/devel/nmbug/nmbug
+++ b/devel/nmbug/nmbug
@@ -39,6 +39,11 @@ my %command = (
 status = \do_status,
 );
 
+# Convert prefix into form suitable for literal matching against
+# notmuch dump --format=batch-tag output.
+my $ENCPREFIX = encode_for_fs ($TAGPREFIX);
+$ENCPREFIX =~ s/:/%3a/g;
+
 my $subcommand = shift || usage ();
 
 if (!exists $command{$subcommand}) {
@@ -203,9 +208,9 @@ sub index_tags {
 
   my $index = $NMBGIT.'/nmbug.index';
 
-  my $query = join ' ', map (tag:$_, get_tags ($TAGPREFIX));
+  my $query = join ' ', map (tag:\$_\, get_tags ($TAGPREFIX));
 
-  my $fh = spawn ('-|', qw/notmuch dump --/, $query)
+  my $fh = spawn ('-|', qw/notmuch dump --format=batch-tag --/, $query)
 or die notmuch dump: $!;
 
   git ('read-tree', '--empty');
@@ -214,22 +219,30 @@ sub index_tags {
 or die 'git update-index';
 
   while ($fh) {
-m/ ( [^ ]* ) \s+ \( ([^\)]* ) \) /x || die 'syntax error in dump';
-my ($id,$rest) = ($1,$2);
 
-#strip prefixes before writing
-my @tags = grep { s/^$TAGPREFIX//; } split (' ', $rest);
+chomp();
+my ($rest,$id) = split(/ -- id:/);
+
+if ($id =~ s/^(.*)\s*$/$1/) {
+  # xapian quoted string, dequote.
+  $id =~ s///g;
+}
+
+#strip prefixes from tags before writing
+my @tags = grep { s/^[+]$ENCPREFIX//; } split (' ', $rest);
 index_tags_for_msg ($git,$id, 'A', @tags);
   }
   unless (close $git) {
 die 'git update-index --index-info' exited with nonzero value\n;
   }
   unless (close $fh) {
-die 'notmuch dump -- $query' exited with nonzero value\n;
+die 'notmuch dump --format=batch-tag -- $query' exited with nonzero 
value\n;
   }
   return $index;
 }
 
+# update the git index to either create or delete an empty file.
+# Neither argument should be encoded/escaped.
 sub index_tags_for_msg {
   my $fh = shift;
   my $msgid = shift;
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch