From ae0754ac9a24afa2693246222fc078fe9c133b3a Mon Sep 17 00:00:00 2001 From: Simon Sasburg Date: Wed, 19 Sep 2007 00:33:34 +0200 Subject: [PATCH 01/17] git-gui: Avoid using bold text in entire gui for some fonts Signed-off-by: Shawn O. Pearce --- git-gui.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-gui.sh b/git-gui.sh index f789e91b66..28d7c21692 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -1648,7 +1648,7 @@ proc apply_config {} { set font [lindex $option 1] if {[catch { foreach {cn cv} $repo_config(gui.$name) { - font configure $font $cn $cv + font configure $font $cn $cv -weight normal } } err]} { error_popup "Invalid font specified in gui.$name:\n\n$err" From 183a1d1496921e16d316ac523146385af39fcdb0 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 21 Sep 2007 10:58:02 -0400 Subject: [PATCH 02/17] git-gui: Display message box when we cannot find git in $PATH If we cannot find the git executable in the user's $PATH then we cannot function correctly. Because we need that to get the version so we can load our library correctly we cannot rely on the library function "error_popup" here, as this is all running before the library path has been configured, so error_popup is not available to us. Signed-off-by: Shawn O. Pearce --- git-gui.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/git-gui.sh b/git-gui.sh index 28d7c21692..10710e26c6 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -498,7 +498,11 @@ proc rmsel_tag {text} { set _git [_which git] if {$_git eq {}} { catch {wm withdraw .} - error_popup "Cannot find git in PATH." + tk_messageBox \ + -icon error \ + -type ok \ + -title [mc "git-gui: fatal error"] \ + -message [mc "Cannot find git in PATH."] exit 1 } From 299077fb40eac1e128b7bc09d5d992960e6f11c2 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 21 Sep 2007 11:08:50 -0400 Subject: [PATCH 03/17] git-gui: Handle starting on mapped shares under Cygwin I really cannot explain Cygwin's behavior here but if we start git-gui through Cygwin on a local drive it appears that Cygwin is leaving $env(PATH) in Unix style, even if it started a native (non-Cygwin) Tcl/Tk process to run git-gui. Yet starting that same git-gui and Tcl/Tk combination through Cygwin on a network share causes it to automatically convert $env(PATH) into Windows style, which broke our internal "which" implementation. Signed-off-by: Shawn O. Pearce --- git-gui.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-gui.sh b/git-gui.sh index 10710e26c6..62e1652276 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -305,7 +305,7 @@ proc _which {what} { global env _search_exe _search_path if {$_search_path eq {}} { - if {[is_Cygwin]} { + if {[is_Cygwin] && [regexp {^(/|\.:)} $env(PATH)]} { set _search_path [split [exec cygpath \ --windows \ --path \ From 2fe167b67a479b19e52b974f9518436565e6793b Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 21 Sep 2007 11:44:23 -0400 Subject: [PATCH 04/17] git-gui: Ensure .git/info/exclude is honored in Cygwin workdirs If we are using Cygwin and the git repository is actually a workdir (by way of git-new-workdir) but this Tcl process is a native Tcl/Tk and not the Cygwin Tcl/Tk then we are unable to traverse the .git/info path as it is a Cygwin symlink and not a standard Windows directory. So we actually need to start a Cygwin process that can do the path translation for us and let it test for .git/info/exclude so we know if we can include that file in our git-ls-files or not. Signed-off-by: Shawn O. Pearce --- git-gui.sh | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 62e1652276..c8375029dd 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -907,6 +907,35 @@ proc rescan {after {honor_trustmtime 1}} { } } +if {[is_Cygwin]} { + set is_git_info_link {} + set is_git_info_exclude {} + proc have_info_exclude {} { + global is_git_info_link is_git_info_exclude + + if {$is_git_info_link eq {}} { + set is_git_info_link [file isfile [gitdir info.lnk]] + } + + if {$is_git_info_link} { + if {$is_git_info_exclude eq {}} { + if {[catch {exec test -f [gitdir info exclude]}]} { + set is_git_info_exclude 0 + } else { + set is_git_info_exclude 1 + } + } + return $is_git_info_exclude + } else { + return [file readable [gitdir info exclude]] + } + } +} else { + proc have_info_exclude {} { + return [file readable [gitdir info exclude]] + } +} + proc rescan_stage2 {fd after} { global rescan_active buf_rdi buf_rdf buf_rlo @@ -917,9 +946,8 @@ proc rescan_stage2 {fd after} { } set ls_others [list --exclude-per-directory=.gitignore] - set info_exclude [gitdir info exclude] - if {[file readable $info_exclude]} { - lappend ls_others "--exclude-from=$info_exclude" + if {[have_info_exclude]} { + lappend ls_others "--exclude-from=[gitdir info exclude]" } set user_exclude [get_config core.excludesfile] if {$user_exclude ne {} && [file readable $user_exclude]} { From 501e4c6f23378aca2ce14ba4bc3eebeccb92e8d7 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Tue, 2 Oct 2007 12:24:44 -0400 Subject: [PATCH 05/17] git-gui: Allow gitk to be started on Cygwin with native Tcl/Tk gitk expects $env(GIT_DIR) to be valid as both a path that core Git and Tcl/Tk can resolve to a valid directory, but it has no special handling for Cygwin style UNIX paths and Windows style paths. So we need to do that for gitk and ensure that only relative paths are fed to it, thus allowing both Cygwin style and UNIX style paths to be resolved. Signed-off-by: Shawn O. Pearce --- git-gui.sh | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/git-gui.sh b/git-gui.sh index c8375029dd..9682418e12 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -1468,7 +1468,27 @@ proc do_gitk {revs} { if {! [file exists $exe]} { error_popup "Unable to start gitk:\n\n$exe does not exist" } else { + global env + + if {[info exists env(GIT_DIR)]} { + set old_GIT_DIR $env(GIT_DIR) + } else { + set old_GIT_DIR {} + } + + set pwd [pwd] + cd [file dirname [gitdir]] + set env(GIT_DIR) [file tail [gitdir]] + eval exec $cmd $revs & + + if {$old_GIT_DIR eq {}} { + unset env(GIT_DIR) + } else { + set env(GIT_DIR) $old_GIT_DIR + } + cd $pwd + ui_status $::starting_gitk_msg after 10000 { ui_ready $starting_gitk_msg From 906ab7f6c03764423adef6c0e4d77442405adc23 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Tue, 2 Oct 2007 12:27:32 -0400 Subject: [PATCH 06/17] git-gui: Don't crash when starting gitk from a browser session If the user has started git-gui from the command line as a browser we offer the gitk menu options but we didn't create the main status bar widget in the "." toplevel. Trying to access it while starting gitk just results in Tcl errors. Signed-off-by: Shawn O. Pearce --- git-gui.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 9682418e12..cf88a0d824 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -1125,11 +1125,17 @@ proc mapdesc {state path} { } proc ui_status {msg} { - $::main_status show $msg + global main_status + if {[info exists main_status]} { + $main_status show $msg + } } proc ui_ready {{test {}}} { - $::main_status show {Ready.} $test + global main_status + if {[info exists main_status]} { + $main_status show [mc "Ready."] $test + } } proc escape_path {path} { From 96f11953c0b0b9b0a0c75fb98bdf239dce5ff582 Mon Sep 17 00:00:00 2001 From: Steffen Prohaska Date: Sat, 6 Oct 2007 15:27:22 +0200 Subject: [PATCH 07/17] git-gui: accept versions containing text annotations, like 1.5.3.mingw.1 This commit teaches git-gui to accept versions with annotations that start with text and optionally end with a dot followed by a number. This is needed by the current versioning scheme of msysgit, which uses versions like 1.5.3.mingw.1. However, the changes is not limited to this use case. Any version of the form .. would be parsed and only the starting used for validation. [sp: Minor edit to remove unnecessary group matching] Signed-off-by: Steffen Prohaska Signed-off-by: Shawn O. Pearce --- git-gui.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/git-gui.sh b/git-gui.sh index cf88a0d824..9335a9761b 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -538,6 +538,7 @@ regsub -- {-dirty$} $_git_version {} _git_version regsub {\.[0-9]+\.g[0-9a-f]+$} $_git_version {} _git_version regsub {\.rc[0-9]+$} $_git_version {} _git_version regsub {\.GIT$} $_git_version {} _git_version +regsub {\.[a-zA-Z]+\.[0-9]+$} $_git_version {} _git_version if {![regexp {^[1-9]+(\.[0-9]+)+$} $_git_version]} { catch {wm withdraw .} From 5eb73581679abb41dcdf9ad7fa63fa156e078790 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 Oct 2007 05:04:00 -0400 Subject: [PATCH 08/17] send-pack: respect '+' on wildcard refspecs When matching source and destination refs, we were failing to pull the 'force' parameter from wildcard refspecs (but not explicit ones) and attach it to the ref struct. This adds a test for explicit and wildcard refspecs; the latter fails without this patch. Signed-off-by: Jeff King Signed-off-by: Shawn O. Pearce --- remote.c | 2 ++ t/t5400-send-pack.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/remote.c b/remote.c index bb774d0bcc..cdbbdcb00d 100644 --- a/remote.c +++ b/remote.c @@ -626,6 +626,8 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail, hashcpy(dst_peer->new_sha1, src->new_sha1); } dst_peer->peer_ref = src; + if (pat) + dst_peer->force = pat->force; free_name: free(dst_name); } diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh index 57c6397be1..2d0c07fd6a 100755 --- a/t/t5400-send-pack.sh +++ b/t/t5400-send-pack.sh @@ -123,4 +123,52 @@ test_expect_success \ git-branch -a >branches && ! grep -q origin/master branches ' +rewound_push_setup() { + rm -rf parent child && + mkdir parent && cd parent && + git-init && echo one >file && git-add file && git-commit -m one && + echo two >file && git-commit -a -m two && + cd .. && + git-clone parent child && cd child && git-reset --hard HEAD^ +} + +rewound_push_succeeded() { + cmp ../parent/.git/refs/heads/master .git/refs/heads/master +} + +rewound_push_failed() { + if rewound_push_succeeded + then + false + else + true + fi +} + +test_expect_success \ + 'pushing explicit refspecs respects forcing' ' + rewound_push_setup && + if git-send-pack ../parent/.git refs/heads/master:refs/heads/master + then + false + else + true + fi && rewound_push_failed && + git-send-pack ../parent/.git +refs/heads/master:refs/heads/master && + rewound_push_succeeded +' + +test_expect_success \ + 'pushing wildcard refspecs respects forcing' ' + rewound_push_setup && + if git-send-pack ../parent/.git refs/heads/*:refs/heads/* + then + false + else + true + fi && rewound_push_failed && + git-send-pack ../parent/.git +refs/heads/*:refs/heads/* && + rewound_push_succeeded +' + test_done From 1d5bf7fcee6122931bd04c97269d64f8f282ec55 Mon Sep 17 00:00:00 2001 From: Ralf Wildenhues Date: Fri, 19 Oct 2007 19:24:43 +0200 Subject: [PATCH 09/17] gitk.txt: Fix markup. For the manpage, avoid generating an em dash in code. Signed-off-by: Ralf Wildenhues Signed-off-by: Shawn O. Pearce --- Documentation/gitk.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt index e9f82b97b9..8dbfb0d5a3 100644 --- a/Documentation/gitk.txt +++ b/Documentation/gitk.txt @@ -69,7 +69,7 @@ gitk --since="2 weeks ago" \-- gitk:: The "--" is necessary to avoid confusion with the *branch* named 'gitk' -gitk --max-count=100 --all -- Makefile:: +gitk --max-count=100 --all \-- Makefile:: Show at most 100 changes made to the file 'Makefile'. Instead of only looking for changes in the current branch look in all branches. From 8af52d7a83a78c5f3a60d5bd5099a5073655339a Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 20 Oct 2007 01:42:01 -0400 Subject: [PATCH 10/17] git-gui: Correctly report failures from git-write-tree If git-write-tree fails (such as if the index file is currently locked and it wants to write to it) we were not getting the error message as $tree_id was always the empty string so we shortcut through the catch and never got the output from stderr. Signed-off-by: Shawn O. Pearce --- lib/commit.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/commit.tcl b/lib/commit.tcl index f857a2ff5b..57238129e4 100644 --- a/lib/commit.tcl +++ b/lib/commit.tcl @@ -253,7 +253,7 @@ proc commit_committree {fd_wt curHEAD msg} { global repo_config gets $fd_wt tree_id - if {$tree_id eq {} || [catch {close $fd_wt} err]} { + if {[catch {close $fd_wt} err]} { error_popup "write-tree failed:\n\n$err" ui_status {Commit failed.} unlock_index From bea6878be2fb73e50407f93f2bcd71b961510c19 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 20 Oct 2007 14:16:15 -0400 Subject: [PATCH 11/17] git-gui: Handle progress bars from newer gits Post Git 1.5.3 a new style progress bar has been introduced that uses only one line rather than two. The formatting of the completed and total section is also slightly different so we must adjust our regexp to match. Unfortunately both styles are in active use by different versions of Git so we need to look for both. Signed-off-by: Shawn O. Pearce --- lib/status_bar.tcl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/status_bar.tcl b/lib/status_bar.tcl index 72a8fe1fd3..3bf79eb6e0 100644 --- a/lib/status_bar.tcl +++ b/lib/status_bar.tcl @@ -69,7 +69,10 @@ method update_meter {buf} { set prior [string range $meter 0 $r] set meter [string range $meter [expr {$r + 1}] end] - if {[regexp "\\((\\d+)/(\\d+)\\)\\s+done\r\$" $prior _j a b]} { + set p "\\((\\d+)/(\\d+)\\)" + if {[regexp ":\\s*\\d+% $p\(?:, done.\\s*\n|\\s*\r)\$" $prior _j a b]} { + update $this $a $b + } elseif {[regexp "$p\\s+done\r\$" $prior _j a b]} { update $this $a $b } } From bbbadf6e58f72ac6bf739d2a1109cbd872eb1083 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 20 Oct 2007 20:42:01 -0400 Subject: [PATCH 12/17] git-gui: Don't display CR within console windows Git progress bars from tools like git-push and git-fetch use CR to skip back to the start of the current line and redraw it with an updated progress. We were doing this in our Tk widget but had failed to skip the CR, which Tk doesn't draw well. Signed-off-by: Shawn O. Pearce --- lib/console.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/console.tcl b/lib/console.tcl index 6f718fbac3..b038a78358 100644 --- a/lib/console.tcl +++ b/lib/console.tcl @@ -122,7 +122,7 @@ method _read {fd after} { } else { $w.m.t delete $console_cr end $w.m.t insert end "\n" - $w.m.t insert end [string range $buf $c $cr] + $w.m.t insert end [string range $buf $c [expr {$cr - 1}]] set c $cr incr c } From 2005dbe2a4e3eba5ecddb7d3c46339d2bc1241d4 Mon Sep 17 00:00:00 2001 From: Julian Phillips Date: Sat, 20 Oct 2007 17:15:38 +0100 Subject: [PATCH 13/17] fast-import: Fix argument order to die in file_change_m The arguments to the "Not a blob" die call in file_change_m were transposed, so that the command was printed as the type, and the type as the command. Switch them around so that the error message comes out correctly. Signed-off-by: Julian Phillips Signed-off-by: Shawn O. Pearce --- fast-import.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fast-import.c b/fast-import.c index 078079d404..c07e3d8ef0 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1859,7 +1859,7 @@ static void file_change_m(struct branch *b) } else if (oe) { if (oe->type != OBJ_BLOB) die("Not a blob (actually a %s): %s", - command_buf.buf, typename(oe->type)); + typename(oe->type), command_buf.buf); } else { enum object_type type = sha1_object_info(sha1, NULL); if (type < 0) From 0b8293f6776a7fc600c924ca71e4bba81d396409 Mon Sep 17 00:00:00 2001 From: Joakim Tjernlund Date: Sat, 20 Oct 2007 21:31:46 +0200 Subject: [PATCH 14/17] Improve receive-pack error message about funny ref creation receive-pack is only executed remotely so when reporting errors, say so. Signed-off-by: Joakim Tjernlund Signed-off-by: Shawn O. Pearce --- receive-pack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/receive-pack.c b/receive-pack.c index d3c422be58..1521d0b2de 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -166,7 +166,7 @@ static const char *update(struct command *cmd) struct ref_lock *lock; if (!prefixcmp(name, "refs/") && check_ref_format(name + 5)) { - error("refusing to create funny ref '%s' locally", name); + error("refusing to create funny ref '%s' remotely", name); return "funny refname"; } From 07134421fc5765d6c96c548e70e461c290143762 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 19 Oct 2007 10:59:22 -0700 Subject: [PATCH 15/17] Fix directory scanner to correctly ignore files without d_type On Fri, 19 Oct 2007, Todd T. Fries wrote: > If DT_UNKNOWN exists, then we have to do a stat() of some form to > find out the right type. That happened in the case of a pathname that was ignored, and we did not ask for "dir->show_ignored". That test used to be *together* with the "DTYPE(de) != DT_DIR", but splitting the two tests up means that we can do that (common) test before we even bother to calculate the real dtype. Of course, that optimization only matters for systems that don't have, or don't fill in DTYPE properly. I also clarified the real relationship between "exclude" and "dir->show_ignored". It used to do if (exclude != dir->show_ignored) { .. which wasn't exactly obvious, because it triggers for two different cases: - the path is marked excluded, but we are not interested in ignored files: ignore it - the path is *not* excluded, but we *are* interested in ignored files: ignore it unless it's a directory, in which case we might have ignored files inside the directory and need to recurse into it). so this splits them into those two cases, since the first case doesn't even care about the type. I also made a the DT_UNKNOWN case a separate helper function, and added some commentary to the cases. Linus Signed-off-by: Shawn O. Pearce --- dir.c | 52 ++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/dir.c b/dir.c index eb6c3abd30..f843c4dd20 100644 --- a/dir.c +++ b/dir.c @@ -443,6 +443,24 @@ static int in_pathspec(const char *path, int len, const struct path_simplify *si return 0; } +static int get_dtype(struct dirent *de, const char *path) +{ + int dtype = DTYPE(de); + struct stat st; + + if (dtype != DT_UNKNOWN) + return dtype; + if (lstat(path, &st)) + return dtype; + if (S_ISREG(st.st_mode)) + return DT_REG; + if (S_ISDIR(st.st_mode)) + return DT_DIR; + if (S_ISLNK(st.st_mode)) + return DT_LNK; + return dtype; +} + /* * Read a directory tree. We currently ignore anything but * directories, regular files and symlinks. That's because git @@ -466,7 +484,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co exclude_stk = push_exclude_per_directory(dir, base, baselen); while ((de = readdir(fdir)) != NULL) { - int len; + int len, dtype; int exclude; if ((de->d_name[0] == '.') && @@ -486,24 +504,30 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co if (exclude && dir->collect_ignored && in_pathspec(fullname, baselen + len, simplify)) dir_add_ignored(dir, fullname, baselen + len); - if (exclude != dir->show_ignored) { - if (!dir->show_ignored || DTYPE(de) != DT_DIR) { + + /* + * Excluded? If we don't explicitly want to show + * ignored files, ignore it + */ + if (exclude && !dir->show_ignored) + continue; + + dtype = get_dtype(de, fullname); + + /* + * Do we want to see just the ignored files? + * We still need to recurse into directories, + * even if we don't ignore them, since the + * directory may contain files that we do.. + */ + if (!exclude && dir->show_ignored) { + if (dtype != DT_DIR) continue; - } } - switch (DTYPE(de)) { - struct stat st; + switch (dtype) { default: continue; - case DT_UNKNOWN: - if (lstat(fullname, &st)) - continue; - if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) - break; - if (!S_ISDIR(st.st_mode)) - continue; - /* fallthrough */ case DT_DIR: memcpy(fullname + baselen + len, "/", 2); len++; From 6dd4b66fdecc2ffdc68758b6c4e059fcaaca512b Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sat, 20 Oct 2007 12:31:31 -0700 Subject: [PATCH 16/17] Fix diffcore-break total breakage Ok, so on the kernel list, some people noticed that "git log --follow" doesn't work too well with some files in the x86 merge, because a lot of files got renamed in very special ways. In particular, there was a pattern of doing single commits with renames that looked basically like - rename "filename.h" -> "filename_64.h" - create new "filename.c" that includes "filename_32.h" or "filename_64.h" depending on whether we're 32-bit or 64-bit. which was preparatory for smushing the two trees together. Now, there's two issues here: - "filename.c" *remained*. Yes, it was a rename, but there was a new file created with the old name in the same commit. This was important, because we wanted each commit to compile properly, so that it was bisectable, so splitting the rename into one commit and the "create helper file" into another was *not* an option. So we need to break associations where the contents change too much. Fine. We have the -B flag for that. When we break things up, then the rename detection will be able to figure out whether there are better alternatives. - "git log --follow" didn't with with -B. Now, the second case was really simple: we use a different "diffopt" structure for the rename detection than the basic one (which we use for showing the diffs). So that second case is trivially fixed by a trivial one-liner that just copies the break_opt values from the "real" diffopts to the one used for rename following. So now "git log -B --follow" works fine: diff --git a/tree-diff.c b/tree-diff.c index 26bdbdd..7c261fd 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -319,6 +319,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co diff_opts.detect_rename = DIFF_DETECT_RENAME; diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT; diff_opts.single_follow = opt->paths[0]; + diff_opts.break_opt = opt->break_opt; paths[0] = NULL; diff_tree_setup_paths(paths, &diff_opts); if (diff_setup_done(&diff_opts) < 0) however, the end result does *not* work. Because our diffcore-break.c logic is totally bogus! In particular: - it used to do if (base_size < MINIMUM_BREAK_SIZE) return 0; /* we do not break too small filepair */ which basically says "don't bother to break small files". But that "base_size" is the *smaller* of the two sizes, which means that if some large file was rewritten into one that just includes another file, we would look at the (small) result, and decide that it's smaller than the break size, so it cannot be worth it to break it up! Even if the other side was ten times bigger and looked *nothing* like the samell file! That's clearly bogus. I replaced "base_size" with "max_size", so that we compare the *bigger* of the filepair with the break size. - It calculated a "merge_score", which was the score needed to merge it back together if nothing else wanted it. But even if it was *so* different that we would never want to merge it back, we wouldn't consider it a break! That makes no sense. So I added if (*merge_score_p > break_score) return 1; to make it clear that if we wouldn't want to merge it at the end, it was *definitely* a break. - It compared the whole "extent of damage", counting all inserts and deletes, but it based this score on the "base_size", and generated the damage score with delta_size = src_removed + literal_added; damage_score = delta_size * MAX_SCORE / base_size; but that makes no sense either, since quite often, this will result in a number that is *bigger* than MAX_SCORE! Why? Because base_size is (again) the smaller of the two files we compare, and when you start out from a small file and add a lot (or start out from a large file and remove a lot), the base_size is going to be much smaller than the damage! Again, the fix was to replace "base_size" with "max_size", at which point the damage actually becomes a sane percentage of the whole. With these changes in place, not only does "git log -B --follow" work for the case that triggered this in the first place, ie now git log -B --follow arch/x86/kernel/vmlinux_64.lds.S actually gives reasonable results. But I also wanted to verify it in general, by doing a full-history git log --stat -B -C on my kernel tree with the old code and the new code. There's some tweaking to be done, but generally, the new code generates much better results wrt breaking up files (and then finding better rename candidates). Here's a few examples of the "--stat" output: - This: include/asm-x86/Kbuild | 2 - include/asm-x86/debugreg.h | 79 +++++++++++++++++++++++++++++++++++------ include/asm-x86/debugreg_32.h | 64 --------------------------------- include/asm-x86/debugreg_64.h | 65 --------------------------------- 4 files changed, 68 insertions(+), 142 deletions(-) Becomes: include/asm-x86/Kbuild | 2 - include/asm-x86/{debugreg_64.h => debugreg.h} | 9 +++- include/asm-x86/debugreg_32.h | 64 ------------------------- 3 files changed, 7 insertions(+), 68 deletions(-) - This: include/asm-x86/bug.h | 41 +++++++++++++++++++++++++++++++++++++++-- include/asm-x86/bug_32.h | 37 ------------------------------------- include/asm-x86/bug_64.h | 34 ---------------------------------- 3 files changed, 39 insertions(+), 73 deletions(-) Becomes include/asm-x86/{bug_64.h => bug.h} | 20 +++++++++++++----- include/asm-x86/bug_32.h | 37 ----------------------------------- 2 files changed, 14 insertions(+), 43 deletions(-) Now, in some other cases, it does actually turn a rename into a real "delete+create" pair, and then the diff is usually bigger, so truth in advertizing: it doesn't always generate a nicer diff. But for what -B was meant for, I think this is a big improvement, and I suspect those cases where it generates a bigger diff are tweakable. So I think this diff fixes a real bug, but we might still want to tweak the default values and perhaps the exact rules for when a break happens. Signed-off-by: Linus Torvalds Signed-off-by: Shawn O. Pearce --- diffcore-break.c | 11 +++++++---- tree-diff.c | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/diffcore-break.c b/diffcore-break.c index ae8a7d03e2..c71a22621a 100644 --- a/diffcore-break.c +++ b/diffcore-break.c @@ -45,8 +45,8 @@ static int should_break(struct diff_filespec *src, * The value we return is 1 if we want the pair to be broken, * or 0 if we do not. */ - unsigned long delta_size, base_size, src_copied, literal_added, - src_removed; + unsigned long delta_size, base_size, max_size; + unsigned long src_copied, literal_added, src_removed; *merge_score_p = 0; /* assume no deletion --- "do not break" * is the default. @@ -63,7 +63,8 @@ static int should_break(struct diff_filespec *src, return 0; /* error but caught downstream */ base_size = ((src->size < dst->size) ? src->size : dst->size); - if (base_size < MINIMUM_BREAK_SIZE) + max_size = ((src->size > dst->size) ? src->size : dst->size); + if (max_size < MINIMUM_BREAK_SIZE) return 0; /* we do not break too small filepair */ if (diffcore_count_changes(src, dst, @@ -89,12 +90,14 @@ static int should_break(struct diff_filespec *src, * less than the minimum, after rename/copy runs. */ *merge_score_p = (int)(src_removed * MAX_SCORE / src->size); + if (*merge_score_p > break_score) + return 1; /* Extent of damage, which counts both inserts and * deletes. */ delta_size = src_removed + literal_added; - if (delta_size * MAX_SCORE / base_size < break_score) + if (delta_size * MAX_SCORE / max_size < break_score) return 0; /* If you removed a lot without adding new material, that is diff --git a/tree-diff.c b/tree-diff.c index 26bdbdd2bf..7c261fd7c3 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -319,6 +319,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co diff_opts.detect_rename = DIFF_DETECT_RENAME; diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT; diff_opts.single_follow = opt->paths[0]; + diff_opts.break_opt = opt->break_opt; paths[0] = NULL; diff_tree_setup_paths(paths, &diff_opts); if (diff_setup_done(&diff_opts) < 0) From 2ee52eb17c26323b3f62538a44510aae75f09d6f Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 20 Oct 2007 23:40:06 -0400 Subject: [PATCH 17/17] Describe more 1.5.3.5 fixes in release notes Signed-off-by: Shawn O. Pearce --- Documentation/RelNotes-1.5.3.5.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Documentation/RelNotes-1.5.3.5.txt b/Documentation/RelNotes-1.5.3.5.txt index 49e52264ba..9581e03c49 100644 --- a/Documentation/RelNotes-1.5.3.5.txt +++ b/Documentation/RelNotes-1.5.3.5.txt @@ -4,6 +4,8 @@ GIT v1.5.3.5 Release Notes Fixes since v1.5.3.4 -------------------- + * Comes with git-gui 0.8.4. + * "git-config" silently ignored options after --list; now it will error out with a usage message. @@ -27,12 +29,20 @@ Fixes since v1.5.3.4 * "git-log --follow" did not work unless diff generation (e.g. -p) was also requested. + * "git-log --follow -B" did not work at all. Fixed. + + * "git-log -M -B" did not correctly handle cases of very large files + being renamed and replaced by very small files in the same commit. + * "git-log" printed extra newlines between commits when a diff was generated internally (e.g. -S or --follow) but not displayed. * "git-push" error message is more helpful when pushing to a repository with no matching refs and none specified. + * "git-push" now respects + (force push) on wildcard refspecs, + matching the behavior of git-fetch. + * "git-filter-branch" now updates the working directory when it has finished filtering the current branch. @@ -46,6 +56,11 @@ Fixes since v1.5.3.4 not impact correctness, only user perception. The bogus error is no longer printed. + * "git-ls-files --ignored" did not properly descend into non-ignored + directories that themselves contained ignored files if d_type + was not supported by the filesystem. This bug impacted systems + such as AFS. Fixed. + * Git segfaulted when reading an invalid .gitattributes file. Fixed. * post-receive-email example hook fixed was fixed for