doc/todo/headless_git_branches.mdwn

   1 Ikiwiki should really survive being asked to work with a git branch that has no existing commits.
   2
   3     mkdir iki-gittest
   4     cd iki-gittest
   5     GIT_DIR=barerepo.git git init
   6     git clone barerepo.git srcdir
   7     ikiwiki --rcs=git srcdir destdir
   8
   9 I've fixed this initial construction case, and, based on my testing, I've
  10 also fixed the post-update executing on a new master, and ikiwiki.cgi
  11 executing on a non-existent master cases.
  12
  13 Please commit so my users stop whining at me about having clean branches to push to, the big babies.
  14
  15 Summary: Change three scary loud failure cases related to empty branches into three mostly quiet success cases.
  16
  17 [[!tag patch]]
  18
  19 > FWIW, [[The_TOVA_Company]] apparently wants this feature (and I hope
  20 > I don't mind that I mention they were willing to pay someone for it,
  21 > but I told them I'd not done any of the work. :) )
  22 >
  23 > Code review follows, per hunk.. --[[Joey]]
  24
  25 <pre>
  26 diff --git a/IkiWiki/Plugin/git.pm b/IkiWiki/Plugin/git.pm
  27 index cf7fbe9..e5bafcf 100644
  28 --- a/IkiWiki/Plugin/git.pm
  29 +++ b/IkiWiki/Plugin/git.pm
  30 @@ -439,17 +439,21 @@ sub git_commit_info ($;$) {
  31
  32         my @opts;
  33         push @opts, "--max-count=$num" if defined $num;
  34 -
  35 -       my @raw_lines = run_or_die('git', 'log', @opts,
  36 -               '--pretty=raw', '--raw', '--abbrev=40', '--always', '-c',
  37 -               '-r', $sha1, '--', '.');
  38 -
  39 +       my @raw_lines;
  40         my @ci;
  41 -       while (my $parsed = parse_diff_tree(\@raw_lines)) {
  42 -               push @ci, $parsed;
  43 -       }
  44 +
  45 +       # Test to see if branch actually exists yet.
  46 +       if (run_or_non('git', 'show-ref', '--quiet', '--verify', '--', 'refs/heads/' . $config{gitmaster_branch}) ) {
  47 +               @raw_lines = run_or_die('git', 'log', @opts,
  48 +                       '--pretty=raw', '--raw', '--abbrev=40', '--always', '-c',
  49 +                       '-r', $sha1, '--', '.');
  50 +
  51 +               while (my $parsed = parse_diff_tree(\@raw_lines)) {
  52 +                       push @ci, $parsed;
  53 +               }
  54
  55 -       warn "Cannot parse commit info for '$sha1' commit" if !@ci;
  56 +               warn "Cannot parse commit info for '$sha1' commit" if !@ci;
  57 +       };
  58
  59         return wantarray ? @ci : $ci[0];
  60  }
  61 </pre>
  62
  63 My concern is that this adds a bit of slowdown (git show-ref is fast, but
  64 It's still extra work) to a very hot code path that is run to eg,
  65 update recentchanges after every change.
  66
  67 Seems not ideal to do extra work every time to handle a case
  68 that will literally happen a maximum of once in the entire lifecycle of a
  69 wiki (and zero times more typically, since the setup automator puts in a
  70 .gitignore file that works around this problem).
  71
  72 So as to not just say "no" ... what if it always tried to run git log,
  73 and if it failed (or returned no parsed lines), then it could look
  74 at git show-ref to deduce whether to throw an error or not.
  75 --[[Joey]]
  76
  77 > Ah, but then git-log would still complain "bad revision 'HEAD'"
  78 > --[[Joey]]
  79
  80     jrayhawk@piny:/srv/git/jrayhawk.git$ time perl -e 'for( $i = 1; $i < 10000; $i++) { system("git", "show-ref", "--quiet", "--verify", "--", "refs/heads/master"); }'
  81
  82     real  0m10.988s
  83     user  0m0.120s
  84     sys   0m1.210s
  85
  86 > FWIW, "an extra millisecond per edit" vs "full git coverage" is no
  87 > contest for me; I use that patch on seven different systems, including
  88 > freedesktop.org, because I've spent more time explaining to users either
  89 > why Ikiwiki won't work on their empty repositories or why their
  90 > repositories need useless initial commits (a la Branchable) that make
  91 > pushing not work and why denyNonFastForwards=0 and git push -f are
  92 > necessary than all the milliseconds that could've been saved in the
  93 > world.
  94 >
  95 > But, since we're having fun rearranging deck chairs on the RMS Perl
  96 > (toot toot)...
  97 >
  98 > There's some discrepency here I wasn't expecting:
  99
 100     jrayhawk@piny:/srv/git/jrayhawk.git$ time dash -c 'i=0; while [ $i -lt 10000 ]; do i=$((i+1)); git show-ref --quiet --verify -- refs/heads/master; done'
 101
 102     real  0m9.986s
 103     user  0m0.170s
 104     sys   0m0.940s
 105
 106 > While looking around in the straces, I notice Perl, unlike {b,d}ash
 107 > appears to do PATH lookup on every invocation of git, adding up to
 108 > around 110 microseconds apiece on a post-2.6.38 16-thread QPI system:
 109
 110     29699      0.000112 execve("/home/jrayhawk/bin/git", ["git", "show-ref", "--quiet", "--verify", "--", "refs/heads/master"], [/* 17 vars */]) = -1 ENOENT (No such file or directory)
 111     29699      0.000116 execve("/usr/local/bin/git", ["git", "show-ref", "--quiet", "--verify", "--", "refs/heads/master"], [/* 17 vars */]) = -1 ENOENT (No such file or directory)
 112     29699      0.000084 execve("/usr/bin/git", ["git", "show-ref", "--quiet", "--verify", "--", "refs/heads/master"], [/* 17 vars */]) = 0
 113
 114 > You can probably save a reasonable number of context switches and
 115 > RCU-heavy (or, previously, lock-heavy) dentry lookups by doing a Perl
 116 > equivalent of `which git` and using the result. It might even add up to
 117 > a whole millisecond in some circumstances!
 118 >
 119 > No idea where the rest of that time is going. Probably cache misses
 120 > on the giant Perl runtime or something.
 121 >
 122 > ...
 123 >
 124 > Now I feel dirty for having spent more time talking about optimization
 125 > than that optimization is likely to save. This must be what being an
 126 > engineer feels like.
 127 > --jrayhawk
 128
 129 <pre>
 130 @@ -474,7 +478,10 @@ sub rcs_update () {
 131         # Update working directory.
 132
 133         if (length $config{gitorigin_branch}) {
 134 -               run_or_cry('git', 'pull', '--prune', $config{gitorigin_branch});
 135 +               run_or_cry('git', 'fetch', '--prune', $config{gitorigin_branch});
 136 +               if (run_or_non('git', 'show-ref', '--quiet', '--verify', '--', 'refs/remotes/' . $config{gitorigin_branch} . '/' . $config{gitmaster_branch}) ) {
 137 +                       run_or_cry('git', 'merge', $config{gitorigin_branch} . '/' . $config{gitmaster_branch});
 138 +               }
 139         }
 140  }
 141
 142 </pre>
 143
 144 Same concern here about extra work. Code path is nearly as hot, being
 145 called on every refresh. Probably could be dealt with similarly as above.
 146
 147 Also, is there any point in breaking the pull up into a
 148 fetch followed by a merge? --[[Joey]]
 149
 150 > The same benchmarking applies, at least.
 151 >
 152 > Re: fetch/merge: We can't test for the nonexistence of the origin branch
 153 > without fetching it, and we can't merge it if it is, indeed,
 154 > nonexistant.
 155 >
 156 > Unless you're implying that it would be better to just spam stderr with
 157 > unnecessary scary messages and/or ignore/suppress them and lose the
 158 > ability to respond appropriately to every other error condition. As
 159 > maintainer, you deal with a disproportionate amount of the resulting
 160 > support fallout, so I'm perfectly satisfied letting you make that call.
 161 > --jrayhawk
 162
 163 <pre>
 164 @@ -559,7 +566,7 @@ sub rcs_commit_helper (@) {
 165         # So we should ignore its exit status (hence run_or_non).
 166         if (run_or_non('git', 'commit', '-m', $params{message}, '-q', @opts)) {
 167                 if (length $config{gitorigin_branch}) {
 168 -                       run_or_cry('git', 'push', $config{gitorigin_branch});
 169 +                       run_or_cry('git', 'push', $config{gitorigin_branch}, $config{gitmaster_branch});
 170                 }
 171         }
 172
 173 </pre>
 174
 175 This seems fine to apply. --[[Joey]]
 176
 177 > Hooray!
 178 > --jrayhawk