* [PATCH 5/5] gitweb: Add 'status_str' to parse_difftree_raw_line output
From: Jakub Narebski @ 2007-09-21 21:41 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 459 bytes --]
Add 'status_str' to diffinfo output, which stores status (also for
merge commit) as a string. This allows for easy checking if there is
given status among all for merge commit, e.g.
$diffinfo->{'status_str'} =~ /D/
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is to simplify _future_ code, so I won't cry if it is not
accepted without some code that needs it.
gitweb/gitweb.perl | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
[-- Attachment #2: 0005-gitweb-Add-status_str-to-parse_difftree_raw_line.patch --]
[-- Type: application/octet-stream, Size: 1501 bytes --]
From d5d0886111e98dc281d8503daa1ce655ea7729bf Mon Sep 17 00:00:00 2001
From: Jakub Narebski <jnareb@gmail.com>
Date: Thu, 13 Sep 2007 21:57:20 +0200
Subject: [PATCH 5/5] gitweb: Add 'status_str' to parse_difftree_raw_line output
Add 'status_str' to diffinfo output, which stores status (also for
merge commit) as a string. This allows for easy checking if there is
given status among all for merge commit, e.g.
$diffinfo->{'status_str'} =~ /D/
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is to simplify _future_ code, so I won't cry if it is not
accepted without some code that needs it.
gitweb/gitweb.perl | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 6c79a95..adc1b32 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1963,7 +1963,7 @@ sub parse_difftree_raw_line {
$res{'to_mode'} = $2;
$res{'from_id'} = $3;
$res{'to_id'} = $4;
- $res{'status'} = $5;
+ $res{'status'} = $res{'status_str'} = $5;
$res{'similarity'} = $6;
if ($res{'status'} eq 'R' || $res{'status'} eq 'C') { # renamed or copied
($res{'from_file'}, $res{'to_file'}) = map { unquote($_) } split("\t", $7);
@@ -1979,6 +1979,7 @@ sub parse_difftree_raw_line {
$res{'to_mode'} = pop @{$res{'from_mode'}};
$res{'from_id'} = [ split(' ', $3) ];
$res{'to_id'} = pop @{$res{'from_id'}};
+ $res{'status_str'} = $4;
$res{'status'} = [ split('', $4) ];
$res{'to_file'} = unquote($5);
}
--
1.5.3
^ permalink raw reply related
* [PATCH 4/5] gitweb: Always set 'from_file' and 'to_file' in parse_difftree_raw_line
From: Jakub Narebski @ 2007-09-21 21:39 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 546 bytes --]
Always set 'from_file' and 'to_file' keys when parsing raw diff output
format line, even if filename didn't change (file was not renamed).
This allows for less code (and no problems with file named '0'); use
this.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This simplifies code a bit _and_ fortifies gitweb against filenames
like '0' (although it does remove _fragments_ of lines, not lines
themselves, so it is not visible in diffstat).
gitweb/gitweb.perl | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)
[-- Attachment #2: 0004-gitweb-Always-set-from_file-and-to_file-in-pars.patch --]
[-- Type: application/octet-stream, Size: 3488 bytes --]
From 9753f8b6a67239a4ed1ad5489965db37f7cbea89 Mon Sep 17 00:00:00 2001
From: Jakub Narebski <jnareb@gmail.com>
Date: Thu, 13 Sep 2007 21:57:03 +0200
Subject: [PATCH 4/5] gitweb: Always set 'from_file' and 'to_file' in parse_difftree_raw_line
Always set 'from_file' and 'to_file' keys when parsing raw diff output
format line, even if filename didn't change (file was not renamed).
This allows for less code (and no problems with file named '0'); use
this.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This simplifies code a bit _and_ fortifies gitweb against filenames
like '0' (although it does remove _fragments_ of lines, not lines
themselves, so it is not visible in diffstat).
gitweb/gitweb.perl | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1b5642a..6c79a95 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1968,7 +1968,7 @@ sub parse_difftree_raw_line {
if ($res{'status'} eq 'R' || $res{'status'} eq 'C') { # renamed or copied
($res{'from_file'}, $res{'to_file'}) = map { unquote($_) } split("\t", $7);
} else {
- $res{'file'} = unquote($7);
+ $res{'from_file'} = $res{'to_file'} = $res{'file'} = unquote($7);
}
}
# '::100755 100755 100755 60e79ca1b01bc8b057abe17ddab484699a7f5fdb 94067cc5f73388f33722d52ae02f44692bc07490 94067cc5f73388f33722d52ae02f44692bc07490 MR git-gui/git-gui.sh'
@@ -2035,7 +2035,10 @@ sub parse_from_to_diffinfo {
fill_from_file_info($diffinfo, @parents)
unless exists $diffinfo->{'from_file'};
for (my $i = 0; $i < $diffinfo->{'nparents'}; $i++) {
- $from->{'file'}[$i] = $diffinfo->{'from_file'}[$i] || $diffinfo->{'to_file'};
+ $from->{'file'}[$i] =
+ defined $diffinfo->{'from_file'}[$i] ?
+ $diffinfo->{'from_file'}[$i] :
+ $diffinfo->{'to_file'};
if ($diffinfo->{'status'}[$i] ne "A") { # not new (added) file
$from->{'href'}[$i] = href(action=>"blob",
hash_base=>$parents[$i],
@@ -2047,7 +2050,7 @@ sub parse_from_to_diffinfo {
}
} else {
# ordinary (not combined) diff
- $from->{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
+ $from->{'file'} = $diffinfo->{'from_file'};
if ($diffinfo->{'status'} ne "A") { # not new (added) file
$from->{'href'} = href(action=>"blob", hash_base=>$hash_parent,
hash=>$diffinfo->{'from_id'},
@@ -2057,7 +2060,7 @@ sub parse_from_to_diffinfo {
}
}
- $to->{'file'} = $diffinfo->{'to_file'} || $diffinfo->{'file'};
+ $to->{'file'} = $diffinfo->{'to_file'};
if (!is_deleted($diffinfo)) { # file exists in result
$to->{'href'} = href(action=>"blob", hash_base=>$hash,
hash=>$diffinfo->{'to_id'},
@@ -2802,7 +2805,7 @@ sub is_patch_split {
my ($diffinfo, $patchinfo) = @_;
return defined $diffinfo && defined $patchinfo
- && ($diffinfo->{'to_file'} || $diffinfo->{'file'}) eq $patchinfo->{'to_file'};
+ && $diffinfo->{'to_file'} eq $patchinfo->{'to_file'};
}
@@ -4637,8 +4640,8 @@ sub git_blobdiff {
}
%diffinfo = parse_difftree_raw_line($difftree[0]);
- $file_parent ||= $diffinfo{'from_file'} || $file_name || $diffinfo{'file'};
- $file_name ||= $diffinfo{'to_file'} || $diffinfo{'file'};
+ $file_parent ||= $diffinfo{'from_file'} || $file_name;
+ $file_name ||= $diffinfo{'to_file'};
$hash_parent ||= $diffinfo{'from_id'};
$hash ||= $diffinfo{'to_id'};
--
1.5.3
^ permalink raw reply related
* [PATCH 3/5] gitweb: Fix and simplify "split patch" detection
From: Jakub Narebski @ 2007-09-21 21:38 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 2746 bytes --]
There are some cases when one line from "raw" git-diff output (raw
format) corresponds to more than one patch in the patchset git-diff
output; we call this situation "split patch". Old code misdetected
subsequent patches (for different files) with the same pre-image and
post-image as fragments of "split patch", leading to mislabeled
from-file/to-file diff header etc.
Old code used pre-image and post-image SHA-1 identifier ('from_id' and
'to_id') to check if current patch corresponds to old raw diff format
line, to find if one difftree raw line coresponds to more than one
patch in the patch format. Now we use post-image filename for that.
This assumes that post-image filename alone can be used to identify
difftree raw line. In the case this changes (which is unlikely
considering current diff engine) we can add 'from_id' and 'to_id'
to detect "patch splitting" together with 'to_file'.
Because old code got pre-image and post-image SHA-1 identifier for the
patch from the "index" line in extended diff header, diff header had
to be buffered. New code takes post-image filename from "git diff"
header, which is first line of a patch; this allows to simplify
git_patchset_body code. A side effect of resigning diff header
buffering is that there is always "diff extended_header" div, even
if extended diff header is empty.
Alternate solution would be to check when git splits patches, and do
not check if parsed info from current patch corresponds to current or
next raw diff format output line. Git splits patches only for 'T'
(typechange) status filepair, and there always two patches
corresponding to one raw diff line. It was not used because it would
tie gitweb code to minute details of git diff output.
While at it, use newly introduced parsed_difftree_line wrapper
subroutine in git_difftree_body.
Noticed-by: Yann Dirson <ydirson@altern.org>
Diagnosed-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Junio has decided in
Message-ID: <7vmyw4ob7z.fsf@gitster.siamese.dyndns.org>
that tying gitweb to minute details of git diff output, namely that
we have "split patch" (two patches of patchset diff format for single
line of raw ditt format) only for typechange (status 'T') diffs.
We make other assumption instead, that post-image filename uniquely
defines line of raw git diff output. Currently diffcore is not
capable of producing other output; see Junio in
Message-ID: <7vtzqcj9ni.fsf@gitster.siamese.dyndns.org>
This assumption simplifies git_patchset_body, as we no longer need
to cache extended diff header for "split patch" detection.
gitweb/gitweb.perl | 152 +++++++++++++++++++++++-----------------------------
1 files changed, 67 insertions(+), 85 deletions(-)
[-- Attachment #2: 0003-gitweb-Fix-and-simplify-split-patch-detection.patch --]
[-- Type: application/octet-stream, Size: 11563 bytes --]
From a4dc45e0cb307d352fd9116507a7e128f8e0d503 Mon Sep 17 00:00:00 2001
From: Jakub Narebski <jnareb@gmail.com>
Date: Sun, 9 Sep 2007 21:34:10 +0200
Subject: [PATCH 3/5] gitweb: Fix and simplify "split patch" detection
There are some cases when one line from "raw" git-diff output (raw
format) corresponds to more than one patch in the patchset git-diff
output; we call this situation "split patch". Old code misdetected
subsequent patches (for different files) with the same pre-image and
post-image as fragments of "split patch", leading to mislabeled
from-file/to-file diff header etc.
Old code used pre-image and post-image SHA-1 identifier ('from_id' and
'to_id') to check if current patch corresponds to old raw diff format
line, to find if one difftree raw line coresponds to more than one
patch in the patch format. Now we use post-image filename for that.
This assumes that post-image filename alone can be used to identify
difftree raw line. In the case this changes (which is unlikely
considering current diff engine) we can add 'from_id' and 'to_id'
to detect "patch splitting" together with 'to_file'.
Because old code got pre-image and post-image SHA-1 identifier for the
patch from the "index" line in extended diff header, diff header had
to be buffered. New code takes post-image filename from "git diff"
header, which is first line of a patch; this allows to simplify
git_patchset_body code. A side effect of resigning diff header
buffering is that there is always "diff extended_header" div, even
if extended diff header is empty.
Alternate solution would be to check when git splits patches, and do
not check if parsed info from current patch corresponds to current or
next raw diff format output line. Git splits patches only for 'T'
(typechange) status filepair, and there always two patches
corresponding to one raw diff line. It was not used because it would
tie gitweb code to minute details of git diff output.
While at it, use newly introduced parsed_difftree_line wrapper
subroutine in git_difftree_body.
Noticed-by: Yann Dirson <ydirson@altern.org>
Diagnosed-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Junio has decided in
Message-ID: <7vmyw4ob7z.fsf@gitster.siamese.dyndns.org>
that tying gitweb to minute details of git diff output, namely that
we have "split patch" (two patches of patchset diff format for single
line of raw ditt format) only for typechange (status 'T') diffs.
We make other assumption instead, that post-image filename uniquely
defines line of raw git diff output. Currently diffcore is not
capable of producing other output; see Junio in
Message-ID: <7vtzqcj9ni.fsf@gitster.siamese.dyndns.org>
This assumption simplifies git_patchset_body, as we no longer need
to cache extended diff header for "split patch" detection.
gitweb/gitweb.perl | 152 +++++++++++++++++++++++-----------------------------
1 files changed, 67 insertions(+), 85 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3064298..1b5642a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1990,6 +1990,19 @@ sub parse_difftree_raw_line {
return wantarray ? %res : \%res;
}
+# wrapper: return parsed line of git-diff-tree "raw" output
+# (the argument might be raw line, or parsed info)
+sub parsed_difftree_line {
+ my $line_or_ref = shift;
+
+ if (ref($line_or_ref) eq "HASH") {
+ # pre-parsed (or generated by hand)
+ return $line_or_ref;
+ } else {
+ return parse_difftree_raw_line($line_or_ref);
+ }
+}
+
# parse line of git-ls-tree output
sub parse_ls_tree_line ($;%) {
my $line = shift;
@@ -2033,6 +2046,7 @@ sub parse_from_to_diffinfo {
}
}
} else {
+ # ordinary (not combined) diff
$from->{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
if ($diffinfo->{'status'} ne "A") { # not new (added) file
$from->{'href'} = href(action=>"blob", hash_base=>$hash_parent,
@@ -2756,6 +2770,7 @@ sub git_print_tree_entry {
## ......................................................................
## functions printing large fragments of HTML
+# get pre-image filenames for merge (combined) diff
sub fill_from_file_info {
my ($diff, @parents) = @_;
@@ -2772,28 +2787,25 @@ sub fill_from_file_info {
return $diff;
}
-# parameters can be strings, or references to arrays of strings
-sub from_ids_eq {
- my ($a, $b) = @_;
-
- if (ref($a) eq "ARRAY" && ref($b) eq "ARRAY" && @$a == @$b) {
- for (my $i = 0; $i < @$a; ++$i) {
- return 0 unless ($a->[$i] eq $b->[$i]);
- }
- return 1;
- } elsif (!ref($a) && !ref($b)) {
- return $a eq $b;
- } else {
- return 0;
- }
-}
-
+# is current raw difftree line of file deletion
sub is_deleted {
my $diffinfo = shift;
return $diffinfo->{'to_id'} eq ('0' x 40);
}
+# does patch correspond to [previous] difftree raw line
+# $diffinfo - hashref of parsed raw diff format
+# $patchinfo - hashref of parsed patch diff format
+# (the same keys as in $diffinfo)
+sub is_patch_split {
+ my ($diffinfo, $patchinfo) = @_;
+
+ return defined $diffinfo && defined $patchinfo
+ && ($diffinfo->{'to_file'} || $diffinfo->{'file'}) eq $patchinfo->{'to_file'};
+}
+
+
sub git_difftree_body {
my ($difftree, $hash, @parents) = @_;
my ($parent) = $parents[0];
@@ -2830,13 +2842,7 @@ sub git_difftree_body {
my $alternate = 1;
my $patchno = 0;
foreach my $line (@{$difftree}) {
- my $diff;
- if (ref($line) eq "HASH") {
- # pre-parsed (or generated by hand)
- $diff = $line;
- } else {
- $diff = parse_difftree_raw_line($line);
- }
+ my $diff = parsed_difftree_line($line);
if ($alternate) {
print "<tr class=\"dark\">\n";
@@ -3107,10 +3113,12 @@ sub git_patchset_body {
my ($fd, $difftree, $hash, @hash_parents) = @_;
my ($hash_parent) = $hash_parents[0];
+ my $is_combined = (@hash_parents > 1);
my $patch_idx = 0;
my $patch_number = 0;
my $patch_line;
my $diffinfo;
+ my $to_name;
my (%from, %to);
print "<div class=\"patchset\">\n";
@@ -3124,73 +3132,46 @@ sub git_patchset_body {
PATCH:
while ($patch_line) {
- my @diff_header;
- my ($from_id, $to_id);
-
- # git diff header
- #assert($patch_line =~ m/^diff /) if DEBUG;
- #assert($patch_line !~ m!$/$!) if DEBUG; # is chomp-ed
- $patch_number++;
- push @diff_header, $patch_line;
-
- # extended diff header
- EXTENDED_HEADER:
- while ($patch_line = <$fd>) {
- chomp $patch_line;
- last EXTENDED_HEADER if ($patch_line =~ m/^--- |^diff /);
-
- if ($patch_line =~ m/^index ([0-9a-fA-F]{40})..([0-9a-fA-F]{40})/) {
- $from_id = $1;
- $to_id = $2;
- } elsif ($patch_line =~ m/^index ((?:[0-9a-fA-F]{40},)+[0-9a-fA-F]{40})..([0-9a-fA-F]{40})/) {
- $from_id = [ split(',', $1) ];
- $to_id = $2;
- }
-
- push @diff_header, $patch_line;
+ # parse "git diff" header line
+ if ($patch_line =~ m/^diff --git (\"(?:[^\\\"]*(?:\\.[^\\\"]*)*)\"|[^ "]*) (.*)$/) {
+ # $1 is from_name, which we do not use
+ $to_name = unquote($2);
+ $to_name =~ s!^b/!!;
+ } elsif ($patch_line =~ m/^diff --(cc|combined) ("?.*"?)$/) {
+ # $1 is 'cc' or 'combined', which we do not use
+ $to_name = unquote($2);
+ } else {
+ $to_name = undef;
}
- my $last_patch_line = $patch_line;
# check if current patch belong to current raw line
# and parse raw git-diff line if needed
- if (defined $diffinfo &&
- defined $from_id && defined $to_id &&
- from_ids_eq($diffinfo->{'from_id'}, $from_id) &&
- $diffinfo->{'to_id'} eq $to_id) {
+ if (is_patch_split($diffinfo, { 'to_file' => $to_name })) {
# this is continuation of a split patch
print "<div class=\"patch cont\">\n";
} else {
# advance raw git-diff output if needed
$patch_idx++ if defined $diffinfo;
- # compact combined diff output can have some patches skipped
- # find which patch (using pathname of result) we are at now
- my $to_name;
- if ($diff_header[0] =~ m!^diff --cc "?(.*)"?$!) {
- $to_name = $1;
- }
-
- do {
- # read and prepare patch information
- if (ref($difftree->[$patch_idx]) eq "HASH") {
- # pre-parsed (or generated by hand)
- $diffinfo = $difftree->[$patch_idx];
- } else {
- $diffinfo = parse_difftree_raw_line($difftree->[$patch_idx]);
- }
+ # read and prepare patch information
+ $diffinfo = parsed_difftree_line($difftree->[$patch_idx]);
- # check if current raw line has no patch (it got simplified)
- if (defined $to_name && $to_name ne $diffinfo->{'to_file'}) {
+ # compact combined diff output can have some patches skipped
+ # find which patch (using pathname of result) we are at now;
+ if ($is_combined) {
+ while ($to_name ne $diffinfo->{'to_file'}) {
print "<div class=\"patch\" id=\"patch". ($patch_idx+1) ."\">\n" .
format_diff_cc_simplified($diffinfo, @hash_parents) .
"</div>\n"; # class="patch"
$patch_idx++;
$patch_number++;
+
+ last if $patch_idx > $#$difftree;
+ $diffinfo = parsed_difftree_line($difftree->[$patch_idx]);
}
- } until (!defined $to_name || $to_name eq $diffinfo->{'to_file'} ||
- $patch_idx > $#$difftree);
+ }
# modifies %from, %to hashes
parse_from_to_diffinfo($diffinfo, \%from, \%to, @hash_parents);
@@ -3200,30 +3181,36 @@ sub git_patchset_body {
print "<div class=\"patch\" id=\"patch". ($patch_idx+1) ."\">\n";
}
+ # git diff header
+ #assert($patch_line =~ m/^diff /) if DEBUG;
+ #assert($patch_line !~ m!$/$!) if DEBUG; # is chomp-ed
+ $patch_number++;
# print "git diff" header
- $patch_line = shift @diff_header;
print format_git_diff_header_line($patch_line, $diffinfo,
\%from, \%to);
# print extended diff header
- print "<div class=\"diff extended_header\">\n" if (@diff_header > 0);
+ print "<div class=\"diff extended_header\">\n";
EXTENDED_HEADER:
- foreach $patch_line (@diff_header) {
+ while ($patch_line = <$fd>) {
+ chomp $patch_line;
+
+ last EXTENDED_HEADER if ($patch_line =~ m/^--- |^diff /);
+
print format_extended_diff_header_line($patch_line, $diffinfo,
\%from, \%to);
}
- print "</div>\n" if (@diff_header > 0); # class="diff extended_header"
+ print "</div>\n"; # class="diff extended_header"
# from-file/to-file diff header
- $patch_line = $last_patch_line;
if (! $patch_line) {
print "</div>\n"; # class="patch"
last PATCH;
}
next PATCH if ($patch_line =~ m/^diff /);
#assert($patch_line =~ m/^---/) if DEBUG;
- #assert($patch_line eq $last_patch_line) if DEBUG;
+ my $last_patch_line = $patch_line;
$patch_line = <$fd>;
chomp $patch_line;
#assert($patch_line =~ m/^\+\+\+/) if DEBUG;
@@ -3248,16 +3235,11 @@ sub git_patchset_body {
# for compact combined (--cc) format, with chunk and patch simpliciaction
# patchset might be empty, but there might be unprocessed raw lines
- for ($patch_idx++ if $patch_number > 0;
+ for (++$patch_idx if $patch_number > 0;
$patch_idx < @$difftree;
- $patch_idx++) {
+ ++$patch_idx) {
# read and prepare patch information
- if (ref($difftree->[$patch_idx]) eq "HASH") {
- # pre-parsed (or generated by hand)
- $diffinfo = $difftree->[$patch_idx];
- } else {
- $diffinfo = parse_difftree_raw_line($difftree->[$patch_idx]);
- }
+ $diffinfo = parsed_difftree_line($difftree->[$patch_idx]);
# generate anchor for "patch" links in difftree / whatchanged part
print "<div class=\"patch\" id=\"patch". ($patch_idx+1) ."\">\n" .
--
1.5.3
^ permalink raw reply related
* [PATCH 2/5] gitweb: No difftree output for trivial merge
From: Jakub Narebski @ 2007-09-21 21:36 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 504 bytes --]
In 'commitdiff' view, for the merge commit, there is an extra header
for the difftree table, with links to commitdiffs to individual
parents. Do not show such header when there is nothing to show, for
trivial merges.
This means that for trivial merge you have to go to 'commit' view
to get links to diffs to each parent.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
It looked a bit stupid, lone "_1_ _2_" links.
gitweb/gitweb.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
[-- Attachment #2: 0002-gitweb-No-difftree-output-for-trivial-merge.patch --]
[-- Type: application/octet-stream, Size: 1193 bytes --]
From e895e5b63a580a54c24509a0df631ff4d3dff658 Mon Sep 17 00:00:00 2001
From: Jakub Narebski <jnareb@gmail.com>
Date: Sat, 8 Sep 2007 21:54:28 +0200
Subject: [PATCH 2/5] gitweb: No difftree output for trivial merge
In 'commitdiff' view, for the merge commit, there is an extra header
for the difftree table, with links to commitdiffs to individual
parents. Do not show such header when there is nothing to show, for
trivial merges.
This means that for trivial merge you have to go to 'commit' view
to get links to diffs to each parent.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
It looked a bit stupid, lone "_1_ _2_" links.
gitweb/gitweb.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c18339f..3064298 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2809,7 +2809,7 @@ sub git_difftree_body {
"diff_tree\">\n";
# header only for combined diff in 'commitdiff' view
- my $has_header = @parents > 1 && $action eq 'commitdiff';
+ my $has_header = @$difftree && @parents > 1 && $action eq 'commitdiff';
if ($has_header) {
# table header
print "<thead><tr>\n" .
--
1.5.3
^ permalink raw reply related
* [PATCH 1/5] gitweb: Remove parse_from_to_diffinfo code from git_patchset_body
From: Jakub Narebski @ 2007-09-21 21:35 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 704 bytes --]
In commit 90921740bd00029708370673fdc537522aa48e6f
"gitweb: Split git_patchset_body into separate subroutines"
a part of git_patchset_body code was separated into parse_from_to_diffinfo
subroutine. But instead of replacing the separated code by the call to
mentioned subroutine, the call to subroutine was placed before the separated
code. This patch removes parse_from_to_diffinfo code from git_patchset_body
subroutine.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
My bad...
I hope the explanation is clean enough. It does make reading
git_patchset_body code much easier.
gitweb/gitweb.perl | 36 +-----------------------------------
1 files changed, 1 insertions(+), 35 deletions(-)
[-- Attachment #2: 0001-gitweb-Remove-parse_from_to_diffinfo-code-from-git_.patch --]
[-- Type: application/octet-stream, Size: 2986 bytes --]
From 5016a1968b651ca971968f917e22dd7ce988e23d Mon Sep 17 00:00:00 2001
From: Jakub Narebski <jnareb@gmail.com>
Date: Sat, 8 Sep 2007 21:49:11 +0200
Subject: [PATCH 1/5] gitweb: Remove parse_from_to_diffinfo code from git_patchset_body
In commit 90921740bd00029708370673fdc537522aa48e6f
"gitweb: Split git_patchset_body into separate subroutines"
a part of git_patchset_body code was separated into parse_from_to_diffinfo
subroutine. But instead of replacing the separated code by the call to
mentioned subroutine, the call to subroutine was placed before the separated
code. This patch removes parse_from_to_diffinfo code from git_patchset_body
subroutine.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
My bad...
I hope the explanation is clean enough. It does make reading
git_patchset_body code much easier.
gitweb/gitweb.perl | 36 +-----------------------------------
1 files changed, 1 insertions(+), 35 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b2bae1b..c18339f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3191,44 +3191,10 @@ sub git_patchset_body {
}
} until (!defined $to_name || $to_name eq $diffinfo->{'to_file'} ||
$patch_idx > $#$difftree);
+
# modifies %from, %to hashes
parse_from_to_diffinfo($diffinfo, \%from, \%to, @hash_parents);
- if ($diffinfo->{'nparents'}) {
- # combined diff
- $from{'file'} = [];
- $from{'href'} = [];
- fill_from_file_info($diffinfo, @hash_parents)
- unless exists $diffinfo->{'from_file'};
- for (my $i = 0; $i < $diffinfo->{'nparents'}; $i++) {
- $from{'file'}[$i] = $diffinfo->{'from_file'}[$i] || $diffinfo->{'to_file'};
- if ($diffinfo->{'status'}[$i] ne "A") { # not new (added) file
- $from{'href'}[$i] = href(action=>"blob",
- hash_base=>$hash_parents[$i],
- hash=>$diffinfo->{'from_id'}[$i],
- file_name=>$from{'file'}[$i]);
- } else {
- $from{'href'}[$i] = undef;
- }
- }
- } else {
- $from{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
- if ($diffinfo->{'status'} ne "A") { # not new (added) file
- $from{'href'} = href(action=>"blob", hash_base=>$hash_parent,
- hash=>$diffinfo->{'from_id'},
- file_name=>$from{'file'});
- } else {
- delete $from{'href'};
- }
- }
- $to{'file'} = $diffinfo->{'to_file'} || $diffinfo->{'file'};
- if (!is_deleted($diffinfo)) { # file exists in result
- $to{'href'} = href(action=>"blob", hash_base=>$hash,
- hash=>$diffinfo->{'to_id'},
- file_name=>$to{'file'});
- } else {
- delete $to{'href'};
- }
# this is first patch for raw difftree line with $patch_idx index
# we index @$difftree array from 0, but number patches from 1
print "<div class=\"patch\" id=\"patch". ($patch_idx+1) ."\">\n";
--
1.5.3
^ permalink raw reply related
* [PATCH 0/5] gitweb: Fixes and improvements related to diffs
From: Jakub Narebski @ 2007-09-21 21:31 UTC (permalink / raw)
To: git
This series of patches adds fixes, simplifications and improvements
related to diff handling in gitweb ('commit', 'commitdiff' views).
Becaus I didn't have access to Internet, this series is based
on 1e61b7640d09015213dbcae3564fa27ac6a8c151 (v1.5.3.1-1-g1e61b76).
As I am sending it via GMail WWW interface, patches are send as
attachements.
I send this series now, even without good Internet access,
because it contains two bugfixes, and one code fix. I'll try to
resend it rebase on top of current master later...
Shortlog:
=========
Jakub Narebski (5):
gitweb: Remove parse_from_to_diffinfo code from git_patchset_body
gitweb: No difftree output for trivial merge
gitweb: Fix and simplify "split patch" detection
gitweb: Always set 'from_file' and 'to_file' in parse_difftree_raw_line
gitweb: Add 'status_str' to parse_difftree_raw_line output
Diffstat:
=========
gitweb/gitweb.perl | 208 ++++++++++++++++++++--------------------------------
1 files changed, 80 insertions(+), 128 deletions(-)
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] [git-p4] Detect exec bit in more cases.
From: David Brown @ 2007-09-21 21:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Simon Hausmann, git
In-Reply-To: <7vfy17iuu9.fsf@gitster.siamese.dyndns.org>
On Fri, Sep 21, 2007 at 02:15:10PM -0700, Junio C Hamano wrote:
>Simon Hausmann <simon@lst.de> writes:
>
>> On Friday 21 September 2007 00:53:52 Junio C Hamano wrote:
>>> David Brown <git@davidb.org> writes:
>>> > On Wed, Sep 19, 2007 at 09:03:50PM +0200, Simon Hausmann wrote:
>>> >>On Wednesday 19 September 2007 20:15:03 David Brown wrote:
>>> >>> git-p4 was missing the execute bit setting if the file had other
>>> >>> attribute bits set.
>>> >>> ---
>>> >>
>>> >>I'm fine with this, so unless you find a better way:
>>> >>
>>> >>Acked-By: Simon Hausmann <simon@lst.de>
>>> >
>>> > I sent out an improved version of this patch yesterday
>>> > <1190232768445-git-send-email-git@davidb.org> that I'd like to get
>>> > approved. I guess I'm not quite sure what happens at this point with a
>>> > patch.
>>>
>>> I still have that *768445* message as "the last one proposed as
>>> better than previous ones" in my mbox.
>>>
>>> Simon?
>>
>> Indeed, the new improved version is much better :)
>>
>> Acked-By: Simon Hausmann <simon@lst.de>
>
>Thanks. This should go to 'maint' (part of v1.5.3.3) right?
Sounds good by me. I've been using it on a few other repos, and haven't
had any problems.
David
^ permalink raw reply
* Re: [PATCH] [git-p4] Detect exec bit in more cases.
From: Junio C Hamano @ 2007-09-21 21:15 UTC (permalink / raw)
To: Simon Hausmann; +Cc: David Brown, git
In-Reply-To: <200709211220.05434.simon@lst.de>
Simon Hausmann <simon@lst.de> writes:
> On Friday 21 September 2007 00:53:52 Junio C Hamano wrote:
>> David Brown <git@davidb.org> writes:
>> > On Wed, Sep 19, 2007 at 09:03:50PM +0200, Simon Hausmann wrote:
>> >>On Wednesday 19 September 2007 20:15:03 David Brown wrote:
>> >>> git-p4 was missing the execute bit setting if the file had other
>> >>> attribute bits set.
>> >>> ---
>> >>
>> >>I'm fine with this, so unless you find a better way:
>> >>
>> >>Acked-By: Simon Hausmann <simon@lst.de>
>> >
>> > I sent out an improved version of this patch yesterday
>> > <1190232768445-git-send-email-git@davidb.org> that I'd like to get
>> > approved. I guess I'm not quite sure what happens at this point with a
>> > patch.
>>
>> I still have that *768445* message as "the last one proposed as
>> better than previous ones" in my mbox.
>>
>> Simon?
>
> Indeed, the new improved version is much better :)
>
> Acked-By: Simon Hausmann <simon@lst.de>
Thanks. This should go to 'maint' (part of v1.5.3.3) right?
^ permalink raw reply
* Re: cvsimport bug on branches [was: conversion to git]
From: Johannes Schindelin @ 2007-09-21 21:12 UTC (permalink / raw)
To: Robin Rosenberg
Cc: Linus Torvalds, Steffen Prohaska, Eric Blake, m4-patches,
Jim Meyering, git
In-Reply-To: <200709212242.53131.robin.rosenberg.lists@dewire.com>
Hi,
On Fri, 21 Sep 2007, Robin Rosenberg wrote:
> fredag 21 september 2007 skrev Johannes Schindelin:
> > Hi,
> >
> > On Fri, 21 Sep 2007, Robin Rosenberg wrote:
> >
> > > fredag 21 september 2007 skrev Linus Torvalds:
> > > >
> > > > The big advantage of git-cvsimport is that it can do incremental
> > > > imports, which I don't think the other methods do. But if there is any
> > > > choice at all, and especially if you're not that interested in the
> > > > incremental feature (ie you can cut over to git, and perhaps use
> > > > git-cvsserver to "supprt" CVS users) the other CVS importers are
> > > > likely to be much better.
> > >
> > > fromcvs does incremental import and it's very fast and uses much less
> > > memory than cvsimport. It needs the rcs files however and will not
> > > convert non-branch tags.
> >
> > Plus you have to install Ruby. Just wanted people to know.
>
> You don't like Ruby, do you? It worth it, really.
No, it's yet another dependency. And the quality of the code still
depends on the programmer, not the language.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 4/4] t6000lib: workaround a possible dash bug
From: Eric Wong @ 2007-09-21 20:45 UTC (permalink / raw)
To: Herbert Xu; +Cc: Junio C Hamano, git
In-Reply-To: <20070921132808.GB9778@gondor.apana.org.au>
Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Hi Eric:
Hi Herbert,
> On Thu, May 25, 2006 at 07:06:18PM -0700, Eric Wong wrote:
> > pdksh doesn't need this patch, of course bash works fine since
> > that what most users use.
> >
> > Normally, 'var=val command' seems to work fine with dash, but
> > perhaps there's something weird going on with "$@". dash is
> > pretty widespread, so it'll be good to support this even though
> > it does seem like a bug in dash.
>
> Just going through dash issues right now. Do you recall
> what the bug is in this case? Doing a quick test doesn't
> seem to show much:
>
> dash -c 'set -- env; a=b "$@"'
I tried to reproduce it on a quick script using shell functions,
multiple arguments, spaces in the $a variable.., but haven't
been successful. However, reverting the below patch still
causes errors in the latest git test suite.
> > diff --git a/t/t6000lib.sh b/t/t6000lib.sh
> > index c6752af..d402621 100755
> > --- a/t/t6000lib.sh
> > +++ b/t/t6000lib.sh
> > @@ -69,7 +69,9 @@ on_committer_date()
> > {
> > _date=$1
> > shift 1
> > - GIT_COMMITTER_DATE=$_date "$@"
> > + export GIT_COMMITTER_DATE="$_date"
> > + "$@"
> > + unset GIT_COMMITTER_DATE
> > }
> >
> > # Execute a command and suppress any error output.
> > --
> > 1.3.2.g7d11
>
> Thanks,
I'm using dash 0.5.3-7 from Debian Etch on x86-32.
(git @ 17ed158021ead9cb056f692fc35ff3fcde96a747)
*** t6003-rev-list-topo-order.sh ***
* ok 1: rev-list has correct number of entries
* ok 2: simple topo order
* ok 3: two diamonds topo order (g6)
* FAIL 4: multiple heads
check_output multiple-heads "git rev-list --topo-order a3 b3 c3"
* FAIL 5: multiple heads, prune at a1
check_output multiple-heads-prune-at-a1 "git rev-list --topo-order a3 b3 c3 ^a1"
* FAIL 6: multiple heads, prune at l1
check_output multiple-heads-prune-at-l1 "git rev-list --topo-order a3 b3 c3 ^l1"
* ok 7: cross-epoch, head at l5, prune at l1
* ok 8: duplicated head arguments
* ok 9: prune near topo
* ok 10: head has no parent
* ok 11: two nodes - one head, one base
* ok 12: three nodes one head, one internal, one base
* ok 13: linear prune l2 ^root
* ok 14: linear prune l2 ^l0
* ok 15: linear prune l2 ^l1
* ok 16: linear prune l5 ^a4
* ok 17: linear prune l5 ^l3
* ok 18: linear prune l5 ^l4
* ok 19: max-count 10 - topo order
* FAIL 20: max-count 10 - non topo order
check_output max-count-10-non-topo-order "git rev-list --max-count=10 l5"
* FAIL 21: --max-age=c3, no --topo-order
check_output max-age-c3-no-topo-order "git rev-list --max-age=1190407285 l5"
* ok 22: one specified head reachable from another a4, c3, --topo-order
* ok 23: one specified head reachable from another c3, a4, --topo-order
* ok 24: one specified head reachable from another a4, c3, no --topo-order
* ok 25: one specified head reachable from another c3, a4, no --topo-order
* ok 26: graph with c3 and a4 parents of head
* ok 27: graph with a4 and c3 parents of head
* ok 28: head ^head --topo-order
* ok 29: head ^head no --topo-order
* ok 30: simple topo order (l5r1)
* ok 31: simple topo order (r1l5)
* ok 32: don't print things unreachable from one branch
* ok 33: --topo-order a4 l3
* failed 5 among 33 test(s)
make: *** [t6003-rev-list-topo-order.sh] Error 1
--
Eric Wong
^ permalink raw reply
* Re: [PATCH] Use "" instead of "<unknown>" for placeholders
From: Johannes Schindelin @ 2007-09-21 20:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michal Vitecek, git
In-Reply-To: <7vk5qjixqy.fsf@gitster.siamese.dyndns.org>
Hi,
On Fri, 21 Sep 2007, Junio C Hamano wrote:
> Michal Vitecek <fuf@mageo.cz> writes:
>
> [jc: Added Dscho back on CC: list]
Thanks. I missed that one. My comment would have been exactly the same.
Ciao,
Dscho
^ permalink raw reply
* Re: cvsimport bug on branches [was: conversion to git]
From: Robin Rosenberg @ 2007-09-21 20:42 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Linus Torvalds, Steffen Prohaska, Eric Blake, m4-patches,
Jim Meyering, git
In-Reply-To: <Pine.LNX.4.64.0709212121330.28395@racer.site>
fredag 21 september 2007 skrev Johannes Schindelin:
> Hi,
>
> On Fri, 21 Sep 2007, Robin Rosenberg wrote:
>
> > fredag 21 september 2007 skrev Linus Torvalds:
> > >
> > > The big advantage of git-cvsimport is that it can do incremental
> > > imports, which I don't think the other methods do. But if there is any
> > > choice at all, and especially if you're not that interested in the
> > > incremental feature (ie you can cut over to git, and perhaps use
> > > git-cvsserver to "supprt" CVS users) the other CVS importers are
> > > likely to be much better.
> >
> > fromcvs does incremental import and it's very fast and uses much less
> > memory than cvsimport. It needs the rcs files however and will not
> > convert non-branch tags.
>
> Plus you have to install Ruby. Just wanted people to know.
You don't like Ruby, do you? It worth it, really.
-- robin
^ permalink raw reply
* Re: [PATCH] post-checkout hook, and related docs and tests
From: Josh England @ 2007-09-21 20:35 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <1190406421-15620-1-git-send-email-jjengla@sandia.gov>
Junk. Sorry about the address. My mailer went retarded.
-JE
On Fri, 2007-09-21 at 14:27 -0600, root wrote:
> Signed-off-by: Josh England <jjengla@sandia.gov>
> ---
> Documentation/hooks.txt | 10 +++++++
> git-checkout.sh | 5 +++
> t/t5403-post-checkout-hook.sh | 61 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 76 insertions(+), 0 deletions(-)
> create mode 100755 t/t5403-post-checkout-hook.sh
>
> diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
> index c39edc5..e78f91a 100644
> --- a/Documentation/hooks.txt
> +++ b/Documentation/hooks.txt
> @@ -87,6 +87,16 @@ parameter, and is invoked after a commit is made.
> This hook is meant primarily for notification, and cannot affect
> the outcome of `git-commit`.
>
> +post-checkout
> +-----------
> +
> +This hook is invoked when a `git-checkout` is run on a local repository.
> +The hook is given two parameters: the ref of the previous HEAD, and the ref of
> +the new HEAD. This hook cannot affect the outcome of `git-checkout`.
> +
> +This hook can be used to perform repository validity checks, auto-display
> +differences from the previous HEAD, or set working dir metadata properties.
> +
> [[pre-receive]]
> pre-receive
> -----------
> diff --git a/git-checkout.sh b/git-checkout.sh
> index 17f4392..0cff36c 100755
> --- a/git-checkout.sh
> +++ b/git-checkout.sh
> @@ -284,3 +284,8 @@ if [ "$?" -eq 0 ]; then
> else
> exit 1
> fi
> +
> +# Run a post-checkout hook
> +if test -x "$GIT_DIR"/hooks/post-checkout; then
> + "$GIT_DIR"/hooks/post-checkout $old $new
> +fi
> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> new file mode 100755
> index 0000000..aa0216a
> --- /dev/null
> +++ b/t/t5403-post-checkout-hook.sh
> @@ -0,0 +1,61 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2006 Josh England
> +#
> +
> +test_description='Test the post-checkout hook.'
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> + echo Data for commit0. >a &&
> + git update-index --add a &&
> + tree0=$(git write-tree) &&
> + commit0=$(echo setup | git commit-tree $tree0) &&
> + git update-ref refs/heads/master $commit0 &&
> + git-clone ./. clone1 &&
> + git-clone ./. clone2 &&
> + GIT_DIR=clone2/.git git branch -a new2 &&
> + echo Data for commit1. >clone2/b &&
> + GIT_DIR=clone2/.git git add clone2/b &&
> + GIT_DIR=clone2/.git git commit -m new2
> +'
> +
> +for clone in 1 2; do
> + cat >clone${clone}/.git/hooks/post-checkout <<'EOF'
> +#!/bin/sh
> +echo $@ > $GIT_DIR/post-checkout.args
> +EOF
> + chmod u+x clone${clone}/.git/hooks/post-checkout
> +done
> +
> +test_expect_success 'post-checkout runs as expected ' '
> + GIT_DIR=clone1/.git git checkout master &&
> + test -e clone1/.git/post-checkout.args
> +'
> +
> +test_expect_success 'post-checkout receives the right arguments with HEAD unchanged ' '
> + old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
> + new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
> + test $old = $new
> +'
> +
> +test_expect_success 'post-checkout runs as expected ' '
> + GIT_DIR=clone1/.git git checkout master &&
> + test -e clone1/.git/post-checkout.args
> +'
> +
> +test_expect_success 'post-checkout args are correct with git checkout -b ' '
> + GIT_DIR=clone1/.git git checkout -b new1 &&
> + old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
> + new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
> + test $old = $new
> +'
> +
> +test_expect_success 'post-checkout receives the right arguments with HEAD changed ' '
> + GIT_DIR=clone2/.git git checkout new2 &&
> + old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
> + new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
> + test $old != $new
> +'
> +
> +test_done
^ permalink raw reply
* Re: cvsimport bug on branches [was: conversion to git]
From: Johannes Schindelin @ 2007-09-21 20:22 UTC (permalink / raw)
To: Robin Rosenberg
Cc: Linus Torvalds, Steffen Prohaska, Eric Blake, m4-patches,
Jim Meyering, git
In-Reply-To: <200709211915.35642.robin.rosenberg.lists@dewire.com>
Hi,
On Fri, 21 Sep 2007, Robin Rosenberg wrote:
> fredag 21 september 2007 skrev Linus Torvalds:
> >
> > The big advantage of git-cvsimport is that it can do incremental
> > imports, which I don't think the other methods do. But if there is any
> > choice at all, and especially if you're not that interested in the
> > incremental feature (ie you can cut over to git, and perhaps use
> > git-cvsserver to "supprt" CVS users) the other CVS importers are
> > likely to be much better.
>
> fromcvs does incremental import and it's very fast and uses much less
> memory than cvsimport. It needs the rcs files however and will not
> convert non-branch tags.
Plus you have to install Ruby. Just wanted people to know.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse
From: Dan Libby @ 2007-09-21 20:21 UTC (permalink / raw)
To: Matthias Urlichs; +Cc: Junio C Hamano, git
In-Reply-To: <20070921102420.GJ11204@kiste.smurf.noris.de>
Hi,
I saw this, so I haven't run the strace command you mentioned. No need now,
right?
I'm no expert on these things, but I'd think that it should be replacing (or
escaping) any characters (not just spaces) that are not allowed by
git-check-ref-format.
For us, replacing any such characters with _ should work fine.
regards,
On Friday 21 September 2007 04:24, Matthias Urlichs wrote:
> Hi,
>
> Junio C Hamano:
> > Matthias Urlichs <smurf@smurf.noris.de> writes:
> > >> we do not like 'Cristian new code' as a tag name.
> > >
> > > Duh? That's a perfectly valid tag name.
> >
> > Is it?
> >
> > $ man git-check-ref-format
>
> Bah, stupid me. You're right, obviously.
>
> I'll replace them with underscores. :-/
--
Dan Libby
Open Source Consulting
San Jose, Costa Rica
http://osc.co.cr
phone: 011 506 223 7382
Fax: 011 506 223 7359
^ permalink raw reply
* Re: [PATCH] Use "" instead of "<unknown>" for placeholders
From: Junio C Hamano @ 2007-09-21 20:12 UTC (permalink / raw)
To: Michal Vitecek; +Cc: git, Johannes Schindelin
In-Reply-To: <20070921140500.GB9072@mageo.cz>
Michal Vitecek <fuf@mageo.cz> writes:
[jc: Added Dscho back on CC: list]
>>> >> I made it because I want to use my own pretty format which currently
>>> >> only allows '%s' for subject and '%b' for body. But '%b' is
>>> >> substituted with <undefined> if the body is "missing" which I
>>> >> obviously don't like :)
>>> >
>>> >Then you should fix %b not to show "<undefined>".
>>>
>>> I'll do it if it is okay. Shall I do the same for the other
>>> placeholders as well?
>>
>>Yeah. Don't know why I did it that way.
>
> Here comes the big patch :)
Now, this breaks t6006 which needs this patch.
Looking at this patch, I am not sure if your change is really a
desirable one --- shouldn't it be removing the line itself, not
just <unknown> token?
---
t/t6006-rev-list-format.sh | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index ad6d0b8..2be323c 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -79,9 +79,9 @@ EOF
test_format encoding %e <<'EOF'
commit 131a310eb913d107dd3c09a65d1651175898735d
-<unknown>
+
commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-<unknown>
+
EOF
test_format subject %s <<'EOF'
@@ -93,9 +93,9 @@ EOF
test_format body %b <<'EOF'
commit 131a310eb913d107dd3c09a65d1651175898735d
-<unknown>
+
commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-<unknown>
+
EOF
test_format colors %Credfoo%Cgreenbar%Cbluebaz%Cresetxyzzy <<'EOF'
@@ -121,9 +121,9 @@ test_format complex-encoding %e <<'EOF'
commit f58db70b055c5718631e5c61528b28b12090cdea
iso8859-1
commit 131a310eb913d107dd3c09a65d1651175898735d
-<unknown>
+
commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-<unknown>
+
EOF
test_format complex-subject %s <<'EOF'
@@ -142,9 +142,9 @@ and it will be encoded in iso8859-1. We should therefore
include an iso8859 character: 臓bueno!
commit 131a310eb913d107dd3c09a65d1651175898735d
-<unknown>
+
commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-<unknown>
+
EOF
test_done
^ permalink raw reply related
* Re: [PATCH] Conjugate "search" correctly in the git-prune-packed man page.
From: Junio C Hamano @ 2007-09-21 19:54 UTC (permalink / raw)
To: Matt Kraai; +Cc: git
In-Reply-To: <11903854383085-git-send-email-kraai@ftbfs.org>
Thanks; this and your other patch will be in 'maint'.
^ permalink raw reply
* Re: [PATCH] Move option parsing code to parse-options.[ch].
From: Junio C Hamano @ 2007-09-21 19:44 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <1190401278-2869-1-git-send-email-krh@redhat.com>
Kristian Høgsberg <krh@redhat.com> writes:
> Signed-off-by: Kristian Høgsberg <krh@redhat.com>
> ---
> Makefile | 2 +-
> builtin-commit.c | 117 ++++++++----------------------------------------------
> parse-options.c | 74 ++++++++++++++++++++++++++++++++++
> parse-options.h | 29 +++++++++++++
> 4 files changed, 121 insertions(+), 101 deletions(-)
> create mode 100644 parse-options.c
> create mode 100644 parse-options.h
Hmmmmmmm. Is it too much to ask to pretend as if the previous
"builtin-commit.c" that had these parts that did not belong to
it in the first place never happened?
^ permalink raw reply
* Re: [PATCH 7/7] Implement git commit as a builtin command.
From: Junio C Hamano @ 2007-09-21 19:32 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <1190395088.31494.55.camel@hinata.boston.redhat.com>
Kristian Høgsberg <krh@redhat.com> writes:
>> > +
>> > + /* update the user index file */
>> > + add_files_to_cache(fd, files, prefix);
>> > +
>> > + if (!initial_commit) {
>> > + tree = parse_tree_indirect(head_sha1);
>> > + if (!tree)
>> > + die("failed to unpack HEAD tree object");
>> > + if (read_tree(tree, 0, NULL))
>> > + die("failed to read HEAD tree object");
>> > + }
>>
>> Huh? Doesn't this read_tree() defeat the add_files_to_cache()
>> you did earlier?
>
> This is the case where we add the files on the command line
> to .git/index, but commit from a clean index file corresponding to HEAD
> with the files from the command line added (partial commit?). The first
> add_files_to_cache() updates .git/index, then we do read_tree() to build
> a tmp index from HEAD and then we add the files again. The tmp index is
> written to a tmp index file.
Still, if you are doing read_tree() that reads into the same
in-core cache you have just prepared in the add_fiels_to_cache()
above, potentially overwriting whatever you did, doesn't it?
That was what I was puzzled about...
> ... As for just using an in-memory
> index, I wanted to do it that way originally, but you have to write it
> to disk after all for the pre-commit hook.
Ah, I completely forgot about the hook. Ok, scratch the idea of
not using a temporary index file. The is not much potential for
performance gain anyway.
^ permalink raw reply
* [PATCH] Share add_files_to_cache() with builtin-add.c
From: Kristian Høgsberg @ 2007-09-21 19:01 UTC (permalink / raw)
To: git; +Cc: Kristian Høgsberg
In-Reply-To: <1190401278-2869-1-git-send-email-krh@redhat.com>
This removes the extra copy in builtin-commit.c and uses
the update() function from builtin-add.c.
Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
builtin-add.c | 8 +++---
builtin-commit.c | 65 +++++++++++------------------------------------------
commit.h | 2 +
3 files changed, 20 insertions(+), 55 deletions(-)
diff --git a/builtin-add.c b/builtin-add.c
index 5e30380..966e145 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -107,7 +107,7 @@ static void update_callback(struct diff_queue_struct *q,
}
}
-static void update(int verbose, const char *prefix, const char **files)
+void add_files_to_cache(int verbose, const char *prefix, const char **files)
{
struct rev_info rev;
init_revisions(&rev, prefix);
@@ -116,8 +116,6 @@ static void update(int verbose, const char *prefix, const char **files)
rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = update_callback;
rev.diffopt.format_callback_data = &verbose;
- if (read_cache() < 0)
- die("index file corrupt");
run_diff_files(&rev, 0);
}
@@ -218,7 +216,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)
}
if (take_worktree_changes) {
- update(verbose, prefix, argv + i);
+ if (read_cache() < 0)
+ die("index file corrupt");
+ add_files_to_cache(verbose, prefix, argv + i);
goto finish;
}
diff --git a/builtin-commit.c b/builtin-commit.c
index 90f23de..3768b53 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -56,53 +56,6 @@ static struct option commit_options[] = {
{ OPTION_STRING, "template", 't', &template_file },
};
-/* FIXME: Taken from builtin-add, should be shared. */
-
-static void update_callback(struct diff_queue_struct *q,
- struct diff_options *opt, void *cbdata)
-{
- int i, verbose;
-
- verbose = *((int *)cbdata);
- for (i = 0; i < q->nr; i++) {
- struct diff_filepair *p = q->queue[i];
- const char *path = p->one->path;
- switch (p->status) {
- default:
- die("unexpacted diff status %c", p->status);
- case DIFF_STATUS_UNMERGED:
- case DIFF_STATUS_MODIFIED:
- case DIFF_STATUS_TYPE_CHANGED:
- add_file_to_cache(path, verbose);
- break;
- case DIFF_STATUS_DELETED:
- remove_file_from_cache(path);
- if (verbose)
- printf("remove '%s'\n", path);
- break;
- }
- }
-}
-
-static void
-add_files_to_cache(int fd, const char **files, const char *prefix)
-{
- struct rev_info rev;
-
- init_revisions(&rev, "");
- setup_revisions(0, NULL, &rev, NULL);
- rev.prune_data = get_pathspec(prefix, files);
- rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
- rev.diffopt.format_callback = update_callback;
- rev.diffopt.format_callback_data = &verbose;
-
- run_diff_files(&rev, 0);
- refresh_cache(REFRESH_QUIET);
-
- if (write_cache(fd, active_cache, active_nr) || close(fd))
- die("unable to write new index file");
-}
-
static char *
prepare_index(const char **files, const char *prefix)
{
@@ -115,10 +68,16 @@ prepare_index(const char **files, const char *prefix)
die("index file corrupt");
if (all) {
- add_files_to_cache(fd, files, NULL);
+ add_files_to_cache(0, prefix, files);
+ if (write_cache(fd, active_cache, active_nr) || close(fd))
+ die("unable to write new index file");
+
return lock_file.filename;
} else if (also) {
- add_files_to_cache(fd, files, prefix);
+ add_files_to_cache(0, prefix, files);
+ if (write_cache(fd, active_cache, active_nr) || close(fd))
+ die("unable to write new index file");
+
return lock_file.filename;
}
@@ -146,7 +105,9 @@ prepare_index(const char **files, const char *prefix)
*/
/* update the user index file */
- add_files_to_cache(fd, files, prefix);
+ add_files_to_cache(0, prefix, files);
+ if (write_cache(fd, active_cache, active_nr) || close(fd))
+ die("unable to write new index file");
if (!initial_commit) {
tree = parse_tree_indirect(head_sha1);
@@ -160,7 +121,9 @@ prepare_index(const char **files, const char *prefix)
next_index_lock = xmalloc(sizeof(*next_index_lock));
fd = hold_lock_file_for_update(next_index_lock,
git_path("next-index-%d", getpid()), 1);
- add_files_to_cache(fd, files, prefix);
+ add_files_to_cache(0, prefix, files);
+ if (write_cache(fd, active_cache, active_nr) || close(fd))
+ die("unable to write new index file");
return next_index_lock->filename;
}
diff --git a/commit.h b/commit.h
index cc8d890..89caa12 100644
--- a/commit.h
+++ b/commit.h
@@ -130,6 +130,8 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
int in_merge_bases(struct commit *, struct commit **, int);
extern int interactive_add(void);
+extern void add_files_to_cache(int verbose,
+ const char *prefix, const char **files);
extern int rerere(void);
#endif /* COMMIT_H */
--
1.5.2.5
^ permalink raw reply related
* [PATCH] Move option parsing code to parse-options.[ch].
From: Kristian Høgsberg @ 2007-09-21 19:01 UTC (permalink / raw)
To: git; +Cc: Kristian Høgsberg
Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
Makefile | 2 +-
builtin-commit.c | 117 ++++++++----------------------------------------------
parse-options.c | 74 ++++++++++++++++++++++++++++++++++
parse-options.h | 29 +++++++++++++
4 files changed, 121 insertions(+), 101 deletions(-)
create mode 100644 parse-options.c
create mode 100644 parse-options.h
diff --git a/Makefile b/Makefile
index 69ebc7a..2612465 100644
--- a/Makefile
+++ b/Makefile
@@ -310,7 +310,7 @@ LIB_OBJS = \
alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
- transport.o bundle.o
+ transport.o bundle.o parse-options.o
BUILTIN_OBJS = \
builtin-add.o \
diff --git a/builtin-commit.c b/builtin-commit.c
index 3e826ca..90f23de 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -18,6 +18,7 @@
#include "log-tree.h"
#include "strbuf.h"
#include "utf8.h"
+#include "parse-options.h"
static const char builtin_commit_usage[] =
"[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [--template <file>] [[-i | -o] <path>...]";
@@ -27,90 +28,6 @@ static char *use_message_buffer;
static const char commit_editmsg[] = "COMMIT_EDITMSG";
static struct lock_file lock_file;
-enum option_type {
- OPTION_NONE,
- OPTION_STRING,
- OPTION_INTEGER,
- OPTION_LAST,
-};
-
-struct option {
- enum option_type type;
- const char *long_name;
- char short_name;
- void *value;
-};
-
-static int scan_options(const char ***argv, struct option *options)
-{
- const char *value, *eq;
- int i;
-
- if (**argv == NULL)
- return 0;
- if ((**argv)[0] != '-')
- return 0;
- if (!strcmp(**argv, "--"))
- return 0;
-
- value = NULL;
- for (i = 0; options[i].type != OPTION_LAST; i++) {
- if ((**argv)[1] == '-') {
- if (!prefixcmp(options[i].long_name, **argv + 2)) {
- if (options[i].type != OPTION_NONE)
- value = *++(*argv);
- goto match;
- }
-
- eq = strchr(**argv + 2, '=');
- if (eq && options[i].type != OPTION_NONE &&
- !strncmp(**argv + 2,
- options[i].long_name, eq - **argv - 2)) {
- value = eq + 1;
- goto match;
- }
- }
-
- if ((**argv)[1] == options[i].short_name) {
- if ((**argv)[2] == '\0') {
- if (options[i].type != OPTION_NONE)
- value = *++(*argv);
- goto match;
- }
-
- if (options[i].type != OPTION_NONE) {
- value = **argv + 2;
- goto match;
- }
- }
- }
-
- usage(builtin_commit_usage);
-
- match:
- switch (options[i].type) {
- case OPTION_NONE:
- *(int *)options[i].value = 1;
- break;
- case OPTION_STRING:
- if (value == NULL)
- die("option %s requires a value.", (*argv)[-1]);
- *(const char **)options[i].value = value;
- break;
- case OPTION_INTEGER:
- if (value == NULL)
- die("option %s requires a value.", (*argv)[-1]);
- *(int *)options[i].value = atoi(value);
- break;
- default:
- assert(0);
- }
-
- (*argv)++;
-
- return 1;
-}
-
static char *logfile, *force_author, *message, *template_file;
static char *edit_message, *use_message;
static int all, edit_flag, also, interactive, only, no_verify, amend, signoff;
@@ -120,24 +37,23 @@ static int no_edit, initial_commit, in_merge;
const char *only_include_assumed;
static struct option commit_options[] = {
- { OPTION_STRING, "file", 'F', (void *) &logfile },
- { OPTION_NONE, "all", 'a', &all },
- { OPTION_STRING, "author", 0, (void *) &force_author },
- { OPTION_NONE, "edit", 'e', &edit_flag },
- { OPTION_NONE, "include", 'i', &also },
- { OPTION_NONE, "interactive", 0, &interactive },
- { OPTION_NONE, "only", 'o', &only },
+ { OPTION_STRING, "file", 'F', &logfile },
+ { OPTION_BOOLEAN, "all", 'a', &all },
+ { OPTION_STRING, "author", 0, &force_author },
+ { OPTION_BOOLEAN, "edit", 'e', &edit_flag },
+ { OPTION_BOOLEAN, "include", 'i', &also },
+ { OPTION_BOOLEAN, "interactive", 0, &interactive },
+ { OPTION_BOOLEAN, "only", 'o', &only },
{ OPTION_STRING, "message", 'm', &message },
- { OPTION_NONE, "no-verify", 'n', &no_verify },
- { OPTION_NONE, "amend", 0, &amend },
+ { OPTION_BOOLEAN, "no-verify", 'n', &no_verify },
+ { OPTION_BOOLEAN, "amend", 0, &amend },
{ OPTION_STRING, "reedit-message", 'c', &edit_message },
{ OPTION_STRING, "reuse-message", 'C', &use_message },
- { OPTION_NONE, "signoff", 's', &signoff },
- { OPTION_NONE, "quiet", 'q', &quiet },
- { OPTION_NONE, "verbose", 'v', &verbose },
- { OPTION_NONE, "untracked-files", 0, &untracked_files },
+ { OPTION_BOOLEAN, "signoff", 's', &signoff },
+ { OPTION_BOOLEAN, "quiet", 'q', &quiet },
+ { OPTION_BOOLEAN, "verbose", 'v', &verbose },
+ { OPTION_BOOLEAN, "untracked-files", 0, &untracked_files },
{ OPTION_STRING, "template", 't', &template_file },
- { OPTION_LAST },
};
/* FIXME: Taken from builtin-add, should be shared. */
@@ -432,8 +348,9 @@ static void parse_and_validate_options(const char ***argv)
int f = 0;
(*argv)++;
- while (scan_options(argv, commit_options))
- ;
+ while (parse_options(argv, commit_options, ARRAY_SIZE(commit_options),
+ builtin_commit_usage))
+ ;
if (logfile || message || use_message)
no_edit = 1;
diff --git a/parse-options.c b/parse-options.c
new file mode 100644
index 0000000..2fb30cd
--- /dev/null
+++ b/parse-options.c
@@ -0,0 +1,74 @@
+#include "git-compat-util.h"
+#include "parse-options.h"
+
+int parse_options(const char ***argv,
+ struct option *options, int count,
+ const char *usage_string)
+{
+ const char *value, *eq;
+ int i;
+
+ if (**argv == NULL)
+ return 0;
+ if ((**argv)[0] != '-')
+ return 0;
+ if (!strcmp(**argv, "--"))
+ return 0;
+
+ value = NULL;
+ for (i = 0; i < count; i++) {
+ if ((**argv)[1] == '-') {
+ if (!prefixcmp(options[i].long_name, **argv + 2)) {
+ if (options[i].type != OPTION_BOOLEAN)
+ value = *++(*argv);
+ goto match;
+ }
+
+ eq = strchr(**argv + 2, '=');
+ if (eq && options[i].type != OPTION_BOOLEAN &&
+ !strncmp(**argv + 2,
+ options[i].long_name, eq - **argv - 2)) {
+ value = eq + 1;
+ goto match;
+ }
+ }
+
+ if ((**argv)[1] == options[i].short_name) {
+ if ((**argv)[2] == '\0') {
+ if (options[i].type != OPTION_BOOLEAN)
+ value = *++(*argv);
+ goto match;
+ }
+
+ if (options[i].type != OPTION_BOOLEAN) {
+ value = **argv + 2;
+ goto match;
+ }
+ }
+ }
+
+ usage(usage_string);
+
+ match:
+ switch (options[i].type) {
+ case OPTION_BOOLEAN:
+ *(int *)options[i].value = 1;
+ break;
+ case OPTION_STRING:
+ if (value == NULL)
+ die("option %s requires a value.", (*argv)[-1]);
+ *(const char **)options[i].value = value;
+ break;
+ case OPTION_INTEGER:
+ if (value == NULL)
+ die("option %s requires a value.", (*argv)[-1]);
+ *(int *)options[i].value = atoi(value);
+ break;
+ default:
+ assert(0);
+ }
+
+ (*argv)++;
+
+ return 1;
+}
diff --git a/parse-options.h b/parse-options.h
new file mode 100644
index 0000000..39399c3
--- /dev/null
+++ b/parse-options.h
@@ -0,0 +1,29 @@
+#ifndef PARSE_OPTIONS_H
+#define PARSE_OPTIONS_H
+
+enum option_type {
+ OPTION_BOOLEAN,
+ OPTION_STRING,
+ OPTION_INTEGER,
+ OPTION_LAST,
+};
+
+struct option {
+ enum option_type type;
+ const char *long_name;
+ char short_name;
+ void *value;
+};
+
+/* Parse the given options against the list of known options. The
+ * order of the option structs matters, in that ambiguous
+ * abbreviations (eg, --in could be short for --include or
+ * --interactive) are matched by the first option that share the
+ * prefix.
+ */
+
+extern int parse_options(const char ***argv,
+ struct option *options, int count,
+ const char *usage_string);
+
+#endif
--
1.5.2.5
^ permalink raw reply related
* [PATCH] Add test-script for git-merge porcelain
From: Lars Hjemli @ 2007-09-21 18:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This test-script tries to excercise the porcelainish aspects of git-merge.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
t/t7600-merge.sh | 317 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 317 insertions(+), 0 deletions(-)
create mode 100644 t/t7600-merge.sh
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
new file mode 100644
index 0000000..9e58636
--- /dev/null
+++ b/t/t7600-merge.sh
@@ -0,0 +1,317 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Lars Hjemli
+#
+
+test_description='git-merge
+
+Testing basic merge operations/option parsing.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo "
+1
+2
+3
+4
+5
+6
+7
+8
+9
+" > file &&
+ git add file &&
+ git commit -m "commit 0" &&
+ git tag c0 &&
+ c0=$(git rev-parse HEAD) &&
+ echo "
+1 X
+2
+3
+4
+5
+6
+7
+8
+9
+" > file &&
+ git add file &&
+ git commit -m "commit 1" &&
+ git tag c1 &&
+ c1=$(git rev-parse HEAD) &&
+ git reset --hard "$c0" &&
+ echo "
+1
+2
+3
+4
+5 X
+6
+7
+8
+9
+" > file &&
+ git add file &&
+ git commit -m "commit 2" &&
+ git tag c2 &&
+ c2=$(git rev-parse HEAD) &&
+ git reset --hard "$c0" &&
+ echo "
+1
+2
+3
+4
+5
+6
+7
+8
+9 X
+" > file &&
+ git add file &&
+ git commit -m "commit 3" &&
+ git tag c3 &&
+ c3=$(git rev-parse HEAD)
+ git reset --hard "$c0"
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'test option parsing' '
+ if git merge -$ c1
+ then
+ echo "[OOPS] -$ accepted"
+ false
+ fi &&
+ if git merge --no-such c1
+ then
+ echo "[OOPS] --no-such accepted"
+ false
+ fi &&
+ if git merge -s foobar c1
+ then
+ echo "[OOPS] -s foobar accepted"
+ false
+ fi &&
+ if git merge -s=foobar c1
+ then
+ echo "[OOPS] -s=foobar accepted"
+ false
+ fi &&
+ if git merge -m
+ then
+ echo "[OOPS] missing commit msg accepted"
+ false
+ fi &&
+ if git merge
+ then
+ echo "[OOPS] missing commit references accepted"
+ false
+ fi
+'
+
+test_expect_success 'merge c0 with c1' '
+ git reset --hard c0 &&
+ git merge c1 &&
+ test "$c1" = "$(git rev-parse HEAD)"
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'verify merge result' '
+ echo "
+1 X
+2
+3
+4
+5
+6
+7
+8
+9
+" > result.1 &&
+ cmp -s file result.1
+'
+
+test_expect_success 'merge c1 with c2' '
+ git reset --hard c1 &&
+ git merge c2 &&
+ test "$c1" = "$(git rev-parse HEAD^1)" &&
+ test "$c2" = "$(git rev-parse HEAD^2)"
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'verify merge result' '
+ echo "
+1 X
+2
+3
+4
+5 X
+6
+7
+8
+9
+" > result.1-5 &&
+ cmp -s file result.1-5
+'
+
+test_expect_success 'merge c1 with c2 and c3' '
+ git reset --hard c1 &&
+ git merge c2 c3 &&
+ test "$c1" = "$(git rev-parse HEAD^1)" &&
+ test "$c2" = "$(git rev-parse HEAD^2)" &&
+ test "$c3" = "$(git rev-parse HEAD^3)"
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'verify merge result' '
+ echo "
+1 X
+2
+3
+4
+5 X
+6
+7
+8
+9 X
+" > result.1-5-9 &&
+ cmp -s file result.1-5-9
+'
+
+test_expect_success 'merge c0 with c1 (no-commit)' '
+ git reset --hard c0 &&
+ git merge --no-commit c1 &&
+ if test "$c1" != "$(git rev-parse HEAD)"
+ then
+ echo "[OOPS] fast-forward not performed"
+ false
+ fi &&
+ if ! cmp -s file result.1
+ then
+ echo "[OOPS] merge result is wrong"
+ false
+ fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c2 (no-commit)' '
+ git reset --hard c1 &&
+ git merge --no-commit c2 &&
+ if test "$c1" != "$(git rev-parse HEAD)"
+ then
+ echo "[OOPS] HEAD changed"
+ false
+ fi &&
+ if ! cmp -s file result.1-5
+ then
+ echo "[OOPS] merge result is wrong"
+ false
+ fi &&
+ if test "$c2" != "$(cat .git/MERGE_HEAD)"
+ then
+ echo "[OOPS] MERGE_HEAD is wrong"
+ false
+ fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c2 and c3 (no-commit)' '
+ git reset --hard c1 &&
+ git merge --no-commit c2 c3 &&
+ if test "$c1" != "$(git rev-parse HEAD)"
+ then
+ echo "[OOPS] HEAD changed"
+ false
+ fi &&
+ if ! cmp -s file result.1-5-9
+ then
+ echo "[OOPS] merge result is wrong"
+ false
+ fi &&
+ if ! grep -q "$c2" .git/MERGE_HEAD
+ then
+ echo "[OOPS] c2 not in MERGE_HEAD"
+ false
+ fi &&
+ if ! grep -q "$c3" .git/MERGE_HEAD
+ then
+ echo "[OOPS] c3 not in MERGE_HEAD"
+ false
+ fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c0 with c1 (squash)' '
+ git reset --hard c0 &&
+ git merge --squash c1 &&
+ if test "$c0" != "$(git rev-parse HEAD)"
+ then
+ echo "[OOPS] HEAD changed"
+ false
+ fi &&
+ if ! cmp -s file result.1
+ then
+ echo "[OOPS] merge result is wrong"
+ false
+ fi &&
+ if test -f .git/MERGE_HEAD
+ then
+ echo "[OOPS] MERGE_HEAD exists"
+ false
+ fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c2 (squash)' '
+ git reset --hard c1 &&
+ git merge --squash c2 &&
+ if test "$c1" != "$(git rev-parse HEAD)"
+ then
+ echo "[OOPS] new commit created"
+ false
+ fi &&
+ if ! cmp -s file result.1-5
+ then
+ echo "[OOPS] merge result is wrong"
+ false
+ fi &&
+ if test -f .git/MERGE_HEAD
+ then
+ echo "[OOPS] MERGE_HEAD exists"
+ false
+ fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c2 and c3 (squash)' '
+ git reset --hard c1 &&
+ git merge --squash c2 c3 &&
+ if test "$c1" != "$(git rev-parse HEAD)"
+ then
+ echo "[OOPS] HEAD changed"
+ false
+ fi &&
+ if ! cmp -s file result.1-5-9
+ then
+ echo "[OOPS] merge result is wrong"
+ false
+ fi &&
+ if test -f .git/MERGE_HEAD
+ then
+ echo "[OOPS] MERGE_HEAD exists"
+ false
+ fi
+'
+
+test_debug 'gitk --all'
+
+test_done
--
1.5.3.2.82.g75c8d
^ permalink raw reply related
* Re: [PATCH] new test from the submodule chapter of the user manual
From: Junio C Hamano @ 2007-09-21 18:04 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Johannes Schindelin, J. Bruce Fields
In-Reply-To: <20070921130908.GF16235@genesis.frugalware.org>
That's horrible. Please do not depend on object SHA1's to stay
the same. If somebody makes a fix to the test to add a new file
in a sample subproject it would break all the rest. Also please
do not depend on the progress output.
In short, please do not try to get away with quick-and-dirty.
^ permalink raw reply
* Re: [PATCH 6/7] Export rerere() and launch_editor().
From: Kristian Høgsberg @ 2007-09-21 18:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr6kunrh7.fsf@gitster.siamese.dyndns.org>
On Wed, 2007-09-19 at 16:52 -0700, Junio C Hamano wrote:
> Kristian Høgsberg <krh@redhat.com> writes:
>
> > +/* Export for builtin-commit. */
> > +int rerere(void)
> > +{
> > + struct path_list merge_rr = { NULL, 0, 0, 1 };
> > + int fd;
> > +
> > + git_config(git_rerere_config);
> > + if (!is_rerere_enabled())
> > + return 0;
> > +
> > + merge_rr_path = xstrdup(git_path("rr-cache/MERGE_RR"));
> > + fd = hold_lock_file_for_update(&write_lock, merge_rr_path, 1);
> > + read_rr(&merge_rr);
> > + return do_plain_rerere(&merge_rr, fd);
> > +}
>
> Is it just me who sees a suboptimal cut and paste here?
>
> BTW, [1-5/7] look good so far.
>
> diff --git a/builtin-rerere.c b/builtin-rerere.c
> index 29d057c..2f51ae0 100644
> --- a/builtin-rerere.c
> +++ b/builtin-rerere.c
> @@ -415,18 +415,39 @@ static int is_rerere_enabled(void)
> return 1;
> }
>
> -int cmd_rerere(int argc, const char **argv, const char *prefix)
> +static int setup_rerere(struct path_list *merge_rr)
> {
> - struct path_list merge_rr = { NULL, 0, 0, 1 };
> - int i, fd = -1;
> + int fd;
>
> git_config(git_rerere_config);
> if (!is_rerere_enabled())
> - return 0;
> + return -1;
>
> merge_rr_path = xstrdup(git_path("rr-cache/MERGE_RR"));
> fd = hold_lock_file_for_update(&write_lock, merge_rr_path, 1);
> - read_rr(&merge_rr);
> + read_rr(merge_rr);
> + return fd;
> +}
> +
> +int rerere(void)
> +{
> + struct path_list merge_rr = { NULL, 0, 0, 1 };
> + int fd;
> +
> + fd = setup_rerere(&merge_rr);
> + if (fd < 0)
> + return 0;
> + return do_plain_rerere(&merge_rr, fd);
> +}
> +
> +int cmd_rerere(int argc, const char **argv, const char *prefix)
> +{
> + struct path_list merge_rr = { NULL, 0, 0, 1 };
> + int i, fd;
> +
> + fd = setup_rerere(&merge_rr);
> + if (fd < 0)
> + return 0;
>
> if (argc < 2)
> return do_plain_rerere(&merge_rr, fd);
That looks better, yes.
Kristian
^ permalink raw reply
* Re: [PATCH 7/7] Implement git commit as a builtin command.
From: Kristian Høgsberg @ 2007-09-21 17:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk5qmm8hq.fsf@gitster.siamese.dyndns.org>
On Wed, 2007-09-19 at 18:27 -0700, Junio C Hamano wrote:
> Kristian Høgsberg <krh@redhat.com> writes:
>
> > diff --git a/builtin-commit.c b/builtin-commit.c
> > new file mode 100644
> > index 0000000..ee98de9
> > --- /dev/null
> > +++ b/builtin-commit.c
> > @@ -0,0 +1,740 @@
> > +/*
> > + * Builtin "git commit"
> > + *
> > + * Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>
> > + * Based on git-commit.sh by Junio C Hamano and Linus Torvalds
> > + */
> > +
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <unistd.h>
> > +
>
> With 85023577a8f4b540aa64aa37f6f44578c0c305a3 (simplify
> inclusion of system header files.), we introduced a rule against
> these lines. We probably would want a Coding Guideline (aka
> "Hacking Git") document somewhere.
Ok. I see you removed them in the pu commit, thanks.
> > +#include "cache.h"
> > +#include "cache-tree.h"
> > +#include "builtin.h"
> > +#include "diff.h"
> > +#include "diffcore.h"
> > +#include "commit.h"
> > +#include "revision.h"
> > +#include "wt-status.h"
> > +#include "run-command.h"
> > +#include "refs.h"
> > +#include "log-tree.h"
> > +#include "strbuf.h"
> > +#include "utf8.h"
> > +
> > +static const char builtin_commit_usage[] =
> > + "[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [--template <file>] [[-i | -o] <path>...]";
> > +
> > +static unsigned char head_sha1[20], merge_head_sha1[20];
> > +static char *use_message_buffer;
> > +static const char commit_editmsg[] = "COMMIT_EDITMSG";
> > +static struct lock_file lock_file;
> > +
> > +enum option_type {
> > + OPTION_NONE,
> > + OPTION_STRING,
> > + OPTION_INTEGER,
> > + OPTION_LAST,
> > +};
> > +
> > +struct option {
> > + enum option_type type;
> > + const char *long_name;
> > + char short_name;
> > + void *value;
> > +};
> > +
> > +static int scan_options(const char ***argv, struct option *options)
> > +{
> > + const char *value, *eq;
> > + int i;
> > +
> > + if (**argv == NULL)
> > + return 0;
> > + if ((**argv)[0] != '-')
> > + return 0;
> > + if (!strcmp(**argv, "--"))
> > + return 0;
> > +
> > + value = NULL;
> > + for (i = 0; options[i].type != OPTION_LAST; i++) {
> > + if ((**argv)[1] == '-') {
> > + if (!prefixcmp(options[i].long_name, **argv + 2)) {
> > + if (options[i].type != OPTION_NONE)
> > + value = *++(*argv);
> > + goto match;
> > + }
> > +
> > + eq = strchr(**argv + 2, '=');
> > + if (eq && options[i].type != OPTION_NONE &&
> > + !strncmp(**argv + 2,
> > + options[i].long_name, eq - **argv - 2)) {
> > + value = eq + 1;
> > + goto match;
> > + }
> > + }
> > +
> > + if ((**argv)[1] == options[i].short_name) {
> > + if ((**argv)[2] == '\0') {
> > + if (options[i].type != OPTION_NONE)
> > + value = *++(*argv);
> > + goto match;
> > + }
> > +
> > + if (options[i].type != OPTION_NONE) {
> > + value = **argv + 2;
> > + goto match;
> > + }
> > + }
> > + }
>
> How do you disambiguate between "--author <me>" and "--amend"?
> "The order in *options list matters" is an acceptable answer but
> it needs to be documented.
Yes, the order matters. I made sure the C version has the same
precedence as the shell script. I'll add a comment in parse-options.h.
> > +
> > + usage(builtin_commit_usage);
> > +
> > + match:
> > + switch (options[i].type) {
> > + case OPTION_NONE:
> > + *(int *)options[i].value = 1;
> > + break;
> > + case OPTION_STRING:
> > + if (value == NULL)
> > + die("option %s requires a value.", (*argv)[-1]);
> > + *(const char **)options[i].value = value;
> > + break;
> > + case OPTION_INTEGER:
> > + if (value == NULL)
> > + die("option %s requires a value.", (*argv)[-1]);
> > + *(int *)options[i].value = atoi(value);
> > + break;
> > + default:
> > + assert(0);
> > + }
> > +
> > + (*argv)++;
> > +
> > + return 1;
> > +}
>
> I do not particularly like this OPTION_LAST convention, but that
> is a minor detail. I also suspect in the longer term we might
> be better off using getopt() or popt(), but that would be a
> larger project.
>
> In any case, if you want to use this option parser, you would
> need to add a new file, perhaps parse_options.c, and move this
> part there, so that other parts of the system can reuse it.
I'll split it out in parse-options.[ch]. I still don't understand why
using getopt is better; parse-options.c is a 75 line file and it's
simple code. It does more work that getopt, since for most options it
automatically writes back the option argument into a global. All I have
to do then is validate that no illegal combination of options was
passed.
> And the same comment goes for the launch_editor still in
> builtin-tag.c. We should move it to editor.c or something.
Yeah.
> > +
> > +static char *logfile, *force_author, *message, *template_file;
> > +static char *edit_message, *use_message;
> > +static int all, edit_flag, also, interactive, only, no_verify, amend, signoff;
> > +static int quiet, verbose, untracked_files;
> > +
> > +static int no_edit, initial_commit, in_merge;
> > +const char *only_include_assumed;
> > +
> > +static struct option commit_options[] = {
> > + { OPTION_STRING, "file", 'F', (void *) &logfile },
> > + { OPTION_NONE, "all", 'a', &all },
> > + { OPTION_STRING, "author", 0, (void *) &force_author },
> > + { OPTION_NONE, "edit", 'e', &edit_flag },
>
> Why do some get casted to (void*) and others don't? It doesn't
> seem to have any pattern. I am puzzled...
Yes, oops, not sure what that was about.
> > + { OPTION_NONE, "include", 'i', &also },
> > + { OPTION_NONE, "interactive", 0, &interactive },
> > + { OPTION_NONE, "only", 'o', &only },
> > + { OPTION_STRING, "message", 'm', &message },
> > + { OPTION_NONE, "no-verify", 'n', &no_verify },
> > + { OPTION_NONE, "amend", 0, &amend },
> > + { OPTION_STRING, "reedit-message", 'c', &edit_message },
> > + { OPTION_STRING, "reuse-message", 'C', &use_message },
> > + { OPTION_NONE, "signoff", 's', &signoff },
> > + { OPTION_NONE, "quiet", 'q', &quiet },
> > + { OPTION_NONE, "verbose", 'v', &verbose },
> > + { OPTION_NONE, "untracked-files", 0, &untracked_files },
> > + { OPTION_STRING, "template", 't', &template_file },
> > + { OPTION_LAST },
> > +};
> > +
> > +/* FIXME: Taken from builtin-add, should be shared. */
>
> You're darn right. I thought you have some other patch that
> touches builtin-add already...
Yeah, I'll fix that.
> > +
> > +static void update_callback(struct diff_queue_struct *q,
> > + struct diff_options *opt, void *cbdata)
> > +{
> > + int i, verbose;
> > +
> > + verbose = *((int *)cbdata);
> > + for (i = 0; i < q->nr; i++) {
> > + struct diff_filepair *p = q->queue[i];
> > + const char *path = p->one->path;
> > + switch (p->status) {
> > + default:
> > + die("unexpacted diff status %c", p->status);
> > + case DIFF_STATUS_UNMERGED:
> > + case DIFF_STATUS_MODIFIED:
> > + case DIFF_STATUS_TYPE_CHANGED:
> > + add_file_to_cache(path, verbose);
> > + break;
> > + case DIFF_STATUS_DELETED:
> > + remove_file_from_cache(path);
> > + cache_tree_invalidate_path(active_cache_tree, path);
>
> I've updated remove_file_from_cache() to invalidate the path in
> cache-tree so this does not hurt but is no longer necessary. I
> removed this line in the version I queued in 'pu'.
>
> > + if (verbose)
> > + printf("remove '%s'\n", path);
> > + break;
> > + }
> > + }
> > +}
> > +
> > +static void
> > +add_files_to_cache(int fd, const char **files, const char *prefix)
> > +{
> > + struct rev_info rev;
> > +
> > + init_revisions(&rev, "");
> > + setup_revisions(0, NULL, &rev, NULL);
> > + rev.prune_data = get_pathspec(prefix, files);
> > + rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
> > + rev.diffopt.format_callback = update_callback;
> > + rev.diffopt.format_callback_data = &verbose;
> > +
> > + run_diff_files(&rev, 0);
> > + refresh_cache(REFRESH_QUIET);
>
> Your update_callback() does add_file_to_cache() which picks up
> the stat information from the filesystem already, so I do not
> see much point calling refresh_cache() afterwards. On the other
> hand, refreshing before running diff_files() might make sense,
> as you can avoid a lot of unnecessary work when you are told to
> do "git commit ."
I didn't have refresh_cache() in there originally, there was some test
case that didn't pass until I added it. The git-commit.sh shell script
calls git update-index --refresh after the prepare_index() part, which
is where I got it from. Will investigate.
> Have you tested this with an unmerged index?
Not yet.
> > +
> > + if (write_cache(fd, active_cache, active_nr) || close(fd))
> > + die("unable to write new index file");
> > +}
> > +
> > +static char *
> > +prepare_index(const char **files, const char *prefix)
> > +{
> > + int fd;
> > + struct tree *tree;
> > + struct lock_file *next_index_lock;
> > +
> > + fd = hold_locked_index(&lock_file, 1);
> > + if (read_cache() < 0)
> > + die("index file corrupt");
> > +
> > + if (all) {
> > + add_files_to_cache(fd, files, NULL);
> > + return lock_file.filename;
>
> Should you be passing files, which is used as pathspec to decide
> if your update_callback() should be called, under --all option?
Oh... hmm, if 'all' is set there are no paths, but yes, that is
confusing.
> > + } else if (also) {
> > + add_files_to_cache(fd, files, prefix);
> > + return lock_file.filename;
> > + }
> > +
> > + if (interactive)
> > + interactive_add();
>
> Don't you need to
>
> (1) abort if the user aborts out of interactive add with ^C?
> (2) re-read the index after interactive_add() returns?
Uhm, yes, good points.
> > + if (*files == NULL) {
> > + /* Commit index as-is. */
> > + rollback_lock_file(&lock_file);
> > + return get_index_file();
> > + }
> > +
> > + /*
> > + * FIXME: Warn on unknown files. Shell script does
> > + *
> > + * commit_only=`git-ls-files --error-unmatch -- "$@"`
> > + */
>
> This should be much easier to do than from the shell script, as
> you have active_cache[] (aka "the_index.cache[]") in-core.
>
> > + /*
> > + * FIXME: shell script does
> > + *
> > + * git-read-tree --index-output="$TMP_INDEX" -i -m HEAD
> > + *
> > + * which warns about unmerged files in the index.
> > + */
>
> I think "unmerged warning" is an unintended side effect. The
> point of the command is to have a copy of index, as there is no
> way for shell script to work with more than one index at a time
> without using a temporary file.
Yes, we talked about this in IRC a few weeks back, and agreed that just
read_tree() should be fine. I'll just remove that FIXME.
> > +
> > + /* update the user index file */
> > + add_files_to_cache(fd, files, prefix);
> > +
> > + if (!initial_commit) {
> > + tree = parse_tree_indirect(head_sha1);
> > + if (!tree)
> > + die("failed to unpack HEAD tree object");
> > + if (read_tree(tree, 0, NULL))
> > + die("failed to read HEAD tree object");
> > + }
>
> Huh? Doesn't this read_tree() defeat the add_files_to_cache()
> you did earlier?
This is the case where we add the files on the command line
to .git/index, but commit from a clean index file corresponding to HEAD
with the files from the command line added (partial commit?). The first
add_files_to_cache() updates .git/index, then we do read_tree() to build
a tmp index from HEAD and then we add the files again. The tmp index is
written to a tmp index file.
> > +
> > + /* Uh oh, abusing lock_file to create a garbage collected file */
> > + next_index_lock = xmalloc(sizeof(*next_index_lock));
> > + fd = hold_lock_file_for_update(next_index_lock,
> > + git_path("next-index-%d", getpid()), 1);
>
> That's not an abuse, but I wonder what happened to the fd you
> got at the beginning of the function.
Ugh, yeah, that's a bit ugly... it gets closed in add_files_to_cache()
once we've written the index. The patch I have here uses the
add_files_to_cache() from builtin-add.c which doesn't close the fd or
write the index. That now happens in prepare_index(), so it's a little
clearer what's going on.
> > + add_files_to_cache(fd, files, prefix);
>
> and then this is puzzling to me.
>
> I am starting to suspect that you might be better off if you do
> not follow the use of temporary index file that was in the shell
> script version to the letter. You can use more than one index
> in the core at the same time, now you are built-in.
As described above, we need to add the files to the user index and the
temporary index we're committing from. As for just using an in-memory
index, I wanted to do it that way originally, but you have to write it
to disk after all for the pre-commit hook. Maybe doing it in-memory is
better after all, and then only write it to disk if we actually have a
pre-commit hook.
> > +
> > + return next_index_lock->filename;
> > +}
>
> ... and if we were to go that route, wt_status structure would
> have a pointer to the "struct index_state" instead of the
> filename of the index... oops, wt_status_print() will discard
> and re-read the cache from file, so that approach would not work
> right now without fixing wt-status first. Hmm.
Yep, that was the other reason. I suppose a short term fix for this
would be to make run_status() take a struct index_state and write it to
a temp file and run against that. We can then clean that up to not use
a temp file in a second patch.
> > +static int run_status(FILE *fp, const char *index_file)
> > +{
> > + struct wt_status s;
> > +
> > + wt_status_prepare(&s);
> > +
> > + if (amend) {
> > + s.amend = 1;
> > + s.reference = "HEAD^1";
> > + }
> > + s.verbose = verbose;
> > + s.untracked = untracked_files;
> > + s.index_file = index_file;
> > + s.fp = fp;
> > +
> > + wt_status_print(&s);
> > +
> > + return s.commitable;
> > +}
>
> I did not look at the rest of the patch; it should be either
> obviously correct or outright incorrect --- anybody would notice
> the breakage immediately.
>
> The prepare_index() part is the most (and only) "interesting"
> part of the puzzle. I need to look at this again and think
> about it a bit more. It needs to:
>
> * save the current index and restore in case the whole
> "git-commit" is aborted in any way (e.g. ^C, empty log
> message from the editor); This is easy to do from C with
> hold_locked_index().
This is working in the current version. I use hold_locked_index() in
the beginning of prepare_index() and calls commit_locked_index() to
write the new index at the end of cmd_commit() once the commit is done.
> * when doing a partial commit,
> - read HEAD in a temporary index, if not initial commit;
> - update the temporary index with the given paths;
> - write out the temporary index as a tree to build a commit;
>
> - update the real index the same way with the given paths;
> - if all goes well, commit the updated real index; again,
> this is easy in C with commit_locked_index();
prepare_index() does this, that's the case you pointed out above, where
I'm calling add_files_to_cache() twice.
> * when not doing a partial commit,
> - update the real index with given paths (if "--also") or all
> what diff-files reports (if "--all");
> - write out the real index as a tree to build a commit;
> - if all goes well, commit the updated real index; again,
> this is easy in C with commit_locked_index();
This is also in prepare_index() - the 'if (all)' and 'if (also)' cases
in the beginning of the function does that, except commiting the the
updated real index, that's done at the end of cmd_commit(), share among
all cases.
> A thing to keep in mind is that you can write out the temporary
> index as a tree from the core without writing it out first to a
> temporary index file at all. Perhaps the code should be
> structured like this.
>
> - parse options and all the other necessary setup;
>
> - hold_locked_index() to lock the "real" index;
>
> - prepare_index() is responsibile for doing two things
>
> 1. build, write out and return the tree object to be
> committed;
>
> 2. leave the_index[] in the state to become the "real"
> index if this commit command is not aborted;
>
> - take the tree object, put commit log message around it and
> make a commit object;
>
> - commit_locked_index() to write out the "real" index.
>
> Now, the task #1 for prepare_index() is simpler for "also" and
> "all" case. You do the equivalent of "git-add -u" or "git add
> <path>" and run cache_tree_update() to write out the tree, and
> when you are done, both the_index.cache and the_index.cache_tree
> are up-to-date, ready to be written out by the caller at the
> very end.
Yes, but isn't that what the code is doing now? I write out the tree in
cmd_commit(), not in prepare_index(), but prepare_index() is also used
in cmd_status(). It seems better to only write the tree if we're going
to do a commit. Other than that, the flow of the code follows your
description pretty much step by step.
> For a partial commit, the task is a bit more convoluted, as
> writing out the tree needs to be done from outside the_index. I
> would say something like this:
>
> int prepare_index(unsigned char *sha1) {
> if (partial commit) {
> struct index_state tmp_index;
>
> /* "temporary index" only in-core */
> memset(&tmp_index, 0, sizeof(tmp_index));
> read_index(&tmp_index);
> add_files_to_index(files, prefix, &tmp_index);
> write_index_as_tree(&tmp_index, sha1);
> discard_index(&tmp_index);
>
> /* update the index the same way */
> add_files_to_index(files, prefix, &the_index);
> return ok;
> }
> /* otherwise */
> if (files && *files)
> add_files_to_index(files, prefix, &the_index);
> else if (all)
> add_files_to_index(NULL, prefix, &the_index);
> write_index_as_tree(&the_index, sha1);
> return ok;
> }
>
> where
>
> * add_files_to_index() is similar to your add_files_to_cache()
> but takes struct index_state; the callback can still diff
> with the_index.cache (aka active_cache) as that diff is what
> we are interested in updating anyway;
>
> * write_index_as_tree() would be a new function that takes
> struct index_state and writes that as a tree. It would
> essentially be the builtin-write-tree.c::write_tree()
> function except the opening and reading the index (or
> updating the cache-tree) part, something like:
>
> write_index_as_tree(struct index_state *istate, unsigned char *sha1)
> {
> if (!cache_tree_fully_valid(istate->cache_tree))
> cache_tree_update(istate->cache_tree,
> istate->cache,
> istate->cache_nr, 0, 0);
> hashcpy(sha1, istate->cache_tree->sha1);
> }
That looks nicer, the add_files_to_index() function is a pretty clean
solution. But again, we can't write the tree in prepare_index(); we
need the temporary index for the pre-commit hook, and prepare_index() is
used in cmd_status(). But we can just return an struct index_state
pointer from prepare_index().
Thanks for reviewing,
Kristian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox