{"thread":{"id":"40760","subject":"[PATCH 0/2] git-candidate: git based patch tracking and review","startedAt":"2015-11-10T12:56:36Z","lastAt":"2016-01-06T20:50:14Z","messageCount":14,"participants":["Richard Ipsum","David Turner","Michael Haggerty","Jeff King","Junio C Hamano","Jonathan Nieder","Dave Borowitz","Sebastian Schuberth"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"273146","messageId":"1447160198-23296-1-git-send-email-richard.ipsum@codethink.co.uk","threadId":"40760","inReplyTo":null,"subject":"[PATCH 0/2] git-candidate: git based patch tracking and review","fromName":"Richard Ipsum","fromEmail":"richard.ipsum@codethink.co.uk","sentAt":"2015-11-10T12:56:36Z","receivedAt":"2015-11-10T12:56:36Z","isPatch":true,"sender":{"key":"richard.ipsum@codethink.co.uk","avatar":null},"body":"Hi,\n\nI've continued my work[1] to add patch tracking and candidate review capability\nto git.\n\ngit-candidate now has a more git-like user interface, so remote candidates\ncan now be specified in a similar way to remote refs (e.g. origin/candidate)\nas well as various other improvements, such as versioned metadata.\n\n[1]: http://www.mail-archive.com/git%40vger.kernel.org/msg79461.html\n\nRichard Ipsum (2):\n contrib: Add git-candidate subcommand\n contrib/git-candidate: Add README\n\n contrib/git-candidate/GitUtils.pm | 207 +++\n contrib/git-candidate/README.md | 154 ++\n contrib/git-candidate/git-candidate.perl | 2541 ++++++++++++++++++++++++++++++\n 3 files changed, 2902 insertions(+)\n create mode 100644 contrib/git-candidate/GitUtils.pm\n create mode 100644 contrib/git-candidate/README.md\n create mode 100755 contrib/git-candidate/git-candidate.perl\n\n-- \n2.1.4\n"},{"id":"273148","messageId":"1447160198-23296-2-git-send-email-richard.ipsum@codethink.co.uk","threadId":"40760","inReplyTo":"1447160198-23296-1-git-send-email-richard.ipsum@codethink.co.uk","subject":"[PATCH 1/2] contrib: Add git-candidate subcommand","fromName":"Richard Ipsum","fromEmail":"richard.ipsum@codethink.co.uk","sentAt":"2015-11-10T12:56:37Z","receivedAt":"2015-11-10T12:56:37Z","isPatch":true,"sender":{"key":"richard.ipsum@codethink.co.uk","avatar":null},"body":"git-candidate provides candidate review and patch tracking,\nallowing distributed comment and review facilities with\nall content stored in git.\n\nSigned-off-by: Richard Ipsum \n---\n contrib/git-candidate/GitUtils.pm | 207 +++\n contrib/git-candidate/git-candidate.perl | 2541 ++++++++++++++++++++++++++++++\n 2 files changed, 2748 insertions(+)\n create mode 100644 contrib/git-candidate/GitUtils.pm\n create mode 100755 contrib/git-candidate/git-candidate.perl\n\ndiff --git a/contrib/git-candidate/GitUtils.pm b/contrib/git-candidate/GitUtils.pm\nnew file mode 100644\nindex 0000000..24c9abe\n--- /dev/null\n+++ b/contrib/git-candidate/GitUtils.pm\n@@ -0,0 +1,207 @@\n+# Copyright 2015 Codethink Limited\n+#\n+# GPL v2 (See COPYING)\n+\n+package GitUtils;\n+\n+use Exporter 'import';\n+use Error qw(:try);\n+use Git;\n+\n+our @EXPORT_OK = qw(require_clean_work_tree git_editor _head_symbolic_ref\n+ _command_input_pipe_capture_output_oneline cat_file\n+ is_object_present is_commit_merged get_head_branch\n+ get_head_commit get_land_for_commit get_sha_at_ref\n+ get_tracking_branch remove_refs inject_json\n+ looks_like_sha validate_kwargs);\n+\n+use Carp;\n+sub validate_kwargs\n+{\n+ my ($kwargs_ref, @mandatory_args) = @_;\n+\n+ for my $k (@mandatory_args) {\n+ confess \"No '$k' provided\" unless exists $kwargs_ref->{$k};\n+ }\n+}\n+\n+sub looks_like_sha\n+{\n+ my $x = shift;\n+ return $x =~ /[0-9a-z]{40}/;\n+}\n+\n+sub inject_json\n+{\n+ my ($repo, $json) = @_;\n+ my $fh = File::Temp->new();\n+\n+ print $fh $json;\n+ close $fh;\n+ my $hash = $repo->hash_and_insert_object($fh->filename);\n+\n+ return $hash;\n+}\n+\n+sub require_clean_work_tree {\n+ my ($repo, $action) = @_;\n+ $action //= 'proceed';\n+ my $errno = 0;\n+\n+ try {\n+ $repo->command_oneline('diff-files', '--quiet', '--ignore-submodules');\n+ }\n+ catch Git::Error::Command with {\n+ print STDERR \"Cannot $action: You have unstaged changes.\\n\";\n+ $errno = 1;\n+ };\n+\n+ try {\n+ $repo->command('diff-index', '--cached', '--quiet',\n+ '--ignore-submodules', 'HEAD');\n+ }\n+ catch Git::Error::Command with {\n+ if ($errno == 0) {\n+ print STDERR \"Cannot $action: Your index contains uncommitted changes.\\n\";\n+ $errno = 1;\n+ }\n+ else {\n+ print STDERR \"Additonally, your index contains uncommitted changes.\\n\";\n+ }\n+ };\n+\n+ exit 1 if $errno;\n+}\n+\n+sub git_editor\n+{\n+ my $editor = $ENV{GIT_EDITOR} // `git var GIT_EDITOR`;\n+ chomp($editor);\n+\n+ system($editor, @_);\n+}\n+\n+sub _head_symbolic_ref\n+{\n+ my $repo = shift;\n+ return $repo->command_oneline(['symbolic-ref', 'HEAD'], STDERR => 0);\n+}\n+\n+sub _command_input_pipe_capture_output_oneline\n+{\n+ my ($repo, $command_input, $command, @arguments) = @_;\n+\n+ confess \"No repo defined\" unless defined $repo;\n+ confess \"No command input defined\" unless defined $command_input;\n+ confess \"No command defined\" unless defined $command;\n+\n+ my ($pid, $pipe_out, $pipe_in, $ctx) = $repo->command_bidi_pipe($command,\n+ @arguments);\n+ print $pipe_in $command_input;\n+ close $pipe_in;\n+\n+ my $output = readline $pipe_out;\n+ chomp($output) if $output;\n+\n+ $repo->command_close_bidi_pipe($pid, $pipe_out, undef, $ctx);\n+\n+ return $output;\n+}\n+\n+sub cat_file\n+{\n+ my ($repo, $sha, $filename, %kwargs) = @_;\n+ return $repo->command(['cat-file', '-p',\n+ defined $filename ? \"$sha:$filename\" : \"$sha\"], %kwargs);\n+}\n+\n+sub is_object_present\n+{\n+ my ($repo, $sha) = @_;\n+\n+ return try {\n+ my $output = $repo->command_oneline(['cat-file', '-t', $sha],\n+ STDERR => 0);\n+ return 1;\n+ }\n+ catch Git::Error::Command with {\n+ return 0;\n+ };\n+}\n+\n+sub is_commit_merged\n+{\n+ my ($repo, $what, $where) = @_;\n+ my @lines = $repo->command('rev-list', $where);\n+ return grep { /$what/ } @lines;\n+}\n+\n+sub get_head_branch\n+{\n+ my $repo = shift;\n+\n+ return try {\n+ my $symbolic_ref = _head_symbolic_ref($repo);\n+ return $repo->command_oneline('for-each-ref',\n+ '--format=%(refname:short)', $symbolic_ref);\n+ }\n+ catch Git::Error::Command with {\n+ return undef;\n+ };\n+}\n+\n+sub get_head_commit\n+{\n+ my $repo = shift;\n+\n+ return try {\n+ return $repo->command_oneline('rev-parse', 'HEAD');\n+ }\n+ catch Git::Error::Command with {\n+ return undef;\n+ };\n+}\n+\n+sub get_land_for_commit\n+{\n+ my ($repo, $commit) = @_;\n+\n+ return try {\n+ my $symbolic_ref = _head_symbolic_ref($repo);\n+ my $line = $repo->command_oneline('for-each-ref',\n+ '--format=%(upstream) %(upstream:short)', $symbolic_ref);\n+ my ($unambig, $land) = split(/ /, $line);\n+ my $ontop = $repo->command_oneline('merge-base', $commit, $unambig);\n+ print \"returning hash...\";\n+ return {ontop => $ontop, landinto => $land};\n+ }\n+ catch Git::Error::Command with {\n+ return undef;\n+ };\n+}\n+\n+sub get_sha_at_ref\n+{\n+ my ($repo, $ref) = @_;\n+\n+ return $repo->command_oneline('show-ref', '--hash', $ref);\n+}\n+\n+sub get_tracking_branch\n+{\n+ my ($repo, $branch) = @_;\n+\n+ return $repo->config(\"branch.$branch.remote\");\n+}\n+\n+sub remove_refs\n+{\n+ my ($repo, @refs) = @_;\n+\n+ for my $ref (@refs) {\n+ print \"ref: $ref\\n\";\n+ $repo->command('update-ref', '-d', $ref);\n+ }\n+}\n+\n+1;\ndiff --git a/contrib/git-candidate/git-candidate.perl b/contrib/git-candidate/git-candidate.perl\nnew file mode 100755\nindex 0000000..7660bd0\n--- /dev/null\n+++ b/contrib/git-candidate/git-candidate.perl\n@@ -0,0 +1,2541 @@\n+#!/usr/bin/env perl\n+#\n+# Copyright 2015 Codethink Limited\n+#\n+# GPL v2 (See COPYING)\n+\n+use 5.008;\n+use strict;\n+use warnings;\n+use Error qw(:try);\n+use Getopt::Long;\n+use Git;\n+use FindBin;\n+use lib \"$FindBin::Bin\";\n+use GitUtils qw(require_clean_work_tree git_editor _head_symbolic_ref\n+ _command_input_pipe_capture_output_oneline cat_file\n+ is_object_present is_commit_merged get_head_branch\n+ get_head_commit get_land_for_commit get_sha_at_ref\n+ get_tracking_branch remove_refs inject_json\n+ looks_like_sha validate_kwargs);\n+\n+use Hash::Util qw(lock_keys);\n+use JSON;\n+use File::Temp qw(tempdir);\n+use Cwd;\n+use POSIX;\n+use Carp;\n+\n+my $DEBUG = $ENV{PR_DEBUG} // 0;\n+my $user_checkout;\n+\n+sub get_repo\n+{\n+ my %kwargs = @_;\n+\n+ my $with_user_worktree = $kwargs{with_user_worktree};\n+ my $repo = try {\n+ # We first run a git command if this succeeds then we're in a git repo\n+ Git::command_oneline(['rev-parse'], STDERR => $DEBUG ? undef : 0);\n+\n+ my $repo;\n+ if ($with_user_worktree) {\n+ return Git->repository;\n+ }\n+ else {\n+ my $tmpdir = tempdir(CLEANUP => 1);\n+\n+ mkdir \"$tmpdir/.git\";\n+ $ENV{GIT_INDEX_FILE} = \"$tmpdir/.git/index\";\n+ my $repo = Git->repository(Repository => getcwd . \"/.git\",\n+ WorkingCopy => $tmpdir);\n+\n+ my $gothead = try {\n+ $repo->command(['rev-parse', 'HEAD'],\n+ STDERR => $DEBUG ? undef : 0);\n+ }\n+ catch Git::Error::Command with {\n+ print STDERR \"Warning: no HEAD \" .\n+ \"nothing to populate index with.\\n\" if $DEBUG;\n+ return undef;\n+ };\n+\n+ if ($gothead) {\n+ $repo->command(['reset', '--hard', 'HEAD'],\n+ STDERR => $DEBUG ? undef : 0); # populate index\n+ }\n+\n+ return $repo;\n+ }\n+ }\n+ catch Git::Error::Command with {\n+ return undef;\n+ };\n+\n+ # user_checkout is a global that stores the user's HEAD\n+ # prior to the exec of git-candidate, so we can restore the correct\n+ # HEAD if we're interrupted.\n+ #\n+ # user_checkout shall only ever be set once,\n+ # by the first call to get_repo.\n+ unless (defined $user_checkout) {\n+ my $head_branch = get_head_branch($repo);\n+ $user_checkout = defined $head_branch ? $head_branch :\n+ $repo->command_oneline('rev-parse', 'HEAD');\n+ }\n+\n+ return $repo;\n+}\n+\n+sub catch_sigint\n+{\n+ if (defined $user_checkout) {\n+ my $repo = get_repo;\n+ $repo->command(['checkout', $user_checkout],\n+ STDERR => $DEBUG ? undef : 0);\n+ }\n+ exit 1;\n+}\n+\n+$SIG{INT} = \\&catch_sigint;\n+\n+my $MAX_COMMENT_LEN = 512;\n+my $REMOTE_REF_PREFIX = 'refs/candidates/remotes';\n+my $LOCAL_REF_PREFIX = 'refs/candidates/heads';\n+\n+my @SUPPORTED_VERSIONS = (0, );\n+if (defined $ENV{CANDIDATE_TEST} and $ENV{CANDIDATE_TEST} eq \"1\") {\n+ push @SUPPORTED_VERSIONS, 256;\n+}\n+\n+my $CANDIDATE_VERSION = defined $ENV{CANDIDATE_VERSION} ?\n+ $ENV{CANDIDATE_VERSION} + 0 : $SUPPORTED_VERSIONS[-1];\n+\n+sub debug_print\n+{\n+ print @_ if $DEBUG;\n+}\n+\n+sub indent_print\n+{\n+ my %kwargs = @_;\n+\n+ die \"No string provided\" unless defined $kwargs{string};\n+ die \"No indent provided\" unless defined $kwargs{indent};\n+\n+ my $indent = $kwargs{indent};\n+ my $string = $kwargs{string};\n+\n+ my @lines = split(/\\n/, $string);\n+ for my $line (@lines) {\n+ printf(\"%*s\\n\", length($line) + $indent, $line);\n+ }\n+}\n+\n+package GitFileObject\n+{\n+ use GitUtils qw(validate_kwargs);\n+\n+ sub new\n+ {\n+ my ($class, %kwargs) = @_;\n+ validate_kwargs(\\%kwargs, 'objtype', 'repo');\n+\n+ bless {_mode => $kwargs{mode}, _objtype => $kwargs{objtype},\n+ _objsum => $kwargs{objsum}, _path => $kwargs{path},\n+ _repo => $kwargs{repo}}, $class;\n+ }\n+\n+ sub mode { return $_[0]->{_mode}; }\n+ sub path { return $_[0]->{_path}; }\n+ sub objsum { return $_[0]->{_objsum}; }\n+ sub objtype { return $_[0]->{_objtype}; }\n+\n+ sub new_regular_blob\n+ {\n+ my ($class, %kwargs) = @_;\n+ validate_kwargs(\\%kwargs, 'repo');\n+ my ($repo, $sha, $path) = @kwargs{'repo', 'sha', 'path'};\n+\n+ my $self = GitFileObject->new(repo => $repo, mode => '100644',\n+ objtype => 'blob', objsum => $sha,\n+ path => $path);\n+ bless $self, $class;\n+ }\n+\n+ sub new_tree\n+ {\n+ my ($class, %kwargs) = @_;\n+ validate_kwargs(\\%kwargs, 'repo');\n+ my ($repo, $sha, $path) = @kwargs{'repo', 'sha', 'path'};\n+\n+ my $self = GitFileObject->new(repo => $repo, mode => '040000',\n+ objtype => 'tree', objsum => $sha,\n+ path => $path);\n+ bless $self, $class;\n+ }\n+\n+ sub tree_entry\n+ {\n+ my $self = shift;\n+ my ($mode, $objtype, $objsum, $path) = ($self->mode, $self->objtype,\n+ $self->objsum, $self->path);\n+\n+ return (defined $mode and defined $objtype\n+ and defined $objsum and defined $path) ?\n+ \"$mode $objtype $objsum\\t$path\" : undef;\n+ }\n+\n+ sub set_objsum\n+ {\n+ my ($self, $objsum) = @_;\n+\n+ $self->{_objsum} = $objsum;\n+ }\n+}\n+\n+package GitCommitObject\n+{\n+ @GitCommitObject::ISA = 'GitFileObject';\n+ use GitUtils qw(cat_file validate_kwargs);\n+\n+ sub _parse_commit_obj\n+ {\n+ my ($class, $repo, $sha) = @_;\n+\n+ my @lines = cat_file($repo, $sha);\n+\n+ my ($timberline, ) = grep { /^tree/ } @lines;\n+ $timberline =~ /^tree ([a-z0-9]+)/;\n+ my $tree_sha = $1;\n+\n+ return $tree_sha;\n+ }\n+\n+ sub new\n+ {\n+ my ($class, %kwargs) = @_;\n+\n+ validate_kwargs(\\%kwargs, 'repo');\n+\n+ my $self = $class->SUPER::new(repo => $kwargs{repo},\n+ objtype => 'commit', objsum => $kwargs{sha});\n+ $self->{_tree} = $class->_parse_commit_obj($kwargs{repo},\n+ $kwargs{sha});\n+\n+ bless $self, $class;\n+ }\n+\n+ sub tree { return $_[0]->{_tree}; }\n+}\n+\n+package GitTreeObject\n+{\n+ @GitTreeObject::ISA = 'GitFileObject';\n+ use GitUtils qw(cat_file _command_input_pipe_capture_output_oneline\n+ validate_kwargs);\n+ use Carp;\n+\n+ sub debug_print { print @_ if $DEBUG; }\n+\n+ sub _git_object_factory\n+ {\n+ my %kwargs = @_;\n+ my @mandatory = qw(tree_entry repo);\n+ validate_kwargs(\\%kwargs, @mandatory);\n+ my ($repo, $tree_entry) = @kwargs{'repo', 'tree_entry'};\n+\n+ debug_print(\"git_object_factory called on entry '$tree_entry'\");\n+\n+ if ($tree_entry =~ m/^([0-9]+) (blob|tree|commit) ([0-9a-z]+)\\t(.*)/) {\n+ my ($mode, $type, $sha, $path) = ($1, $2, $3, $4);\n+ return $type eq 'tree' ?\n+ GitTreeObject->new(repo => $repo, sha => $sha, path => $path)\n+ : GitFileObject->new(repo => $repo, mode => $mode,\n+ objtype => $type, objsum => $sha,\n+ path => $path);\n+ }\n+ else {\n+ die \"Could not parse tree entry: '$tree_entry'\";\n+ }\n+ }\n+\n+ sub new\n+ {\n+ my ($class, %kwargs) = @_;\n+ my @mandatory = ('repo');\n+ validate_kwargs(\\%kwargs, @mandatory);\n+ my ($sha, $path, $repo) = @kwargs{'sha', 'path', 'repo'};\n+\n+ my $self = $class->SUPER::new_tree(repo => $repo, sha => $sha,\n+ path => $path);\n+ my %children;\n+\n+ if (defined $sha) {\n+ my @tree_entries = cat_file($repo, $sha);\n+ %children = map {\n+ my $o = _git_object_factory(repo => $repo, tree_entry => $_);\n+ $o->path => $o;\n+ } @tree_entries;\n+ }\n+\n+ $self->{_children} = \\%children;\n+ $self->{parent} = undef;\n+\n+ bless $self, $class;\n+ }\n+\n+ sub add_child\n+ {\n+ my ($self, $git_file_obj) = @_;\n+\n+ unless ($git_file_obj->isa('GitFileObject')) {\n+ die \"Child must be of type 'GitFileObject'\";\n+ }\n+\n+ # Do we already have an object with the same path?\n+ # If we do then we just need to update the sha in the existing file obj\n+ my $objpath = $git_file_obj->path;\n+ if (exists $self->{_children}->{$objpath}) {\n+ $self->{_children}->{$objpath}->set_objsum($git_file_obj->objsum);\n+ }\n+ else {\n+ $self->{_children}->{$objpath} = $git_file_obj;\n+ }\n+ }\n+\n+ sub child_exists\n+ {\n+ my ($self, $path) = @_;\n+ return defined $self->{_children}->{$path};\n+ }\n+\n+ sub get_child\n+ {\n+ my ($self, $path) = @_;\n+\n+ my $obj = $self->{_children}->{$path};\n+\n+ unless (defined $obj) {\n+ confess \"Tried to get non-existent child at path '$path' from $self\";\n+ }\n+\n+ return $obj;\n+ }\n+\n+ sub get_subtree\n+ {\n+ my ($self, $path) = @_;\n+ my $child = $self->get_child($path);\n+\n+ unless ($child->isa('GitTreeObject')) {\n+ die \"Child at path '$path' is not a subtree\";\n+ }\n+\n+ return $child;\n+ }\n+\n+ sub _make_tree\n+ {\n+ my ($self, @git_file_objs) = @_;\n+\n+ my @tree_entries = map { $_->tree_entry } @git_file_objs;\n+ my $cmdinput = join(\"\\n\", @tree_entries);\n+\n+ my $treesum = try {\n+ return _command_input_pipe_capture_output_oneline($self->{_repo},\n+ $cmdinput, 'mktree');\n+ }\n+ catch Git::Error::Command with {\n+ return undef;\n+ };\n+\n+ return $treesum;\n+ }\n+\n+ sub _write_tree\n+ {\n+ # Write the tree to git, recursively.\n+ my $self = shift;\n+ my @git_file_objs;\n+\n+ while (my ($path, $child) = each %{$self->{_children}}) {\n+ if ($child->isa('GitTreeObject')) {\n+ my $subtree_sum = $child->_write_tree;\n+ $child->set_objsum($subtree_sum);\n+ }\n+\n+ push @git_file_objs, $child;\n+ }\n+\n+ my $treesum = $self->_make_tree(@git_file_objs);\n+ return $treesum;\n+ }\n+\n+ sub commit\n+ {\n+ my ($self, %kwargs) = @_;\n+ my $commit_msg = $kwargs{commitmsg};\n+ my $parentsum = $kwargs{parentsha};\n+\n+ my $treesum = $self->_write_tree;\n+\n+ debug_print(\"parentsum: $parentsum\\n\") if defined $parentsum;\n+\n+ my $commitsum = try {\n+ if ($parentsum) {\n+ return _command_input_pipe_capture_output_oneline(\n+ $self->{_repo}, $commit_msg, 'commit-tree',\n+ '-p', $parentsum, $treesum);\n+ }\n+ else {\n+ return _command_input_pipe_capture_output_oneline(\n+ $self->{_repo}, $commit_msg, 'commit-tree', $treesum);\n+ }\n+ }\n+ catch Git::Error::Command with {\n+ return undef;\n+ };\n+ debug_print(\"commitsum: $commitsum\\n\");\n+\n+ return $commitsum;\n+ }\n+\n+ # Returns a copy of the hash of children,\n+ # modifying the result of this hash will not change the children\n+ sub get_children\n+ {\n+ my $self = shift;\n+ my %children = %{$self->{_children}};\n+ return %children;\n+ }\n+}\n+\n+package Candidate\n+{\n+ use GitUtils qw(looks_like_sha validate_kwargs);\n+ use Error qw(:try);\n+\n+ sub new\n+ {\n+ my ($class, %kwargs) = @_;\n+ my @kwarg_keys = keys %kwargs;\n+\n+ my @mandatory = qw(name repo);\n+ validate_kwargs(\\%kwargs, @mandatory);\n+\n+ my $self = {};\n+ $self->{_name} = $kwargs{name};\n+ $self->{_repo} = $kwargs{repo};\n+ $self->{_tree} = GitTreeObject->new(repo => $kwargs{repo});\n+ bless $self, $class;\n+ }\n+\n+ sub new_from_sha\n+ {\n+ my ($class, %kwargs) = @_;\n+\n+ my @mandatory = ('repo', 'sha', 'name', 'ref');\n+ validate_kwargs(\\%kwargs, @mandatory);\n+\n+ my $self = {};\n+ $self->{_tree} = GitTreeObject->new(repo => $kwargs{repo},\n+ sha => GitCommitObject->new(repo => $kwargs{repo},\n+ sha => $kwargs{sha})->tree\n+ );\n+\n+ $self->{_repo} = $kwargs{repo};\n+ $self->{_name} = $kwargs{name};\n+ $self->{_sha} = $kwargs{sha};\n+ $self->{_ref} = $kwargs{ref};\n+ bless $self, $class;\n+ }\n+\n+ sub name { return $_[0]->{_name}; }\n+ sub sha { return $_[0]->{_sha}; }\n+ sub ref { return $_[0]->{_ref}; }\n+ sub land { return $_[0]->get_metadata->landinto; }\n+ sub ontop { return $_[0]->get_metadata->ontop; }\n+\n+ sub add_child\n+ {\n+ my ($self, @args) = @_;\n+ return $self->{_tree}->add_child(@args);\n+ }\n+\n+ sub revision_tree_exists\n+ {\n+ return $_[0]->{_tree}->child_exists('revisions');\n+ }\n+\n+ sub get_revision_tree\n+ {\n+ return $_[0]->{_tree}->get_subtree('revisions');\n+ }\n+\n+ sub review_tree_exists\n+ {\n+ return $_[0]->{_tree}->child_exists('reviews');\n+ }\n+\n+ sub get_review_tree\n+ {\n+ return $_[0]->{_tree}->get_subtree('reviews');\n+ }\n+\n+ sub comment_tree_exists\n+ {\n+ return $_[0]->{_tree}->child_exists('comments');\n+ }\n+\n+ sub get_comment_tree\n+ {\n+ return $_[0]->{_tree}->get_subtree('comments');\n+ }\n+\n+ sub get_revision\n+ {\n+ my ($self, $sha) = @_;\n+\n+ die \"Revision should be a sha\" unless looks_like_sha($sha);\n+\n+ my $revisions = $self->get_revision_tree;\n+ my $revision_obj = $revisions->get_child(\"revision.$sha\");\n+ my $revision = Revision->new_from_sha(repo => $self->{_repo},\n+ sha => $revision_obj->objsum);\n+ }\n+\n+ # Get an array of revisions stored in reverse order of submission\n+ # (most recent submission first)\n+ sub get_revisions\n+ {\n+ my $self = shift;\n+\n+ my @revisions;\n+ my $rev = $self->get_revision($self->get_metadata->latestrevision);\n+\n+ while (1) {\n+ push @revisions, $rev;\n+ last if !defined $rev->parentrevision;\n+ $rev = $self->get_revision($rev->parentrevision);\n+ }\n+\n+ return @revisions;\n+ }\n+\n+ sub revision_exists\n+ {\n+ my ($self, $revision_sha) = @_;\n+\n+ return 0 unless $self->revision_tree_exists;\n+\n+ my %revisions = $self->get_revision_tree->get_children;\n+ return exists $revisions{\"revision.$revision_sha\"};\n+ }\n+\n+ sub get_subtree_for_revision\n+ {\n+ my ($self, $revision_sha, $path) = @_;\n+\n+ return undef unless $self->{_tree}->child_exists($path);\n+ my $tree = $self->{_tree}->get_child($path);\n+\n+ return ($tree->child_exists($revision_sha)) ?\n+ $tree->get_subtree($revision_sha) : undef;\n+ }\n+\n+ sub get_reviews_for_revision\n+ {\n+ my ($self, $revision_sha) = @_;\n+ my $review_tree = $self->get_subtree_for_revision($revision_sha, 'reviews');\n+ return undef unless defined $review_tree;\n+ my %review_objs = $review_tree->get_children;\n+\n+ my @reviews = map { Review->new_from_sha(repo => $self->{_repo},\n+ sha => $review_objs{$_}->objsum);\n+ } keys %review_objs;\n+\n+ return \\@reviews;\n+ }\n+\n+ sub get_comments_for_revision\n+ {\n+ my ($self, $revision_sha) = @_;\n+ return $self->get_subtree_for_revision($revision_sha, 'comments');\n+ }\n+\n+ sub get_metaobj\n+ {\n+ my $self = shift;\n+ return $self->{_tree}->get_child('candidate.meta');\n+ }\n+\n+ sub get_metadata\n+ {\n+ my $self = shift;\n+\n+ my $metaobj = $self->get_metaobj;\n+\n+ return Metadata->new_from_sha(repo => $self->{_repo},\n+ sha => $self->get_metaobj->objsum);\n+ }\n+\n+ sub commit\n+ {\n+ my ($self, %kwargs) = @_;\n+\n+ return $self->{_tree}->commit(%kwargs);\n+ }\n+\n+ sub get_head_commit_for_revision\n+ {\n+ my ($self, $revision_sha) = @_;\n+\n+ my $tree = $self->get_revision_tree;\n+ my $revision_obj = $tree->get_child(\"revision.$revision_sha\");\n+\n+ my $revision = Revision->new_from_sha(repo => $self->{_repo},\n+ sha => $revision_obj->objsum);\n+ return $revision->headcommit;\n+ }\n+\n+ sub _update_friendly_ref\n+ {\n+ my ($self, %kwargs) = @_;\n+ my @mandatory = ('sha');\n+ for my $k (@mandatory) {\n+ die \"No '$k' provided\" unless defined $kwargs{$k};\n+ }\n+\n+ my $suffix = defined $kwargs{suffix} ? \"_\" . $kwargs{suffix} : '';\n+ my $candidate_name = $self->{_name};\n+ my $remote = defined $kwargs{remote} ? $kwargs{remote} . \"/\" : '';\n+\n+ my $namespace = $self->get_checkout_ref_namespace;\n+ $self->{_repo}->command_oneline('update-ref',\n+ \"refs/$namespace/${remote}${candidate_name}$suffix\", $kwargs{sha});\n+ }\n+\n+ sub update_friendly_refs\n+ {\n+ my ($self, %kwargs) = @_;\n+ my $remote = $kwargs{remote};\n+\n+ my @revisions = $self->get_revisions;\n+ my $i = @revisions;\n+ $self->_update_friendly_ref(sha => $revisions[0]->headcommit,\n+ remote => $remote);\n+\n+ for my $revision (@revisions) {\n+ $self->_update_friendly_ref(\n+ sha => $revision->headcommit, suffix => \"v$i\",\n+ remote => $remote);\n+ $i--;\n+ }\n+ }\n+\n+ sub get_checkout_ref_namespace\n+ {\n+ my $self = shift;\n+\n+ my $namespace = try {\n+ $self->{_repo}->config('candidates.namespace');\n+ } catch Git::Error::Command with {\n+ return undef\n+ };\n+\n+ # Assume 'candidates' namespace if no namespace defined in config\n+ return $namespace // 'candidates';\n+ }\n+}\n+\n+sub _no_pr_err\n+{\n+ my ($pr, $remote) = @_;\n+\n+ err(\"No such candidate '$pr'\"\n+ . (defined $remote ? \" on remote '$remote'\" : \"\"));\n+}\n+\n+package MetaBase\n+{\n+ use GitUtils qw (inject_json cat_file validate_kwargs);\n+ use JSON;\n+ use Error qw(:try);\n+\n+ sub load_hash_from_sha\n+ {\n+ my ($class, $repo, $sha) = @_;\n+\n+ my $contents = try {\n+ return cat_file($repo, $sha, undef);\n+ }\n+ catch Git::Error::Command with {\n+ debug_print(\"exception caught in load_hash_from_sha\\n\");\n+ return undef;\n+ };\n+\n+ return undef unless defined $contents;\n+ return decode_json($contents);\n+ }\n+\n+ sub new\n+ {\n+ my ($class, %kwargs) = @_;\n+ bless {_sha => $kwargs{sha}}, $class;\n+ }\n+\n+ sub new_from_sha\n+ {\n+ my ($class, %kwargs) = @_;\n+\n+ validate_kwargs(\\%kwargs, 'repo', 'sha');\n+\n+ my $repo = $kwargs{repo};\n+ my $sha = $kwargs{sha};\n+\n+ my $self = $class->new(sha => $sha,\n+ %{$class->load_hash_from_sha($repo, $sha)});\n+ bless $self, $class;\n+ }\n+\n+ sub to_hash\n+ {\n+ ...\n+ }\n+\n+ sub json\n+ {\n+ my $self = shift;\n+\n+ my $j = JSON->new->utf8(1)->pretty(1)->canonical(1);\n+ my $json = $j->encode($self->to_hash);\n+ return $json;\n+ }\n+\n+ sub hash_and_insert\n+ {\n+ my ($self, $repo) = @_;\n+ return inject_json($repo, $self->json);\n+ }\n+\n+ sub sha { return $_[0]->{_sha}; }\n+}\n+\n+package Comment\n+{\n+ @Comment::ISA = 'MetaBase';\n+\n+ sub new\n+ {\n+ my ($class, %kwargs) = @_;\n+\n+ my $self = $class->SUPER::new(%kwargs);\n+ $self->{_msg} = $kwargs{msg};\n+ $self->{_line} = $kwargs{line};\n+ $self->{_author} = $kwargs{author};\n+ $self->{_time} = $kwargs{time};\n+ $self->{_timezone} = $kwargs{timezone};\n+\n+ bless $self, $class;\n+ }\n+\n+ sub to_hash\n+ {\n+ my $self = shift;\n+\n+ return {\n+ msg => $self->{_msg},\n+ line => $self->{_line},\n+ author => $self->{_author},\n+ time => $self->{_time},\n+ timezone => $self->{_timezone}\n+ };\n+ }\n+\n+ sub msg { return $_[0]->{_msg}; }\n+ sub line { return $_[0]->{_line}; }\n+ sub author { return $_[0]->{_author}; }\n+ sub time { return $_[0]->{_time}; } # seconds since epoch\n+\n+ # hours east of UTC (as a string e.g. BST is +0100, EST is -0500)\n+ sub timezone { return $_[0]->{_timezone}; }\n+}\n+\n+package Review\n+{\n+ @Review::ISA = 'Comment';\n+\n+ sub new\n+ {\n+ my ($class, %kwargs) = @_;\n+\n+ return undef if (defined $kwargs{vote}\n+ and !($kwargs{vote} =~ /(\\+|-)[012]/));\n+\n+ my $self = $class->SUPER::new(%kwargs);\n+ $self->{_vote} = $kwargs{vote};\n+ bless $self, $class;\n+ }\n+\n+ sub to_hash\n+ {\n+ my $self = shift;\n+ my $h = $self->SUPER::to_hash;\n+ $h->{vote} = $self->{_vote};\n+\n+ return $h;\n+ }\n+\n+ sub vote { return $_[0]->{_vote}; };\n+}\n+\n+package Revision\n+{\n+ @Revision::ISA = 'MetaBase';\n+ use GitUtils qw(looks_like_sha);\n+\n+ sub new\n+ {\n+ my ($class, %kwargs) = @_;\n+\n+ my @mandatory = ('headcommit', 'coverletter');\n+ for my $k (@mandatory) {\n+ die \"Invalid revision data $k\" unless defined $kwargs{$k};\n+ }\n+\n+ unless (looks_like_sha($kwargs{headcommit})) {\n+ die \"headcommit should be a sha\";\n+ }\n+\n+ my $self = $class->SUPER::new(%kwargs);\n+ $self->{_headcommit} = $kwargs{headcommit};\n+ $self->{_coverletter} = $kwargs{coverletter};\n+ $self->{_parentrevision} = $kwargs{parentrevision};\n+\n+ bless $self, $class;\n+ }\n+\n+ sub to_hash\n+ {\n+ my $self = shift;\n+\n+ return {\n+ headcommit => $self->{_headcommit},\n+ coverletter => $self->{_coverletter},\n+ parentrevision => $self->{_parentrevision},\n+ }\n+ }\n+\n+ sub headcommit { return $_[0]->{_headcommit}; }\n+ sub coverletter { return $_[0]->{_coverletter}; }\n+ sub parentrevision { return $_[0]->{_parentrevision}; }\n+}\n+\n+package Metadata\n+{\n+ @Metadata::ISA = 'MetaBase';\n+ use GitUtils qw (looks_like_sha validate_kwargs);\n+\n+ sub new\n+ {\n+ my ($class, %kwargs) = @_;\n+\n+ my @mandatory = ('state', 'landinto', 'ontop',\n+ 'latestrevision', 'metadataversion');\n+ validate_kwargs(\\%kwargs, @mandatory);\n+\n+ my $self = $class->SUPER::new(%kwargs);\n+ $self->{_state} = $self->set_state($kwargs{state});\n+ $self->{_landinto} = $kwargs{landinto};\n+ $self->{_ontop} = $kwargs{ontop};\n+ $self->{_latestrevision} = $kwargs{latestrevision};\n+ $self->{_metadataversion} = $kwargs{metadataversion};\n+\n+ bless $self, $class;\n+ }\n+\n+ sub new_from_sha\n+ {\n+ my $class = shift;\n+ my $self = $class->SUPER::new_from_sha(@_);\n+\n+ my $n = grep { $self->metadataversion eq $_ } @SUPPORTED_VERSIONS;\n+ if ($n == 0) {\n+ print STDERR \"Cannot load candidate, unsupported version: \"\n+ . $self->metadataversion . \".\\n\";\n+ exit 1\n+ }\n+\n+ bless $self, $class;\n+ }\n+\n+ sub new_from_candidate_sha\n+ {\n+ my ($class, %kwargs) = @_;\n+\n+ die \"No repo provided\" unless defined $kwargs{repo};\n+ die \"No sha provided\" unless defined $kwargs{sha};\n+\n+ my $repo = $kwargs{repo};\n+ my $sha = $kwargs{sha};\n+ my $metasha = $repo->command_oneline('rev-parse',\n+ \"$sha:candidate.meta\");\n+\n+ my $self = $class->SUPER::new_from_sha(repo => $repo, sha => $metasha);\n+\n+ bless $self, $class;\n+ }\n+\n+ sub to_hash\n+ {\n+ my $self = shift;\n+\n+ return {\n+ state => $self->{_state},\n+ landinto => $self->{_landinto},\n+ ontop => $self->{_ontop},\n+ latestrevision => $self->{_latestrevision},\n+ metadataversion => $self->{_metadataversion}\n+ };\n+ }\n+\n+ sub state { return $_[0]->{_state}; }\n+ sub landinto { return $_[0]->{_landinto}; }\n+ sub ontop { return $_[0]->{_ontop}; }\n+ sub latestrevision { return $_[0]->{_latestrevision}; }\n+ sub metadataversion { return $_[0]->{_metadataversion}; }\n+\n+ sub set_latestrevision\n+ {\n+ my ($self, $revision) = @_;\n+\n+ die \"Revision must be a sha\" unless looks_like_sha($revision);\n+ $self->{_latestrevision} = $revision;\n+ }\n+\n+ sub set_state\n+ {\n+ my ($self, $state) = @_;\n+\n+ my %valid_states = (merged => 1, active => 1);\n+\n+ unless (exists $valid_states{$state}) {\n+ die \"'$state' is not a valid state.\";\n+ }\n+\n+ $self->{_state} = $state;\n+ }\n+}\n+\n+sub err\n+{\n+ my $msg = shift;\n+ print STDERR \"$msg\\n\";\n+ exit 1\n+}\n+\n+sub candidate_already_exists\n+{\n+ my %kwargs = @_;\n+ my @mandatory = qw(repo name);\n+ validate_kwargs(\\%kwargs, @mandatory);\n+ my ($repo, $name) = @kwargs{'repo', 'name'};\n+\n+ return try {\n+ $repo->command('show-ref', \"$LOCAL_REF_PREFIX/${name}__meta\");\n+ return 1;\n+ }\n+ catch Git::Error::Command with {\n+ return 0;\n+ };\n+}\n+\n+sub _pr_revise_edit_msg\n+{\n+ my $pr = shift;\n+\n+ my $PR_EDIT_MSG = <new(CLEANUP => 1);\n+\n+ my $presum = $repo->hash_object('blob', $editmsg_fh->filename);\n+ if (defined $preplaced_text) {\n+ $preplaced_text =~ s/\\s+$/\\n\\n/;\n+ print $editmsg_fh $preplaced_text . $msg;\n+ }\n+ else {\n+ print $editmsg_fh $msg;\n+ }\n+\n+ git_editor $editmsg_fh->filename;\n+ my $postsum = $repo->hash_object('blob', $editmsg_fh->filename);\n+\n+ if ($presum eq $postsum) {\n+ print STDERR \"Message unchanged, aborting.\\n\";\n+ exit 1;\n+ }\n+\n+ if (!$allow_empty_msg and !file_has_none_whitespace_chars($editmsg_fh)) {\n+ err(\"Message contains only whitespace, aborting.\");\n+ }\n+\n+ my $s;\n+ seek $editmsg_fh, SEEK_SET, 0;\n+\n+ while (my $line = readline $editmsg_fh) {\n+ $s .= $line unless $line =~ /^#/;\n+ }\n+\n+ return $s;\n+}\n+\n+sub get_candidate_refs\n+{\n+ my %kwargs = @_;\n+ my @mandatory = ('repo');\n+ validate_kwargs(\\%kwargs, @mandatory);\n+ my ($repo, $name, $suffix, $remote) = @kwargs{'repo', 'name', 'suffix', 'remote'};\n+\n+ $name //= '[^\\/]+';\n+\n+ my $ref_pattern = defined $remote ?\n+ qr/refs\\/candidates\\/remotes\\/(\\Q$remote\\E)\\/${name}__$suffix/ :\n+ qr/refs\\/candidates\\/heads\\/${name}__$suffix/;\n+\n+ my @lines = $repo->command('show-ref');\n+ my @matching_refs = grep { /$ref_pattern/ } @lines;\n+ my %metarefs;\n+\n+ for my $ref (@matching_refs) {\n+ debug_print(\"a matching ref: $ref\\n\");\n+ my ($sha, $ref) = split(' ', $ref);\n+ $ref =~ /$ref_pattern/;\n+ $metarefs{$ref} = {sha => $sha};\n+ }\n+\n+ return \\%metarefs;\n+}\n+\n+sub get_candidate_meta_refs\n+{\n+ my %kwargs = @_;\n+ my ($repo, $name, $remote) = @kwargs{'repo', 'name', 'remote'};\n+ validate_kwargs(\\%kwargs, 'repo');\n+\n+ return get_candidate_refs(repo => $repo, suffix => 'meta',\n+ name => $name, remote => $remote);\n+}\n+\n+sub get_candidate_anchor_refs\n+{\n+ my %kwargs = @_;\n+ validate_kwargs(\\%kwargs, 'repo');\n+ my ($repo, $name, $anchor_suffix, $remote) = @kwargs{'repo', 'name',\n+ 'anchorsuffix', 'remote'};\n+ $anchor_suffix //= '';\n+\n+ return get_candidate_refs(repo => $repo, name => $name,\n+ suffix => \"anchor__$anchor_suffix\",\n+ remote => $remote);\n+}\n+\n+sub get_candidate_anchor_ref\n+{\n+ my %kwargs = @_;\n+\n+ my $h = get_candidate_anchor_refs(%kwargs);\n+ my $numof_refs = keys %$h;\n+ confess \"Multiple refs where only one expected\" if $numof_refs > 1;\n+ my ($ref, $ref_attrs) = each %$h;\n+ return ($ref, $ref_attrs);\n+}\n+\n+sub _name_from_pr_ref\n+{\n+ my $ref = shift;\n+\n+ $ref =~ /.*\\/(.*)__meta/;\n+ my $pr_name = $1;\n+ return $pr_name;\n+}\n+\n+sub display_meta_summary\n+{\n+ my ($metadata, $ref, $remote) = @_;\n+\n+ unless ($metadata) {\n+ print STDERR \"Warning: couldn't display summary for pr\"\n+ . \"with ref $ref\\n\";\n+ }\n+\n+ my $state = $metadata->{state};\n+ my $landinto = $metadata->{landinto};\n+\n+ my $pr = _name_from_pr_ref($ref);\n+\n+ $remote = defined $remote ? \" ($remote)\" : '';\n+ print \"$pr\\t$state\\t$landinto$remote\\n\";\n+}\n+\n+sub find_remote_pr\n+{\n+ my %kwargs = @_;\n+ validate_kwargs(\\%kwargs, 'repo', 'name', 'remote');\n+ my ($repo, $pr, $remote) = @kwargs{'repo', 'name', 'remote'};\n+\n+ my $href = get_candidate_meta_refs(repo => $repo,\n+ name => $pr, remote => $remote);\n+\n+ my ($ref, $ref_attrs) = each %$href;\n+ return ($ref, $ref_attrs);\n+}\n+\n+sub find_local_pr\n+{\n+ my %kwargs = @_;\n+ validate_kwargs(\\%kwargs, 'repo', 'name');\n+ my ($repo, $pr) = @kwargs{'repo', 'name'};\n+\n+ my $ref = \"$LOCAL_REF_PREFIX/${pr}__meta\";\n+ my $ref_attrs = get_candidate_meta_refs(repo => $repo)->{$ref};\n+\n+ return undef unless defined $ref_attrs;\n+ return ($ref, $ref_attrs);\n+}\n+\n+sub parse_candidate_ref\n+{\n+ # Candidate refs may be local or remotes,\n+ # since candidate names cannot contain a / character\n+ # the remote can easily be parsed\n+\n+ my $ref = shift;\n+\n+ my $remote;\n+ my $candidate_name;\n+\n+ if ($ref =~ /(?:([\\/\\w]+)\\/)?(\\w+(@\\w+)?)$/) {\n+ $remote = $1;\n+ $candidate_name = \"$2\";\n+ }\n+ else {\n+ err(\"Invalid candidate ref '$ref'.\");\n+ }\n+\n+ return ($remote, $candidate_name);\n+}\n+\n+sub parse_input_name\n+{\n+ my %kwargs = @_;\n+ my @mandatory = qw(repo name);\n+ validate_kwargs(\\%kwargs, @mandatory);\n+ my ($repo, $name) = @kwargs{'repo', 'name'};\n+\n+ my $revision;\n+\n+ # Assume the latest revision by default\n+ # else we will accept @ also @\n+ if ($name =~ /^(.*)@(.*)$/) {\n+ $name = $1;\n+ $revision = $repo->command_oneline('rev-parse', $2);\n+\n+ unless (looks_like_sha($revision)) {\n+ err(\"Could not parse revision from '$name'.\");\n+ }\n+ }\n+\n+ return ($name, $revision);\n+}\n+\n+sub maybe_update_ref\n+{\n+ my %kwargs = @_;\n+ validate_kwargs(\\%kwargs, 'repo', 'ref', 'sha');\n+ my ($repo, $ref, $sha) = @kwargs{'repo', 'ref', 'sha'};\n+\n+ if (is_object_present($repo, $ref)) {\n+ print STDERR \"localise_remote_pr: refusing to update existing ref '$ref' with '$sha'.\";\n+ return 0;\n+ }\n+\n+ debug_print(\"Running update-ref on ref $ref for sha $sha\");\n+ $repo->command('update-ref', $ref, $sha);\n+\n+ return 1;\n+}\n+\n+sub update_ref\n+{\n+ my %kwargs = @_;\n+ validate_kwargs(\\%kwargs, 'repo', 'ref', 'sha');\n+ my ($repo, $ref, $sha) = @kwargs{'repo', 'ref', 'sha'};\n+\n+ $repo->command('update-ref', $ref, $sha);\n+ return 1;\n+}\n+\n+sub localise_anchor_refs\n+{\n+ my %kwargs = @_;\n+ my @mandatory = ('repo', 'local_candidate_name',\n+ 'remote_candidate_name', 'remote', 'refs');\n+ validate_kwargs(\\%kwargs, @mandatory);\n+ my ($repo, $local_candidate_name, $remote_candidate_name,\n+ $remote, $remote_anchor_refs, $ignore_existing) = @kwargs{@mandatory, 'ignore_existing'};\n+\n+ my $update_fun = $ignore_existing ? \\&update_ref : \\&maybe_update_ref;\n+\n+ while (my ($ref, $ref_attrs) = each %$remote_anchor_refs) {\n+ my $sha = $ref_attrs->{sha};\n+ $ref =~ /$REMOTE_REF_PREFIX\\/$remote\\/${remote_candidate_name}__anchor__([0-9a-z]{40})/;\n+ my $anchor_sha = $1;\n+ my $update_complete = $update_fun->(repo => $repo,\n+ ref => \"$LOCAL_REF_PREFIX/${local_candidate_name}__anchor__${anchor_sha}\",\n+ sha => $sha);\n+\n+ return 0 unless $update_complete;\n+ }\n+\n+ return 1;\n+}\n+\n+sub localise_remote_candidate\n+{\n+ my %kwargs = @_;\n+ my @mandatory = qw(repo remote_candidate_name remote);\n+\n+ validate_kwargs(\\%kwargs, @mandatory);\n+\n+ my ($repo, $remote_pr_name, $remote, $local_pr_name) = @kwargs{'repo',\n+ 'remote_candidate_name', 'remote', 'local_candidate_name'};\n+ return undef unless (defined $remote_pr_name and defined $remote);\n+\n+ # if no local pr name provided then\n+ # assume the remote pr name as the local pr name\n+ $local_pr_name //= $remote_pr_name;\n+\n+ # Take all pr refs from remote and add them under our local prefix\n+ my $remote_meta_refs = get_candidate_meta_refs(repo => $repo,\n+ name => $remote_pr_name, remote => $remote);\n+ my $remote_anchor_refs = get_candidate_anchor_refs(repo => $repo,\n+ name => $remote_pr_name, remote => $remote);\n+\n+ my $numof_meta_refs = keys %$remote_meta_refs;\n+ my $numof_anchor_refs = keys %$remote_anchor_refs;\n+\n+ unless ($numof_meta_refs > 0 and $numof_anchor_refs > 0) {\n+ print STDERR \"failed to localise remote pr '$remote_pr_name' at '$remote'\\n\";\n+ return 0;\n+ }\n+\n+ my ($pr_meta_ref, $pr_meta_ref_attrs) = each %$remote_meta_refs;\n+ my $pr_meta_sha = $pr_meta_ref_attrs->{sha};\n+\n+ my $update_complete = maybe_update_ref(repo => $repo,\n+ ref => \"$LOCAL_REF_PREFIX/${local_pr_name}__meta\",\n+ sha => $pr_meta_sha);\n+\n+ $update_complete = localise_anchor_refs(repo => $repo,\n+ local_candidate_name => $local_pr_name, remote => $remote,\n+ remote_candidate_name => $remote_pr_name, refs => $remote_anchor_refs);\n+\n+ return 0 unless $update_complete;\n+\n+ my $metadata = Metadata->new_from_candidate_sha(repo => $repo, sha => $pr_meta_sha);\n+ my $revision = Revision->new_from_sha(repo => $repo,\n+ sha => $metadata->latestrevision);\n+\n+ return find_local_pr(repo => $repo, name => $local_pr_name);\n+}\n+\n+sub cmd_list\n+{\n+ my %kwargs = @_;\n+ confess \"No 'repo' provided\" unless $kwargs{repo};\n+ my ($repo, $remote) = @kwargs{'repo', 'remote'};\n+ my $metarefs = get_candidate_meta_refs(repo => $repo, remote => $remote);\n+\n+ my @sorted_refs = sort {\n+ my $pr_a = _name_from_pr_ref($a);\n+ my $pr_b = _name_from_pr_ref($b);\n+ return $pr_a cmp $pr_b;\n+ } keys %$metarefs;\n+\n+ for my $ref (@sorted_refs) {\n+ my $ref_attrs = $metarefs->{$ref};\n+\n+ my $metadata = Metadata->new_from_candidate_sha(repo => $repo,\n+ sha => $ref_attrs->{sha});\n+\n+ my $state = $metadata->state;\n+ my $landinto = $metadata->landinto;\n+ my $pr = _name_from_pr_ref($ref);\n+\n+ print \"$pr\\t$state\\t$landinto\\n\";\n+ }\n+}\n+\n+sub validate_candidate_name\n+{\n+ my $name = shift;\n+\n+ if ($name =~ /\\//) {\n+ err(\"The name of a candidate may not contain a '/'.\");\n+ }\n+}\n+\n+sub cmd_create\n+{\n+ my %kwargs = @_;\n+ my @mandatory = qw(name land msg allow_empty_msg repo);\n+ validate_kwargs(\\%kwargs, @mandatory);\n+\n+ my ($name, $land, $msg, $allow_empty_msg, $repo) = @kwargs{'name', 'land',\n+ 'msg', 'allow_empty_msg',\n+ 'repo'};\n+\n+ my $ontop = get_sha_at_ref($repo, $land);\n+\n+ validate_candidate_name($name);\n+ if (candidate_already_exists(repo => $repo, name => $name)) {\n+ err(\"A candidate with that name already exists\");\n+ }\n+\n+ require_clean_work_tree($repo);\n+\n+ my $head_commit = get_head_commit($repo);\n+ my $cover_letter = defined $msg ? \"$msg\\n\" :\n+ prompt_user(repo => $repo,\n+ msg => _pr_create_edit_msg($name, $head_commit,\n+ $land, $ontop),\n+ allow_empty_msg => $allow_empty_msg);\n+\n+ my $candidate = Candidate->new(name => $name, repo => $repo);\n+ my $revisiontree = GitTreeObject->new(repo => $repo,\n+ sha => undef, path => 'revisions');\n+\n+ my $revisionsum = Revision->new(headcommit => $head_commit,\n+ coverletter => $cover_letter,\n+ parentrevision => undef)->hash_and_insert($repo);\n+\n+ my $revisionobj = GitFileObject->new_regular_blob(repo => $repo,\n+ sha => $revisionsum, path => \"revision.$revisionsum\");\n+ $revisiontree->add_child($revisionobj);\n+\n+ my $metasum = Metadata->new(state => 'active', landinto => $land,\n+ ontop => $ontop,\n+ latestrevision => $revisionsum,\n+ metadataversion => $CANDIDATE_VERSION)->hash_and_insert($repo);\n+ my $metafileobj = GitFileObject->new_regular_blob(repo => $repo,\n+ sha => $metasum, path => 'candidate.meta');\n+ $candidate->add_child($metafileobj);\n+ $candidate->add_child($revisiontree);\n+\n+ my $commitsum = $candidate->commit(commitmsg => \"Create candidate $name\",\n+ parentsha => undef);\n+\n+ $repo->command_oneline('update-ref', '-m', \"Creation of candidate $name\",\n+ \"refs/candidates/heads/${name}__meta\", $commitsum);\n+\n+ $repo->command_oneline('update-ref', '-m',\n+ \"Copying anchor for candidate $name\",\n+ \"refs/candidates/heads/${name}__anchor__$revisionsum\", $head_commit);\n+\n+ $candidate->update_friendly_refs;\n+ print \"Candidate $name created successfully.\\n\";\n+}\n+\n+sub is_remote\n+{\n+ my %kwargs = @_;\n+ validate_kwargs(\\%kwargs, 'repo', 'remote');\n+\n+ my ($repo, $remote) = @kwargs{'repo', 'remote'};\n+\n+ my @remotes = $repo->command('remote');\n+ my $n = grep { $_ eq $remote } @remotes;\n+ return $n;\n+}\n+\n+sub cmd_submit\n+{\n+ my %kwargs = @_;\n+ my @mandatory = qw(worktreerepo repo name);\n+ for my $k (@mandatory) {\n+ die \"No '$k' provided\" unless defined $kwargs{$k};\n+ }\n+ my ($worktree_repo, $repo, $pr, $remote, $name) = @kwargs{\n+ 'worktreerepo', 'repo', 'name', 'remote', 'new_name'};\n+\n+ unless (is_remote(repo => $repo, remote => $remote)) {\n+ err(\"Failed to submit candidate: '$remote' is not a remote.\");\n+ }\n+\n+ # Get our current branch so we can switch back after the rebase\n+ my $initial_branch = get_head_branch($repo);\n+ unless (defined $initial_branch) {\n+ # HEAD is detached, just get the sha\n+ $initial_branch = $repo->command_oneline('rev-parse', 'HEAD');\n+ }\n+\n+ my ($pr_meta_ref, $ref_attrs) = find_local_pr(repo => $repo, name => $pr);\n+ unless ($pr_meta_ref) {\n+ err(\"candidate '$pr' does not exist\");\n+ }\n+ my $remote_pr_meta_ref = defined $name ?\n+ \"$LOCAL_REF_PREFIX/${name}__meta\" : $pr_meta_ref;\n+\n+ my $metadata = Metadata->new_from_candidate_sha(repo => $repo,\n+ sha => $ref_attrs->{sha});\n+ my $latest_revision = $metadata->latestrevision;\n+\n+ my ($pr_anchor_ref, ) = get_candidate_anchor_ref(repo => $repo, name => $pr,\n+ anchorsuffix => $latest_revision);\n+ unless ($pr_anchor_ref) {\n+ err(\"Could not find anchor ref for candidate \"\n+ . \"'$pr' revision $latest_revision\\n\"\n+ . \"candidate was not submitted.\");\n+ }\n+\n+ my $remote_anchor_ref = $pr_anchor_ref;\n+ if (defined $name) {\n+ $remote_anchor_ref =~ /$LOCAL_REF_PREFIX\\/${pr}_anchor__([0-9a-z]{40})/;\n+ my $sha = $1;\n+ $remote_anchor_ref = \"$LOCAL_REF_PREFIX/${name}_anchor__$sha\";\n+ }\n+\n+ # candidates must be uniquely named\n+ try {\n+ # 1. Determine if ref already exists on remote\n+ my @remote_refs = $worktree_repo->command('ls-remote',\n+ $remote, $pr_meta_ref);\n+ my $numof_matching_refs = @remote_refs;\n+ if ($numof_matching_refs == 0) {\n+ # This candidate hasn't been submitted before,\n+ # so we just push and we're done\n+ goto \"push\";\n+ }\n+\n+ # 2. Fetch from $remote's refs/candidates/heads/candidate__meta\n+ # to $REMOTE_REF_PREFIX//candidate__meta\n+ #\n+ my $remote_ref = \"$REMOTE_REF_PREFIX/$remote/${pr}__meta\";\n+ debug_print(\"running git fetch $remote $pr_meta_ref:$remote_ref\\n\");\n+ $worktree_repo->command(['fetch', $remote, \"$pr_meta_ref:$remote_ref\"],\n+ STDERR => $DEBUG ? undef : 0);\n+\n+ # 3. Rebase refs/candidates/heads/candidate__meta\n+ # on top of $REMOTE_REF_PREFIX//candidate__meta\n+ # (this cannot conflict)\n+ debug_print(\"running git rebase $remote_ref $pr_meta_ref\\n\");\n+\n+ try {\n+ my @rebase_output = $repo->command(\n+ ['rebase', $remote_ref, $pr_meta_ref],\n+ STDERR => $DEBUG ? undef : 0);\n+ }\n+ catch Git::Error::Command with {\n+ $repo->command(['rebase', '--abort'], STDERR => $DEBUG ? undef : 0);\n+ $repo->command(['checkout', $initial_branch], STDERR => $DEBUG ? undef : 0);\n+ err(\"Candidate cannot be submitted.\\n\"\n+ . \"The latest revision of '$pr' does not seem to be based\\n\"\n+ . \"on the latest revision of '$pr' at remote '$remote'\");\n+ };\n+\n+ debug_print(\"running git update-ref $pr_meta_ref HEAD\");\n+ $repo->command(['update-ref', $pr_meta_ref, 'HEAD'], STDERR => $DEBUG ? undef : 0);\n+\n+ # Git rebase will checkout $remote_ref, so we need to switch back\n+ # to whatever was checked out before here\n+ debug_print(\"switching back to initial branch\");\n+ $repo->command(['checkout', $initial_branch], STDERR => $DEBUG ? undef : 0);\n+push:\n+ # 4. Push refs/candidates/heads/candidate__meta to\n+ # refs/candidates/heads/candidate__meta on the remote\n+ # and refs/candidates/heads/candidate__anchor__$sha to\n+ # refs/candidates/heads/candidate__anchor__$sha on the remote\n+ debug_print(\"running git push $remote $pr_anchor_ref:$remote_anchor_ref\\n\");\n+ $worktree_repo->command('push', $remote, \"$pr_anchor_ref:$remote_anchor_ref\",\n+ \"$pr_meta_ref:$remote_pr_meta_ref\");\n+ }\n+ catch Git::Error::Command with {\n+ err(\"Candidate was not submitted.\");\n+ };\n+\n+ print \"Candidate was submitted successfully.\\n\";\n+}\n+\n+sub cmd_revise\n+{\n+ my %kwargs = @_;\n+ my @mandatory = qw(repo name);\n+ validate_kwargs(\\%kwargs, @mandatory);\n+\n+ my ($repo, $inputpr, $remote, $msg, $allow_empty_msg) = @kwargs{'repo',\n+ 'name', 'remote', 'msg', 'allow_empty_msg'};\n+\n+ my $candidate = find_candidate(repo => $repo, name => $inputpr,\n+ remote => $remote);\n+ _no_pr_err($inputpr, $remote) unless defined $candidate;\n+ my $name = $candidate->name;\n+\n+ my $head_commit = get_head_commit($repo);\n+ my $metadata = $candidate->get_metadata;\n+ my $latestrevision = $metadata->latestrevision;\n+ my $current_revision = $candidate->get_revision($latestrevision);\n+ if ($head_commit eq $current_revision->headcommit) {\n+ err(\"This revision looks the same as the current revision. Exiting.\");\n+ }\n+\n+ my $cover_letter = defined $msg ? \"$msg\\n\" :\n+ prompt_user(repo => $repo,\n+ msg => _pr_revise_edit_msg($name),\n+ allow_empty_msg => $allow_empty_msg,\n+ preplaced_text => $current_revision->coverletter);\n+\n+ my $revisionsum = Revision->new(headcommit => $head_commit,\n+ coverletter => $cover_letter,\n+ parentrevision => $latestrevision)->hash_and_insert($repo);\n+ my $revisionobj = GitFileObject->new_regular_blob(repo => $repo,\n+ sha => $revisionsum, path => \"revision.$revisionsum\");\n+\n+ my $revisiontree = $candidate->get_revision_tree;\n+ $revisiontree->add_child($revisionobj);\n+ $metadata->set_latestrevision($revisionsum);\n+ my $metasum = $metadata->hash_and_insert($repo);\n+ $candidate->get_metaobj->set_objsum($metasum);\n+\n+ my $commitsum = $candidate->commit(commitmsg => \"Revise $name\",\n+ parentsha => $candidate->sha);\n+\n+ $repo->command_oneline('update-ref', '-m', \"Revision of candidate $name\",\n+ \"refs/candidates/heads/${name}__meta\", $commitsum);\n+\n+ $repo->command_oneline('update-ref', '-m',\n+ \"Copying anchor for candidate $name\",\n+ \"refs/candidates/heads/${name}__anchor__$revisionsum\", $head_commit);\n+\n+ $candidate->update_friendly_refs;\n+\n+ print \"Candidate \" . $candidate->name . \" revised successfully.\\n\";\n+}\n+\n+sub _candidate_review_edit_msg\n+{\n+ my ($candidate, $revision) = @_;\n+\n+ my $CANDIDATE_EDIT_MSG = < $repo,\n+ name => $candidate_name, remote => $remote);\n+\n+ unless (defined $remote_ref) {\n+ err(\"candidate '$candidate_name' does not exist in remote '$remote'.\");\n+ }\n+\n+ # Get the latest revision for the local candidate_name\n+ my $local_sha = $local_ref_attrs->{sha};\n+ my $local_pr_metadata = Metadata->new_from_candidate_sha(repo => $repo,\n+ sha => $local_sha);\n+ my $local_latest_revision = $local_pr_metadata->latestrevision;\n+ debug_print(\"local_latest_revision: $local_latest_revision\\n\");\n+\n+ # Get the latest revision for the remote candidate_name\n+ my $remote_sha = $remote_ref_attrs->{sha};\n+ my $remote_pr_metadata = Metadata->new_from_candidate_sha(repo => $repo,\n+ sha => $remote_sha);\n+ my $remote_latest_revision = $remote_pr_metadata->latestrevision;\n+ debug_print(\"remote_latest_revision: $remote_latest_revision\\n\");\n+ my $remote_candidate = Candidate->new_from_sha(repo => $repo,\n+ sha => $remote_sha, name => $candidate_name, ref => $remote_ref);\n+ my $remote_revision_tree = $remote_candidate->get_revision_tree;\n+\n+ unless (defined $remote_revision_tree) {\n+ err(\"Could not build revision tree for candidate '$candidate_name'\");\n+ }\n+\n+ my %revision_objs = $remote_revision_tree->get_children;\n+ my %revisions;\n+ while (my ($path, $revision_obj) = each %revision_objs) {\n+ $revisions{$revision_obj->objsum} = Revision->new_from_sha(\n+ repo => $repo, sha => $revision_obj->objsum);\n+ }\n+\n+ # a kind of dummy revision with a parent pointing to\n+ # the current latest revision...\n+ my $rev = Revision->new(headcommit => sprintf(\"%040s\", ''),\n+ coverletter => 'dummy',\n+ parentrevision => $remote_latest_revision);\n+ my $found = 0;\n+\n+ while (defined $rev->parentrevision) {\n+ if ($rev->parentrevision eq $local_latest_revision) {\n+ $found = 1;\n+ last;\n+ }\n+\n+ $rev = $revisions{$rev->parentrevision};\n+ }\n+\n+ if ($found) {\n+ # The remote candidate_name is based off the current local candidate_name,\n+ # so merge the remote into the local\n+\n+ my $original_branch = get_head_branch($repo);\n+ unless (defined $original_branch) {\n+ # HEAD is detached, just get the sha\n+ $original_branch = $repo->command_oneline('rev-parse', 'HEAD');\n+ }\n+\n+ $repo->command(['checkout', $local_ref], STDERR => $DEBUG ? undef : 0);\n+ $repo->command(['merge', $remote_ref], STDERR => $DEBUG ? undef : 0);\n+ $repo->command(['update-ref', $local_ref, 'HEAD'], STDERR => $DEBUG ? undef : 0);\n+ $repo->command(['checkout', $original_branch], STDERR => $DEBUG ? undef : 0);\n+\n+ my $remote_anchor_refs = get_candidate_anchor_refs(repo => $repo,\n+ name => $candidate_name, remote => $remote);\n+\n+ my $update_complete = localise_anchor_refs(repo => $repo,\n+ ignore_existing => 1,\n+ local_candidate_name => $candidate_name, remote => $remote,\n+ remote_candidate_name => $candidate_name, refs => $remote_anchor_refs);\n+\n+ return ($candidate_name,\n+ find_local_pr(repo => $repo, name => $candidate_name));\n+ }\n+ else {\n+ # The remote candidate_name isn't related to the current local candidate_name\n+ # so get new name from --rename option,\n+ # we must error here if the user hasn't specified another name\n+ # with --rename\n+\n+ my $new_candidate_name;\n+ if (defined $rename) {\n+ $new_candidate_name = $rename;\n+ }\n+ else {\n+ err(\"A local candidate called '$candidate_name' already exists\\n\"\n+ . \"but the candidate '$candidate_name' at '$remote' does not seem \"\n+ . \"to be related to it.\\nRun with --rename to \"\n+ . \"localise '$candidate_name' under a new name.\");\n+ }\n+\n+ my ($r, ) = find_local_pr(repo => $repo, name => $new_candidate_name);\n+\n+ validate_candidate_name($new_candidate_name);\n+\n+ my $is_localised = localise_remote_candidate(\n+ remote_candidate_name => $candidate_name, remote => $remote,\n+ local_canidate_name => $new_candidate_name);\n+\n+ return undef unless $is_localised;\n+\n+ print \"candidate '$candidate_name' from '$remote' \"\n+ . \"is now known as '$new_candidate_name' . \\n\";\n+\n+ return ($new_candidate_name,\n+ find_local_pr(repo => $repo, name => $new_candidate_name));\n+ }\n+}\n+\n+sub copy_candidate_from_remote\n+{\n+ my %kwargs = @_;\n+ my @mandatory = qw(repo name remote);\n+ validate_kwargs(\\%kwargs, @mandatory);\n+ my ($repo, $candidate_name, $remote) = @kwargs{@mandatory};\n+\n+ # a. if there's no local version of this candidate_name,\n+ # we just copy the candidate_name from the remote\n+\n+ my ($ref, $ref_attrs) = find_remote_pr(repo => $repo,\n+ name => $candidate_name, remote => $remote);\n+ return undef unless defined $ref;\n+\n+ ($ref, $ref_attrs) = localise_remote_candidate(repo => $repo,\n+ remote_candidate_name => $candidate_name, remote => $remote);\n+\n+ unless (defined $ref) {\n+ err(\"Failed to localise candidate '$candidate_name' from remote '$remote'\");\n+ }\n+\n+ return ($candidate_name, $ref, $ref_attrs);\n+}\n+\n+sub find_candidate\n+{\n+ my %kwargs = @_;\n+ validate_kwargs(\\%kwargs, 'repo', 'name');\n+ my ($repo, $candidate_name, $remote, $rename) = @kwargs{'repo', 'name',\n+ 'remote', 'rename'};\n+\n+ my ($local_ref, $local_ref_attrs) = find_local_pr(repo => $repo,\n+ name => $candidate_name);\n+\n+ # 1. If we have no remote then things are very simple,\n+ # check for the candidate_name locally,\n+ # if it doesn't exist then we have no candidate_name, and we're done.\n+ unless (defined $remote) {\n+ return defined $local_ref_attrs->{sha} ?\n+ Candidate->new_from_sha(repo => $repo,\n+ sha => $local_ref_attrs->{sha},\n+ name => $candidate_name, ref => $local_ref) : undef;\n+ }\n+\n+ # 2. If we have a remote, then, find the candidate from the given remote\n+ my ($new_candidate_name, $ref, $ref_attrs) = defined $local_ref ?\n+ merge_remote_candidate_into_local(repo => $repo,\n+ name => $candidate_name,\n+ remote => $remote, local_ref => $local_ref,\n+ local_ref_attrs => $local_ref_attrs, rename => $rename) :\n+ copy_candidate_from_remote(repo => $repo, name => $candidate_name,\n+ remote => $remote);\n+\n+ # 3. We're note quite finished, we also need to set up the \"friendly refs\"\n+ # locally, to allow the user to checkout any revision of this newly\n+ # localised candidate.\n+ unless (defined $ref_attrs->{sha}) {\n+ return undef; # it might just not exist\n+ }\n+\n+ my $candidate = Candidate->new_from_sha(repo => $repo, ref => $ref,\n+ name => $new_candidate_name, sha => $ref_attrs->{sha});\n+\n+ $candidate->update_friendly_refs(remote => $remote);\n+ return $candidate;\n+}\n+\n+sub cmd_review\n+{\n+ my %kwargs = @_;\n+ my @mandatory = qw(repo name);\n+ for my $k (@mandatory) {\n+ die \"No '$k' provided\" unless defined $kwargs{$k};\n+ }\n+\n+ my ($repo, $unparsed_input_name, $remote, $vote, $msg, $rename,\n+ $allow_empty_msg) = @kwargs{'repo', 'name', 'remote', 'vote',\n+ 'msg', 'rename', 'allow_empty_msg'};\n+\n+ my ($input_name, $revision_sha) = parse_input_name(repo => $repo,\n+ name => $unparsed_input_name);\n+ my $candidate = find_candidate(repo => $repo, name => $input_name,\n+ remote => $remote, rename => $rename);\n+ _no_pr_err($input_name, $remote) unless defined $candidate;\n+ my $name = $candidate->name;\n+\n+ my $metadata = $candidate->get_metadata;\n+ $revision_sha //= $metadata->latestrevision;\n+\n+ my $comment = defined $msg ? \"$msg\\n\" :\n+ prompt_user(repo => $repo,\n+ msg => _candidate_review_edit_msg($name, $revision_sha),\n+ allow_empty_msg => $allow_empty_msg);\n+\n+ my $author_name = $repo->config('user.name');\n+ my $author_email = $repo->config('user.email');\n+\n+ my $current_time = time;\n+ my $timezone = $repo->get_tz_offset;\n+\n+ unless ($candidate->revision_exists($revision_sha)) {\n+ err(\"candidate '$name' does not have revision $revision_sha.\");\n+ }\n+\n+ my $reviewsum = Review->new(msg => $comment, line => undef,\n+ author => \"$author_name <$author_email>\",\n+ time => $current_time, timezone => $timezone,\n+ vote => $vote)->hash_and_insert($repo);\n+ my $review_file_obj = GitFileObject->new_regular_blob(repo => $repo,\n+ sha => $reviewsum, path => \"review.$reviewsum\");\n+\n+ unless ($candidate->review_tree_exists) {\n+ $candidate->add_child(\n+ GitTreeObject->new(repo => $repo, sha => undef, path => 'reviews'));\n+ }\n+ my $review_tree = $candidate->get_review_tree;\n+\n+ unless ($review_tree->child_exists($revision_sha)) {\n+ $review_tree->add_child(\n+ GitTreeObject->new(repo => $repo, sha => undef, path => $revision_sha));\n+ }\n+ $review_tree->get_child($revision_sha)->add_child($review_file_obj);\n+\n+ my $commitsum = $candidate->commit(commitmsg => 'Add review',\n+ parentsha => $candidate->sha);\n+ unless ($commitsum) {\n+ err(\"Failed to commit review\");\n+ }\n+\n+ $repo->command_oneline('update-ref',\n+ \"refs/candidates/heads/${name}__meta\", $commitsum);\n+\n+ $candidate->update_friendly_refs;\n+\n+ print \"Review added successfully\\n\";\n+}\n+\n+sub cmd_fetch\n+{\n+ my %kwargs = @_;\n+ my @mandatory =('repo', 'remote');\n+ validate_kwargs(\\%kwargs, @mandatory);\n+\n+ my ($repo, $remote) = @kwargs{'repo', 'remote'};\n+\n+ unless (is_remote(repo => $repo, remote => $remote)) {\n+ err(\"Failed to fetch candidates: '$remote' is not a remote.\");\n+ }\n+\n+ git_cmd_try {\n+ my $output = $repo->command('fetch', $remote,\n+ \"+refs/candidates/heads/*:$REMOTE_REF_PREFIX/$remote/*\");\n+ } \"Failed to fetch candidates\";\n+\n+ # Now for every candidate in $remote, create a ref in refs/heads/$pr_space\n+ my $remote_meta_refs = get_candidate_meta_refs(repo => $repo,\n+ remote => $remote);\n+\n+ while (my ($ref, $ref_attrs) = each %$remote_meta_refs) {\n+ my $name = _name_from_pr_ref($ref);\n+ my $metadata = Metadata->new_from_candidate_sha(repo => $repo,\n+ sha => $ref);\n+ my $revision = Revision->new_from_sha(repo => $repo,\n+ sha => $metadata->latestrevision);\n+\n+ my $candidate = Candidate->new_from_sha(repo => $repo,\n+ sha => $ref_attrs->{sha}, name => $name, ref => $ref);\n+ $candidate->update_friendly_refs(remote => $remote);\n+ }\n+}\n+\n+sub get_comments_from_tree\n+{\n+ my %kwargs = @_;\n+ my @mandatory = ('repo', 'tree', 'path', 'hashref');\n+ validate_kwargs(\\%kwargs, @mandatory);\n+ my ($repo, $tree, $path, $hashref) = @kwargs{@mandatory};\n+\n+ debug_print(\"in get_comments_from_tree: tree: $tree, path: $path\\n\");\n+\n+ my %children = $tree->get_children;\n+\n+ while (my ($k, $child) = each %children) {\n+ debug_print(\"child: $child\\n\");\n+\n+ if ($child->objtype eq 'tree') {\n+ get_comments_from_tree(repo => $repo, tree => $child,\n+ path => ($path ? \"$path/\" : $path) . $child->path,\n+ hashref => $hashref);\n+ }\n+ elsif ($child->objtype eq 'blob') {\n+ $hashref->{$path} //= ();\n+ my $c = Comment->new_from_sha(repo => $repo,\n+ sha => $child->objsum);\n+ debug_print(\"c: $c\\n\");\n+ push @{$hashref->{$path}}, $c;\n+ }\n+ else {\n+ err('commit obj in the comment tree. this is very unexpected...');\n+ }\n+ }\n+}\n+\n+sub get_comments_hashref\n+{\n+ my %kwargs = @_;\n+ my @mandatory = ('repo', 'tree');\n+ validate_kwargs(\\%kwargs, @mandatory);\n+ my ($repo, $tree) = @kwargs{'repo', 'tree'};\n+\n+ my $h = {};\n+ get_comments_from_tree(repo => $repo, tree=> $tree,\n+ path => \"\", hashref => $h);\n+ return $h;\n+}\n+\n+sub cmd_status\n+{\n+ my %kwargs = @_;\n+ my @mandatory = qw(repo name);\n+ validate_kwargs(\\%kwargs, @mandatory);\n+\n+ my ($repo, $unparsed_input_pr, $remote) = @kwargs{'repo', 'name', 'remote'};\n+\n+ my ($pr, $revision) = parse_input_name(repo => $repo,\n+ name => $unparsed_input_pr);\n+ my ($ref, $ref_attrs) = (defined $remote) ?\n+ find_remote_pr(repo => $repo, name => $pr, remote => $remote) :\n+ find_local_pr(repo => $repo, name => $pr);\n+\n+ unless (defined $ref) {\n+ _no_pr_err($pr, $remote);\n+ }\n+\n+ my $candidate = Candidate->new_from_sha(repo => $repo,\n+ sha => $ref_attrs->{sha}, name => $pr, ref => $ref);\n+ my $metadata = $candidate->get_metadata;\n+ $revision //= $metadata->latestrevision;\n+\n+ print \"Revision: $revision\\n\";\n+\n+ my $checkout_ref_namespace = $candidate->get_checkout_ref_namespace;\n+ my $p = (defined $remote ? \"$remote/\" : \"\") . $pr;\n+ print \"Ref: $checkout_ref_namespace/$p\\n\";\n+ print \"Status: \" . $metadata->state . \"\\n\";\n+ print \"Land: \" . $metadata->landinto . \"\\n\";\n+\n+ my $r = $candidate->get_revision($revision);\n+ my $headcommit = $r->headcommit;\n+\n+ if (defined $r->coverletter) {\n+ print \"\\n\";\n+ indent_print(string => $r->coverletter, indent => 0);\n+ print \"\\n\";\n+ }\n+\n+ my $diff_stat = $repo->command('diff', '--stat',\n+ $metadata->ontop, $headcommit);\n+ print \"$diff_stat\\n\";\n+\n+ my $reviews_for_revision = $candidate->get_reviews_for_revision($revision);\n+\n+ if (defined $reviews_for_revision) {\n+ print \"-\"x80;\n+ my @sorted_reviews = sort {\n+ $a->time <=> $b->time\n+ } @$reviews_for_revision;\n+\n+ my $numof_reviews = @sorted_reviews;\n+ print \"\\n$numof_reviews \" . ($numof_reviews == 1 ? \"review\" : \"reviews\") . \"\\n\";\n+ print \"-\"x80 . \"\\n\\n\";\n+\n+ for my $review (@sorted_reviews) {\n+ my $author = $review->author;\n+ my $vote = $review->vote;\n+ my $timezone = $review->timezone;\n+\n+ indent_print(string => \"Author: $author\", indent => 4);\n+\n+ my $datetime = strftime(\"%a %b %d %H:%M:%S %Y\", localtime($review->time));\n+ indent_print(string => \"Date: $datetime $timezone\", indent => 4);\n+ if (defined $review->vote) {\n+ indent_print(string => \"Vote: $vote\", indent => 4);\n+ }\n+\n+ my $msg = $review->msg;\n+ print \"\\n\";\n+ indent_print(string => $msg, indent => 4);\n+\n+ print \"\\n\" . '-'x80 . \"\\n\\n\"; # separate reviews\n+ }\n+ }\n+\n+ my $comments_for_revision = $candidate->get_comments_for_revision($revision);\n+\n+ if (defined $comments_for_revision) {\n+ print \"-\"x80;\n+ my $comments = get_comments_hashref(repo => $repo,\n+ tree => $comments_for_revision);\n+ my @files = sort { $a cmp $b } keys %$comments;\n+\n+ my $numof_comments;\n+ map { $numof_comments += @{$comments->{$_}} } @files;\n+ print \"\\n$numof_comments \" . ($numof_comments == 1 ? \"comment\" : \"comments\") . \"\\n\"; print \"-\"x80 . \"\\n\\n\";\n+\n+ for my $file (@files) {\n+ my $cs = $comments->{$file};\n+\n+ my @comments_on_file = sort { $a->time <=> $b->time } @$cs;\n+\n+ for my $comment (@comments_on_file) {\n+ print \"$file:\\n\";\n+ my $datetime = strftime(\"%a %b %d %H:%M:%S %Y\",\n+ localtime($comment->time));\n+ my $author = $comment->author;\n+ my $timezone = $comment->timezone;\n+\n+ indent_print(string => \"Author: $author\", indent => 4);\n+ indent_print(string => \"Date: $datetime $timezone\", indent => 4);\n+ print \"\\n\";\n+ indent_print(string => $comment->msg, indent => 4);\n+ print \"\\n\" . '-'x80 . \"\\n\\n\"; # separate reviews\n+ }\n+ }\n+ }\n+}\n+\n+sub cmd_comment_file\n+{\n+ my %kwargs = @_;\n+ my @mandatory = qw(repo name file);\n+ validate_kwargs(\\%kwargs, @mandatory);\n+\n+ my ($repo, $unparsed_input_name, $file_path, $remote, $msg,\n+ $rename, $allow_empty_msg) = @kwargs{'repo', 'name', 'file', 'remote',\n+ 'msg', 'rename', 'allow_empty_msg'};\n+\n+ my ($input_name, $revision_sha) = parse_input_name(repo => $repo,\n+ name => $unparsed_input_name);\n+ my $candidate = find_candidate(repo => $repo, name => $input_name,\n+ remote => $remote, rename => $rename);\n+\n+ _no_pr_err($input_name, $remote) unless defined $candidate;\n+\n+ my $name = $candidate->name;\n+ my $metadata = $candidate->get_metadata;\n+ # If no revision specified then assume latest\n+ $revision_sha //= $metadata->latestrevision;\n+\n+ # Check that this revision actually exists in the candidate tree first >.>\n+ unless ($candidate->revision_exists($revision_sha)) {\n+ err(\"candidate '$name' does not have revision $revision_sha.\");\n+ }\n+\n+ my $author_name = $repo->config('user.name');\n+ err(\"Could not get 'user.name' from config.\") unless defined $author_name;\n+\n+ my $author_email = $repo->config('user.email');\n+ err(\"Could not get 'user.email' from config.\") unless defined $author_email;\n+\n+ my $current_time = time;\n+ err(\"Could not get current time\") unless defined $current_time;\n+\n+ my $timezone = $repo->get_tz_offset;\n+ err(\"Could not get time zone\") unless defined $timezone;\n+\n+ # We need to have validated the file path argument *before*\n+ # we pop open a comment window\n+ my $revision = Revision->new_from_sha(repo => $repo, sha => $revision_sha);\n+ my $revision_head = $revision->headcommit;\n+ my $parent_revision;\n+\n+ if (defined $revision->parentrevision) {\n+ $parent_revision = Revision->new_from_sha(repo => $repo,\n+ sha => $revision->parentrevision);\n+ }\n+\n+ my @lines;\n+\n+ try {\n+ @lines = cat_file($repo, $revision_head, $file_path, STDERR => 0);\n+ }\n+ catch Git::Error::Command with {\n+ if (defined $parent_revision) {\n+ try {\n+ @lines = cat_file($repo, $parent_revision->headcommit,\n+ $file_path, STDERR => 0);\n+ }\n+ catch Git::Error::Command with {\n+ err(\"No such file '$file_path'\");\n+ };\n+ }\n+ else {\n+ # Check land\n+ try {\n+ @lines = cat_file($repo, $candidate->ontop, $file_path, STDERR => 0);\n+ }\n+ catch Git::Error::Command with {\n+ err(\"No such file '$file_path'\");\n+ };\n+ }\n+ };\n+\n+ my $comment = defined $msg ? \"$msg\\n\" :\n+ prompt_user(repo => $repo,\n+ msg => _pr_comment_edit_msg($name, $revision_sha),\n+ allow_empty_msg => $allow_empty_msg);\n+\n+ open(my $fh, '<', \\$comment);\n+ my @comment_lines;\n+ # Read comment from stdin (limit comment length)\n+ my $count = 0;\n+ while (my $line = readline($fh)) {\n+ push @comment_lines, $line;\n+ last if $count++ >= $MAX_COMMENT_LEN;\n+ }\n+\n+ my $comment_text = join('', @comment_lines);\n+ debug_print(\"comment_text: $comment_text\");\n+\n+ my $comment_line = -1;\n+ my $commentsum = Comment->new(msg => $comment_text, line => $comment_line,\n+ author => \"$author_name <$author_email>\", time => $current_time,\n+ timezone => $timezone)->hash_and_insert($repo);\n+ my $comment_file_obj = GitFileObject->new_regular_blob(repo => $repo,\n+ sha => $commentsum, path => \"comment.$commentsum\");\n+\n+ # Construct tree if necessary, and insert comment into it.\n+ unless ($candidate->comment_tree_exists) {\n+ $candidate->add_child(\n+ GitTreeObject->new(repo => $repo, sha => undef, path => 'comments'));\n+ }\n+ my $commenttree = $candidate->get_comment_tree;\n+\n+ unless ($commenttree->child_exists($revision_sha)) {\n+ $commenttree->add_child(\n+ GitTreeObject->new(repo => $repo, sha => undef, path => $revision_sha));\n+ }\n+ my $comment_subtree = $commenttree->get_child($revision_sha);\n+\n+ # What if our file is in a directory?\n+ # We can either escape the forward-slash\n+ # or have a tree for each directory...\n+ my $filetree;\n+ my $file;\n+ my $filetree_parent;\n+\n+ if ($file_path =~ /.*\\/.*/) {\n+ # Set up directory trees if needed\n+\n+ my @components = split('/', $file_path);\n+ my $numof_components = @components;\n+\n+ my @head = @components[0 .. $numof_components - 2];\n+ my $tail = $components[-1];\n+\n+ my $prev = $comment_subtree;\n+\n+ for my $component (@head) {\n+ unless ($prev->child_exists($component)) {\n+ $prev->add_child(\n+ GitTreeObject->new(repo => $repo, sha => undef, path => $component));\n+ }\n+ $prev = $prev->get_child($component);\n+ }\n+\n+ $file = $tail;\n+ $filetree_parent = $prev;\n+ }\n+ else {\n+ $file = $file_path;\n+ $filetree_parent = $comment_subtree;\n+ }\n+\n+ unless ($filetree_parent->child_exists($file)) {\n+ $filetree_parent->add_child(\n+ GitTreeObject->new(repo => $repo, sha => undef, path => $file));\n+ }\n+ $filetree_parent->get_child($file)->add_child($comment_file_obj);\n+\n+ my $commitsum = $candidate->commit(commitmsg => 'Add comment',\n+ parentsha => $candidate->sha);\n+\n+ unless ($commitsum) {\n+ err(\"Failed to commit comment\");\n+ }\n+\n+ $repo->command_oneline('update-ref', '-m', \"Add comment\",\n+ $candidate->ref, $commitsum);\n+\n+ print \"Comment added successfully\\n\";\n+}\n+\n+sub cmd_show_revisions\n+{\n+ my %kwargs = @_;\n+ my @mandatory = qw(repo name);\n+ for my $k (@mandatory) {\n+ die \"No '$k' provided\" unless defined $kwargs{$k};\n+ }\n+ my ($repo, $pr, $remote) = @kwargs{'repo', 'name', 'remote'};\n+\n+ my ($ref, $ref_attrs) = defined $remote ?\n+ find_remote_pr(repo => $repo, name => $pr, remote => $remote) :\n+ find_local_pr(repo => $repo, name => $pr);\n+\n+ _no_pr_err($pr, $remote) unless defined $ref;\n+\n+ my $candidate = Candidate->new_from_sha(repo => $repo,\n+ sha => $ref_attrs->{sha}, ref => $ref, name => $pr);\n+ my @revisions = $candidate->get_revisions;\n+ my $numof_revisions = @revisions;\n+ for my $rev (@revisions) {\n+ print $rev->sha . \" (v$numof_revisions)\\n\";\n+ $numof_revisions--;\n+ }\n+}\n+\n+sub cmd_remove\n+{\n+ my %kwargs = @_;\n+ my @mandatory = qw(name repo);\n+ for my $k (@mandatory) {\n+ die \"No '$k' provided\" unless defined $kwargs{$k};\n+ }\n+ my ($repo, $name) = @kwargs{'repo', 'name'};\n+\n+ # All we want to do is remove the refs associated with a candidate\n+ my ($ref, $ref_attrs) = find_local_pr(repo => $repo, name => $name);\n+\n+ _no_pr_err($name, undef) unless defined $ref;\n+\n+ # The anchors, what are they,\n+ # find the revisions, find the anchors\n+ my $candidate = Candidate->new_from_sha(repo => $repo,\n+ sha => $ref_attrs->{sha}, ref => $ref, name => $name);\n+ my @revisions = $candidate->get_revisions;\n+ my @anchor_refs = map {\n+ my($anchor_ref, ) = get_candidate_anchor_ref(repo => $repo, name => $name,\n+ anchorsuffix => $_->sha);\n+ $anchor_ref;\n+ } @revisions;\n+ remove_refs($repo, @anchor_refs, $ref);\n+\n+ print \"Candidate '$name' removed successfully.\\n\";\n+}\n+\n+sub cmd_gc_refs\n+{\n+ my %kwargs = @_;\n+ validate_kwargs(\\%kwargs, 'repo');\n+ my $repo = $kwargs{repo};\n+\n+ my @remotes = $repo->command('remote');\n+\n+ my @refs = map {\n+ my ($sha, $ref) = split(' ', $_);\n+ $ref;\n+ } $repo->command('show-ref');\n+ my @filtered_refs = grep { /^refs\\/candidates\\/remotes/ } @refs;\n+\n+ for my $ref (@filtered_refs) {\n+ my @bools = map {\n+ my $remote = $_;\n+ $ref =~ /^refs\\/candidates\\/remotes\\/$remote/;\n+ } @remotes;\n+\n+ my $any = grep { $_ eq 1 } @bools;\n+\n+ unless ($any) {\n+ print \"remove $ref\\n\" unless $any;\n+ $repo->command('update-ref', '-d', $ref);\n+ }\n+ }\n+}\n+\n+my %usages = (\n+ create => [\n+ 'create CANDIDATE LAND [-m ]', 'Create a new candidate'\n+ ],\n+ revise => [\n+ 'revise CANDIDATE [-m ] [--rename ]',\n+ 'Make a new revision of a candidate'\n+ ],\n+ review => [\n+ 'review CANDIDATE [--rename ] [--vote -2|-1|-0|+0|+1|+2] [-m ]',\n+ 'Review a candidate'\n+ ],\n+ submit => [\n+ 'submit REMOTE CANDIDATE [NAME]', 'Submit a candidate'\n+ ],\n+ fetch => [\n+ 'fetch REMOTE', 'Fetch candidates'\n+ ],\n+ list => [\n+ 'list [REMOTE]', 'List candidates along with their status'\n+ ],\n+ status => [\n+ 'status CANDIDATE', 'Show status of a candidate'\n+ ],\n+ 'comment-file' => [\n+ 'comment-file CANDIDATE FILE [--rename ] [-m ]',\n+ 'Comment on a file in a candidate'\n+ ],\n+ 'show-revisions' => [\n+ 'show-revisions CANDIDATE', 'List revisions (most recent first)'\n+ ],\n+ remove => [\n+ 'remove CANDIDATE', 'Remove a candidate'\n+ ],\n+ 'gc-refs' => [\n+ 'gc-refs', 'Removes any remote candidates that belong to remotes' .\n+ 'that no longer exist.'\n+ ]\n+);\n+\n+sub usage\n+{\n+ my $cmd = shift;\n+\n+ if ($cmd) {\n+ my ($usage, ) = @{$usages{$cmd}};\n+ print \"usage: git candidate \" . $usage . \"\\n\";\n+ }\n+ else {\n+ print \"usage: git candidate [options] [args]\\n\\n\"\n+ . \"Primary sub-commands are:\\n\";\n+\n+ my $max_len = 0;\n+\n+ my @xs = sort {\n+ my ($cmd_a, ) = @$a; my ($cmd_b, ) = @$b;\n+ $cmd_a cmp $cmd_b\n+ } values %usages;\n+\n+ for my $x (@xs) {\n+ my ($cmd_usage, $cmd_description) = @$x;\n+ $max_len = length($cmd_usage) > $max_len ?\n+ length($cmd_usage) : $max_len;\n+ }\n+\n+ # There must be max_len + gap padding between usage and description\n+ my $gap = 5;\n+ for my $x (@xs) {\n+ my ($cmd_usage, $cmd_description) = @$x;\n+ printf(\"%*s%s\\n\", ($max_len + $gap) * -1,\n+ $cmd_usage, $cmd_description); # -ve for left-justification\n+ }\n+ }\n+\n+ exit 1;\n+}\n+\n+sub git_repo_check\n+{\n+ my $repo = shift;\n+ # Yes I know this is awful\n+ unless (defined $repo) {\n+ try {\n+ Git::command_oneline('rev-parse')\n+ }\n+ catch Git::Error::Command with {\n+ # the rev-parse process prints the desired err msg for us\n+ exit 1;\n+ };\n+ }\n+}\n+\n+sub runcmd {\n+ my ($fun, %kwargs) = @_;\n+\n+ # Perform whatever global checks we need\n+ git_repo_check($kwargs{repo});\n+\n+ $fun->(%kwargs);\n+}\n+\n+my %cmds = (\n+ create => sub {\n+ my $msg;\n+ my $allow_empty_msg;\n+ GetOptions(\"m=s\" => \\$msg,\n+ \"allow-empty-message\" => \\$allow_empty_msg);\n+ my $argc = @ARGV;\n+ usage('create') unless $argc == 3;\n+\n+ my ($name, $land) = @ARGV[1, 2];\n+ my $repo = get_repo(with_user_worktree => 1);\n+\n+ runcmd(\\&cmd_create, name => $name, land => $land, msg => $msg,\n+ allow_empty_msg => $allow_empty_msg, repo => $repo);\n+ },\n+ list => sub {\n+ my $argc = @ARGV;\n+ usage('list') unless ($argc == 1 or $argc == 2);\n+\n+ my $remote = $ARGV[1];\n+ my $repo = get_repo;\n+\n+ runcmd(\\&cmd_list, repo => get_repo, remote => $remote);\n+ },\n+ submit => sub {\n+ my $argc = @ARGV;\n+ usage('submit') unless ($argc == 3 or $argc == 4);\n+\n+ my ($cmd, $remote, $name, $new_name) = @ARGV;\n+ runcmd(\\&cmd_submit, repo => get_repo(with_user_worktree => 0),\n+ worktreerepo => get_repo(with_user_worktree => 1),\n+ name => $name, remote => $remote, new_name => $new_name);\n+ },\n+ fetch => sub {\n+ my $argc = @ARGV;\n+ usage('fetch') unless $argc == 2;\n+\n+ my $remote = $ARGV[1];\n+ # Working tree is required since remotes are stored in .git/config\n+ # which is probably not tracked by the repo\n+ runcmd(\\&cmd_fetch, repo => get_repo(with_user_worktree => 1), remote => $remote);\n+ },\n+ revise => sub {\n+ my $msg;\n+ my $allow_empty_msg;\n+ GetOptions(\"m=s\" => \\$msg,\n+ 'allow-empty-message' => \\$allow_empty_msg);\n+ my $argc = @ARGV;\n+ usage('revise') unless $argc == 2;\n+\n+ my ($remote, $name) = parse_candidate_ref($ARGV[1]);\n+\n+ runcmd(\\&cmd_revise, repo => get_repo, name => $name,\n+ remote => $remote, msg => $msg,\n+ allow_empty_msg => $allow_empty_msg);\n+ },\n+ review => sub {\n+ my $msg;\n+ my $vote;\n+ my $rename;\n+ my $allow_empty_msg;\n+ GetOptions(\"m=s\" => \\$msg, \"vote=s\" => \\$vote, \"rename=s\" => \\$rename,\n+ 'allow-empty-message' => \\$allow_empty_msg);\n+ if (defined $vote and !($vote =~ /(\\+|-)[012]/)) {\n+ err(\"Invalid vote option. \"\n+ . \"(a vote must be one of -2, -1, -0, +0, +1, +2)\");\n+ }\n+\n+ my $argc = @ARGV;\n+ usage('review') unless $argc == 2;\n+\n+ my ($remote, $name) = parse_candidate_ref($ARGV[1]);\n+ runcmd(\\&cmd_review, repo => get_repo, name => $name,\n+ remote => $remote, vote => $vote, msg => $msg,\n+ rename => $rename, allow_empty_msg => $allow_empty_msg);\n+ },\n+ status => sub {\n+ my $argc = @ARGV;\n+ usage('status') unless $argc == 2;\n+\n+ my ($remote, $name) = parse_candidate_ref($ARGV[1]);\n+ runcmd(\\&cmd_status, repo => get_repo, name => $name,\n+ remote => $remote);\n+ },\n+ 'comment-file' => sub {\n+ my $msg;\n+ my $rename;\n+ my $allow_empty_msg;\n+ GetOptions(\"m=s\" => \\$msg, \"rename=s\" => \\$rename,\n+ 'allow-empty-message' => \\$allow_empty_msg);\n+ my $argc = @ARGV;\n+ usage('comment-file') unless ($argc == 2 or $argc == 3);\n+\n+ my ($remote, $name) = parse_candidate_ref($ARGV[1]);\n+\n+ my $file = $ARGV[2];\n+ runcmd(\\&cmd_comment_file, name => $name, file => $file,\n+ remote => $remote, msg => $msg, rename => $rename,\n+ allow_empty_msg => $allow_empty_msg, repo => get_repo);\n+ },\n+ 'show-revisions' => sub {\n+ my $argc = @ARGV;\n+ usage('show-revisions') unless $argc == 2;\n+\n+ my ($remote, $name) = parse_candidate_ref($ARGV[1]);\n+ runcmd(\\&cmd_show_revisions, repo => get_repo, name => $name,\n+ remote => $remote);\n+ },\n+ remove => sub {\n+ my $argc = @ARGV;\n+ usage('remove') unless ($argc == 2);\n+\n+ my $name = $ARGV[1];\n+ runcmd(\\&cmd_remove, repo => get_repo, name => $name);\n+ },\n+ 'gc-refs' => sub { runcmd(\\&cmd_gc_refs, repo => get_repo); }\n+);\n+\n+my $argc = @ARGV;\n+usage unless $argc >= 1;\n+\n+my $cmd = $ARGV[0];\n+usage unless exists $cmds{$cmd};\n+\n+$cmds{$cmd}->();\n-- \n2.1.4\n"},{"id":"273147","messageId":"1447160198-23296-3-git-send-email-richard.ipsum@codethink.co.uk","threadId":"40760","inReplyTo":"1447160198-23296-1-git-send-email-richard.ipsum@codethink.co.uk","subject":"[PATCH 2/2] contrib/git-candidate: Add README","fromName":"Richard Ipsum","fromEmail":"richard.ipsum@codethink.co.uk","sentAt":"2015-11-10T12:56:38Z","receivedAt":"2015-11-10T12:56:38Z","isPatch":true,"sender":{"key":"richard.ipsum@codethink.co.uk","avatar":null},"body":"Describes motivation for git-candidate and shows an example workflow.\n\nSigned-off-by: Richard Ipsum \n---\n contrib/git-candidate/README.md | 154 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 154 insertions(+)\n create mode 100644 contrib/git-candidate/README.md\n\ndiff --git a/contrib/git-candidate/README.md b/contrib/git-candidate/README.md\nnew file mode 100644\nindex 0000000..d2d4437\n--- /dev/null\n+++ b/contrib/git-candidate/README.md\n@@ -0,0 +1,154 @@\n+git-candidate\n+=============\n+\n+git-candidate provides candidate review and patch tracking,\n+it differs from other tools that provide this by storing _all_\n+content within git.\n+\n+## Why?\n+\n+Existing tools such as Github's pull-requests and Gerrit are already\n+in wide use, why bother with something new?\n+\n+We are concerned that whilst git is a distributed version control\n+system the systems used to store comments and reviews for content\n+under version control are usually centralised,\n+git-candidate aims to solve this by storing\n+all patch-tracking data in git proper.\n+\n+## Example review process\n+\n+### Contributor - Submits a candidate\n+\n+\t(hack hack hack)\n+\n+\t(feature)$ git commit -m \"Add archived repo\"\n+\t(feature)$ git candidate create archivedrepo master\n+\t\t-m \"Add support for archived repo\"\n+\tCandidate archivedrepo created successfully.\n+\t(feature)$ git candidate submit origin archivedrepo\n+\tCandidate was submitted successfully.\n+\n+### Upstream - Reviews candidate\n+\n+\t(master)$ git candidate fetch origin\n+\t(master)$ git candidate status origin/archiverepo\n+\tRevision: 6239bd72d597357af901718becae91cee2a32b73\n+\tRef: candidates/origin/archiverepo\n+\tStatus: active\n+\tLand: master\n+\n+\tAdd archived repo support\n+\n+\t lib/gitano/command.lua | 28 ++++++++++++++++++++++------\n+\t 1 file changed, 22 insertions(+), 6 deletions(-)\n+\n+\t(master)$ git show candidates/origin/archiverepo\n+\tcommit 2db28539c8fa7b81122382bcc526c6706c9e113a\n+\tAuthor: Richard Ipsum \n+\tDate: Thu Oct 8 10:43:22 2015 +0100\n+\n+\t Add support for archived repository masking in `ls`\n+\n+\t By setting `project.archived` to something truthy, a repository\n+\t is thusly masked from `ls` output unless --all is passed in.\n+\n+\t Signed-off-by: Richard Ipsum \n+\t....\n+\t....\n+\n+\n+\t(master)$ git candidate review origin/archiverepo --vote -1\n+\t\t-m \"Sorry, I'll need to see tests before I can accept this\"\n+\t(master)$ git candidate submit origin archiverepo\n+\tReview added successfully\n+\n+### Contributor - Revises candidate\n+\n+\t(master)$ git candidate fetch origin\n+\t(master)$ git candidate status origin/archiverepo\n+\tRevision: 6239bd72d597357af901718becae91cee2a32b73\n+\tRef: candidates/origin/archiverepo\n+\tStatus: active\n+\tLand: master\n+\n+\tAdd archived repo support\n+\n+\t lib/gitano/command.lua | 28 ++++++++++++++++++++++------\n+\t 1 file changed, 22 insertions(+), 6 deletions(-)\n+\n+\t--------------------------------------------------------------------------------\n+\t1 review\n+\t--------------------------------------------------------------------------------\n+\n+\t Author: Emmet Hikory \n+\t Date: Tue Oct 13 10:09:45 2015 +0100\n+\t Vote: -1\n+\n+\t Sorry, I'll need to see tests before I can accept this\n+\n+\t--------------------------------------------------------------------------------\n+\n+\t(hack hack hack add tests)\n+\n+\t(feature_v2)$ git log --oneline -1\n+\tEnsure the `ls` yarn checks for archived repos\n+\n+\t(feature_v2)$ git candidate revise origin/archiverepo\n+\t\t-m \"Add archived repo support with tests\"\n+\tCandidate archiverepo revised successfully.\n+\n+\t(feature_v2)$ git candidate submit origin archiverepo\n+\tCandidate was submitted successfully.\n+\n+### Upstream - Merges candidate\n+\n+\t(master)$ git candidate fetch origin\n+\t(master)$ git candidate status origin/archiverepo\n+\tRevision: 4cd3d1197d399005a713ca55f126a9086356a072\n+\tRef: candidates/origin/archiverepo\n+\tStatus: active\n+\tLand: master\n+\n+\tAdd archived repo support with tests\n+\n+\t lib/gitano/command.lua | 28 ++++++++++++++++++++++------\n+\t testing/02-commands-ls.yarn | 19 +++++++++++++++++++\n+\t 2 files changed, 41 insertions(+), 6 deletions(-)\n+\n+\t(master)$ git candidate review origin/archiverepo --vote +2\n+\t\t-m \"Looks good, merging. Thanks for your efforts\"\n+\tReview added successfully\n+\n+\t(master)$ git candidate submit origin archiverepo\n+\tCandidate was submitted successfully.\n+\n+\t(master)$ git merge candidates/origin/archiverepo\n+\t(master)$ git push origin master\n+\n+### Contributor - Observes candidate has been accepted\n+\n+\t(feature_v2)$ git candidate fetch origin\n+\t(feature_v2)$ git candidate status origin/archiverepo\n+\tRevision: 4cd3d1197d399005a713ca55f126a9086356a072\n+\tRef: candidates/origin/archiverepo\n+\tStatus: active\n+\tLand: master\n+\n+\tAdd archived repo support with tests\n+\n+\t lib/gitano/command.lua | 28 ++++++++++++++++++++++------\n+\t testing/02-commands-ls.yarn | 19 +++++++++++++++++++\n+\t 2 files changed, 41 insertions(+), 6 deletions(-)\n+\n+\t--------------------------------------------------------------------------------\n+\t1 review\n+\t--------------------------------------------------------------------------------\n+\n+\t Author: Emmet Hikory \n+\t Date: Tue Oct 13 10:35:00 2015 +0100\n+\t Vote: +2\n+\n+\t\tLooks good, merging. Thanks for your efforts\n+\n+\t--------------------------------------------------------------------------------\n-- \n2.1.4\n"},{"id":"273158","messageId":"1447186751.20147.24.camel@twopensource.com","threadId":"40760","inReplyTo":"1447160198-23296-3-git-send-email-richard.ipsum@codethink.co.uk","subject":"Re: [PATCH 2/2] contrib/git-candidate: Add README","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-11-10T20:19:11Z","receivedAt":"2015-11-10T20:19:11Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"I didn't actually read the code. Instead, I started with the README and\ndecided to provide both text and UX comments all mixed up. These are\nmostly my personal preferences; take them or leave them as you choose. \n\nI'm really excited about this tool and I think it's got great potential!\n\nOn Tue, 2015-11-10 at 12:56 +0000, Richard Ipsum wrote:\n> Describes motivation for git-candidate and shows an example workflow.\n> \n> Signed-off-by: Richard Ipsum \n> ---\n> contrib/git-candidate/README.md | 154 ++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 154 insertions(+)\n> create mode 100644 contrib/git-candidate/README.md\n> \n> diff --git a/contrib/git-candidate/README.md b/contrib/git-candidate/README.md\n> new file mode 100644\n> index 0000000..d2d4437\n> --- /dev/null\n> +++ b/contrib/git-candidate/README.md\n> @@ -0,0 +1,154 @@\n> +git-candidate\n\nI have not heard the name \"candidate\" used this way. What about \"git\ncodereview\"? \n\n> +=============\n> +\n> +git-candidate provides candidate review and patch tracking,\n> +it differs from other tools that provide this by storing _all_\n> +content within git.\n> +\n> +## Why?\n\nI've made a few suggestions below that you might think are out of scope.\nIf they are, it might be good to have a \"non-goals\" section so that\npeople know what the scope of the tool is.\n\n> +Existing tools such as Github's pull-requests and Gerrit are already\n> +in wide use, why bother with something new?\n> +\n> +We \n\nwho? \n\n> are concerned that whilst \n\nToday I learned: \"whilst\" can be used in the sense of \"although\" (I had\npreviously thought only \"while\" could be used this way, but I was wrong!\n)\n\n> git is a distributed version control\n> +system the systems used to store comments and reviews for content\n\ninsert comma after \"system\"\n\n> +under version control are usually centralised,\n\nreplace comma with period.\n\n> +git-candidate aims to solve this by storing\n> +all patch-tracking data in git proper.\n\ns/tracking/tracking and review/ ? Or something\n\n> +## Example review process\n> +\n> +### Contributor - Submits a candidate\n> +\n> +\t(hack hack hack)\n> +\n> +\t(feature)$ git commit -m \"Add archived repo\"\n> +\t(feature)$ git candidate create archivedrepo master\n> +\t\t-m \"Add support for archived repo\"\n> +\tCandidate archivedrepo created successfully.\n> +\t(feature)$ git candidate submit origin archivedrepo\n> +\tCandidate was submitted successfully.\n> +### Upstream - Reviews candidate\n\nWhat happens if a third party wants to review candidate? OR is this\njust the same as if upstream does it?\n\n> +\t(master)$ git candidate fetch origin\n> +\t(master)$ git candidate status origin/archiverepo\n> +\tRevision: 6239bd72d597357af901718becae91cee2a32b73\n> +\tRef: candidates/origin/archiverepo\n> +\tStatus: active\n> +\tLand: master\n\nCould this be \"Merge: master\"? Or something that doesn't invent a new\nterm?\n\n> +\tAdd archived repo support\n> +\n> +\t lib/gitano/command.lua | 28 ++++++++++++++++++++++------\n> +\t 1 file changed, 22 insertions(+), 6 deletions(-)\n> +\n> +\t(master)$ git show candidates/origin/archiverepo\n> +\tcommit 2db28539c8fa7b81122382bcc526c6706c9e113a\n> +\tAuthor: Richard Ipsum \n\nProbably better to use example.com addresses in the README rather than\nreal people. Git traditionally uses \"A U Thor\" as the fake name.\n\n> +\tDate: Thu Oct 8 10:43:22 2015 +0100\n> +\n> +\t Add support for archived repository masking in `ls`\n> +\n> +\t By setting `project.archived` to something truthy, a repository\n> +\t is thusly masked from `ls` output unless --all is passed in.\n> +\n> +\t Signed-off-by: Richard Ipsum \n> +\t....\n> +\t....\n> +\n> +\n> +\t(master)$ git candidate review origin/archiverepo --vote -1\n> +\t\t-m \"Sorry, I'll need to see tests before I can accept this\"\n\nAre per-line or per-commit comments supported? If so, please add an\nexample of this.\n\n> +\t(master)$ git candidate submit origin archiverepo\n> +\tReview added successfully\n\nIs the contributor automatically (optionally) emailed on this? If not,\nconsider this a feature request for this.\n\n> +### Contributor - Revises candidate\n> +\n> +\t(master)$ git candidate fetch origin\n> +\t(master)$ git candidate status origin/archiverepo\n> +\tRevision: 6239bd72d597357af901718becae91cee2a32b73\n> +\tRef: candidates/origin/archiverepo\n> +\tStatus: active\n> +\tLand: master\n> +\n> +\tAdd archived repo support\n> +\n> +\t lib/gitano/command.lua | 28 ++++++++++++++++++++++------\n> +\t 1 file changed, 22 insertions(+), 6 deletions(-)\n> +\n> +\t--------------------------------------------------------------------------------\n> +\t1 review\n> +\t--------------------------------------------------------------------------------\n> +\n> +\t Author: Emmet Hikory \n> +\t Date: Tue Oct 13 10:09:45 2015 +0100\n> +\t Vote: -1\n> +\n> +\t Sorry, I'll need to see tests before I can accept this\n> +\n> +\t--------------------------------------------------------------------------------\n> +\n> +\t(hack hack hack add tests)\n> +\n> +\t(feature_v2)$ git log --oneline -1\n> +\tEnsure the `ls` yarn checks for archived repos\n> +\n> +\t(feature_v2)$ git candidate revise origin/archiverepo\n> +\t\t-m \"Add archived repo support with tests\"\n> +\tCandidate archiverepo revised successfully.\n> +\n> +\t(feature_v2)$ git candidate submit origin archiverepo\n> +\tCandidate was submitted successfully.\n> +\n> +### Upstream - Merges candidate\n> +\n> +\t(master)$ git candidate fetch origin\n> +\t(master)$ git candidate status origin/archiverepo\n> +\tRevision: 4cd3d1197d399005a713ca55f126a9086356a072\n> +\tRef: candidates/origin/archiverepo\n> +\tStatus: active\n> +\tLand: master\n> +\n> +\tAdd archived repo support with tests\n> +\n> +\t lib/gitano/command.lua | 28 ++++++++++++++++++++++------\n> +\t testing/02-commands-ls.yarn | 19 +++++++++++++++++++\n> +\t 2 files changed, 41 insertions(+), 6 deletions(-)\n\n\"git candidate diff\" might be nice too to show the diff between v1 and\nv2. You might even have \"git candidate commit-diff\" (or some better\nname) so you can see which commit has changed in a changeset containing\nmultiple commits. \n\n> +\t(master)$ git candidate review origin/archiverepo --vote +2\n> +\t\t-m \"Looks good, merging. Thanks for your efforts\"\n> +\tReview added successfully\n\nIs that +2 \"+1 because I like it, +1 because I previously -1'd it?\" If\nso, it might be nice to have --replace-vote so you don't have to track,\n\"wait, I did -1, then +1, then -1 again...\"\n\n> +\t(master)$ git candidate submit origin archiverepo\n> +\tCandidate was submitted successfully.\n\nI don't understand what the verb \"submit\" means here. Is it \"mark this\nas accepted\"? If so, \"accept\" might be a better word. \n\n> +\t(master)$ git merge candidates/origin/archiverepo\n\nI would like \"git candidate merge\" to do a submit+merge the way that\npull does a fetch+merge. It seems like the common case. Also, if it\nturns out at this point that there's a merge conflict, I might want to\nback out the acceptance.\n\n> +\t(master)$ git push origin master\n> +\n> +### Contributor - Observes candidate has been accepted\n> +\n> +\t(feature_v2)$ git candidate fetch origin\n> +\t(feature_v2)$ git candidate status origin/archiverepo\n> +\tRevision: 4cd3d1197d399005a713ca55f126a9086356a072\n> +\tRef: candidates/origin/archiverepo\n> +\tStatus: active\n> +\tLand: master\n> +\n> +\tAdd archived repo support with tests\n> +\n> +\t lib/gitano/command.lua | 28 ++++++++++++++++++++++------\n> +\t testing/02-commands-ls.yarn | 19 +++++++++++++++++++\n> +\t 2 files changed, 41 insertions(+), 6 deletions(-)\n> +\n> +\t--------------------------------------------------------------------------------\n> +\t1 review\n> +\t--------------------------------------------------------------------------------\n> +\n> +\t Author: Emmet Hikory \n> +\t Date: Tue Oct 13 10:35:00 2015 +0100\n> +\t Vote: +2\n> +\n> +\t\tLooks good, merging. Thanks for your efforts\n> +\n> +\t--------------------------------------------------------------------------------\n\nYou should include here \"git candidate remove archiverepo\". And\nsomewhere an example of \"git candidate list\".\n"},{"id":"273180","messageId":"20151111094816.GA2949@salo","threadId":"40760","inReplyTo":"1447186751.20147.24.camel@twopensource.com","subject":"Re: [PATCH 2/2] contrib/git-candidate: Add README","fromName":"Richard Ipsum","fromEmail":"richard.ipsum@codethink.co.uk","sentAt":"2015-11-11T09:48:17Z","receivedAt":"2015-11-11T09:48:17Z","isPatch":true,"sender":{"key":"richard.ipsum@codethink.co.uk","avatar":null},"body":"On Tue, Nov 10, 2015 at 03:19:11PM -0500, David Turner wrote:\n> I didn't actually read the code. Instead, I started with the README and\n> decided to provide both text and UX comments all mixed up. These are\n> mostly my personal preferences; take them or leave them as you choose. \n> \n> I'm really excited about this tool and I think it's got great potential!\n\nIt's great to hear that, I think there is a need for a tool like this.\n\n> \n> On Tue, 2015-11-10 at 12:56 +0000, Richard Ipsum wrote:\n> > Describes motivation for git-candidate and shows an example workflow.\n> > \n[snip]\n> \n> I have not heard the name \"candidate\" used this way. What about \"git\n> codereview\"? \n\nI admit to being quite bad at naming things,\noriginally we were going to call this git-pull-request after the\ninitial concept provided by Daniel Silverstone[1]. We later realised\nthat we'd created something more flexible than pull-requests:\ngit-candidate can be used with a pull-request model or a more tranditional\npatch submission model.\n\nI certainly have nothing against renaming this tool if there's some\nagreement on a new name, though I will point out that it's possible\nthat the content of a candidate is not code.\n\n> \n> > +=============\n> > +\n> > +git-candidate provides candidate review and patch tracking,\n> > +it differs from other tools that provide this by storing _all_\n> > +content within git.\n> > +\n> > +## Why?\n> \n> I've made a few suggestions below that you might think are out of scope.\n> If they are, it might be good to have a \"non-goals\" section so that\n> people know what the scope of the tool is.\n> \n> > +Existing tools such as Github's pull-requests and Gerrit are already\n> > +in wide use, why bother with something new?\n> > +\n> > +We \n> \n> who? \n\nAt the moment the 'we' is Codethink who are sponsoring this work,\nbut I'd like to resolve the text to not need that.\n\n> \n> > are concerned that whilst \n> \n> Today I learned: \"whilst\" can be used in the sense of \"although\" (I had\n> previously thought only \"while\" could be used this way, but I was wrong!\n> )\n\n:)\n\n> \n> > git is a distributed version control\n> > +system the systems used to store comments and reviews for content\n> \n> insert comma after \"system\"\n\nack\n\n> \n> > +under version control are usually centralised,\n> \n> replace comma with period.\n\nack\n\n> \n> > +git-candidate aims to solve this by storing\n> > +all patch-tracking data in git proper.\n> \n> s/tracking/tracking and review/ ? Or something\n\n\"all patch-tracking and review data in git proper\" would probably be better.\n\n> \n> > +## Example review process\n> > +\n> > +### Contributor - Submits a candidate\n> > +\n> > +\t(hack hack hack)\n> > +\n> > +\t(feature)$ git commit -m \"Add archived repo\"\n> > +\t(feature)$ git candidate create archivedrepo master\n> > +\t\t-m \"Add support for archived repo\"\n> > +\tCandidate archivedrepo created successfully.\n> > +\t(feature)$ git candidate submit origin archivedrepo\n> > +\tCandidate was submitted successfully.\n> > +### Upstream - Reviews candidate\n> \n> What happens if a third party wants to review candidate? OR is this\n> just the same as if upstream does it?\n\nExactly, the third party follows the same process as upstream.\n\n> \n> > +\t(master)$ git candidate fetch origin\n> > +\t(master)$ git candidate status origin/archiverepo\n> > +\tRevision: 6239bd72d597357af901718becae91cee2a32b73\n> > +\tRef: candidates/origin/archiverepo\n> > +\tStatus: active\n> > +\tLand: master\n> \n> Could this be \"Merge: master\"? Or something that doesn't invent a new\n> term?\n\nConsider it done. :)\n\n> \n> > +\tAdd archived repo support\n> > +\n> > +\t lib/gitano/command.lua | 28 ++++++++++++++++++++++------\n> > +\t 1 file changed, 22 insertions(+), 6 deletions(-)\n> > +\n> > +\t(master)$ git show candidates/origin/archiverepo\n> > +\tcommit 2db28539c8fa7b81122382bcc526c6706c9e113a\n> > +\tAuthor: Richard Ipsum \n> \n> Probably better to use example.com addresses in the README rather than\n> real people. Git traditionally uses \"A U Thor\" as the fake name.\n\nWill do.\n\n> \n> > +\tDate: Thu Oct 8 10:43:22 2015 +0100\n> > +\n> > +\t Add support for archived repository masking in `ls`\n> > +\n> > +\t By setting `project.archived` to something truthy, a repository\n> > +\t is thusly masked from `ls` output unless --all is passed in.\n> > +\n> > +\t Signed-off-by: Richard Ipsum \n> > +\t....\n> > +\t....\n> > +\n> > +\n> > +\t(master)$ git candidate review origin/archiverepo --vote -1\n> > +\t\t-m \"Sorry, I'll need to see tests before I can accept this\"\n> \n> Are per-line or per-commit comments supported? If so, please add an\n> example of this.\n\nThat's work in progress, there will soon be a --line option to the\n'comment-file' command, the status command will then render per-line\ncomments.\n\n> \n> > +\t(master)$ git candidate submit origin archiverepo\n> > +\tReview added successfully\n> \n> Is the contributor automatically (optionally) emailed on this? If not,\n> consider this a feature request for this.\n\nThere's no server integration of any kind at the moment,\nthis is clearly something we will want to add.\n\n> \n> > +### Contributor - Revises candidate\n> > +\n> > +\t(master)$ git candidate fetch origin\n[snip]\n> > +\t(master)$ git candidate status origin/archiverepo\n> > +\tRevision: 4cd3d1197d399005a713ca55f126a9086356a072\n> > +\tRef: candidates/origin/archiverepo\n> > +\tStatus: active\n> > +\tLand: master\n> > +\n> > +\tAdd archived repo support with tests\n> > +\n> > +\t lib/gitano/command.lua | 28 ++++++++++++++++++++++------\n> > +\t testing/02-commands-ls.yarn | 19 +++++++++++++++++++\n> > +\t 2 files changed, 41 insertions(+), 6 deletions(-)\n> \n> \"git candidate diff\" might be nice too to show the diff between v1 and\n> v2. You might even have \"git candidate commit-diff\" (or some better\n> name) so you can see which commit has changed in a changeset containing\n> multiple commits. \n\nYes, we definitely want that. I think \"git candidate diff\" to diff\nbetween revisions would be sufficient, and it could take a list of files\nto diff as an arg?\n\n> \n> > +\t(master)$ git candidate review origin/archiverepo --vote +2\n> > +\t\t-m \"Looks good, merging. Thanks for your efforts\"\n> > +\tReview added successfully\n> \n> Is that +2 \"+1 because I like it, +1 because I previously -1'd it?\" If\n> so, it might be nice to have --replace-vote so you don't have to track,\n> \"wait, I did -1, then +1, then -1 again...\"\n\nVotes are per-review, perhaps they should simply be per-revision?\nThen --vote sets the vote for the revision and there's no need for\na --replace-vote option?\nThis would use user.name and user.email as identification.\n\n> \n> > +\t(master)$ git candidate submit origin archiverepo\n> > +\tCandidate was submitted successfully.\n> \n> I don't understand what the verb \"submit\" means here. Is it \"mark this\n> as accepted\"? If so, \"accept\" might be a better word. \n\nI'm tempted to change this to 'push', 'submit' comes from gerrit.\n\n> \n> > +\t(master)$ git merge candidates/origin/archiverepo\n> \n> I would like \"git candidate merge\" to do a submit+merge the way that\n> pull does a fetch+merge. It seems like the common case. Also, if it\n> turns out at this point that there's a merge conflict, I might want to\n> back out the acceptance.\n\nThere is currently no git-candidate-merge, I removed this recently\nbecause I decided that you can merge candidates with git-merge\nand that this is more flexible. Often a candidate will be rebased\nbefore it is merged, it would be nice to avoid having to create\na merge command that needs to handle all the different cases for\nmerging a candidate.\n\n> \n> > +\t(master)$ git push origin master\n> > +\n> > +### Contributor - Observes candidate has been accepted\n> > +\n[snip]\n> \n> You should include here \"git candidate remove archiverepo\". And\n> somewhere an example of \"git candidate list\".\n> \n\nGood point, will do.\n\nThank you for taking the time to provide this excellent feedback. :)\n\n[1]: https://www.gitano.org.uk/ideas/git-pull-request/\n"},{"id":"273182","messageId":"5643107B.20501@alum.mit.edu","threadId":"40760","inReplyTo":"1447160198-23296-1-git-send-email-richard.ipsum@codethink.co.uk","subject":"Re: [PATCH 0/2] git-candidate: git based patch tracking and review","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2015-11-11T09:55:07Z","receivedAt":"2015-11-11T09:55:07Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/10/2015 01:56 PM, Richard Ipsum wrote:\n> I've continued my work[1] to add patch tracking and candidate review capability\n> to git.\n> \n> git-candidate now has a more git-like user interface, so remote candidates\n> can now be specified in a similar way to remote refs (e.g. origin/candidate)\n> as well as various other improvements, such as versioned metadata.\n\nThis is a really interesting project. I've seen a blog post or two\nproposing to store bug tracker information in Git in a distributed way,\nbut I don't recall anything about doing the same for code review\ninformation.\n\nI would be interested to hear about the design of your system at an\nabstract technical level. What do you store in Git and in what layout?\nDo you need to record any extra metadata within the commits that are\nmerged to master? How do you merge and/or reconcile code review comments\nthat come from multiple sources (or are they just tabulated)? Can your\nsystem handle the rebasing of topic branches? What about nonlinear topic\nbranches (branches branches that themselves include merges)?\n\nAll that being said, my gut feeling is that a system like this should\nnot be developed within the Git project itself. Code review is a\ncomplicated thing, and I expect that different people will have very\ndifferent ideas about how it should work. It would be a bad idea for the\nGit project to \"bless\" one system by including it in our source tree.\n(Earlier in the Git's history it was easier to get something accepted\ninto \"contrib\", but that has gotten much harder over time.)\n\nIf, someday, one system becomes crushingly dominant, then conceivably it\nwould make sense for it to be distributed along with Git for the\nconvenience of users. Or if a bunch of review systems standardize on a\nsingle data model for storing review information in a Git repo, it might\nmake sense for the plumbing for handling that data to reside in git-core\nfor performance and data integrity reasons. Until then, I think it would\nbe better for code review systems to live on their own, as independent\nprojects.\n\nIn my opinion it would be fine to discuss the design of your system and\nsolicit feedback about the design on the Git mailing list, and also to\npublish occasional announcement emails when you release new versions or\nwhatever. You might also want to list your system on the Git SCM wiki,\nfor example here [1].\n\nYours,\nMichael\n\n[1] https://git.wiki.kernel.org/index.php/Interfaces,_frontends,_and_tools\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"273196","messageId":"20151111151204.GA4249@salo","threadId":"40760","inReplyTo":"5643107B.20501@alum.mit.edu","subject":"Re: [PATCH 0/2] git-candidate: git based patch tracking and review","fromName":"Richard Ipsum","fromEmail":"richard.ipsum@codethink.co.uk","sentAt":"2015-11-11T15:12:05Z","receivedAt":"2015-11-11T15:12:05Z","isPatch":true,"sender":{"key":"richard.ipsum@codethink.co.uk","avatar":null},"body":"On Wed, Nov 11, 2015 at 10:55:07AM +0100, Michael Haggerty wrote:\n> On 11/10/2015 01:56 PM, Richard Ipsum wrote:\n> > I've continued my work[1] to add patch tracking and candidate review capability\n> > to git.\n> > \n> > git-candidate now has a more git-like user interface, so remote candidates\n> > can now be specified in a similar way to remote refs (e.g. origin/candidate)\n> > as well as various other improvements, such as versioned metadata.\n> \n> This is a really interesting project. I've seen a blog post or two\n> proposing to store bug tracker information in Git in a distributed way,\n> but I don't recall anything about doing the same for code review\n> information.\n> \n> I would be interested to hear about the design of your system at an\n> abstract technical level. What do you store in Git and in what layout?\n> Do you need to record any extra metadata within the commits that are\n> merged to master? How do you merge and/or reconcile code review comments\n> that come from multiple sources (or are they just tabulated)? Can your\n> system handle the rebasing of topic branches? What about nonlinear topic\n> branches (branches branches that themselves include merges)?\n\nExtra metadata tracks the latest revision of a candidate\nand the metadata version, the metadata objects for the revisions and reviews\nare stored in trees, each object is keyed on the hash of the content\nthat is stored within the object, so reviews from multiple sources\ncannot conflict. There's no ordering of reviews or comments,\nthe status command simply sorts reviews based on their timestamp.\n\nThe system reserves a portion of the refspace under refs/candidates/\nthe root tree object is stored in a commit under refs/candidate/heads/foo__meta\nwhere foo is the name of the candidate. Anchors are also stored in this space,\nan anchor is made for every revision of a candidate to ensure that commits\nreferred to by a candidate are not garbage collected.\n\nThe operations performed on a topic branch don't matter, all the candidate\nknows about is the head sha of the branch we want to merge. Merging a\ncandidate is no different from merging any other git branch, you have\na ref candidate/foo so you can choose to merge that, or you can\ncheck it out rebase it and then merge it.\n\n> \n> All that being said, my gut feeling is that a system like this should\n> not be developed within the Git project itself. Code review is a\n> complicated thing, and I expect that different people will have very\n> different ideas about how it should work. It would be a bad idea for the\n> Git project to \"bless\" one system by including it in our source tree.\n> (Earlier in the Git's history it was easier to get something accepted\n> into \"contrib\", but that has gotten much harder over time.)\n\nThe aim is not to bless one particular system but to eventually\nprovide a common data model that all review systems can share,\nso that it is possible to do distributed reviews with arbitrary UIs\nin a widely compatible way.\n\nIf we add git-candidate to contrib then it can act as a reference\nimplementation, so that this data model can be validated and tested\nby additional developers.\n\n> \n> If, someday, one system becomes crushingly dominant, then conceivably it\n> would make sense for it to be distributed along with Git for the\n> convenience of users. Or if a bunch of review systems standardize on a\n> single data model for storing review information in a Git repo, it might\n> make sense for the plumbing for handling that data to reside in git-core\n> for performance and data integrity reasons. Until then, I think it would\n> be better for code review systems to live on their own, as independent\n> projects.\n\nI don't feel too strongly either way, that said I do think it may be\nadvantageous for all concerned to have this data model, we don't necessarily\nneed to wait for a particular system to become crushingly dominant.\n\n> \n> In my opinion it would be fine to discuss the design of your system and\n> solicit feedback about the design on the Git mailing list, and also to\n> publish occasional announcement emails when you release new versions or\n> whatever. You might also want to list your system on the Git SCM wiki,\n> for example here [1].\n\nThanks, it would be good to add git-candidate to the list there.\n\nThanks,\nRichard Ipsum\n"},{"id":"273222","messageId":"1447272954.20147.36.camel@twopensource.com","threadId":"40760","inReplyTo":"20151111094816.GA2949@salo","subject":"Re: [PATCH 2/2] contrib/git-candidate: Add README","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-11-11T20:15:54Z","receivedAt":"2015-11-11T20:15:54Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2015-11-11 at 09:48 +0000, Richard Ipsum wrote:\n> > \n> > > +\t(master)$ git candidate submit origin archiverepo\n> > > +\tReview added successfully\n> > \n> > Is the contributor automatically (optionally) emailed on this? If not,\n> > consider this a feature request for this.\n> \n> There's no server integration of any kind at the moment,\n> this is clearly something we will want to add.\n\nI don't think this needs server integration. It could just work like\ngit send-email and send the email from the local machine.\n\n> > \"git candidate diff\" might be nice too to show the diff between v1 and\n> > v2. You might even have \"git candidate commit-diff\" (or some better\n> > name) so you can see which commit has changed in a changeset containing\n> > multiple commits. \n> \n> Yes, we definitely want that. I think \"git candidate diff\" to diff\n> between revisions would be sufficient, and it could take a list of files\n> to diff as an arg?\n\nThat's a good start, but often I want to review per-patch, so it would\nbe nice (if complicated) to track the evolution of a patchset.\n\n> > > +\t(master)$ git candidate review origin/archiverepo --vote +2\n> > > +\t\t-m \"Looks good, merging. Thanks for your efforts\"\n> > > +\tReview added successfully\n> > \n> > Is that +2 \"+1 because I like it, +1 because I previously -1'd it?\" If\n> > so, it might be nice to have --replace-vote so you don't have to track,\n> > \"wait, I did -1, then +1, then -1 again...\"\n> \n> Votes are per-review, perhaps they should simply be per-revision?\n> Then --vote sets the vote for the revision and there's no need for\n> a --replace-vote option?\n> This would use user.name and user.email as identification.\n\nI like votes being per-revision.\n\n> > > +\t(master)$ git candidate submit origin archiverepo\n> > > +\tCandidate was submitted successfully.\n> > \n> > I don't understand what the verb \"submit\" means here. Is it \"mark this\n> > as accepted\"? If so, \"accept\" might be a better word. \n> \n> I'm tempted to change this to 'push', 'submit' comes from gerrit.\n\nSGTM.\n\n> > > +\t(master)$ git merge candidates/origin/archiverepo\n> > \n> > I would like \"git candidate merge\" to do a submit+merge the way that\n> > pull does a fetch+merge. It seems like the common case. Also, if it\n> > turns out at this point that there's a merge conflict, I might want to\n> > back out the acceptance.\n> \n> There is currently no git-candidate-merge, I removed this recently\n> because I decided that you can merge candidates with git-merge\n> and that this is more flexible. Often a candidate will be rebased\n> before it is merged, it would be nice to avoid having to create\n> a merge command that needs to handle all the different cases for\n> merging a candidate.\n\nI like the convenience, but it could always be added later.\n\nOne more random note: it might be nice to have a Documentation/technical\narticle describing review storage.\n"},{"id":"273328","messageId":"20151114081707.GA32739@sigill.intra.peff.net","threadId":"40760","inReplyTo":"20151111151204.GA4249@salo","subject":"Re: [PATCH 0/2] git-candidate: git based patch tracking and review","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-11-14T08:17:08Z","receivedAt":"2015-11-14T08:17:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 11, 2015 at 03:12:05PM +0000, Richard Ipsum wrote:\n\n> > All that being said, my gut feeling is that a system like this should\n> > not be developed within the Git project itself. Code review is a\n> > complicated thing, and I expect that different people will have very\n> > different ideas about how it should work. It would be a bad idea for the\n> > Git project to \"bless\" one system by including it in our source tree.\n> > (Earlier in the Git's history it was easier to get something accepted\n> > into \"contrib\", but that has gotten much harder over time.)\n> \n> The aim is not to bless one particular system but to eventually\n> provide a common data model that all review systems can share,\n> so that it is possible to do distributed reviews with arbitrary UIs\n> in a widely compatible way.\n\nI think that's a laudable goal, but I didn't see any discussion or\ndocumentation of the data model in your patches. Maybe that would be a\ngood place to start.\n\n> If we add git-candidate to contrib then it can act as a reference\n> implementation, so that this data model can be validated and tested\n> by additional developers.\n\nThat can happen outside of git's contrib/ directory, too.\n\nI think Michael's \"bless\" argument applies to the data model, too. Is\nyour data model a good one? Should other systems adopt it, or is it\nstill a work in progress? We don't know yet.\n\nI think I'd rather see it prove itself before entering the git tree, if\nonly because it doesn't really gain anything by being inside the git\ntree. Once upon a time that was a good way to get publicity and easy\nhosting, but these days it is easy to find git hosting, and I am not\nsure people actually explore contrib/ all that much.\n\n-Peff\n"},{"id":"273331","messageId":"xmqqbnawzq3l.fsf@gitster.mtv.corp.google.com","threadId":"40760","inReplyTo":"20151114081707.GA32739@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] git-candidate: git based patch tracking and review","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-14T13:07:42Z","receivedAt":"2015-11-14T13:07:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Nov 11, 2015 at 03:12:05PM +0000, Richard Ipsum wrote:\n>> \n>> The aim is not to bless one particular system but to eventually\n>> provide a common data model that all review systems can share,\n>> so that it is possible to do distributed reviews with arbitrary UIs\n>> in a widely compatible way.\n>\n> I think that's a laudable goal, but I didn't see any discussion or\n> documentation of the data model in your patches. Maybe that would be a\n> good place to start.\n\nEven before doing that it might be a good idea to pick brains of\nfolks who work on Gerrit, who has already done one such data model\nthat may be generic enough to serve as a good starting point. ISTR\nthat they were migrating to a notes based storage to ease federated\ncode review systems?\n\n>> If we add git-candidate to contrib then it can act as a reference\n>> implementation, so that this data model can be validated and tested\n>> by additional developers.\n>\n> That can happen outside of git's contrib/ directory, too.\n>\n> I think Michael's \"bless\" argument applies to the data model, too. Is\n> your data model a good one? Should other systems adopt it, or is it\n> still a work in progress? We don't know yet.\n>\n> I think I'd rather see it prove itself before entering the git tree, if\n> only because it doesn't really gain anything by being inside the git\n> tree. Once upon a time that was a good way to get publicity and easy\n> hosting, but these days it is easy to find git hosting, and I am not\n> sure people actually explore contrib/ all that much.\n"},{"id":"273875","messageId":"20151201205527.GF18255@google.com","threadId":"40760","inReplyTo":"5643107B.20501@alum.mit.edu","subject":"Re: [PATCH 0/2] git-candidate: git based patch tracking and review","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2015-12-01T20:55:27Z","receivedAt":"2015-12-01T20:55:27Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Michael Haggerty wrote:\n> On 11/10/2015 01:56 PM, Richard Ipsum wrote:\n\n>> I've continued my work[1] to add patch tracking and candidate review capability\n>> to git.\n>>\n>> git-candidate now has a more git-like user interface, so remote candidates\n>> can now be specified in a similar way to remote refs (e.g. origin/candidate)\n>> as well as various other improvements, such as versioned metadata.\n>\n> This is a really interesting project. I've seen a blog post or two\n> proposing to store bug tracker information in Git in a distributed way,\n> but I don't recall anything about doing the same for code review\n> information.\n\nCc-ing dborowitz, who has been working on storing Gerrit's code review\ninformation in Git instead of a separate database (e.g., see [1]).\n\nThanks,\nJonathan\n\n[1] https://storage.googleapis.com/gerrit-talks/summit/2015/NoteDB.pdf\n"},{"id":"273876","messageId":"CAD0k6qQ98ED4xLynRZdnnPX_HDHhEq5SQhNov-5cwG82pqMFGw@mail.gmail.com","threadId":"40760","inReplyTo":"20151201205527.GF18255@google.com","subject":"Re: [PATCH 0/2] git-candidate: git based patch tracking and review","fromName":"Dave Borowitz","fromEmail":"dborowitz@google.com","sentAt":"2015-12-01T21:00:52Z","receivedAt":"2015-12-01T21:00:52Z","isPatch":true,"sender":{"key":"dborowitz@google.com","avatar":"https://avatars.githubusercontent.com/u/194927?v=4"},"body":"On Tue, Dec 1, 2015 at 3:55 PM, Jonathan Nieder wrote:\n> Cc-ing dborowitz, who has been working on storing Gerrit's code review\n> information in Git instead of a separate database (e.g., see [1]).\n\nThanks, we actually already had a thread going that I realize only in\nretrospect did not include the git mailing list.\n"},{"id":"275444","messageId":"20160106154910.GA8375@salo","threadId":"40760","inReplyTo":"CAD0k6qQ98ED4xLynRZdnnPX_HDHhEq5SQhNov-5cwG82pqMFGw@mail.gmail.com","subject":"Re: [PATCH 0/2] git-candidate: git based patch tracking and review","fromName":"Richard Ipsum","fromEmail":"richard.ipsum@codethink.co.uk","sentAt":"2016-01-06T15:49:10Z","receivedAt":"2016-01-06T15:49:10Z","isPatch":true,"sender":{"key":"richard.ipsum@codethink.co.uk","avatar":null},"body":"On Tue, Dec 01, 2015 at 04:00:52PM -0500, Dave Borowitz wrote:\n> On Tue, Dec 1, 2015 at 3:55 PM, Jonathan Nieder wrote:\n> > Cc-ing dborowitz, who has been working on storing Gerrit's code review\n> > information in Git instead of a separate database (e.g., see [1]).\n> \n> Thanks, we actually already had a thread going that I realize only in\n> retrospect did not include the git mailing list.\n\nThanks, we did indeed have a thread going,\nthat probably should have been on-list.\nI'm also working on a little library[1] that should eventually allow other\ntools to use gerrit's notedb in the hope that this might\neventually lead to the development/adoption of a common format for such tools.\n\nOne thing that concerns me about notedb with respect to distributed review\nis the storage format for comments. Within a distributed review system comments\nmay be made in any order, yet the format is designed around the kind of\nlinearisation that can be assumed by a centralised system.\n\nThe problem is that multiple comments in notedb may be stored within\nthe same blob/note, specifically all comments on a particular commit will\nbe stored in the same blob. In a distributed system storing multiple comments\nin the same blob like this will inevitably lead to merge conflicts.\n\nThis problem isn't unsolvable, someone already suggested to me the idea\nof writing a custom merge driver for merging different notes stored in notedb.\nIt would obviously be preferable to have a format that avoided creating\nconflicts in the first place, but a custom merge driver doesn't seem like\nan unreasonable solution.\n\nIf anyone has any thoughts on how else this problem might be solved,\nI'd be very interested to hear them.\n\nThanks again,\nRichard Ipsum\n\n[1]: https://bitbucket.org/richardipsum/perl-notedb\n"},{"id":"275449","messageId":"568D7E06.9030100@gmail.com","threadId":"40760","inReplyTo":"1447160198-23296-3-git-send-email-richard.ipsum@codethink.co.uk","subject":"Re: [PATCH 2/2] contrib/git-candidate: Add README","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2016-01-06T20:50:14Z","receivedAt":"2016-01-06T20:50:14Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On 10.11.2015 13:56, Richard Ipsum wrote:\n\n> +Existing tools such as Github's pull-requests and Gerrit are already\n> +in wide use, why bother with something new?\n> +\n> +We are concerned that whilst git is a distributed version control\n> +system the systems used to store comments and reviews for content\n> +under version control are usually centralised,\n\nI think it's a bit unjust to unconditionally mention Gerrit in this \ncontext as you seem to imply that Gerrit does not store *any* review \ndata in Git.\n\nEven without Dave's upcoming notedb, Gerrit already stores refs/changes \nin Git, and with the reviewnotes plugin [1] also the outcome of a review \nin refs/notes/review.\n\n[1] \nhttps://gerrit.googlesource.com/plugins/reviewnotes/+/refs/heads/master/src/main/resources/Documentation/refs-notes-review.md\n\n-- \nSebastian Schuberth\n"}]}