]> git.vanrenterghem.biz Git - git.ikiwiki.info.git/blob - doc/todo/Attempt_to_extend_Mercurial_backend_support.mdwn
Merge commit '3.20110715' into prv/po
[git.ikiwiki.info.git] / doc / todo / Attempt_to_extend_Mercurial_backend_support.mdwn
1 Using the Mercurial backend, the lack of `rcs_commit_staged` is noticed
2 frequently. I couldn't find any tries to update `mercurial.pm`, so not
3 letting lack of Mercurial AND Perl knowledge bring me down, I copy-pasted
4 from `git.pm` to mimic its behaviour from a Mercurial perspective. I hope
5 it can be a foundation for development by those more proficient in
6 ikiwiki's inner workings. I have doubts that I personally will be able to
7 revise it more, based on my Perl skills.
9 I've tested it briefly. `ikiwiki-calendar` and posting of comments now
10 works with automatic commits, i.e. the `rcs_commit_staged` function works
11 in those cases. Under my current setup, I don't know where else to expect
12 it to work. I would be flabberghasted if there wasn't any problems with it,
13 though.
15 > Absolutely, the [[/rcs]] chart shows mercurial is lagging behind
16 > nearly everything. 
17
18 > I don't think this stuff is hard, or unlikely to work, familiarity with
19 > the rcs's particular details is the main thing. --[[Joey]] 
21 Diff follows, for anyone to annotate. First code version is also available at [my hg-repo](http://510x.se/hg/program/ikiwiki/file/e741fcfd800f/Plugin/mercurial.pm). Latest version should be [here](http://46.239.104.5:81/hg/program/ikiwiki/file/tip/Plugin/mercurial.pm) ([raw format](http://46.239.104.5:81/hg/program/ikiwiki/raw-file/tip/Plugin/mercurial.pm)). I'll notify on this page with "*Done*" remarks when I've actually commited changes to my local repository. I don't know if I should replace the code and the comments below when I've changed something. I'll probably do this when the code feels more mature. --[[Daniel Andersson]]
23 > I've looked over the current version and it looks ok to me. --[[Joey]]
25         diff -r 20c61288d7bd Plugin/mercurial.pm
26         --- a/Plugin/mercurial.pm       Fri Jul 15 02:55:12 2011 +0200
27         +++ b/Plugin/mercurial.pm       Fri Jul 15 03:29:10 2011 +0200
28         @@ -7,6 +7,8 @@
29          use Encode;
30          use open qw{:utf8 :std};
31          
32         +my $hg_dir=undef;
33         +
34          sub import {
35                 hook(type => "checkconfig", id => "mercurial", call => \&checkconfig);
36                 hook(type => "getsetup", id => "mercurial", call => \&getsetup);
38 A corresponding variable is declared for git. It is unused as of yet for
39 Mercurial, but when more advanced merge features become available for
40 `mercurial.pm`, I think it will come into play.
42 > Maybe.. I'd rather avoid unused cruft though. --[[Joey]]
44 >> OK, will be removed. *Done* --[[Daniel Andersson]]
46         @@ -69,6 +71,62 @@
47                         },
48          }
49          
50         +sub safe_hg (&@) {
51         +       # Start a child process safely without resorting to /bin/sh.
52         +       # Returns command output (in list content) or success state
53         +       # (in scalar context), or runs the specified data handler.
54         +
55         +       my ($error_handler, $data_handler, @cmdline) = @_;
56         +
57         +       my $pid = open my $OUT, "-|";
58         +
59         +       error("Cannot fork: $!") if !defined $pid;
60         +
61         +       if (!$pid) {
62         +               # In child.
63         +               # hg commands want to be in wc.
64         +               if (! defined $hg_dir) {
65         +                       chdir $config{srcdir}
66         +                           or error("cannot chdir to $config{srcdir}: $!");
67         +               }
68         +               else {
69         +                       chdir $hg_dir
70         +                           or error("cannot chdir to $hg_dir: $!");
71         +               }
73 > How can this possibly work, since `$hg_dir` is not set? The code
74 > that is being replaced seems to use `-R` to make hg use the right
75 > directory. If it worked for you without specifying the directory,
76 > it's quite likely this is the place the patch fails in some
77 > unusual circumstance.. but I think it could easily use `-R` here.
79 >> It works since `if (! defined $hg_dir)` always hits, and `chdir $config{srcdir}` is well defined. The whole logic is just used in `git.pm` for merge functionality that is not present in `mercurial.pm`, so by the cruft argument above, this should be replaced with just chdir `$config{srcdir}` (which is equivalent to `hg -R` or `hg --cwd` from what I know). Will be removed. *Done* --[[Daniel Andersson]]
81         +               exec @cmdline or error("Cannot exec '@cmdline': $!");
82         +       }
83         +       # In parent.
84         +
85         +       # hg output is probably utf-8 encoded, but may contain
86         +       # other encodings or invalidly encoded stuff. So do not rely
87         +       # on the normal utf-8 IO layer, decode it by hand.
88         +       binmode($OUT);
90 > Is this actually true for hg?
92 >> I don't know. ["hg stores everything internally as UTF-8, except for pathnames"](https://jira.atlassian.com/browse/FE-3198), but output is dependent on the system's locale. The environment variable `HGENCODING=utf-8` can be set to ensure that Mercurial's own output is always UTF-8, but when viewing a diff containing non-UTF-8 changes, the affected lines are nevertheless output in their original encoding. I personally think that this is the correct way to output it, though, unless there is a possibility that someone is running ikiwiki wih a non-UTF-8 locale.
94 >>> *Done*. I removed the `encode_utf8()` part and instead set `HGENCODING=utf-8` where the external `hg` command was called. It seems to have taken care of "all" character encoding issues (but it is an almost infinite error pool to draw from, so some problem might pop up). --[[Daniel Andersson]]
96         +       my @lines;
97         +       while (<$OUT>) {
98         +               $_=decode_utf8($_, 0);
99         +
100         +               chomp;
101         +
102         +               if (! defined $data_handler) {
103         +                       push @lines, $_;
104         +               }
105         +               else {
106         +                       last unless $data_handler->($_);
107         +               }
108         +       }
109         +
110         +       close $OUT;
111         +
112         +       $error_handler->("'@cmdline' failed: $!") if $? && $error_handler;
113         +
114         +       return wantarray ? @lines : ($? == 0);
115         +}
116         +# Convenient wrappers.
117         +sub run_or_die ($@) { safe_hg(\&error, undef, @_) }
118         +sub run_or_cry ($@) { safe_hg(sub { warn @_ }, undef, @_) }
119         +sub run_or_non ($@) { safe_hg(undef, undef, @_) }
120         +
121          sub mercurial_log ($) {
122                 my $out = shift;
123                 my @infos;
124         @@ -116,10 +174,7 @@
125          }
126          
127          sub rcs_update () {
128         -       my @cmdline = ("hg", "-q", "-R", "$config{srcdir}", "update");
129         -       if (system(@cmdline) != 0) {
130         -               warn "'@cmdline' failed: $!";
131         -       }
132         +       run_or_cry('hg', '-q', 'update');
133          }
134          
135          sub rcs_prepedit ($) {
137 With the `run_or_{die,cry,non}()` functions defined as in `git.pm`, some old Mercurial functions can be rewritten more compactly.
139         @@ -129,6 +184,14 @@
140          sub rcs_commit (@) {
141                 my %params=@_;
142          
143         +       return rcs_commit_helper(@_);
144         +}
145         +
146         +sub rcs_commit_helper (@) {
147         +       my %params=@_;
148         +
149         +       my %env=%ENV;
151 > This `%env` stash is unused; `%ENV` is never modified.
153 >> Yes, the code is missing setting the username at all. The local `hgrc` file is always used. I'll add setting of `$ENV{HGUSER}`. *Done* --[[Daniel Andersson]]
155         +
156                 my $user="Anonymous";
157                 if (defined $params{session}) {
158                         if (defined $params{session}->param("name")) {
160 Here comes the `rcs_commit{,_staged}` part. It is modeled on a `rcs_commit_helper` function, as in `git.pm`.
162 Some old `mercurial.pm` logic concerning commiter name is kept instead of transplanting the more elaborate logic from `git.pm`. Maybe it is better to "steal" that as well.
164 > Exactly how to encode the nickname from openid in the commit metadata,
165 > and get it back out in `rcs_recentchanges`, would probably vary from git.
167 >> Yes, right now the long and ugly OpenID strings, e.g. `https://www.google.com/accounts/o8/id?id=AItOawmUIes3yDLfQME0uvZvJKDN0NsdKPx_PTw`, gets recorded as author and are shown as `id [www.google.com/accounts/o8]` in RecentChanges. I see that here on `ikiwiki.info`, my commits, identified by OpenID, are shown as authored by simply `Daniel`. I'll look into it. --[[Daniel Andersson]]
169         @@ -143,43 +206,45 @@
170                         $params{message} = "no message given";
171                 }
172          
173         -       my @cmdline = ("hg", "-q", "-R", $config{srcdir}, "commit", 
174         -                      "-m", IkiWiki::possibly_foolish_untaint($params{message}),
175         -                      "-u", IkiWiki::possibly_foolish_untaint($user));
176         -       if (system(@cmdline) != 0) {
177         -               warn "'@cmdline' failed: $!";
178         +       $params{message} = IkiWiki::possibly_foolish_untaint($params{message});
179         +
180         +       my @opts;
181         +       
182         +       if (exists $params{file}) {
183         +               push @opts, '--', $params{file};
184                 }
185         -
186         +       # hg commit returns non-zero if nothing really changed.
187         +       # So we should ignore its exit status (hence run_or_non).
188         +       run_or_non('hg', 'commit', '-m', $params{message}, '-q', @opts);
189         +       
190         +       %ENV=%env;
191                 return undef; # success
192          }
193          
194          sub rcs_commit_staged (@) {
195                 # Commits all staged changes. Changes can be staged using rcs_add,
196                 # rcs_remove, and rcs_rename.
197         -       my %params=@_;
198         -       
199         -       error("rcs_commit_staged not implemented for mercurial"); # TODO
200         +       return rcs_commit_helper(@_);
201          }
202          
203          sub rcs_add ($) {
204                 my ($file) = @_;
205          
206         -       my @cmdline = ("hg", "-q", "-R", "$config{srcdir}", "add", "$config{srcdir}/$file");
207         -       if (system(@cmdline) != 0) {
208         -               warn "'@cmdline' failed: $!";
209         -       }
210         +       run_or_cry('hg', 'add', $file);
211          }
212          
213          sub rcs_remove ($) {
214         +       # Remove file from archive.
215         +
216                 my ($file) = @_;
217          
218         -       error("rcs_remove not implemented for mercurial"); # TODO
219         +       run_or_cry('hg', 'remove', '-f', $file);
220          }
221          
222          sub rcs_rename ($$) {
223                 my ($src, $dest) = @_;
224          
225         -       error("rcs_rename not implemented for mercurial"); # TODO
226         +       run_or_cry('hg', 'rename', '-f', $src, $dest);
227          }
228          
229          sub rcs_recentchanges ($) {
231 > Remainder seems ok to me. Should probably test that the remove plugin 
232 > works, since this implements `rcs_remove` too and you didn't mention
233 > any tests that would run that code path. --[[Joey]] 
235 >> I tested `rename`. It fails if the page title includes e.g. åäö. Trying to rename a page from "title without special chars" to "title with åäö" renders in `/var/log/apache2/error.log`:
237     [Fri Jul 15 14:58:17 2011] [error] [client 46.239.104.5] transaction abort!, referer: http://46.239.104.5:81/blog/ikiwiki.cgi
238     [Fri Jul 15 14:58:17 2011] [error] [client 46.239.104.5] rollback completed, referer: http://46.239.104.5:81/blog/ikiwiki.cgi
239     [Fri Jul 15 14:58:17 2011] [error] [client 46.239.104.5] abort: decoding near 'itle_with_\xc3\xa5\xc3\xa4\xc3\xb6.mdw': 'ascii' codec can't decode byte 0xc3 in position 66: ordinal not in range(128)!, referer: http://46.239.104.5:81/blog/ikiwiki.cgi
240     [Fri Jul 15 14:58:17 2011] [error] [client 46.239.104.5] 'hg commit -m rename posts/title_without_special_chars.mdwn to posts/title_with_\xc3\xa5\xc3\xa4\xc3\xb6.mdwn -q' failed:  at /usr/share/perl5/IkiWiki/Plugin/mercurial.pm line 123., referer: http://46.239.104.5:81/blog/ikiwiki.cgi
242 >>> I added setting the environment variable `HGENCODING=utf-8` in `rcs_commit_helper`, which took care of these problems. *Done* --[[Daniel Andersson]]
244 >> When this has happened, directly following by `rename` and `remove` doesn't work as it should, since the file is not commited. `hg remove -f` doesn't physically remove files that aren't tracked (no `hg` command does). Perhaps a regular `unlink $file` should be called to not clutter the source dir if `hg remove` failed because the file wasn't tracked. --[[Daniel Andersson]]
246 >> I've also noted that when a post is added or removed, the commit message lacks the page title. It contains the title when the page is renamed though, so it should be an easy fix. I'll look into it. --[[Daniel Andersson]]
248 >>> This is to do with `{rename,remove,editchanges}.pm`. The last two simply don't give a message to `rcs_commit_staged`. Separate issue. --[[Daniel Andersson]]