lei: forget-external support with canonicalization

For proper matching, we'll do a better job canonicalizing
URLs and path names for matching.  Of course, users may edit
the file outside of lei, so ensure we try both the canonicalized
and as-is form provided by the user.

I also don't think we'll need to store externals info in
MiscIdx; just the config file is fine.

diff --git a/MANIFEST b/MANIFEST
index 0de1de4a1a06ef229f7034cb4307c4d286f7af6b..ddee15398b1ca5d0e5c1dd58ecd04491d153a58a 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -339,6 +339,7 @@ t/kqnotify.t
 t/lei-oneshot.t
 t/lei.t
 t/lei_dedupe.t
+t/lei_external.t
 t/lei_overview.t
 t/lei_store.t
 t/lei_to_mail.t

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 37b45a001a29ca0ccc1e305a689a270e59e73f0a..9c3d7279deffb53521229e91a62c74cd1ef2a126 100644 (file)
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -21,7 +21,7 @@ use PublicInbox::Config;
 use PublicInbox::Syscall qw(SFD_NONBLOCK EPOLLIN EPOLLET);
 use PublicInbox::Sigfd;
 use PublicInbox::DS qw(now dwaitpid);
-use PublicInbox::Spawn qw(spawn run_die popen_rd);
+use PublicInbox::Spawn qw(spawn popen_rd);
 use PublicInbox::OnDestroy;
 use Text::Wrap qw(wrap);
 use File::Path qw(mkpath);
@@ -95,7 +95,7 @@ our %CMD = ( # sorted in order of importance/use:
        qw(boost=i quiet|q) ],
 'ls-external' => [ '[FILTER...]', 'list publicinbox|extindex locations',
        qw(format|f=s z|0 local remote quiet|q) ],
-'forget-external' => [ '{URL_OR_PATHNAME|--prune}',
+'forget-external' => [ 'URL_OR_PATHNAME...|--prune',
        'exclude further results from a publicinbox|extindex',
        qw(prune quiet|q) ],
 
@@ -114,7 +114,7 @@ our %CMD = ( # sorted in order of importance/use:
        "exclude message(s) on stdin from `q' search results",
        qw(stdin| oid=s exact by-mid|mid:s quiet|q) ],
 
-'purge-mailsource' => [ '{URL_OR_PATHNAME|--all}',
+'purge-mailsource' => [ 'URL_OR_PATHNAME|--all',
        'remove imported messages from IMAP, Maildirs, and MH',
        qw(exact! all jobs:i indexed) ],
 
@@ -137,7 +137,7 @@ our %CMD = ( # sorted in order of importance/use:
 'forget-watch' => [ '{WATCH_NUMBER|--prune}', 'stop and forget a watch',
        qw(prune) ],
 
-'import' => [ '{URL_OR_PATHNAME|--stdin}',
+'import' => [ 'URL_OR_PATHNAME|--stdin',
        'one-shot import/update from URL or filesystem',
        qw(stdin| offset=i recursive|r exclude=s include=s !flags),
        ],
@@ -468,6 +468,7 @@ sub optparse ($$$) {
                                        last;
                                } # else continue looping
                        }
+                       last if $ok;
                        my $last = pop @or;
                        $err = join(', ', @or) . " or $last must be set";
                } else {
@@ -547,16 +548,23 @@ sub lei_mark {
        my ($self, @argv) = @_;
 }
 
-sub lei_config {
+sub _config {
        my ($self, @argv) = @_;
-       $self->{opt}->{'config-file'} and return fail $self,
-               "config file switches not supported by `lei config'";
        my $env = $self->{env};
        delete local $env->{GIT_CONFIG};
+       delete local $ENV{GIT_CONFIG};
        my $cfg = _lei_cfg($self, 1);
        my $cmd = [ qw(git config -f), $cfg->{'-f'}, @argv ];
        my %rdr = map { $_ => $self->{$_} } (0..2);
-       run_die($cmd, $env, \%rdr);
+       waitpid(spawn($cmd, $env, \%rdr), 0);
+}
+
+sub lei_config {
+       my ($self, @argv) = @_;
+       $self->{opt}->{'config-file'} and return fail $self,
+               "config file switches not supported by `lei config'";
+       _config(@_);
+       x_it($self, $?) if $?;
 }
 
 sub lei_init {

diff --git a/lib/PublicInbox/LeiExternal.pm b/lib/PublicInbox/LeiExternal.pm
index 64faf5a08b2b61b6d6d9d41ce78f5ff21cd8f0c8..21071058a1a43d9aa6823b9092d85527334bd41e 100644 (file)
--- a/lib/PublicInbox/LeiExternal.pm
+++ b/lib/PublicInbox/LeiExternal.pm
@@ -7,6 +7,7 @@ use strict;
 use v5.10.1;
 use parent qw(Exporter);
 our @EXPORT = qw(lei_ls_external lei_add_external lei_forget_external);
+use PublicInbox::Config;
 
 sub _externals_each {
        my ($self, $cb, @arg) = @_;
@@ -30,7 +31,6 @@ sub _externals_each {
 
 sub lei_ls_external {
        my ($self, @argv) = @_;
-       my $stor = $self->_lei_store(0);
        my $out = $self->{1};
        my ($OFS, $ORS) = $self->{opt}->{z} ? ("\0", "\0\0") : (" ", "\n");
        $self->_externals_each(sub {
@@ -39,24 +39,58 @@ sub lei_ls_external {
        });
 }
 
+sub _canonicalize {
+       my ($location) = @_;
+       if ($location !~ m!\Ahttps?://!) {
+               PublicInbox::Config::rel2abs_collapsed($location);
+       } else {
+               require URI;
+               my $uri = URI->new($location)->canonical;
+               my $path = $uri->path . '/';
+               $path =~ tr!/!/!s; # squeeze redundant '/'
+               $uri->path($path);
+               $uri->as_string;
+       }
+}
+
 sub lei_add_external {
-       my ($self, $url_or_dir) = @_;
+       my ($self, $location) = @_;
        my $cfg = $self->_lei_cfg(1);
-       if ($url_or_dir !~ m!\Ahttps?://!) {
-               $url_or_dir = File::Spec->canonpath($url_or_dir);
-       }
        my $new_boost = $self->{opt}->{boost} // 0;
-       my $key = "external.$url_or_dir.boost";
+       $location = _canonicalize($location);
+       my $key = "external.$location.boost";
        my $cur_boost = $cfg->{$key};
        return if defined($cur_boost) && $cur_boost == $new_boost; # idempotent
        $self->lei_config($key, $new_boost);
-       my $stor = $self->_lei_store(1);
-       # TODO: add to MiscIdx
-       $stor->done;
+       $self->_lei_store(1)->done; # just create the store
 }
 
 sub lei_forget_external {
-       # TODO
+       my ($self, @locations) = @_;
+       my $cfg = $self->_lei_cfg(1);
+       my $quiet = $self->{opt}->{quiet};
+       for my $loc (@locations) {
+               my (@unset, @not_found);
+               for my $l ($loc, _canonicalize($loc)) {
+                       my $key = "external.$l.boost";
+                       delete($cfg->{$key});
+                       $self->_config('--unset', $key);
+                       if ($? == 0) {
+                               push @unset, $key;
+                       } elsif (($? >> 8) == 5) {
+                               push @not_found, $key;
+                       } else {
+                               $self->err("# --unset $key error");
+                               return $self->x_it($?);
+                       }
+               }
+               if (@unset) {
+                       next if $quiet;
+                       $self->err("# $_ unset") for @unset;
+               } elsif (@not_found) {
+                       $self->err("# $_ not found") for @not_found;
+               } # else { already exited
+       }
 }
 
 1;

diff --git a/t/lei.t b/t/lei.t
index ef820fe3f0d046769dfb86035b942519330bdf5c..50ad2bb10a4f94f79967946320545f5e9ac58490 100644 (file)
--- a/t/lei.t
+++ b/t/lei.t
@@ -180,6 +180,15 @@ my $test_external = sub {
        });
        $lei->('ls-external');
        like($out, qr/boost=0\n/s, 'ls-external has output');
+       ok($lei->(qw(add-external -q https://EXAMPLE.com/ibx)), 'add remote');
+       is($err, '', 'no warnings after add-external');
+       $lei->('ls-external');
+       like($out, qr!https://example\.com/ibx/!s, 'added canonical URL');
+       is($err, '', 'no warnings on ls-external');
+       ok($lei->(qw(forget-external -q https://EXAMPLE.com/ibx)),
+               'forget');
+       $lei->('ls-external');
+       unlike($out, qr!https://example\.com/ibx/!s, 'removed canonical URL');
 
        ok(!$lei->(qw(q s:prefix -o /dev/null -f maildir)), 'bad maildir');
        like($err, qr!/dev/null exists and is not a directory!,

diff --git a/t/lei_external.t b/t/lei_external.t
new file mode 100644 (file)
index 0000000..1f0048a
--- /dev/null
+++ b/t/lei_external.t
@@ -0,0 +1,18 @@
+#!perl -w
+use strict;
+use v5.10.1;
+use Test::More;
+my $cls = 'PublicInbox::LeiExternal';
+require_ok $cls;
+my $canon = $cls->can('_canonicalize');
+my $exp = 'https://example.com/my-inbox/';
+is($canon->('https://example.com/my-inbox'), $exp, 'trailing slash added');
+is($canon->('https://example.com/my-inbox//'), $exp, 'trailing slash removed');
+is($canon->('https://example.com//my-inbox/'), $exp, 'leading slash removed');
+is($canon->('https://EXAMPLE.com/my-inbox/'), $exp, 'lowercased');
+is($canon->('/this/path/is/nonexistent/'), '/this/path/is/nonexistent',
+       'non-existent pathname canonicalized');
+is($canon->('/this//path/'), '/this/path', 'extra slashes gone');
+is($canon->('/ALL/CAPS'), '/ALL/CAPS', 'caps preserved');
+
+done_testing;