From 9991030c0c97285087e5da86cfdbaf143a8a74cb Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 4 Jun 2008 22:48:42 -0700 Subject: git-blame: refactor code to emit "porcelain format" output Both the --porcelain and --incremental format shared the same output format but implemented with two identical codepaths. This merges them into one shared function. Signed-off-by: Junio C Hamano diff --git a/builtin-blame.c b/builtin-blame.c index 48cc0c1..163d2dc 100644 --- a/builtin-blame.c +++ b/builtin-blame.c @@ -1491,6 +1491,34 @@ static void write_filename_info(const char *path) } /* + * Porcelain/Incremental format wants to show a lot of details per + * commit. Instead of repeating this every line, emit it only once, + * the first time each commit appears in the output. + */ +static int emit_one_suspect_detail(struct origin *suspect) +{ + struct commit_info ci; + + if (suspect->commit->object.flags & METAINFO_SHOWN) + return 0; + + suspect->commit->object.flags |= METAINFO_SHOWN; + get_commit_info(suspect->commit, &ci, 1); + printf("author %s\n", ci.author); + printf("author-mail %s\n", ci.author_mail); + printf("author-time %lu\n", ci.author_time); + printf("author-tz %s\n", ci.author_tz); + printf("committer %s\n", ci.committer); + printf("committer-mail %s\n", ci.committer_mail); + printf("committer-time %lu\n", ci.committer_time); + printf("committer-tz %s\n", ci.committer_tz); + printf("summary %s\n", ci.summary); + if (suspect->commit->object.flags & UNINTERESTING) + printf("boundary\n"); + return 1; +} + +/* * The blame_entry is found to be guilty for the range. Mark it * as such, and show it in incremental output. */ @@ -1505,22 +1533,7 @@ static void found_guilty_entry(struct blame_entry *ent) printf("%s %d %d %d\n", sha1_to_hex(suspect->commit->object.sha1), ent->s_lno + 1, ent->lno + 1, ent->num_lines); - if (!(suspect->commit->object.flags & METAINFO_SHOWN)) { - struct commit_info ci; - suspect->commit->object.flags |= METAINFO_SHOWN; - get_commit_info(suspect->commit, &ci, 1); - printf("author %s\n", ci.author); - printf("author-mail %s\n", ci.author_mail); - printf("author-time %lu\n", ci.author_time); - printf("author-tz %s\n", ci.author_tz); - printf("committer %s\n", ci.committer); - printf("committer-mail %s\n", ci.committer_mail); - printf("committer-time %lu\n", ci.committer_time); - printf("committer-tz %s\n", ci.committer_tz); - printf("summary %s\n", ci.summary); - if (suspect->commit->object.flags & UNINTERESTING) - printf("boundary\n"); - } + emit_one_suspect_detail(suspect); write_filename_info(suspect->path); maybe_flush_or_die(stdout, "stdout"); } @@ -1627,24 +1640,8 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent) ent->s_lno + 1, ent->lno + 1, ent->num_lines); - if (!(suspect->commit->object.flags & METAINFO_SHOWN)) { - struct commit_info ci; - suspect->commit->object.flags |= METAINFO_SHOWN; - get_commit_info(suspect->commit, &ci, 1); - printf("author %s\n", ci.author); - printf("author-mail %s\n", ci.author_mail); - printf("author-time %lu\n", ci.author_time); - printf("author-tz %s\n", ci.author_tz); - printf("committer %s\n", ci.committer); - printf("committer-mail %s\n", ci.committer_mail); - printf("committer-time %lu\n", ci.committer_time); - printf("committer-tz %s\n", ci.committer_tz); - write_filename_info(suspect->path); - printf("summary %s\n", ci.summary); - if (suspect->commit->object.flags & UNINTERESTING) - printf("boundary\n"); - } - else if (suspect->commit->object.flags & MORE_THAN_ONE_PATH) + if (emit_one_suspect_detail(suspect) || + (suspect->commit->object.flags & MORE_THAN_ONE_PATH)) write_filename_info(suspect->path); cp = nth_line(sb, ent->lno); -- cgit v0.10.2-6-g49f6 From 96e117099c0e4f7d508eb071f60b6275038f6f37 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 4 Jun 2008 22:58:40 -0700 Subject: blame: show "previous" information in --porcelain/--incremental format When the final blame is laid for a line to a pair, it also gives a "previous" information to --porcelain and --incremental output format. It gives the parent commit of the blamed commit, _and_ a path in that parent commit that corresponds to the blamed path --- in short, it is the origin that would have been blamed (or passed blame through) for the line _if_ the blamed commit did not change that line. This unfortunately makes sanity checking of refcount quite complex, so I ripped it out for now. Signed-off-by: Junio C Hamano diff --git a/builtin-blame.c b/builtin-blame.c index 163d2dc..e386120 100644 --- a/builtin-blame.c +++ b/builtin-blame.c @@ -73,6 +73,7 @@ static unsigned blame_copy_score; */ struct origin { int refcnt; + struct origin *previous; struct commit *commit; mmfile_t file; unsigned char blob_sha1[20]; @@ -114,6 +115,8 @@ static inline struct origin *origin_incref(struct origin *o) static void origin_decref(struct origin *o) { if (o && --o->refcnt <= 0) { + if (o->previous) + origin_decref(o->previous); free(o->file.ptr); free(o); } @@ -1292,6 +1295,10 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt) struct origin *porigin = sg_origin[i]; if (!porigin) continue; + if (!origin->previous) { + origin_incref(porigin); + origin->previous = porigin; + } if (pass_blame_to_parent(sb, origin, porigin)) goto finish; } @@ -1515,6 +1522,11 @@ static int emit_one_suspect_detail(struct origin *suspect) printf("summary %s\n", ci.summary); if (suspect->commit->object.flags & UNINTERESTING) printf("boundary\n"); + if (suspect->previous) { + struct origin *prev = suspect->previous; + printf("previous %s ", sha1_to_hex(prev->commit->object.sha1)); + write_name_quoted(prev->path, stdout, '\n'); + } return 1; } @@ -1878,36 +1890,6 @@ static void sanity_check_refcnt(struct scoreboard *sb) baa = 1; } } - for (ent = sb->ent; ent; ent = ent->next) { - /* Mark the ones that haven't been checked */ - if (0 < ent->suspect->refcnt) - ent->suspect->refcnt = -ent->suspect->refcnt; - } - for (ent = sb->ent; ent; ent = ent->next) { - /* - * ... then pick each and see if they have the the - * correct refcnt. - */ - int found; - struct blame_entry *e; - struct origin *suspect = ent->suspect; - - if (0 < suspect->refcnt) - continue; - suspect->refcnt = -suspect->refcnt; /* Unmark */ - for (found = 0, e = sb->ent; e; e = e->next) { - if (e->suspect != suspect) - continue; - found++; - } - if (suspect->refcnt != found) { - fprintf(stderr, "%s in %s has refcnt %d, not %d\n", - ent->suspect->path, - sha1_to_hex(ent->suspect->commit->object.sha1), - ent->suspect->refcnt, found); - baa = 2; - } - } if (baa) { int opt = 0160; find_alignment(sb, &opt); -- cgit v0.10.2-6-g49f6 From 06569cd5bef7c8e3925bd36870abc083ae7b93f8 Mon Sep 17 00:00:00 2001 From: Alexander Gavrilov Date: Sat, 24 Jan 2009 00:18:13 +0300 Subject: git-gui: Fix post-commit status with subject in non-locale encoding As pointed out in msysgit bug #181, when a non-locale encoding is used for commits, post-commit status messages display the subject incorrectly. It happens because the file handle is not properly configured before the subject is read back. This patch fixes it by factoring out the code that is used to setup the output handle into a separate function, and calling it from the reading code. Signed-off-by: Alexander Gavrilov Acked-by: Robin Rosenberg Signed-off-by: Shawn O. Pearce diff --git a/lib/commit.tcl b/lib/commit.tcl index 9cc8410..17aba91 100644 --- a/lib/commit.tcl +++ b/lib/commit.tcl @@ -115,6 +115,23 @@ proc create_new_commit {} { rescan ui_ready } +proc setup_commit_encoding {msg_wt {quiet 0}} { + global repo_config + + if {[catch {set enc $repo_config(i18n.commitencoding)}]} { + set enc utf-8 + } + set use_enc [tcl_encoding $enc] + if {$use_enc ne {}} { + fconfigure $msg_wt -encoding $use_enc + } else { + if {!$quiet} { + error_popup [mc "warning: Tcl does not support encoding '%s'." $enc] + } + fconfigure $msg_wt -encoding utf-8 + } +} + proc commit_tree {} { global HEAD commit_type file_states ui_comm repo_config global pch_error @@ -200,16 +217,7 @@ A good commit message has the following format: set msg_p [gitdir GITGUI_EDITMSG] set msg_wt [open $msg_p w] fconfigure $msg_wt -translation lf - if {[catch {set enc $repo_config(i18n.commitencoding)}]} { - set enc utf-8 - } - set use_enc [tcl_encoding $enc] - if {$use_enc ne {}} { - fconfigure $msg_wt -encoding $use_enc - } else { - error_popup [mc "warning: Tcl does not support encoding '%s'." $enc] - fconfigure $msg_wt -encoding utf-8 - } + setup_commit_encoding $msg_wt puts $msg_wt $msg close $msg_wt @@ -362,6 +370,7 @@ A rescan will be automatically started now. append reflogm " ($commit_type)" } set msg_fd [open $msg_p r] + setup_commit_encoding $msg_fd 1 gets $msg_fd subject close $msg_fd append reflogm {: } $subject -- cgit v0.10.2-6-g49f6 From 4e1be63c3b05fd379b51b611b7e869ff6977d8ab Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:25:59 +0100 Subject: Add valgrind support in test scripts This patch adds the ability to use valgrind's memcheck tool to diagnose memory problems in Git while running the test scripts. It requires valgrind 3.4.0 or newer. It works by creating symlinks to a valgrind script, which have the same name as our Git binaries, and then putting that directory in front of the test script's PATH as well as set GIT_EXEC_PATH to that directory. Git scripts are symlinked from that directory directly. That way, Git binaries called by Git scripts are valgrinded, too. Valgrind can be used by specifying "GIT_TEST_OPTS=--valgrind" in the make invocation. Any invocation of git that finds any errors under valgrind will exit with failure code 126. Any valgrind output will go to the usual stderr channel for tests (i.e., /dev/null, unless -v has been specified). If you need to pass options to valgrind -- you might want to run another tool than memcheck, for example -- you can set the environment variable GIT_VALGRIND_OPTIONS. A few default suppressions are included, since libz seems to trigger quite a few false positives. We'll assume that libz works and that we can ignore any errors which are reported there. Note: it is safe to run the valgrind tests in parallel, as the links in t/valgrind/bin/ are created using proper locking. Initial patch and all the hard work by Jeff King. Signed-off-by: Johannes Schindelin Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/t/README b/t/README index f208cf1..7560db5 100644 --- a/t/README +++ b/t/README @@ -39,7 +39,8 @@ this: * passed all 3 test(s) You can pass --verbose (or -v), --debug (or -d), and --immediate -(or -i) command line argument to the test. +(or -i) command line argument to the test, or by setting GIT_TEST_OPTS +appropriately before running "make". --verbose:: This makes the test more verbose. Specifically, the @@ -58,6 +59,11 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate This causes additional long-running tests to be run (where available), for more exhaustive testing. +--valgrind:: + Execute all Git binaries with valgrind and exit with status + 126 on errors (just like regular tests, this will only stop + the test script when running under -i). Valgrind errors + go to stderr, so you might want to pass the -v option, too. Skipping Tests -------------- diff --git a/t/test-lib.sh b/t/test-lib.sh index 6f6244a..da12b0a 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -94,6 +94,8 @@ do --no-python) # noop now... shift ;; + --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind) + valgrind=t; shift ;; *) break ;; esac @@ -492,8 +494,60 @@ test_done () { # Test the binaries we have just built. The tests are kept in # t/ subdirectory and are run in 'trash directory' subdirectory. TEST_DIRECTORY=$(pwd) -PATH=$TEST_DIRECTORY/..:$PATH -GIT_EXEC_PATH=$(pwd)/.. +if test -z "$valgrind" +then + PATH=$TEST_DIRECTORY/..:$PATH + GIT_EXEC_PATH=$TEST_DIRECTORY/.. +else + make_symlink () { + test -h "$2" && + test "$1" = "$(readlink "$2")" || { + # be super paranoid + if mkdir "$2".lock + then + rm -f "$2" && + ln -s "$1" "$2" && + rm -r "$2".lock + else + while test -d "$2".lock + do + say "Waiting for lock on $2." + sleep 1 + done + fi + } + } + + make_valgrind_symlink () { + # handle only executables + test -x "$1" || return + + base=$(basename "$1") + symlink_target=$TEST_DIRECTORY/../$base + # do not override scripts + if test -x "$symlink_target" && + test ! -d "$symlink_target" && + test "#!" != "$(head -c 2 < "$symlink_target")" + then + symlink_target=../valgrind.sh + fi + # create the link, or replace it if it is out of date + make_symlink "$symlink_target" "$GIT_VALGRIND/bin/$base" || exit + } + + # override all git executables in TEST_DIRECTORY/.. + GIT_VALGRIND=$TEST_DIRECTORY/valgrind + mkdir -p "$GIT_VALGRIND"/bin + for file in $TEST_DIRECTORY/../git* $TEST_DIRECTORY/../test-* + do + make_valgrind_symlink $file + done + PATH=$GIT_VALGRIND/bin:$PATH + GIT_EXEC_PATH=$GIT_VALGRIND/bin + export GIT_VALGRIND + + make_symlink ../../../templates "$GIT_VALGRIND"/bin/templates || exit +fi GIT_TEMPLATE_DIR=$(pwd)/../templates/blt unset GIT_CONFIG GIT_CONFIG_NOSYSTEM=1 diff --git a/t/valgrind/.gitignore b/t/valgrind/.gitignore new file mode 100644 index 0000000..d4ae667 --- /dev/null +++ b/t/valgrind/.gitignore @@ -0,0 +1,2 @@ +/bin/ +/templates diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp new file mode 100644 index 0000000..2482b3b --- /dev/null +++ b/t/valgrind/default.supp @@ -0,0 +1,21 @@ +{ + ignore-zlib-errors-cond + Memcheck:Cond + obj:*libz.so* +} + +{ + ignore-zlib-errors-value4 + Memcheck:Value4 + obj:*libz.so* +} + +{ + writing-data-from-zlib-triggers-errors + Memcheck:Param + write(buf) + obj:/lib/ld-*.so + fun:write_in_full + fun:write_buffer + fun:write_loose_object +} diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh new file mode 100755 index 0000000..dc92612 --- /dev/null +++ b/t/valgrind/valgrind.sh @@ -0,0 +1,13 @@ +#!/bin/sh + +base=$(basename "$0") + +exec valgrind -q --error-exitcode=126 \ + --leak-check=no \ + --suppressions="$GIT_VALGRIND/default.supp" \ + --gen-suppressions=all \ + --track-origins=yes \ + --log-fd=4 \ + --input-fd=4 \ + $GIT_VALGRIND_OPTIONS \ + "$GIT_VALGRIND"/../../"$base" "$@" -- cgit v0.10.2-6-g49f6 From 6a7e37c99f733821bd3a17432fa6e4591b63866c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 4 Feb 2009 00:26:03 +0100 Subject: valgrind: ignore ldso and more libz errors On some Linux systems, we get a host of Cond and Addr errors from calls to dlopen that are caused by nss modules. We should be able to safely ignore anything happening in ld-*.so as "not our problem." [Johannes: I added some more... unfortunately using valgrind 3.4.0 syntax] Signed-off-by: Jeff King Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp index 2482b3b..5f341b8 100644 --- a/t/valgrind/default.supp +++ b/t/valgrind/default.supp @@ -5,17 +5,39 @@ } { + ignore-zlib-errors-value8 + Memcheck:Value8 + obj:*libz.so* +} + +{ ignore-zlib-errors-value4 Memcheck:Value4 obj:*libz.so* } { - writing-data-from-zlib-triggers-errors + ignore-ldso-cond + Memcheck:Cond + obj:*ld-*.so +} + +{ + ignore-ldso-addr8 + Memcheck:Addr8 + obj:*ld-*.so +} + +{ + ignore-ldso-addr4 + Memcheck:Addr4 + obj:*ld-*.so +} + +{ + writing-data-from-zlib-triggers-even-more-errors Memcheck:Param write(buf) - obj:/lib/ld-*.so - fun:write_in_full - fun:write_buffer + ... fun:write_loose_object } -- cgit v0.10.2-6-g49f6 From efd92ffd316d03360e1c8a348091e4f50f749d6f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:26:08 +0100 Subject: Valgrind support: check for more than just programming errors This patch makes --valgrind try to override _all_ Git binaries in the PATH, and it makes it an error to call *.sh and *.perl scripts directly. While it is not strictly necessary to look through the whole PATH to find git binaries to override, it is in line with running an expensive test (which valgrind is) to make extra sure that only binaries are tested that actually come from the git.git checkout. In the same spirit, we can test that neither our test suite nor our scripts try to run the *.sh or *.perl scripts directly. It's more like a "because we can" than a "this is tightly connected to valgrind", but in the author's opinion "because we can" is "so we should" in this case. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/t/test-lib.sh b/t/test-lib.sh index da12b0a..5a58356 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -531,6 +531,10 @@ else then symlink_target=../valgrind.sh fi + case "$base" in + *.sh|*.perl) + symlink_target=../unprocessed-script + esac # create the link, or replace it if it is out of date make_symlink "$symlink_target" "$GIT_VALGRIND/bin/$base" || exit } @@ -542,6 +546,17 @@ else do make_valgrind_symlink $file done + OLDIFS=$IFS + IFS=: + for path in $PATH + do + ls "$path"/git-* 2> /dev/null | + while read file + do + make_valgrind_symlink "$file" + done + done + IFS=$OLDIFS PATH=$GIT_VALGRIND/bin:$PATH GIT_EXEC_PATH=$GIT_VALGRIND/bin export GIT_VALGRIND -- cgit v0.10.2-6-g49f6 From 44138559e8b7c89768a2450220b831847059311c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:26:12 +0100 Subject: test-lib.sh: optionally output to test-results/$TEST.out, too When tests are run in parallel and a few tests fail, it does not help that the output of the terminal is totally confusing, as you rarely know which test which line came from. So introduce the option '--tee' which triggers that the output of the tests will be written to t/test-results/$TEST.out in addition to the terminal, where $TEST is the basename of the script. Unfortunately, there seems to be no way to redirect a given file descriptor to a specified subprocess in POSIX shell, only redirection to a file is supported via 'exec > $FILE'. At least with bash, one might think that 'exec >($COMMAND)' would work as intended, but it does not. The common way to work around the lack of proper tools support is to work with named pipes, alas, one of our most beloved platforms does not really support named pipes. Besides, we would need a pipe for every script, as the whole point of this patch is to allow parallel execution. Therefore, we handle the redirection in the following way: when '--tee' was passed to the test script, the variable GIT_TEST_TEE_STARTED is set (to avoid triggering that code path again) and the script is started _again_, in a subshell, redirected to the command "tee". Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/t/README b/t/README index 7560db5..ed1ebb6 100644 --- a/t/README +++ b/t/README @@ -65,6 +65,12 @@ appropriately before running "make". the test script when running under -i). Valgrind errors go to stderr, so you might want to pass the -v option, too. +--tee:: + In addition to printing the test output to the terminal, + write it to files named 't/test-results/$TEST_NAME.out'. + As the names depend on the tests' file names, it is safe to + run the tests with this option in parallel. + Skipping Tests -------------- diff --git a/t/test-lib.sh b/t/test-lib.sh index 5a58356..34f372c 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -3,6 +3,22 @@ # Copyright (c) 2005 Junio C Hamano # +# if --tee was passed, write the output not only to the terminal, but +# additionally to the file test-results/$BASENAME.out, too. +case "$GIT_TEST_TEE_STARTED, $* " in +done,*) + # do not redirect again + ;; +*' --tee '*) + mkdir -p test-results + BASE=test-results/$(basename "$0" .sh) + (GIT_TEST_TEE_STARTED=done ${SHELL-sh} "$0" "$@" 2>&1; + echo $? > $BASE.exit) | tee $BASE.out + test "$(cat $BASE.exit)" = 0 + exit + ;; +esac + # Keep the original TERM for say_color ORIGINAL_TERM=$TERM @@ -96,6 +112,8 @@ do shift ;; --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind) valgrind=t; shift ;; + --tee) + shift ;; # was handled already *) break ;; esac -- cgit v0.10.2-6-g49f6 From 7f6fdea1198cd7599c25cf9435df8540e9378a15 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:26:18 +0100 Subject: t/Makefile: provide a 'valgrind' target It is easy to forget running valgrinded tests without -v, and it is also easy to forget to redirect the output to "tee" (lest the output scroll out of the terminal's buffer). Running "make valgrind" will take care of all that. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/t/Makefile b/t/Makefile index ed49c20..e544493 100644 --- a/t/Makefile +++ b/t/Makefile @@ -38,4 +38,7 @@ full-svn-test: $(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=1 LC_ALL=C $(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=0 LC_ALL=en_US.UTF-8 -.PHONY: pre-clean $(T) aggregate-results clean +valgrind: + GIT_TEST_OPTS='--valgrind -v --tee' $(MAKE) -k + +.PHONY: pre-clean $(T) aggregate-results clean valgrind -- cgit v0.10.2-6-g49f6 From 268fac6919ef673094e50e2d944d09f017f5ad33 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:26:22 +0100 Subject: Add a script to coalesce the valgrind outputs After running the valgrind tests with GIT_TEST_TREE=t, the test output is in the test-results/$TEST.out files. Call ./valgrind/analyze.sh in $GIT_ROOT/t/ to group the valgrind errors by backtrace. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/t/valgrind/analyze.sh b/t/valgrind/analyze.sh new file mode 100755 index 0000000..d8105d9 --- /dev/null +++ b/t/valgrind/analyze.sh @@ -0,0 +1,123 @@ +#!/bin/sh + +out_prefix=$(dirname "$0")/../test-results/valgrind.out +output= +count=0 +total_count=0 +missing_message= +new_line=' +' + +# start outputting the current valgrind error in $out_prefix.++$count, +# and the test case which failed in the corresponding .message file +start_output () { + test -z "$output" || return + + # progress + total_count=$(($total_count+1)) + test -t 2 && printf "\rFound %d errors" $total_count >&2 + + count=$(($count+1)) + output=$out_prefix.$count + : > $output + + echo "*** $1 ***" > $output.message +} + +finish_output () { + test ! -z "$output" || return + output= + + # if a test case has more than one valgrind error, we need to + # copy the last .message file to the previous errors + test -z "$missing_message" || { + while test $missing_message -lt $count + do + cp $out_prefix.$count.message \ + $out_prefix.$missing_message.message + missing_message=$(($missing_message+1)) + done + missing_message= + } +} + +# group the valgrind errors by backtrace +output_all () { + last_line= + j=0 + i=1 + while test $i -le $count + do + # output + echo "$i $(tr '\n' ' ' < $out_prefix.$i)" + i=$(($i+1)) + done | + sort -t ' ' -k 2 | # order by + while read number line + do + # find duplicates, do not output backtrace twice + if test "$line" != "$last_line" + then + last_line=$line + j=$(($j+1)) + printf "\nValgrind error $j:\n\n" + cat $out_prefix.$number + printf "\nfound in:\n" + fi + # print the test case where this came from + printf "\n" + cat $out_prefix.$number.message + done +} + +handle_one () { + OLDIFS=$IFS + IFS="$new_line" + while read line + do + case "$line" in + # backtrace, possibly a new one + ==[0-9]*) + + # Does the current valgrind error have a message yet? + case "$output" in + *.message) + test -z "$missing_message" && + missing_message=$count + output= + esac + + start_output $(basename $1) + echo "$line" | + sed 's/==[0-9]*==/==valgrind==/' >> $output + ;; + # end of backtrace + '}') + test -z "$output" || { + echo "$line" >> $output + test $output = ${output%.message} && + output=$output.message + } + ;; + # end of test case + '') + finish_output + ;; + # normal line; if $output is set, print the line + *) + test -z "$output" || echo "$line" >> $output + ;; + esac + done < $1 + IFS=$OLDIFS + + # just to be safe + finish_output +} + +for test_script in "$(dirname "$0")"/../test-results/*.out +do + handle_one $test_script +done + +output_all -- cgit v0.10.2-6-g49f6 From 3da936523445eb7a74dfe3ab1fe0ce82fac56174 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:26:26 +0100 Subject: Tests: let --valgrind imply --verbose and --tee It does not make much sense to run the (expensive) valgrind tests and not look at the output. To prevent output from scrolling out of reach, the parameter --tee is implied, too. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/t/Makefile b/t/Makefile index e544493..0962341 100644 --- a/t/Makefile +++ b/t/Makefile @@ -39,6 +39,6 @@ full-svn-test: $(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=0 LC_ALL=en_US.UTF-8 valgrind: - GIT_TEST_OPTS='--valgrind -v --tee' $(MAKE) -k + GIT_TEST_OPTS=--valgrind $(MAKE) .PHONY: pre-clean $(T) aggregate-results clean valgrind diff --git a/t/README b/t/README index ed1ebb6..d8f6c7d 100644 --- a/t/README +++ b/t/README @@ -65,6 +65,10 @@ appropriately before running "make". the test script when running under -i). Valgrind errors go to stderr, so you might want to pass the -v option, too. + Since it makes no sense to run the tests with --valgrind and + not see any output, this option implies --verbose. For + convenience, it also implies --tee. + --tee:: In addition to printing the test output to the terminal, write it to files named 't/test-results/$TEST_NAME.out'. diff --git a/t/test-lib.sh b/t/test-lib.sh index 34f372c..bc87936 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -9,7 +9,7 @@ case "$GIT_TEST_TEE_STARTED, $* " in done,*) # do not redirect again ;; -*' --tee '*) +*' --tee '*|*' --va'*) mkdir -p test-results BASE=test-results/$(basename "$0" .sh) (GIT_TEST_TEE_STARTED=done ${SHELL-sh} "$0" "$@" 2>&1; @@ -111,7 +111,7 @@ do # noop now... shift ;; --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind) - valgrind=t; shift ;; + valgrind=t; verbose=t; shift ;; --tee) shift ;; # was handled already *) -- cgit v0.10.2-6-g49f6 From a6d63b749369f3ba9c6d2f382efd27838604b7db Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Feb 2009 00:26:31 +0100 Subject: test-lib: avoid assuming that templates/ are in the GIT_EXEC_PATH Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/t/test-lib.sh b/t/test-lib.sh index bc87936..2021446 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -454,7 +454,7 @@ test_create_repo () { repo="$1" mkdir -p "$repo" cd "$repo" || error "Cannot setup test environment" - "$GIT_EXEC_PATH/git" init "--template=$GIT_EXEC_PATH/templates/blt/" >&3 2>&4 || + "$GIT_EXEC_PATH/git" init "--template=$owd/../templates/blt/" >&3 2>&4 || error "cannot run git init -- have you built things yet?" mv .git/hooks .git/hooks-disabled cd "$owd" @@ -578,8 +578,6 @@ else PATH=$GIT_VALGRIND/bin:$PATH GIT_EXEC_PATH=$GIT_VALGRIND/bin export GIT_VALGRIND - - make_symlink ../../../templates "$GIT_VALGRIND"/bin/templates || exit fi GIT_TEMPLATE_DIR=$(pwd)/../templates/blt unset GIT_CONFIG -- cgit v0.10.2-6-g49f6 From 5caa81d1b4f1b0b9ed73605ab1e4d91ba31a56b4 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 5 Feb 2009 22:03:00 +0100 Subject: valgrind: do not require valgrind 3.4.0 or newer Valgrind 3.4.0 is pretty new, and even if --track-origins is a nice feature, it is not the end of the world if that is not available. So play nice and use that option only when only an older version of valgrind is available. In the same spirit, refrain from the use of '...' in suppression files, which is also a feature only valgrind 3.4 and newer understand. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp index 5f341b8..9e013fa 100644 --- a/t/valgrind/default.supp +++ b/t/valgrind/default.supp @@ -38,6 +38,8 @@ writing-data-from-zlib-triggers-even-more-errors Memcheck:Param write(buf) - ... + obj:/lib/ld-*.so + fun:write_in_full + fun:write_buffer fun:write_loose_object } diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh index dc92612..582b4dc 100755 --- a/t/valgrind/valgrind.sh +++ b/t/valgrind/valgrind.sh @@ -2,11 +2,20 @@ base=$(basename "$0") +TRACK_ORIGINS= + +VALGRIND_VERSION=$(valgrind --version) +VALGRIND_MAJOR=$(expr "$VALGRIND_VERSION" : '[^0-9]*\([0-9]*\)') +VALGRIND_MINOR=$(expr "$VALGRIND_VERSION" : '[^0-9]*[0-9]*\.\([0-9]*\)') +test 3 -gt "$VALGRIND_MAJOR" || +test 3 -eq "$VALGRIND_MAJOR" -a 4 -gt "$VALGRIND_MINOR" || +TRACK_ORIGINS=--track-origins=yes + exec valgrind -q --error-exitcode=126 \ --leak-check=no \ --suppressions="$GIT_VALGRIND/default.supp" \ --gen-suppressions=all \ - --track-origins=yes \ + $TRACK_ORIGINS \ --log-fd=4 \ --input-fd=4 \ $GIT_VALGRIND_OPTIONS \ -- cgit v0.10.2-6-g49f6 From 584fa9ccf4b467e7633c100333cadf92e0c07894 Mon Sep 17 00:00:00 2001 From: Alexander Gavrilov Date: Sat, 7 Feb 2009 19:24:01 +0300 Subject: git-gui: Avoid an infinite rescan loop in handle_empty_diff. If the index update machinery and git diff happen to disagree on whether a particular file is modified, it may cause git-gui to enter an infinite index rescan loop, where an empty diff starts a rescan, which finds the same set of files modified, and tries to display the diff for the first one, which happens to be the empty one. A current example of a possible disagreement point is the autocrlf filter. This patch breaks the loop by using a global counter to track the auto-rescans. The variable is reset whenever a non-empty diff is displayed. Another suggested approach, which is based on giving the --exit-code argument to git diff, cannot be used, because diff-files seems to trust the timestamps in the index, and returns a non-zero code even if the file is actually unchanged, which essentially defeats the purpose of the auto-rescan logic. Signed-off-by: Alexander Gavrilov Signed-off-by: Shawn O. Pearce diff --git a/lib/diff.tcl b/lib/diff.tcl index bbbf15c..925b3f5 100644 --- a/lib/diff.tcl +++ b/lib/diff.tcl @@ -51,11 +51,16 @@ proc force_diff_encoding {enc} { proc handle_empty_diff {} { global current_diff_path file_states file_lists + global diff_empty_count set path $current_diff_path set s $file_states($path) if {[lindex $s 0] ne {_M}} return + # Prevent infinite rescan loops + incr diff_empty_count + if {$diff_empty_count > 1} return + info_popup [mc "No differences detected. %s has no changes. @@ -310,6 +315,7 @@ proc read_diff {fd cont_info} { global ui_diff diff_active global is_3way_diff is_conflict_diff current_diff_header global current_diff_queue + global diff_empty_count $ui_diff conf -state normal while {[gets $fd line] >= 0} { @@ -415,7 +421,10 @@ proc read_diff {fd cont_info} { if {[$ui_diff index end] eq {2.0}} { handle_empty_diff + } else { + set diff_empty_count 0 } + set callback [lindex $cont_info 1] if {$callback ne {}} { eval $callback -- cgit v0.10.2-6-g49f6 From 764369c5ea730894c75f32c2f94a651dd517fc5b Mon Sep 17 00:00:00 2001 From: Alexander Gavrilov Date: Sat, 7 Feb 2009 19:43:57 +0300 Subject: git-gui: Support more git version notations. Recently the msysgit repository has got a '1.6.1-msysgit1' tag, which, when used to build the git version, is not handled gracefully by the git-gui version code. This patch changes the regular expressions to fix it, and removes the hardcoded 'rc' string. Now git-gui can accept a version tail like '.foo123.GIT.bar.456.7.g89ab' Signed-off-by: Alexander Gavrilov Signed-off-by: Shawn O. Pearce diff --git a/git-gui.sh b/git-gui.sh index e018e07..2f1f305 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -769,9 +769,9 @@ if {![regsub {^git version } $_git_version {} _git_version]} { set _real_git_version $_git_version 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 {\.[a-zA-Z]+\.?[0-9]+$} $_git_version {} _git_version regsub {\.GIT$} $_git_version {} _git_version -regsub {\.[a-zA-Z]+\.[0-9]+$} $_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 .} -- cgit v0.10.2-6-g49f6 From 3bec8ff99a7792cae67aaeb5892d832478d7f548 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Sat, 7 Feb 2009 23:53:00 +0200 Subject: config: Add new option to open an editor. The idea was originated by discussion about usability of manually editing the config file in 'special needs' systems such as Windows. Now the user can forget a bit about where the config files actually are. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 19a8917..7d14007 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -22,6 +22,7 @@ SYNOPSIS 'git config' [] [-z|--null] -l | --list 'git config' [] --get-color name [default] 'git config' [] --get-colorbool name [stdout-is-tty] +'git config' [] -e | --edit DESCRIPTION ----------- @@ -157,6 +158,11 @@ See also <>. output. The optional `default` parameter is used instead, if there is no color configured for `name`. +-e:: +--edit:: + Opens an editor to modify the specified config file; either + '--system', '--global', or repository (default). + [[FILES]] FILES ----- diff --git a/builtin-config.c b/builtin-config.c index f710162..6937eaf 100644 --- a/builtin-config.c +++ b/builtin-config.c @@ -3,7 +3,7 @@ #include "color.h" static const char git_config_set_usage[] = -"git config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int | --bool-or-int ] [ -z | --null ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list | --get-color var [default] | --get-colorbool name [stdout-is-tty]"; +"git config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int | --bool-or-int ] [ -z | --null ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list | --get-color var [default] | --get-colorbool name [stdout-is-tty] | --edit | -e ]"; static char *key; static regex_t *key_regexp; @@ -362,6 +362,17 @@ int cmd_config(int argc, const char **argv, const char *prefix) return get_color(argc-2, argv+2); } else if (!strcmp(argv[1], "--get-colorbool")) { return get_colorbool(argc-2, argv+2); + } else if (!strcmp(argv[1], "--edit") || !strcmp(argv[1], "-e")) { + const char *config_filename; + if (argc != 2) + usage(git_config_set_usage); + if (config_exclusive_filename) + config_filename = config_exclusive_filename; + else + config_filename = git_path("config"); + git_config(git_default_config, NULL); + launch_editor(config_filename, NULL, NULL); + return 0; } else break; argc--; -- cgit v0.10.2-6-g49f6 From 60b458b7d31ff2497ed90cbe9f65444d84882cec Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Mon, 9 Feb 2009 21:54:04 +0100 Subject: lstat_cache(): small cleanup and optimisation Simplify the if-else test in longest_match_lstat_cache() such that we only have one simple if test. Instead of testing for 'i == cache.len' or 'i == len', we transform this to a common test for 'i == max_len'. And to further optimise we use 'i >= max_len' instead of 'i == max_len', the reason is that it is now the exact opposite of one part inside the while-loop termination expression 'i < max_len && name[i] == cache.path[i]', and then the compiler can probably reuse a test instruction from it. We also throw away the arguments to reset_lstat_cache(), such that all the safeguard logic inside lstat_cache() is handled at one place. Signed-off-by: Kjetil Barvik Signed-off-by: Junio C Hamano diff --git a/symlinks.c b/symlinks.c index f262b7c..ae57e56 100644 --- a/symlinks.c +++ b/symlinks.c @@ -25,27 +25,30 @@ static inline int longest_match_lstat_cache(int len, const char *name, } i++; } - /* Is the cached path string a substring of 'name'? */ - if (i == cache.len && cache.len < len && name[cache.len] == '/') { - match_len_prev = match_len; - match_len = cache.len; - /* Is 'name' a substring of the cached path string? */ - } else if ((i == len && len < cache.len && cache.path[len] == '/') || - (i == len && len == cache.len)) { + /* + * Is the cached path string a substring of 'name', is 'name' + * a substring of the cached path string, or is 'name' and the + * cached path string the exact same string? + */ + if (i >= max_len && ((len > cache.len && name[cache.len] == '/') || + (len < cache.len && cache.path[len] == '/') || + (len == cache.len))) { match_len_prev = match_len; - match_len = len; + match_len = i; } *previous_slash = match_len_prev; return match_len; } -static inline void reset_lstat_cache(int track_flags, int prefix_len_stat_func) +static inline void reset_lstat_cache(void) { cache.path[0] = '\0'; cache.len = 0; cache.flags = 0; - cache.track_flags = track_flags; - cache.prefix_len_stat_func = prefix_len_stat_func; + /* + * The track_flags and prefix_len_stat_func members is only + * set by the safeguard rule inside lstat_cache() + */ } #define FL_DIR (1 << 0) @@ -77,11 +80,13 @@ static int lstat_cache(int len, const char *name, if (cache.track_flags != track_flags || cache.prefix_len_stat_func != prefix_len_stat_func) { /* - * As a safeguard we clear the cache if the values of - * track_flags and/or prefix_len_stat_func does not - * match with the last supplied values. + * As a safeguard rule we clear the cache if the + * values of track_flags and/or prefix_len_stat_func + * does not match with the last supplied values. */ - reset_lstat_cache(track_flags, prefix_len_stat_func); + reset_lstat_cache(); + cache.track_flags = track_flags; + cache.prefix_len_stat_func = prefix_len_stat_func; match_len = last_slash = 0; } else { /* @@ -153,7 +158,7 @@ static int lstat_cache(int len, const char *name, cache.path[last_slash] = '\0'; cache.len = last_slash; cache.flags = save_flags; - } else if (track_flags & FL_DIR && + } else if ((track_flags & FL_DIR) && last_slash_dir > 0 && last_slash_dir <= PATH_MAX) { /* * We have a separate test for the directory case, @@ -170,7 +175,7 @@ static int lstat_cache(int len, const char *name, cache.len = last_slash_dir; cache.flags = FL_DIR; } else { - reset_lstat_cache(track_flags, prefix_len_stat_func); + reset_lstat_cache(); } return ret_flags; } @@ -190,8 +195,7 @@ void invalidate_lstat_cache(int len, const char *name) cache.len = previous_slash; cache.flags = FL_DIR; } else - reset_lstat_cache(cache.track_flags, - cache.prefix_len_stat_func); + reset_lstat_cache(); } } @@ -200,7 +204,7 @@ void invalidate_lstat_cache(int len, const char *name) */ void clear_lstat_cache(void) { - reset_lstat_cache(0, 0); + reset_lstat_cache(); } #define USE_ONLY_LSTAT 0 -- cgit v0.10.2-6-g49f6 From 148bc06b9101042342a89454a003998fc0e1ded3 Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Mon, 9 Feb 2009 21:54:05 +0100 Subject: lstat_cache(): generalise longest_match_lstat_cache() Rename the function to longst_path_match() and generalise it such that it can also be used by other functions. Signed-off-by: Kjetil Barvik Signed-off-by: Junio C Hamano diff --git a/symlinks.c b/symlinks.c index ae57e56..4596aee 100644 --- a/symlinks.c +++ b/symlinks.c @@ -1,38 +1,30 @@ #include "cache.h" -static struct cache_def { - char path[PATH_MAX + 1]; - int len; - int flags; - int track_flags; - int prefix_len_stat_func; -} cache; - /* * Returns the length (on a path component basis) of the longest - * common prefix match of 'name' and the cached path string. + * common prefix match of 'name_a' and 'name_b'. */ -static inline int longest_match_lstat_cache(int len, const char *name, - int *previous_slash) +static int longest_path_match(const char *name_a, int len_a, + const char *name_b, int len_b, + int *previous_slash) { int max_len, match_len = 0, match_len_prev = 0, i = 0; - max_len = len < cache.len ? len : cache.len; - while (i < max_len && name[i] == cache.path[i]) { - if (name[i] == '/') { + max_len = len_a < len_b ? len_a : len_b; + while (i < max_len && name_a[i] == name_b[i]) { + if (name_a[i] == '/') { match_len_prev = match_len; match_len = i; } i++; } /* - * Is the cached path string a substring of 'name', is 'name' - * a substring of the cached path string, or is 'name' and the - * cached path string the exact same string? + * Is 'name_b' a substring of 'name_a', the other way around, + * or is 'name_a' and 'name_b' the exact same string? */ - if (i >= max_len && ((len > cache.len && name[cache.len] == '/') || - (len < cache.len && cache.path[len] == '/') || - (len == cache.len))) { + if (i >= max_len && ((len_a > len_b && name_a[len_b] == '/') || + (len_a < len_b && name_b[len_a] == '/') || + (len_a == len_b))) { match_len_prev = match_len; match_len = i; } @@ -40,6 +32,14 @@ static inline int longest_match_lstat_cache(int len, const char *name, return match_len; } +static struct cache_def { + char path[PATH_MAX + 1]; + int len; + int flags; + int track_flags; + int prefix_len_stat_func; +} cache; + static inline void reset_lstat_cache(void) { cache.path[0] = '\0'; @@ -94,7 +94,8 @@ static int lstat_cache(int len, const char *name, * the 2 "excluding" path types. */ match_len = last_slash = - longest_match_lstat_cache(len, name, &previous_slash); + longest_path_match(name, len, cache.path, cache.len, + &previous_slash); match_flags = cache.flags & track_flags & (FL_NOENT|FL_SYMLINK); if (match_flags && match_len == cache.len) return match_flags; @@ -188,7 +189,8 @@ void invalidate_lstat_cache(int len, const char *name) { int match_len, previous_slash; - match_len = longest_match_lstat_cache(len, name, &previous_slash); + match_len = longest_path_match(name, len, cache.path, cache.len, + &previous_slash); if (len == match_len) { if ((cache.track_flags & FL_DIR) && previous_slash > 0) { cache.path[previous_slash] = '\0'; -- cgit v0.10.2-6-g49f6 From 571998921d8fd4ee674545406aabb86987921252 Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Mon, 9 Feb 2009 21:54:06 +0100 Subject: lstat_cache(): swap func(length, string) into func(string, length) Swap function argument pair (length, string) into (string, length) to conform with the commonly used order inside the GIT source code. Also, add a note about this fact into the coding guidelines. Signed-off-by: Kjetil Barvik Signed-off-by: Junio C Hamano diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 0d7fa9c..b8bf618 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -129,3 +129,6 @@ For C programs: used in the git core command set (unless your command is clearly separate from it, such as an importer to convert random-scm-X repositories to git). + + - When we pass pair to functions, we should try to + pass them in that order. diff --git a/builtin-add.c b/builtin-add.c index ac98c83..a23ad96 100644 --- a/builtin-add.c +++ b/builtin-add.c @@ -148,7 +148,7 @@ static const char **validate_pathspec(int argc, const char **argv, const char *p if (pathspec) { const char **p; for (p = pathspec; *p; p++) { - if (has_symlink_leading_path(strlen(*p), *p)) { + if (has_symlink_leading_path(*p, strlen(*p))) { int len = prefix ? strlen(prefix) : 0; die("'%s' is beyond a symbolic link", *p + len); } diff --git a/builtin-apply.c b/builtin-apply.c index f312798..106be94 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -2360,7 +2360,7 @@ static int check_to_create_blob(const char *new_name, int ok_if_exists) * In such a case, path "new_name" does not exist as * far as git is concerned. */ - if (has_symlink_leading_path(strlen(new_name), new_name)) + if (has_symlink_leading_path(new_name, strlen(new_name))) return 0; return error("%s: already exists in working directory", new_name); diff --git a/builtin-update-index.c b/builtin-update-index.c index 5604977..6c55527 100644 --- a/builtin-update-index.c +++ b/builtin-update-index.c @@ -195,7 +195,7 @@ static int process_path(const char *path) struct stat st; len = strlen(path); - if (has_symlink_leading_path(len, path)) + if (has_symlink_leading_path(path, len)) return error("'%s' is beyond a symbolic link", path); /* diff --git a/cache.h b/cache.h index 2d889de..80eeeb7 100644 --- a/cache.h +++ b/cache.h @@ -724,10 +724,10 @@ struct checkout { }; extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath); -extern int has_symlink_leading_path(int len, const char *name); -extern int has_symlink_or_noent_leading_path(int len, const char *name); -extern int has_dirs_only_path(int len, const char *name, int prefix_len); -extern void invalidate_lstat_cache(int len, const char *name); +extern int has_symlink_leading_path(const char *name, int len); +extern int has_symlink_or_noent_leading_path(const char *name, int len); +extern int has_dirs_only_path(const char *name, int len, int prefix_len); +extern void invalidate_lstat_cache(const char *name, int len); extern void clear_lstat_cache(void); extern struct alternate_object_database { diff --git a/diff-lib.c b/diff-lib.c index a41e1ec..a3ba20e 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -31,7 +31,7 @@ static int check_removed(const struct cache_entry *ce, struct stat *st) return -1; return 1; } - if (has_symlink_leading_path(ce_namelen(ce), ce->name)) + if (has_symlink_leading_path(ce->name, ce_namelen(ce))) return 1; if (S_ISDIR(st->st_mode)) { unsigned char sub[20]; diff --git a/dir.c b/dir.c index cfd1ea5..8fb5226 100644 --- a/dir.c +++ b/dir.c @@ -720,7 +720,7 @@ int read_directory(struct dir_struct *dir, const char *path, const char *base, i { struct path_simplify *simplify; - if (has_symlink_leading_path(strlen(path), path)) + if (has_symlink_leading_path(path, strlen(path))) return dir->nr; simplify = create_simplify(pathspec); diff --git a/entry.c b/entry.c index 05aa58d..bb6bdb9 100644 --- a/entry.c +++ b/entry.c @@ -20,7 +20,7 @@ static void create_directories(const char *path, const struct checkout *state) * we test the path components of the prefix with the * stat() function instead of the lstat() function. */ - if (has_dirs_only_path(len, buf, state->base_dir_len)) + if (has_dirs_only_path(buf, len, state->base_dir_len)) continue; /* ok, it is already a directory. */ /* diff --git a/symlinks.c b/symlinks.c index 4596aee..5167286 100644 --- a/symlinks.c +++ b/symlinks.c @@ -70,7 +70,7 @@ static inline void reset_lstat_cache(void) * of the prefix, where the cache should use the stat() function * instead of the lstat() function to test each path component. */ -static int lstat_cache(int len, const char *name, +static int lstat_cache(const char *name, int len, int track_flags, int prefix_len_stat_func) { int match_len, last_slash, last_slash_dir, previous_slash; @@ -185,7 +185,7 @@ static int lstat_cache(int len, const char *name, * Invalidate the given 'name' from the cache, if 'name' matches * completely with the cache. */ -void invalidate_lstat_cache(int len, const char *name) +void invalidate_lstat_cache(const char *name, int len) { int match_len, previous_slash; @@ -214,9 +214,9 @@ void clear_lstat_cache(void) /* * Return non-zero if path 'name' has a leading symlink component */ -int has_symlink_leading_path(int len, const char *name) +int has_symlink_leading_path(const char *name, int len) { - return lstat_cache(len, name, + return lstat_cache(name, len, FL_SYMLINK|FL_DIR, USE_ONLY_LSTAT) & FL_SYMLINK; } @@ -225,9 +225,9 @@ int has_symlink_leading_path(int len, const char *name) * Return non-zero if path 'name' has a leading symlink component or * if some leading path component does not exists. */ -int has_symlink_or_noent_leading_path(int len, const char *name) +int has_symlink_or_noent_leading_path(const char *name, int len) { - return lstat_cache(len, name, + return lstat_cache(name, len, FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT) & (FL_SYMLINK|FL_NOENT); } @@ -239,9 +239,9 @@ int has_symlink_or_noent_leading_path(int len, const char *name) * 'prefix_len', thus we then allow for symlinks in the prefix part as * long as those points to real existing directories. */ -int has_dirs_only_path(int len, const char *name, int prefix_len) +int has_dirs_only_path(const char *name, int len, int prefix_len) { - return lstat_cache(len, name, + return lstat_cache(name, len, FL_DIR|FL_FULLPATH, prefix_len) & FL_DIR; } diff --git a/unpack-trees.c b/unpack-trees.c index e547282..2293158 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -61,7 +61,7 @@ static void unlink_entry(struct cache_entry *ce) char *cp, *prev; char *name = ce->name; - if (has_symlink_or_noent_leading_path(ce_namelen(ce), ce->name)) + if (has_symlink_or_noent_leading_path(ce->name, ce_namelen(ce))) return; if (unlink(name)) return; @@ -583,7 +583,7 @@ static int verify_absent(struct cache_entry *ce, const char *action, if (o->index_only || o->reset || !o->update) return 0; - if (has_symlink_or_noent_leading_path(ce_namelen(ce), ce->name)) + if (has_symlink_or_noent_leading_path(ce->name, ce_namelen(ce))) return 0; if (!lstat(ce->name, &st)) { -- cgit v0.10.2-6-g49f6 From 7847892716a3c9a7b8facc076fc056ac425bcfe6 Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Mon, 9 Feb 2009 21:54:07 +0100 Subject: unlink_entry(): introduce schedule_dir_for_removal() Currently inside unlink_entry() if we get a successful removal of one file with unlink(), we try to remove the leading directories each and every time. So if one directory containing 200 files is moved to an other location we get 199 failed calls to rmdir() and 1 successful call. To fix this and avoid some unnecessary calls to rmdir(), we schedule each directory for removal and wait much longer before we do the real call to rmdir(). Since the unlink_entry() function is called with alphabetically sorted names, this new function end up being very effective to avoid unnecessary calls to rmdir(). In some cases over 95% of all calls to rmdir() is removed with this patch. Signed-off-by: Kjetil Barvik Signed-off-by: Junio C Hamano diff --git a/cache.h b/cache.h index 80eeeb7..1bf2d4b 100644 --- a/cache.h +++ b/cache.h @@ -729,6 +729,8 @@ extern int has_symlink_or_noent_leading_path(const char *name, int len); extern int has_dirs_only_path(const char *name, int len, int prefix_len); extern void invalidate_lstat_cache(const char *name, int len); extern void clear_lstat_cache(void); +extern void schedule_dir_for_removal(const char *name, int len); +extern void remove_scheduled_dirs(void); extern struct alternate_object_database { struct alternate_object_database *next; diff --git a/symlinks.c b/symlinks.c index 5167286..1d6b35b 100644 --- a/symlinks.c +++ b/symlinks.c @@ -245,3 +245,62 @@ int has_dirs_only_path(const char *name, int len, int prefix_len) FL_DIR|FL_FULLPATH, prefix_len) & FL_DIR; } + +static struct removal_def { + char path[PATH_MAX]; + int len; +} removal; + +static void do_remove_scheduled_dirs(int new_len) +{ + while (removal.len > new_len) { + removal.path[removal.len] = '\0'; + if (rmdir(removal.path)) + break; + do { + removal.len--; + } while (removal.len > new_len && + removal.path[removal.len] != '/'); + } + removal.len = new_len; + return; +} + +void schedule_dir_for_removal(const char *name, int len) +{ + int match_len, last_slash, i, previous_slash; + + match_len = last_slash = i = + longest_path_match(name, len, removal.path, removal.len, + &previous_slash); + /* Find last slash inside 'name' */ + while (i < len) { + if (name[i] == '/') + last_slash = i; + i++; + } + + /* + * If we are about to go down the directory tree, we check if + * we must first go upwards the tree, such that we then can + * remove possible empty directories as we go upwards. + */ + if (match_len < last_slash && match_len < removal.len) + do_remove_scheduled_dirs(match_len); + /* + * If we go deeper down the directory tree, we only need to + * save the new path components as we go down. + */ + if (match_len < last_slash) { + memcpy(&removal.path[match_len], &name[match_len], + last_slash - match_len); + removal.len = last_slash; + } + return; +} + +void remove_scheduled_dirs(void) +{ + do_remove_scheduled_dirs(0); + return; +} diff --git a/unpack-trees.c b/unpack-trees.c index 2293158..e3c3fa1 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -52,36 +52,17 @@ static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce, add_index_entry(&o->result, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|ADD_CACHE_SKIP_DFCHECK); } -/* Unlink the last component and attempt to remove leading - * directories, in case this unlink is the removal of the - * last entry in the directory -- empty directories are removed. +/* + * Unlink the last component and schedule the leading directories for + * removal, such that empty directories get removed. */ static void unlink_entry(struct cache_entry *ce) { - char *cp, *prev; - char *name = ce->name; - if (has_symlink_or_noent_leading_path(ce->name, ce_namelen(ce))) return; - if (unlink(name)) + if (unlink(ce->name)) return; - prev = NULL; - while (1) { - int status; - cp = strrchr(name, '/'); - if (prev) - *prev = '/'; - if (!cp) - break; - - *cp = 0; - status = rmdir(name); - if (status) { - *cp = '/'; - break; - } - prev = cp; - } + schedule_dir_for_removal(ce->name, ce_namelen(ce)); } static struct checkout state; @@ -117,6 +98,7 @@ static int check_updates(struct unpack_trees_options *o) continue; } } + remove_scheduled_dirs(); for (i = 0; i < index->cache_nr; i++) { struct cache_entry *ce = index->cache[i]; -- cgit v0.10.2-6-g49f6 From 81a9aa60a193f1181149b69920930e15c4e34059 Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Mon, 9 Feb 2009 21:54:08 +0100 Subject: create_directories(): remove some memcpy() and strchr() calls Remove the call to memcpy() and strchr() for each path component tested, and instead add each path component as we go forward inside the while-loop. Impact: small optimisation Signed-off-by: Kjetil Barvik Signed-off-by: Junio C Hamano diff --git a/entry.c b/entry.c index bb6bdb9..cc8f0c6 100644 --- a/entry.c +++ b/entry.c @@ -2,15 +2,19 @@ #include "blob.h" #include "dir.h" -static void create_directories(const char *path, const struct checkout *state) +static void create_directories(const char *path, int path_len, + const struct checkout *state) { - int len = strlen(path); - char *buf = xmalloc(len + 1); - const char *slash = path; - - while ((slash = strchr(slash+1, '/')) != NULL) { - len = slash - path; - memcpy(buf, path, len); + char *buf = xmalloc(path_len + 1); + int len = 0; + + while (len < path_len) { + do { + buf[len] = path[len]; + len++; + } while (len < path_len && path[len] != '/'); + if (len >= path_len) + break; buf[len] = 0; /* @@ -190,6 +194,7 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t memcpy(path, state->base_dir, len); strcpy(path + len, ce->name); + len += ce_namelen(ce); if (!lstat(path, &st)) { unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID); @@ -218,6 +223,6 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t return error("unable to unlink old '%s' (%s)", path, strerror(errno)); } else if (state->not_new) return 0; - create_directories(path, state); + create_directories(path, len, state); return write_entry(ce, path, state, 0); } -- cgit v0.10.2-6-g49f6 From 4857c761e35b07c12ff2ef1140e93b071b8ac4e7 Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Mon, 9 Feb 2009 21:54:50 +0100 Subject: write_entry(): cleanup of some duplicated code The switch-cases for S_IFREG and S_IFLNK was so similar that it will be better to do some cleanup and use the common parts of it. And the entry.c file should now be clean for 'gcc -Wextra' warnings. Signed-off-by: Kjetil Barvik Signed-off-by: Junio C Hamano diff --git a/entry.c b/entry.c index cc8f0c6..1f53588 100644 --- a/entry.c +++ b/entry.c @@ -78,7 +78,7 @@ static int create_file(const char *path, unsigned int mode) return open(path, O_WRONLY | O_CREAT | O_EXCL, mode); } -static void *read_blob_entry(struct cache_entry *ce, const char *path, unsigned long *size) +static void *read_blob_entry(struct cache_entry *ce, unsigned long *size) { enum object_type type; void *new = read_sha1_file(ce->sha1, &type, size); @@ -93,36 +93,51 @@ static void *read_blob_entry(struct cache_entry *ce, const char *path, unsigned static int write_entry(struct cache_entry *ce, char *path, const struct checkout *state, int to_tempfile) { - int fd; - long wrote; - - switch (ce->ce_mode & S_IFMT) { - char *new; - struct strbuf buf; - unsigned long size; - + unsigned int ce_mode_s_ifmt = ce->ce_mode & S_IFMT; + int fd, ret; + char *new; + struct strbuf buf = STRBUF_INIT; + unsigned long size; + size_t wrote, newsize = 0; + + switch (ce_mode_s_ifmt) { case S_IFREG: - new = read_blob_entry(ce, path, &size); + case S_IFLNK: + new = read_blob_entry(ce, &size); if (!new) return error("git checkout-index: unable to read sha1 file of %s (%s)", path, sha1_to_hex(ce->sha1)); + if (ce_mode_s_ifmt == S_IFLNK && has_symlinks && !to_tempfile) { + ret = symlink(new, path); + free(new); + if (ret) + return error("git checkout-index: unable to create symlink %s (%s)", + path, strerror(errno)); + break; + } + /* * Convert from git internal format to working tree format */ - strbuf_init(&buf, 0); - if (convert_to_working_tree(ce->name, new, size, &buf)) { - size_t newsize = 0; + if (ce_mode_s_ifmt == S_IFREG && + convert_to_working_tree(ce->name, new, size, &buf)) { free(new); new = strbuf_detach(&buf, &newsize); size = newsize; } if (to_tempfile) { - strcpy(path, ".merge_file_XXXXXX"); + if (ce_mode_s_ifmt == S_IFREG) + strcpy(path, ".merge_file_XXXXXX"); + else + strcpy(path, ".merge_link_XXXXXX"); fd = mkstemp(path); - } else + } else if (ce_mode_s_ifmt == S_IFREG) { fd = create_file(path, ce->ce_mode); + } else { + fd = create_file(path, 0666); + } if (fd < 0) { free(new); return error("git checkout-index: unable to create file %s (%s)", @@ -135,36 +150,6 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout if (wrote != size) return error("git checkout-index: unable to write file %s", path); break; - case S_IFLNK: - new = read_blob_entry(ce, path, &size); - if (!new) - return error("git checkout-index: unable to read sha1 file of %s (%s)", - path, sha1_to_hex(ce->sha1)); - if (to_tempfile || !has_symlinks) { - if (to_tempfile) { - strcpy(path, ".merge_link_XXXXXX"); - fd = mkstemp(path); - } else - fd = create_file(path, 0666); - if (fd < 0) { - free(new); - return error("git checkout-index: unable to create " - "file %s (%s)", path, strerror(errno)); - } - wrote = write_in_full(fd, new, size); - close(fd); - free(new); - if (wrote != size) - return error("git checkout-index: unable to write file %s", - path); - } else { - wrote = symlink(new, path); - free(new); - if (wrote) - return error("git checkout-index: unable to create " - "symlink %s (%s)", path, strerror(errno)); - } - break; case S_IFGITLINK: if (to_tempfile) return error("git checkout-index: cannot create temporary subproject %s", path); -- cgit v0.10.2-6-g49f6 From e4c7292353dbef39feac1c6a60c5cde9140520a6 Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Mon, 9 Feb 2009 21:54:51 +0100 Subject: write_entry(): use fstat() instead of lstat() when file is open Currently inside write_entry() we do an lstat(path, &st) call on a file which have just been opened inside the exact same function. It should be better to call fstat(fd, &st) on the file while it is open, and it should be at least as fast as the lstat() method. Signed-off-by: Kjetil Barvik Signed-off-by: Junio C Hamano diff --git a/entry.c b/entry.c index 1f53588..5daacc2 100644 --- a/entry.c +++ b/entry.c @@ -94,11 +94,12 @@ static void *read_blob_entry(struct cache_entry *ce, unsigned long *size) static int write_entry(struct cache_entry *ce, char *path, const struct checkout *state, int to_tempfile) { unsigned int ce_mode_s_ifmt = ce->ce_mode & S_IFMT; - int fd, ret; + int fd, ret, fstat_done = 0; char *new; struct strbuf buf = STRBUF_INIT; unsigned long size; size_t wrote, newsize = 0; + struct stat st; switch (ce_mode_s_ifmt) { case S_IFREG: @@ -145,6 +146,11 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout } wrote = write_in_full(fd, new, size); + /* use fstat() only when path == ce->name */ + if (state->refresh_cache && !to_tempfile && !state->base_dir_len) { + fstat(fd, &st); + fstat_done = 1; + } close(fd); free(new); if (wrote != size) @@ -161,8 +167,8 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout } if (state->refresh_cache) { - struct stat st; - lstat(ce->name, &st); + if (!fstat_done) + lstat(ce->name, &st); fill_stat_cache_info(ce, &st); } return 0; -- cgit v0.10.2-6-g49f6 From 91fcbcbdcdd845bd43104c5ac0af4c40da15223b Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Mon, 9 Feb 2009 21:54:52 +0100 Subject: show_patch_diff(): remove a call to fstat() Currently inside show_patch_diff() we have an fstat() call after an ok lstat() call. Since before the call to fstat() we have already tested for the link case with S_ISLNK(), the fstat() can be removed. Signed-off-by: Kjetil Barvik Signed-off-by: Junio C Hamano diff --git a/combine-diff.c b/combine-diff.c index bccc018..4300319 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -713,9 +713,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent, result_size = buf.len; result = strbuf_detach(&buf, NULL); elem->mode = canon_mode(st.st_mode); - } - else if (0 <= (fd = open(elem->path, O_RDONLY)) && - !fstat(fd, &st)) { + } else if (0 <= (fd = open(elem->path, O_RDONLY))) { size_t len = xsize_t(st.st_size); ssize_t done; int is_file, i; -- cgit v0.10.2-6-g49f6 From 7734f04873cfaddd0b148074a633f1f824fd961f Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Mon, 9 Feb 2009 21:54:53 +0100 Subject: lstat_cache(): print a warning if doing ping-pong between cache types This is a debug patch which is only to be used while the lstat_cache() is in the test stage, and should be removed/reverted before the final relase. I think it should be useful to catch these warnings, as I it could be an indication of that the cache would not be very effective if it is doing ping-pong by switching between different cache types too many times. Also, if someone is experimenting with the lstat_cache(), this patch will maybe be useful while debugging. If someone is able to trigger the warning, then send a mail to the GIT mailing list, containing the first 15 lines of the warning, and a short description of the GIT commands to trigger the warnings. I hope someone is willing to use this patch for a while, to be able to catch possible ping-pong's. Signed-off-by: Kjetil Barvik Signed-off-by: Junio C Hamano diff --git a/symlinks.c b/symlinks.c index 1d6b35b..cb255a3 100644 --- a/symlinks.c +++ b/symlinks.c @@ -51,6 +51,11 @@ static inline void reset_lstat_cache(void) */ } +#define SWITCHES_BEFORE_WARNING 10 +static unsigned int cache_switches, number_of_warnings; +static unsigned int current_cache_func, last_cache_func; +static unsigned int total_calls; + #define FL_DIR (1 << 0) #define FL_NOENT (1 << 1) #define FL_SYMLINK (1 << 2) @@ -77,6 +82,7 @@ static int lstat_cache(const char *name, int len, int match_flags, ret_flags, save_flags, max_len, ret; struct stat st; + total_calls++; if (cache.track_flags != track_flags || cache.prefix_len_stat_func != prefix_len_stat_func) { /* @@ -88,6 +94,17 @@ static int lstat_cache(const char *name, int len, cache.track_flags = track_flags; cache.prefix_len_stat_func = prefix_len_stat_func; match_len = last_slash = 0; + cache_switches++; + if (cache_switches > SWITCHES_BEFORE_WARNING) { + if (number_of_warnings < 10 || number_of_warnings % 1000 == 0) + printf("warning from %s:%d cache_switches:%u > %u "\ + "(current:%u last:%u total:%u)\n", + __FILE__, __LINE__, + cache_switches, SWITCHES_BEFORE_WARNING, + current_cache_func, last_cache_func, + total_calls); + number_of_warnings++; + } } else { /* * Check to see if we have a match from the cache for @@ -216,6 +233,8 @@ void clear_lstat_cache(void) */ int has_symlink_leading_path(const char *name, int len) { + last_cache_func = current_cache_func; + current_cache_func = 1; return lstat_cache(name, len, FL_SYMLINK|FL_DIR, USE_ONLY_LSTAT) & FL_SYMLINK; @@ -227,6 +246,8 @@ int has_symlink_leading_path(const char *name, int len) */ int has_symlink_or_noent_leading_path(const char *name, int len) { + last_cache_func = current_cache_func; + current_cache_func = 2; return lstat_cache(name, len, FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT) & (FL_SYMLINK|FL_NOENT); @@ -241,6 +262,8 @@ int has_symlink_or_noent_leading_path(const char *name, int len) */ int has_dirs_only_path(const char *name, int len, int prefix_len) { + last_cache_func = current_cache_func; + current_cache_func = 3; return lstat_cache(name, len, FL_DIR|FL_FULLPATH, prefix_len) & FL_DIR; -- cgit v0.10.2-6-g49f6 From fa26a401bed5967d6118ac430c5c5f4707c54386 Mon Sep 17 00:00:00 2001 From: Ted Pavlic Date: Wed, 11 Feb 2009 13:03:23 -0500 Subject: completion: For consistency, change "git rev-parse" to __gitdir calls Signed-off-by: Ted Pavlic Acked-by: Shawn O. Pearce Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index f44f63c..6bbe09a 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -80,7 +80,7 @@ __gitdir () # returns text to add to bash PS1 prompt (includes branch name) __git_ps1 () { - local g="$(git rev-parse --git-dir 2>/dev/null)" + local g="$(__gitdir)" if [ -n "$g" ]; then local r local b @@ -1797,7 +1797,7 @@ _gitk () __git_has_doubledash && return local cur="${COMP_WORDS[COMP_CWORD]}" - local g="$(git rev-parse --git-dir 2>/dev/null)" + local g="$(__gitdir)" local merge="" if [ -f $g/MERGE_HEAD ]; then merge="--merge" -- cgit v0.10.2-6-g49f6 From ad244d256865c06804afffef32b753239a06119e Mon Sep 17 00:00:00 2001 From: Ted Pavlic Date: Wed, 11 Feb 2009 13:03:24 -0500 Subject: completion: Use consistent if [...] convention, not "test" The local coding convention in bash completion is to use [...] rather than test. Additionally, if [...]; then is preferred over if [...] then and so matching "if [...]\nthen" were changed accordingly. Signed-off-by: Ted Pavlic Acked-by: Shawn O. Pearce Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 6bbe09a..c61576f 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -84,39 +84,30 @@ __git_ps1 () if [ -n "$g" ]; then local r local b - if [ -d "$g/rebase-apply" ] - then - if test -f "$g/rebase-apply/rebasing" - then + if [ -d "$g/rebase-apply" ]; then + if [ -f "$g/rebase-apply/rebasing" ]; then r="|REBASE" - elif test -f "$g/rebase-apply/applying" - then + elif [ -f "$g/rebase-apply/applying" ]; then r="|AM" else r="|AM/REBASE" fi b="$(git symbolic-ref HEAD 2>/dev/null)" - elif [ -f "$g/rebase-merge/interactive" ] - then + elif [ -f "$g/rebase-merge/interactive" ]; then r="|REBASE-i" b="$(cat "$g/rebase-merge/head-name")" - elif [ -d "$g/rebase-merge" ] - then + elif [ -d "$g/rebase-merge" ]; then r="|REBASE-m" b="$(cat "$g/rebase-merge/head-name")" - elif [ -f "$g/MERGE_HEAD" ] - then + elif [ -f "$g/MERGE_HEAD" ]; then r="|MERGING" b="$(git symbolic-ref HEAD 2>/dev/null)" else - if [ -f "$g/BISECT_LOG" ] - then + if [ -f "$g/BISECT_LOG" ]; then r="|BISECTING" fi - if ! b="$(git symbolic-ref HEAD 2>/dev/null)" - then - if ! b="$(git describe --exact-match HEAD 2>/dev/null)" - then + if ! b="$(git symbolic-ref HEAD 2>/dev/null)"; then + if ! b="$(git describe --exact-match HEAD 2>/dev/null)"; then b="$(cut -c1-7 "$g/HEAD")..." fi fi @@ -125,8 +116,8 @@ __git_ps1 () local w local i - if test -n "${GIT_PS1_SHOWDIRTYSTATE-}"; then - if test "$(git config --bool bash.showDirtyState)" != "false"; then + if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then + if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then git diff --no-ext-diff --ignore-submodules \ --quiet --exit-code || w="*" if git rev-parse --quiet --verify HEAD >/dev/null; then -- cgit v0.10.2-6-g49f6 From e5dd864adfeb8b0176b31a132e972d7f7beff32a Mon Sep 17 00:00:00 2001 From: Ted Pavlic Date: Wed, 11 Feb 2009 13:03:25 -0500 Subject: completion: Better __git_ps1 support when not in working directory If .git/HEAD is not readable, __git_ps1 does nothing. If --is-in-git-dir, __git_ps1 returns " (GIT_DIR!)" as a cautionary note. The previous behavior would show the branch name (and would optionally attempt to determine the dirtyState of the directory, which was impossible because a "git diff" was used). If --is-in-work-tree, __git_ps1 returns the branch name. Additionally, if showDirtyState is on, the dirty state is displayed. Signed-off-by: Ted Pavlic Acked-by: Shawn O. Pearce Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index c61576f..aa8eec2 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -108,7 +108,9 @@ __git_ps1 () fi if ! b="$(git symbolic-ref HEAD 2>/dev/null)"; then if ! b="$(git describe --exact-match HEAD 2>/dev/null)"; then - b="$(cut -c1-7 "$g/HEAD")..." + if [ -r "$g/HEAD" ]; then + b="$(cut -c1-7 "$g/HEAD")..." + fi fi fi fi @@ -116,23 +118,29 @@ __git_ps1 () local w local i - if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then - if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then - git diff --no-ext-diff --ignore-submodules \ - --quiet --exit-code || w="*" - if git rev-parse --quiet --verify HEAD >/dev/null; then - git diff-index --cached --quiet \ - --ignore-submodules HEAD -- || i="+" - else - i="#" + if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then + b="GIT_DIR!" + elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then + if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then + if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then + git diff --no-ext-diff --ignore-submodules \ + --quiet --exit-code || w="*" + if git rev-parse --quiet --verify HEAD >/dev/null; then + git diff-index --cached --quiet \ + --ignore-submodules HEAD -- || i="+" + else + i="#" + fi fi fi fi - if [ -n "${1-}" ]; then - printf "$1" "${b##refs/heads/}$w$i$r" - else - printf " (%s)" "${b##refs/heads/}$w$i$r" + if [ -n "$b" ]; then + if [ -n "${1-}" ]; then + printf "$1" "${b##refs/heads/}$w$i$r" + else + printf " (%s)" "${b##refs/heads/}$w$i$r" + fi fi fi } -- cgit v0.10.2-6-g49f6 From 5c9cc64a4a608ab0bbd5eb5c8e405bfe050be309 Mon Sep 17 00:00:00 2001 From: Ted Pavlic Date: Wed, 11 Feb 2009 13:03:26 -0500 Subject: completion: More fixes to prevent unbound variable errors Several functions make use of "[-n ...]" and "[-z ...]". In many cases, the variables being tested were declared with "local." However, several __variables are not, and so they must be replaced with their ${__-} equivalents. Signed-off-by: Ted Pavlic Acked-by: Shawn O. Pearce Signed-off-by: Junio C Hamano diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index aa8eec2..6e8c5b9 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -62,7 +62,7 @@ esac __gitdir () { if [ -z "${1-}" ]; then - if [ -n "$__git_dir" ]; then + if [ -n "${__git_dir-}" ]; then echo "$__git_dir" elif [ -d .git ]; then echo .git @@ -298,7 +298,7 @@ __git_remotes () __git_merge_strategies () { - if [ -n "$__git_merge_strategylist" ]; then + if [ -n "${__git_merge_strategylist-}" ]; then echo "$__git_merge_strategylist" return fi @@ -384,7 +384,7 @@ __git_complete_revlist () __git_all_commands () { - if [ -n "$__git_all_commandlist" ]; then + if [ -n "${__git_all_commandlist-}" ]; then echo "$__git_all_commandlist" return fi @@ -402,7 +402,7 @@ __git_all_commandlist="$(__git_all_commands 2>/dev/null)" __git_porcelain_commands () { - if [ -n "$__git_porcelain_commandlist" ]; then + if [ -n "${__git_porcelain_commandlist-}" ]; then echo "$__git_porcelain_commandlist" return fi -- cgit v0.10.2-6-g49f6 From 1d398a03902aab8ec49197d8827c19f9e2203c98 Mon Sep 17 00:00:00 2001 From: Deskin Miller Date: Thu, 12 Feb 2009 00:19:41 -0500 Subject: add -i: revisit hunk on editor failure Similar to the behaviour for editing a commit message, let terminating the editor with a failure abort the current hunk edit and revisit the option selection for the hunk. Signed-off-by: Deskin Miller Signed-off-by: Junio C Hamano diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 5f129a4..f7b0761 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -753,6 +753,10 @@ EOF || $ENV{VISUAL} || $ENV{EDITOR} || "vi"; system('sh', '-c', $editor.' "$@"', $editor, $hunkfile); + if ($? != 0) { + return undef; + } + open $fh, '<', $hunkfile or die "failed to open hunk edit file for reading: " . $!; my @newtext = grep { !/^#/ } <$fh>; -- cgit v0.10.2-6-g49f6 From 0db5260bd033cc357186cc13fe23be9635ad69e7 Mon Sep 17 00:00:00 2001 From: Jeremy White Date: Thu, 12 Feb 2009 09:51:55 -0600 Subject: Enable setting attach as the default in .gitconfig for git-format-patch. Signed-off-by: Jeremy White Signed-off-by: Junio C Hamano diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 11a7d77..e7ae8cf 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -10,7 +10,8 @@ SYNOPSIS -------- [verse] 'git format-patch' [-k] [-o | --stdout] [--thread] - [--attach[=] | --inline[=]] + [--attach[=] | --inline[=] | + [--no-attach]] [-s | --signoff] [] [-n | --numbered | -N | --no-numbered] [--start-number ] [--numbered-files] @@ -117,6 +118,10 @@ include::diff-options.txt[] which is the commit message and the patch itself in the second part, with "Content-Disposition: attachment". +--no-attach:: + Disable the creation of an attachment, overriding the + configuration setting. + --inline[=]:: Create multipart/mixed attachment, the first part of which is the commit message and the patch itself in the @@ -174,7 +179,8 @@ CONFIGURATION ------------- You can specify extra mail header lines to be added to each message in the repository configuration, new defaults for the subject prefix -and file suffix, and number patches when outputting more than one. +and file suffix, control attachements, and number patches when outputting +more than one. ------------ [format] @@ -183,6 +189,7 @@ and file suffix, and number patches when outputting more than one. suffix = .txt numbered = auto cc = + attach [ = mime-boundary-string ] ------------ diff --git a/builtin-log.c b/builtin-log.c index 2ae39af..8549028 100644 --- a/builtin-log.c +++ b/builtin-log.c @@ -428,6 +428,8 @@ static const char *fmt_patch_suffix = ".patch"; static int numbered = 0; static int auto_number = 1; +static char *default_attach = NULL; + static char **extra_hdr; static int extra_hdr_nr; static int extra_hdr_alloc; @@ -488,6 +490,14 @@ static int git_format_config(const char *var, const char *value, void *cb) auto_number = auto_number && numbered; return 0; } + if (!strcmp(var, "format.attach")) { + if (value && *value) + default_attach = xstrdup(value); + else + default_attach = xstrdup(git_version_string); + return 0; + } + return git_log_config(var, value, cb); } @@ -787,6 +797,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.subject_prefix = fmt_patch_subject_prefix; + if (default_attach) { + rev.mime_boundary = default_attach; + rev.no_inline = 1; + } + /* * Parse the arguments before setup_revisions(), or something * like "git format-patch -o a123 HEAD^.." may fail; a123 is @@ -849,6 +864,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.mime_boundary = argv[i] + 9; rev.no_inline = 1; } + else if (!strcmp(argv[i], "--no-attach")) { + rev.mime_boundary = NULL; + rev.no_inline = 0; + } else if (!strcmp(argv[i], "--inline")) { rev.mime_boundary = git_version_string; rev.no_inline = 0; -- cgit v0.10.2-6-g49f6 From 209d336ae3923f2c1783a42b12fca50f3bee0802 Mon Sep 17 00:00:00 2001 From: Jay Soffian Date: Fri, 13 Feb 2009 04:40:18 -0500 Subject: builtin-branch: improve output when displaying remote branches When encountering a symref (typically refs/remotes//HEAD), display the ref target. When displaying local and remote branches, prefix the remote branch names with "remotes/" to make the remote branches clear from the local branches. If displaying only the remote branches, the prefix is not shown since it would be redundant. Sample output: $ git branch foo -> master * master rather-long-branch-name $ git branch -v foo -> master * master 51cecb2 initial rather-long-branch-name 51cecb2 initial $ git branch -v --no-abbrev foo -> master * master 51cecb2dbb1a1902bb4df79b543c8f951ee59d83 initial rather-long-branch-name 51cecb2dbb1a1902bb4df79b543c8f951ee59d83 initial $ git branch -r frotz/HEAD -> frotz/master frotz/master origin/HEAD -> origin/master origin/UNUSUAL -> refs/heads/master origin/master $ git branch -a foo -> master * master rather-long-branch-name remotes/frotz/HEAD -> frotz/master remotes/frotz/master remotes/origin/HEAD -> origin/master remotes/origin/UNUSUAL -> refs/heads/master remotes/origin/master $ git branch -rv frotz/HEAD -> frotz/master frotz/master e1d8130 added file2 origin/HEAD -> origin/master origin/UNUSUAL -> refs/heads/master origin/master e1d8130 added file2 $ git branch -av foo -> master * master 51cecb2 initial rather-long-branch-name 51cecb2 initial remotes/frotz/HEAD -> frotz/master remotes/frotz/master e1d8130 added file2 remotes/origin/HEAD -> origin/master remotes/origin/UNUSUAL -> refs/heads/master remotes/origin/master e1d8130 added file2 Signed-off-by: Jay Soffian Signed-off-by: Junio C Hamano diff --git a/builtin-branch.c b/builtin-branch.c index 56a1971..7607f6a 100644 --- a/builtin-branch.c +++ b/builtin-branch.c @@ -181,7 +181,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds) struct ref_item { char *name; - unsigned int kind; + char *dest; + unsigned int kind, len; struct commit *commit; }; @@ -193,22 +194,47 @@ struct ref_list { int kinds; }; +static char *resolve_symref(const char *src, const char *prefix) +{ + unsigned char sha1[20]; + int flag; + const char *dst, *cp; + + dst = resolve_ref(src, sha1, 0, &flag); + if (!(dst && (flag & REF_ISSYMREF))) + return NULL; + if (prefix && (cp = skip_prefix(dst, prefix))) + dst = cp; + return xstrdup(dst); +} + static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data) { struct ref_list *ref_list = (struct ref_list*)(cb_data); struct ref_item *newitem; struct commit *commit; - int kind; - int len; + int kind, i; + const char *prefix, *orig_refname = refname; + + static struct { + int kind; + const char *prefix; + int pfxlen; + } ref_kind[] = { + { REF_LOCAL_BRANCH, "refs/heads/", 11 }, + { REF_REMOTE_BRANCH, "refs/remotes/", 13 }, + }; /* Detect kind */ - if (!prefixcmp(refname, "refs/heads/")) { - kind = REF_LOCAL_BRANCH; - refname += 11; - } else if (!prefixcmp(refname, "refs/remotes/")) { - kind = REF_REMOTE_BRANCH; - refname += 13; - } else + for (i = 0; i < ARRAY_SIZE(ref_kind); i++) { + prefix = ref_kind[i].prefix; + if (strncmp(refname, prefix, ref_kind[i].pfxlen)) + continue; + kind = ref_kind[i].kind; + refname += ref_kind[i].pfxlen; + break; + } + if (ARRAY_SIZE(ref_kind) <= i) return 0; commit = lookup_commit_reference_gently(sha1, 1); @@ -239,9 +265,14 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags, newitem->name = xstrdup(refname); newitem->kind = kind; newitem->commit = commit; - len = strlen(newitem->name); - if (len > ref_list->maxwidth) - ref_list->maxwidth = len; + newitem->len = strlen(refname); + newitem->dest = resolve_symref(orig_refname, prefix); + /* adjust for "remotes/" */ + if (newitem->kind == REF_REMOTE_BRANCH && + ref_list->kinds != REF_REMOTE_BRANCH) + newitem->len += 8; + if (newitem->len > ref_list->maxwidth) + ref_list->maxwidth = newitem->len; return 0; } @@ -250,8 +281,10 @@ static void free_ref_list(struct ref_list *ref_list) { int i; - for (i = 0; i < ref_list->index; i++) + for (i = 0; i < ref_list->index; i++) { free(ref_list->list[i].name); + free(ref_list->list[i].dest); + } free(ref_list->list); } @@ -292,11 +325,12 @@ static int matches_merge_filter(struct commit *commit) } static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, - int abbrev, int current) + int abbrev, int current, char *prefix) { char c; int color; struct commit *commit = item->commit; + struct strbuf out = STRBUF_INIT, name = STRBUF_INIT; if (!matches_merge_filter(commit)) return; @@ -319,7 +353,18 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, color = COLOR_BRANCH_CURRENT; } - if (verbose) { + strbuf_addf(&name, "%s%s", prefix, item->name); + if (verbose) + strbuf_addf(&out, "%c %s%-*s%s", c, branch_get_color(color), + maxwidth, name.buf, + branch_get_color(COLOR_BRANCH_RESET)); + else + strbuf_addf(&out, "%c %s%s%s", c, branch_get_color(color), + name.buf, branch_get_color(COLOR_BRANCH_RESET)); + + if (item->dest) + strbuf_addf(&out, " -> %s", item->dest); + else if (verbose) { struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT; const char *sub = " **** invalid ref ****"; @@ -333,28 +378,25 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, if (item->kind == REF_LOCAL_BRANCH) fill_tracking_info(&stat, item->name); - printf("%c %s%-*s%s %s %s%s\n", c, branch_get_color(color), - maxwidth, item->name, - branch_get_color(COLOR_BRANCH_RESET), - find_unique_abbrev(item->commit->object.sha1, abbrev), - stat.buf, sub); + strbuf_addf(&out, " %s %s%s", + find_unique_abbrev(item->commit->object.sha1, abbrev), + stat.buf, sub); strbuf_release(&stat); strbuf_release(&subject); - } else { - printf("%c %s%s%s\n", c, branch_get_color(color), item->name, - branch_get_color(COLOR_BRANCH_RESET)); } + printf("%s\n", out.buf); + strbuf_release(&name); + strbuf_release(&out); } static int calc_maxwidth(struct ref_list *refs) { - int i, l, w = 0; + int i, w = 0; for (i = 0; i < refs->index; i++) { if (!matches_merge_filter(refs->list[i].commit)) continue; - l = strlen(refs->list[i].name); - if (l > w) - w = l; + if (refs->list[i].len > w) + w = refs->list[i].len; } return w; } @@ -394,7 +436,7 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str item.commit = head_commit; if (strlen(item.name) > ref_list.maxwidth) ref_list.maxwidth = strlen(item.name); - print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1); + print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1, ""); free(item.name); } @@ -402,8 +444,11 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str int current = !detached && (ref_list.list[i].kind == REF_LOCAL_BRANCH) && !strcmp(ref_list.list[i].name, head); + char *prefix = (kinds != REF_REMOTE_BRANCH && + ref_list.list[i].kind == REF_REMOTE_BRANCH) + ? "remotes/" : ""; print_ref_item(&ref_list.list[i], ref_list.maxwidth, verbose, - abbrev, current); + abbrev, current, prefix); } free_ref_list(&ref_list); -- cgit v0.10.2-6-g49f6 From b2f82e05de2512ae4adb63b669f6c1fe6cba3148 Mon Sep 17 00:00:00 2001 From: Sverre Rabbelier Date: Fri, 13 Feb 2009 23:48:01 +0100 Subject: Teach rebase to rebase even if upstream is up to date Normally, if the current branch is up to date, the rebase is aborted. However, it may be desirable to allow rebasing even if the current branch is up to date. When using the '--whitespace=fix' option -f is implied. Signed-off-by: Sverre Rabbelier Signed-off-by: Junio C Hamano diff --git a/git-rebase.sh b/git-rebase.sh index 6d3eddb..5d9a393 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -3,7 +3,7 @@ # Copyright (c) 2005 Junio C Hamano. # -USAGE='[--interactive | -i] [-v] [--onto ] [|--root] []' +USAGE='[--interactive | -i] [-v] [--force-rebase | -f] [--onto ] [|--root] []' LONG_USAGE='git-rebase replaces with a new branch of the same name. When the --onto option is provided the new branch starts out with a HEAD equal to , otherwise it is equal to @@ -48,6 +48,7 @@ prec=4 verbose= git_am_opt= rebase_root= +force_rebase= continue_merge () { test -n "$prev_head" || die "prev_head must be defined" @@ -294,6 +295,11 @@ do ;; --whitespace=*) git_am_opt="$git_am_opt $1" + case "$1" in + --whitespace=fix|--whitespace=strip) + force_rebase=t + ;; + esac ;; -C*) git_am_opt="$git_am_opt $1" @@ -301,6 +307,9 @@ do --root) rebase_root=t ;; + -f|--f|--fo|--for|--forc|force|--force-r|--force-re|--force-reb|--force-reba|--force_rebas|--force-rebase) + force_rebase=t + ;; -*) usage ;; @@ -419,10 +428,15 @@ if test "$upstream" = "$onto" && test "$mb" = "$onto" && # linear history? ! (git rev-list --parents "$onto".."$branch" | grep " .* ") > /dev/null then - # Lazily switch to the target branch if needed... - test -z "$switch_to" || git checkout "$switch_to" - echo >&2 "Current branch $branch_name is up to date." - exit 0 + if test -z "$force_rebase" + then + # Lazily switch to the target branch if needed... + test -z "$switch_to" || git checkout "$switch_to" + echo >&2 "Current branch $branch_name is up to date." + exit 0 + else + echo "Current branch $branch_name is up to date, rebase forced." + fi fi if test -n "$verbose" -- cgit v0.10.2-6-g49f6 From dc6ebd4cc5028d59146e02e30f7945ee91974e6e Mon Sep 17 00:00:00 2001 From: Arjen Laarhoven Date: Fri, 13 Feb 2009 22:53:40 +0100 Subject: Clean up use of ANSI color sequences Remove the literal ANSI escape sequences and replace them by readable constants. Signed-off-by: Arjen Laarhoven Signed-off-by: Junio C Hamano diff --git a/builtin-branch.c b/builtin-branch.c index 56a1971..fe139e1 100644 --- a/builtin-branch.c +++ b/builtin-branch.c @@ -32,11 +32,11 @@ static unsigned char head_sha1[20]; static int branch_use_color = -1; static char branch_colors[][COLOR_MAXLEN] = { - "\033[m", /* reset */ - "", /* PLAIN (normal) */ - "\033[31m", /* REMOTE (red) */ - "", /* LOCAL (normal) */ - "\033[32m", /* CURRENT (green) */ + GIT_COLOR_RESET, + GIT_COLOR_NORMAL, /* PLAIN */ + GIT_COLOR_RED, /* REMOTE */ + GIT_COLOR_NORMAL, /* LOCAL */ + GIT_COLOR_GREEN, /* CURRENT */ }; enum color_branch { COLOR_BRANCH_RESET = 0, diff --git a/color.c b/color.c index db4dccf..62977f4 100644 --- a/color.c +++ b/color.c @@ -1,8 +1,6 @@ #include "cache.h" #include "color.h" -#define COLOR_RESET "\033[m" - int git_use_color_default = 0; static int parse_color(const char *name, int len) @@ -54,7 +52,7 @@ void color_parse_mem(const char *value, int value_len, const char *var, int bg = -2; if (!strncasecmp(value, "reset", len)) { - strcpy(dst, "\033[m"); + strcpy(dst, GIT_COLOR_RESET); return; } @@ -175,7 +173,7 @@ static int color_vfprintf(FILE *fp, const char *color, const char *fmt, r += fprintf(fp, "%s", color); r += vfprintf(fp, fmt, args); if (*color) - r += fprintf(fp, "%s", COLOR_RESET); + r += fprintf(fp, "%s", GIT_COLOR_RESET); if (trail) r += fprintf(fp, "%s", trail); return r; @@ -217,7 +215,7 @@ int color_fwrite_lines(FILE *fp, const char *color, char *p = memchr(buf, '\n', count); if (p != buf && (fputs(color, fp) < 0 || fwrite(buf, p ? p - buf : count, 1, fp) != 1 || - fputs(COLOR_RESET, fp) < 0)) + fputs(GIT_COLOR_RESET, fp) < 0)) return -1; if (!p) return 0; diff --git a/color.h b/color.h index 5019df8..6846be1 100644 --- a/color.h +++ b/color.h @@ -4,6 +4,16 @@ /* "\033[1;38;5;2xx;48;5;2xxm\0" is 23 bytes */ #define COLOR_MAXLEN 24 +#define GIT_COLOR_NORMAL "" +#define GIT_COLOR_RESET "\033[m" +#define GIT_COLOR_BOLD "\033[1m" +#define GIT_COLOR_RED "\033[31m" +#define GIT_COLOR_GREEN "\033[32m" +#define GIT_COLOR_YELLOW "\033[33m" +#define GIT_COLOR_BLUE "\033[34m" +#define GIT_COLOR_CYAN "\033[36m" +#define GIT_COLOR_BG_RED "\033[41m" + /* * This variable stores the value of color.ui */ diff --git a/diff.c b/diff.c index be3859e..a3db16e 100644 --- a/diff.c +++ b/diff.c @@ -30,14 +30,14 @@ int diff_auto_refresh_index = 1; static int diff_mnemonic_prefix; static char diff_colors[][COLOR_MAXLEN] = { - "\033[m", /* reset */ - "", /* PLAIN (normal) */ - "\033[1m", /* METAINFO (bold) */ - "\033[36m", /* FRAGINFO (cyan) */ - "\033[31m", /* OLD (red) */ - "\033[32m", /* NEW (green) */ - "\033[33m", /* COMMIT (yellow) */ - "\033[41m", /* WHITESPACE (red background) */ + GIT_COLOR_RESET, + GIT_COLOR_NORMAL, /* PLAIN */ + GIT_COLOR_BOLD, /* METAINFO */ + GIT_COLOR_CYAN, /* FRAGINFO */ + GIT_COLOR_RED, /* OLD */ + GIT_COLOR_GREEN, /* NEW */ + GIT_COLOR_YELLOW, /* COMMIT */ + GIT_COLOR_BG_RED, /* WHITESPACE */ }; static void diff_filespec_load_driver(struct diff_filespec *one); diff --git a/pretty.c b/pretty.c index cc460b5..66bae42 100644 --- a/pretty.c +++ b/pretty.c @@ -567,16 +567,16 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder, return end - placeholder + 1; } if (!prefixcmp(placeholder + 1, "red")) { - strbuf_addstr(sb, "\033[31m"); + strbuf_addstr(sb, GIT_COLOR_RED); return 4; } else if (!prefixcmp(placeholder + 1, "green")) { - strbuf_addstr(sb, "\033[32m"); + strbuf_addstr(sb, GIT_COLOR_GREEN); return 6; } else if (!prefixcmp(placeholder + 1, "blue")) { - strbuf_addstr(sb, "\033[34m"); + strbuf_addstr(sb, GIT_COLOR_BLUE); return 5; } else if (!prefixcmp(placeholder + 1, "reset")) { - strbuf_addstr(sb, "\033[m"); + strbuf_addstr(sb, GIT_COLOR_RESET); return 6; } else return 0; diff --git a/wt-status.c b/wt-status.c index 96ff2f8..dd87339 100644 --- a/wt-status.c +++ b/wt-status.c @@ -15,11 +15,11 @@ int wt_status_relative_paths = 1; int wt_status_use_color = -1; int wt_status_submodule_summary; static char wt_status_colors[][COLOR_MAXLEN] = { - "", /* WT_STATUS_HEADER: normal */ - "\033[32m", /* WT_STATUS_UPDATED: green */ - "\033[31m", /* WT_STATUS_CHANGED: red */ - "\033[31m", /* WT_STATUS_UNTRACKED: red */ - "\033[31m", /* WT_STATUS_NOBRANCH: red */ + GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */ + GIT_COLOR_GREEN, /* WT_STATUS_UPDATED */ + GIT_COLOR_RED, /* WT_STATUS_CHANGED */ + GIT_COLOR_RED, /* WT_STATUS_UNTRACKED */ + GIT_COLOR_RED, /* WT_STATUS_NOBRANCH */ }; enum untracked_status_type show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES; -- cgit v0.10.2-6-g49f6 From 74bb2dfbec15ad47242b9a879b5547b6f4a28318 Mon Sep 17 00:00:00 2001 From: Arjen Laarhoven Date: Fri, 13 Feb 2009 22:53:41 +0100 Subject: builtin-branch.c: Rename branch category color names The branch color constants have the form COLOR_BRANCH_$category. Rename them to BRANCH_COLOR_$category as this conveys their meaning better. Signed-off-by: Arjen Laarhoven Signed-off-by: Junio C Hamano diff --git a/builtin-branch.c b/builtin-branch.c index fe139e1..6d241c8 100644 --- a/builtin-branch.c +++ b/builtin-branch.c @@ -39,11 +39,11 @@ static char branch_colors[][COLOR_MAXLEN] = { GIT_COLOR_GREEN, /* CURRENT */ }; enum color_branch { - COLOR_BRANCH_RESET = 0, - COLOR_BRANCH_PLAIN = 1, - COLOR_BRANCH_REMOTE = 2, - COLOR_BRANCH_LOCAL = 3, - COLOR_BRANCH_CURRENT = 4, + BRANCH_COLOR_RESET = 0, + BRANCH_COLOR_PLAIN = 1, + BRANCH_COLOR_REMOTE = 2, + BRANCH_COLOR_LOCAL = 3, + BRANCH_COLOR_CURRENT = 4, }; static enum merge_filter { @@ -56,15 +56,15 @@ static unsigned char merge_filter_ref[20]; static int parse_branch_color_slot(const char *var, int ofs) { if (!strcasecmp(var+ofs, "plain")) - return COLOR_BRANCH_PLAIN; + return BRANCH_COLOR_PLAIN; if (!strcasecmp(var+ofs, "reset")) - return COLOR_BRANCH_RESET; + return BRANCH_COLOR_RESET; if (!strcasecmp(var+ofs, "remote")) - return COLOR_BRANCH_REMOTE; + return BRANCH_COLOR_REMOTE; if (!strcasecmp(var+ofs, "local")) - return COLOR_BRANCH_LOCAL; + return BRANCH_COLOR_LOCAL; if (!strcasecmp(var+ofs, "current")) - return COLOR_BRANCH_CURRENT; + return BRANCH_COLOR_CURRENT; die("bad config variable '%s'", var); } @@ -303,20 +303,20 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, switch (item->kind) { case REF_LOCAL_BRANCH: - color = COLOR_BRANCH_LOCAL; + color = BRANCH_COLOR_LOCAL; break; case REF_REMOTE_BRANCH: - color = COLOR_BRANCH_REMOTE; + color = BRANCH_COLOR_REMOTE; break; default: - color = COLOR_BRANCH_PLAIN; + color = BRANCH_COLOR_PLAIN; break; } c = ' '; if (current) { c = '*'; - color = COLOR_BRANCH_CURRENT; + color = BRANCH_COLOR_CURRENT; } if (verbose) { @@ -335,14 +335,14 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, printf("%c %s%-*s%s %s %s%s\n", c, branch_get_color(color), maxwidth, item->name, - branch_get_color(COLOR_BRANCH_RESET), + branch_get_color(BRANCH_COLOR_RESET), find_unique_abbrev(item->commit->object.sha1, abbrev), stat.buf, sub); strbuf_release(&stat); strbuf_release(&subject); } else { printf("%c %s%s%s\n", c, branch_get_color(color), item->name, - branch_get_color(COLOR_BRANCH_RESET)); + branch_get_color(BRANCH_COLOR_RESET)); } } -- cgit v0.10.2-6-g49f6 From 900569661bbfe5c0e56cf8b9c014a2f594bb174c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Sat, 14 Feb 2009 23:21:04 +0100 Subject: rerere: remove duplicated functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both rerere.c and builtin-rerere.c define the static functions rr_path() and has_resolution() the exact same way. To eliminate this code duplication this patch turns the functions in rerere.c non-static, and makes builtin-rerere.c use them. Also, since this puts these two functions into the global namespace, rename them to rerere_path() and has_rerere_resolution(), respectively, and rename their "name" parameter to "hex", because it better reflects what that parameter actually is. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano diff --git a/builtin-rerere.c b/builtin-rerere.c index bd8fc77..020af73 100644 --- a/builtin-rerere.c +++ b/builtin-rerere.c @@ -13,28 +13,17 @@ static const char git_rerere_usage[] = static int cutoff_noresolve = 15; static int cutoff_resolve = 60; -static const char *rr_path(const char *name, const char *file) -{ - return git_path("rr-cache/%s/%s", name, file); -} - static time_t rerere_created_at(const char *name) { struct stat st; - return stat(rr_path(name, "preimage"), &st) ? (time_t) 0 : st.st_mtime; -} - -static int has_resolution(const char *name) -{ - struct stat st; - return !stat(rr_path(name, "postimage"), &st); + return stat(rerere_path(name, "preimage"), &st) ? (time_t) 0 : st.st_mtime; } static void unlink_rr_item(const char *name) { - unlink(rr_path(name, "thisimage")); - unlink(rr_path(name, "preimage")); - unlink(rr_path(name, "postimage")); + unlink(rerere_path(name, "thisimage")); + unlink(rerere_path(name, "preimage")); + unlink(rerere_path(name, "postimage")); rmdir(git_path("rr-cache/%s", name)); } @@ -65,7 +54,7 @@ static void garbage_collect(struct string_list *rr) then = rerere_created_at(e->d_name); if (!then) continue; - cutoff = (has_resolution(e->d_name) + cutoff = (has_rerere_resolution(e->d_name) ? cutoff_resolve : cutoff_noresolve); if (then < now - cutoff * 86400) string_list_append(e->d_name, &to_remove); @@ -124,7 +113,7 @@ int cmd_rerere(int argc, const char **argv, const char *prefix) if (!strcmp(argv[1], "clear")) { for (i = 0; i < merge_rr.nr; i++) { const char *name = (const char *)merge_rr.items[i].util; - if (!has_resolution(name)) + if (!has_rerere_resolution(name)) unlink_rr_item(name); } unlink(git_path("rr-cache/MERGE_RR")); @@ -137,7 +126,7 @@ int cmd_rerere(int argc, const char **argv, const char *prefix) for (i = 0; i < merge_rr.nr; i++) { const char *path = merge_rr.items[i].string; const char *name = (const char *)merge_rr.items[i].util; - diff_two(rr_path(name, "preimage"), path, path, path); + diff_two(rerere_path(name, "preimage"), path, path, path); } else usage(git_rerere_usage); diff --git a/rerere.c b/rerere.c index 3518207..713c6e1 100644 --- a/rerere.c +++ b/rerere.c @@ -12,15 +12,15 @@ static int rerere_autoupdate; static char *merge_rr_path; -static const char *rr_path(const char *name, const char *file) +const char *rerere_path(const char *hex, const char *file) { - return git_path("rr-cache/%s/%s", name, file); + return git_path("rr-cache/%s/%s", hex, file); } -static int has_resolution(const char *name) +int has_rerere_resolution(const char *hex) { struct stat st; - return !stat(rr_path(name, "postimage"), &st); + return !stat(rerere_path(hex, "postimage"), &st); } static void read_rr(struct string_list *rr) @@ -208,12 +208,12 @@ static int merge(const char *name, const char *path) mmbuffer_t result = {NULL, 0}; xpparam_t xpp = {XDF_NEED_MINIMAL}; - if (handle_file(path, NULL, rr_path(name, "thisimage")) < 0) + if (handle_file(path, NULL, rerere_path(name, "thisimage")) < 0) return 1; - if (read_mmfile(&cur, rr_path(name, "thisimage")) || - read_mmfile(&base, rr_path(name, "preimage")) || - read_mmfile(&other, rr_path(name, "postimage"))) + if (read_mmfile(&cur, rerere_path(name, "thisimage")) || + read_mmfile(&base, rerere_path(name, "preimage")) || + read_mmfile(&other, rerere_path(name, "postimage"))) return 1; ret = xdl_merge(&base, &cur, "", &other, "", &xpp, XDL_MERGE_ZEALOUS, &result); @@ -291,7 +291,7 @@ static int do_plain_rerere(struct string_list *rr, int fd) string_list_insert(path, rr)->util = hex; if (mkdir(git_path("rr-cache/%s", hex), 0755)) continue; - handle_file(path, NULL, rr_path(hex, "preimage")); + handle_file(path, NULL, rerere_path(hex, "preimage")); fprintf(stderr, "Recorded preimage for '%s'\n", path); } } @@ -307,7 +307,7 @@ static int do_plain_rerere(struct string_list *rr, int fd) const char *path = rr->items[i].string; const char *name = (const char *)rr->items[i].util; - if (has_resolution(name)) { + if (has_rerere_resolution(name)) { if (!merge(name, path)) { if (rerere_autoupdate) string_list_insert(path, &update); @@ -326,7 +326,7 @@ static int do_plain_rerere(struct string_list *rr, int fd) continue; fprintf(stderr, "Recorded resolution for '%s'.\n", path); - copy_file(rr_path(name, "postimage"), path, 0666); + copy_file(rerere_path(name, "postimage"), path, 0666); mark_resolved: rr->items[i].util = NULL; } diff --git a/rerere.h b/rerere.h index f9b0386..13313f3 100644 --- a/rerere.h +++ b/rerere.h @@ -5,5 +5,7 @@ extern int setup_rerere(struct string_list *); extern int rerere(void); +extern const char *rerere_path(const char *hex, const char *file); +extern int has_rerere_resolution(const char *hex); #endif -- cgit v0.10.2-6-g49f6 From c64d84f1452ec56fd1586493a0b0707bf7442c42 Mon Sep 17 00:00:00 2001 From: Jeremy White Date: Thu, 12 Feb 2009 08:58:12 -0600 Subject: imap.preformattedHTML to tell Thunderbird to send non-flowed text Many e-mail based development communities require non-flowed text to carry patches to prevent whitespaces from getting mangled, but there is no easy way to tell Thunderbird MUA not to use format=flowed, unless you configure it to do so unconditionally for all outgoing mails. A workaround for users who use git-imap-send is to wrap the patch in "pre" element in the draft folder as an HTML message, and tell Thunderbird to send "text only". Thunderbird turns such a message into a non-flowed plain text when sending it out, which is what we want for patch e-mails. Signed-off-by: Jeremy White Signed-off-by: Junio C Hamano diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt index 1685f04..024084b 100644 --- a/Documentation/git-imap-send.txt +++ b/Documentation/git-imap-send.txt @@ -64,6 +64,13 @@ imap.sslverify:: used by the SSL/TLS connection. Default is `true`. Ignored when imap.tunnel is set. +imap.preformattedHTML:: + A boolean to enable/disable the use of html encoding when sending + a patch. An html encoded patch will be bracketed with
+	and have a content type of text/html.  Ironically, enabling this
+	option causes Thunderbird to send the patch as a plain/text,
+	format=fixed email.  Default is `false`.
+
 Examples
 ~~~~~~~~
 
diff --git a/imap-send.c b/imap-send.c
index f91293c..cb518eb 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -135,6 +135,7 @@ struct imap_server_conf {
 	char *pass;
 	int use_ssl;
 	int ssl_verify;
+	int use_html;
 };
 
 struct imap_store_conf {
@@ -1263,6 +1264,53 @@ static int imap_store_msg(struct store *gctx, struct msg_data *data, int *uid)
 	return DRV_OK;
 }
 
+static void encode_html_chars(struct strbuf *p)
+{
+	int i;
+	for (i = 0; i < p->len; i++) {
+		if (p->buf[i] == '&')
+			strbuf_splice(p, i, 1, "&", 5);
+		if (p->buf[i] == '<')
+			strbuf_splice(p, i, 1, "<", 4);
+		if (p->buf[i] == '>')
+			strbuf_splice(p, i, 1, ">", 4);
+		if (p->buf[i] == '"')
+			strbuf_splice(p, i, 1, """, 6);
+	}
+}
+static void wrap_in_html(struct msg_data *msg)
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct strbuf **lines;
+	struct strbuf **p;
+	static char *content_type = "Content-Type: text/html;\n";
+	static char *pre_open = "
\n";
+	static char *pre_close = "
\n"; + int added_header = 0; + + strbuf_attach(&buf, msg->data, msg->len, msg->len); + lines = strbuf_split(&buf, '\n'); + strbuf_release(&buf); + for (p = lines; *p; p++) { + if (! added_header) { + if ((*p)->len == 1 && *((*p)->buf) == '\n') { + strbuf_addstr(&buf, content_type); + strbuf_addbuf(&buf, *p); + strbuf_addstr(&buf, pre_open); + added_header = 1; + continue; + } + } + else + encode_html_chars(*p); + strbuf_addbuf(&buf, *p); + } + strbuf_addstr(&buf, pre_close); + strbuf_list_free(lines); + msg->len = buf.len; + msg->data = strbuf_detach(&buf, NULL); +} + #define CHUNKSIZE 0x1000 static int read_message(FILE *f, struct msg_data *msg) @@ -1339,6 +1387,7 @@ static struct imap_server_conf server = { NULL, /* pass */ 0, /* use_ssl */ 1, /* ssl_verify */ + 0, /* use_html */ }; static char *imap_folder; @@ -1377,6 +1426,8 @@ static int git_imap_config(const char *key, const char *val, void *cb) server.tunnel = xstrdup(val); else if (!strcmp("sslverify", key)) server.ssl_verify = git_config_bool(key, val); + else if (!strcmp("preformattedHTML", key)) + server.use_html = git_config_bool(key, val); return 0; } @@ -1439,6 +1490,8 @@ int main(int argc, char **argv) fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total); if (!split_msg(&all_msgs, &msg, &ofs)) break; + if (server.use_html) + wrap_in_html(&msg); r = imap_store_msg(ctx, &msg, &uid); if (r != DRV_OK) break; -- cgit v0.10.2-6-g49f6 From 45e2b6140147d7a8b2cd68399e4d52d5d8d7b5be Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 18 Feb 2009 19:14:59 +0100 Subject: Avoid segfault with 'git branch' when the HEAD is detached A recent addition to the ref_item struct was not taken care of, leading to a segmentation fault when accessing the (uninitialized) "dest" member. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin-branch.c b/builtin-branch.c index 7607f6a..6106a1a 100644 --- a/builtin-branch.c +++ b/builtin-branch.c @@ -432,7 +432,9 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str is_descendant_of(head_commit, with_commit)) { struct ref_item item; item.name = xstrdup("(no branch)"); + item.len = strlen(item.name); item.kind = REF_LOCAL_BRANCH; + item.dest = NULL; item.commit = head_commit; if (strlen(item.name) > ref_list.maxwidth) ref_list.maxwidth = strlen(item.name); -- cgit v0.10.2-6-g49f6 From 7c4c97c0ac0cd66861d0c2b8bd7a47ed3c523ea7 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 16 Feb 2009 13:20:25 +0100 Subject: Turn the flags in struct dir_struct into a single variable By having flags represented as bits in the new member variable 'flags', it will be easier to use parse_options when dir_struct is involved. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin-add.c b/builtin-add.c index ac98c83..c986a3a 100644 --- a/builtin-add.c +++ b/builtin-add.c @@ -104,7 +104,7 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec, /* Set up the default git porcelain excludes */ memset(dir, 0, sizeof(*dir)); if (!ignored_too) { - dir->collect_ignored = 1; + dir->flags |= DIR_COLLECT_IGNORED; setup_standard_excludes(dir); } diff --git a/builtin-checkout.c b/builtin-checkout.c index 20b34ce..5b4921d 100644 --- a/builtin-checkout.c +++ b/builtin-checkout.c @@ -405,7 +405,7 @@ static int merge_working_tree(struct checkout_opts *opts, topts.verbose_update = !opts->quiet; topts.fn = twoway_merge; topts.dir = xcalloc(1, sizeof(*topts.dir)); - topts.dir->show_ignored = 1; + topts.dir->flags |= DIR_SHOW_IGNORED; topts.dir->exclude_per_dir = ".gitignore"; tree = parse_tree_indirect(old->commit->object.sha1); init_tree_desc(&trees[0], tree->buffer, tree->size); diff --git a/builtin-clean.c b/builtin-clean.c index f78c2fb..c5ad33d 100644 --- a/builtin-clean.c +++ b/builtin-clean.c @@ -60,7 +60,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) memset(&dir, 0, sizeof(dir)); if (ignored_only) - dir.show_ignored = 1; + dir.flags |= DIR_SHOW_IGNORED; if (ignored && ignored_only) die("-x and -X cannot be used together"); @@ -69,7 +69,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) die("clean.requireForce%s set and -n or -f not given; " "refusing to clean", config_set ? "" : " not"); - dir.show_other_directories = 1; + dir.flags |= DIR_SHOW_OTHER_DIRECTORIES; if (!ignored) setup_standard_excludes(&dir); diff --git a/builtin-ls-files.c b/builtin-ls-files.c index 9dec282..d36f80b 100644 --- a/builtin-ls-files.c +++ b/builtin-ls-files.c @@ -174,7 +174,8 @@ static void show_files(struct dir_struct *dir, const char *prefix) for (i = 0; i < active_nr; i++) { struct cache_entry *ce = active_cache[i]; int dtype = ce_to_dtype(ce); - if (excluded(dir, ce->name, &dtype) != dir->show_ignored) + if (excluded(dir, ce->name, &dtype) != + !!(dir->flags & DIR_SHOW_IGNORED)) continue; if (show_unmerged && !ce_stage(ce)) continue; @@ -189,7 +190,8 @@ static void show_files(struct dir_struct *dir, const char *prefix) struct stat st; int err; int dtype = ce_to_dtype(ce); - if (excluded(dir, ce->name, &dtype) != dir->show_ignored) + if (excluded(dir, ce->name, &dtype) != + !!(dir->flags & DIR_SHOW_IGNORED)) continue; if (ce->ce_flags & CE_UPDATE) continue; @@ -432,7 +434,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix) continue; } if (!strcmp(arg, "-i") || !strcmp(arg, "--ignored")) { - dir.show_ignored = 1; + dir.flags |= DIR_SHOW_IGNORED; require_work_tree = 1; continue; } @@ -446,11 +448,11 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix) continue; } if (!strcmp(arg, "--directory")) { - dir.show_other_directories = 1; + dir.flags |= DIR_SHOW_OTHER_DIRECTORIES; continue; } if (!strcmp(arg, "--no-empty-directory")) { - dir.hide_empty_directories = 1; + dir.flags |= DIR_HIDE_EMPTY_DIRECTORIES; continue; } if (!strcmp(arg, "-u") || !strcmp(arg, "--unmerged")) { @@ -542,7 +544,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix) ps_matched = xcalloc(1, num); } - if (dir.show_ignored && !exc_given) { + if ((dir.flags & DIR_SHOW_IGNORED) && !exc_given) { fprintf(stderr, "%s: --ignored needs some exclude pattern\n", argv[0]); exit(1); diff --git a/builtin-merge.c b/builtin-merge.c index 6d2160d..4c11935 100644 --- a/builtin-merge.c +++ b/builtin-merge.c @@ -636,7 +636,7 @@ static int checkout_fast_forward(unsigned char *head, unsigned char *remote) memset(&opts, 0, sizeof(opts)); memset(&t, 0, sizeof(t)); memset(&dir, 0, sizeof(dir)); - dir.show_ignored = 1; + dir.flags |= DIR_SHOW_IGNORED; dir.exclude_per_dir = ".gitignore"; opts.dir = &dir; diff --git a/builtin-read-tree.c b/builtin-read-tree.c index 38fef34..8e02738 100644 --- a/builtin-read-tree.c +++ b/builtin-read-tree.c @@ -170,7 +170,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) die("more than one --exclude-per-directory are given."); dir = xcalloc(1, sizeof(*opts.dir)); - dir->show_ignored = 1; + dir->flags |= DIR_SHOW_IGNORED; dir->exclude_per_dir = arg + 24; opts.dir = dir; /* We do not need to nor want to do read-directory diff --git a/dir.c b/dir.c index cfd1ea5..5011cb4 100644 --- a/dir.c +++ b/dir.c @@ -487,14 +487,14 @@ static enum directory_treatment treat_directory(struct dir_struct *dir, return recurse_into_directory; case index_gitdir: - if (dir->show_other_directories) + if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES) return ignore_directory; return show_directory; case index_nonexistent: - if (dir->show_other_directories) + if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES) break; - if (!dir->no_gitlinks) { + if (!(dir->flags & DIR_NO_GITLINKS)) { unsigned char sha1[20]; if (resolve_gitlink_ref(dirname, "HEAD", sha1) == 0) return show_directory; @@ -503,7 +503,7 @@ static enum directory_treatment treat_directory(struct dir_struct *dir, } /* This is the "show_other_directories" case */ - if (!dir->hide_empty_directories) + if (!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES)) return show_directory; if (!read_directory_recursive(dir, dirname, dirname, len, 1, simplify)) return ignore_directory; @@ -601,7 +601,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co dtype = DTYPE(de); exclude = excluded(dir, fullname, &dtype); - if (exclude && dir->collect_ignored + if (exclude && (dir->flags & DIR_COLLECT_IGNORED) && in_pathspec(fullname, baselen + len, simplify)) dir_add_ignored(dir, fullname, baselen + len); @@ -609,7 +609,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co * Excluded? If we don't explicitly want to show * ignored files, ignore it */ - if (exclude && !dir->show_ignored) + if (exclude && !(dir->flags & DIR_SHOW_IGNORED)) continue; if (dtype == DT_UNKNOWN) @@ -621,7 +621,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co * even if we don't ignore them, since the * directory may contain files that we do.. */ - if (!exclude && dir->show_ignored) { + if (!exclude && (dir->flags & DIR_SHOW_IGNORED)) { if (dtype != DT_DIR) continue; } @@ -634,7 +634,8 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co len++; switch (treat_directory(dir, fullname, baselen + len, simplify)) { case show_directory: - if (exclude != dir->show_ignored) + if (exclude != !!(dir->flags + & DIR_SHOW_IGNORED)) continue; break; case recurse_into_directory: diff --git a/dir.h b/dir.h index bdc2d47..541286a 100644 --- a/dir.h +++ b/dir.h @@ -34,11 +34,13 @@ struct exclude_stack { struct dir_struct { int nr, alloc; int ignored_nr, ignored_alloc; - unsigned int show_ignored:1, - show_other_directories:1, - hide_empty_directories:1, - no_gitlinks:1, - collect_ignored:1; + enum { + DIR_SHOW_IGNORED = 1<<0, + DIR_SHOW_OTHER_DIRECTORIES = 1<<1, + DIR_HIDE_EMPTY_DIRECTORIES = 1<<2, + DIR_NO_GITLINKS = 1<<3, + DIR_COLLECT_IGNORED = 1<<4 + } flags; struct dir_entry **entries; struct dir_entry **ignored; diff --git a/wt-status.c b/wt-status.c index 96ff2f8..f2eae20 100644 --- a/wt-status.c +++ b/wt-status.c @@ -250,10 +250,9 @@ static void wt_status_print_untracked(struct wt_status *s) memset(&dir, 0, sizeof(dir)); - if (!s->untracked) { - dir.show_other_directories = 1; - dir.hide_empty_directories = 1; - } + if (!s->untracked) + dir.flags |= + DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES; setup_standard_excludes(&dir); read_directory(&dir, ".", "", 0, NULL); -- cgit v0.10.2-6-g49f6 From ce8e8804068772a24354ee24641ca65ffa417837 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Tue, 17 Feb 2009 15:27:11 +0100 Subject: parse-opt: migrate builtin-ls-files. Signed-off-by: Miklos Vajna Signed-off-by: Junio C Hamano diff --git a/builtin-ls-files.c b/builtin-ls-files.c index d36f80b..1742c0f 100644 --- a/builtin-ls-files.c +++ b/builtin-ls-files.c @@ -10,6 +10,7 @@ #include "dir.h" #include "builtin.h" #include "tree.h" +#include "parse-options.h" static int abbrev; static int show_deleted; @@ -28,6 +29,7 @@ static const char **pathspec; static int error_unmatch; static char *ps_matched; static const char *with_tree; +static int exc_given; static const char *tag_cached = ""; static const char *tag_unmerged = ""; @@ -376,156 +378,139 @@ int report_path_error(const char *ps_matched, const char **pathspec, int prefix_ return errors; } -static const char ls_files_usage[] = - "git ls-files [-z] [-t] [-v] (--[cached|deleted|others|stage|unmerged|killed|modified])* " - "[ --ignored ] [--exclude=] [--exclude-from=] " - "[ --exclude-per-directory= ] [--exclude-standard] " - "[--full-name] [--abbrev] [--] []*"; +static const char * const ls_files_usage[] = { + "git ls-files [options] []*", + NULL +}; + +static int option_parse_z(const struct option *opt, + const char *arg, int unset) +{ + line_terminator = unset ? '\n' : '\0'; + + return 0; +} + +static int option_parse_exclude(const struct option *opt, + const char *arg, int unset) +{ + struct exclude_list *list = opt->value; + + exc_given = 1; + add_exclude(arg, "", 0, list); + + return 0; +} + +static int option_parse_exclude_from(const struct option *opt, + const char *arg, int unset) +{ + struct dir_struct *dir = opt->value; + + exc_given = 1; + add_excludes_from_file(dir, arg); + + return 0; +} + +static int option_parse_exclude_standard(const struct option *opt, + const char *arg, int unset) +{ + struct dir_struct *dir = opt->value; + + exc_given = 1; + setup_standard_excludes(dir); + + return 0; +} int cmd_ls_files(int argc, const char **argv, const char *prefix) { - int i; - int exc_given = 0, require_work_tree = 0; + int require_work_tree = 0, show_tag = 0; struct dir_struct dir; + struct option builtin_ls_files_options[] = { + { OPTION_CALLBACK, 'z', NULL, NULL, NULL, + "paths are separated with NUL character", + PARSE_OPT_NOARG, option_parse_z }, + OPT_BOOLEAN('t', NULL, &show_tag, + "identify the file status with tags"), + OPT_BOOLEAN('v', NULL, &show_valid_bit, + "use lowercase letters for 'assume unchanged' files"), + OPT_BOOLEAN('c', "cached", &show_cached, + "show cached files in the output (default)"), + OPT_BOOLEAN('d', "deleted", &show_deleted, + "show deleted files in the output"), + OPT_BOOLEAN('m', "modified", &show_modified, + "show modified files in the output"), + OPT_BOOLEAN('o', "others", &show_others, + "show other files in the output"), + OPT_BIT('i', "ignored", &dir.flags, + "show ignored files in the output", + DIR_SHOW_IGNORED), + OPT_BOOLEAN('s', "stage", &show_stage, + "show staged contents' object name in the output"), + OPT_BOOLEAN('k', "killed", &show_killed, + "show files on the filesystem that need to be removed"), + OPT_BIT(0, "directory", &dir.flags, + "show 'other' directories' name only", + DIR_SHOW_OTHER_DIRECTORIES), + OPT_BIT(0, "empty-directory", &dir.flags, + "list empty directories", + DIR_HIDE_EMPTY_DIRECTORIES), + OPT_BOOLEAN('u', "unmerged", &show_unmerged, + "show unmerged files in the output"), + { OPTION_CALLBACK, 'x', "exclude", &dir.exclude_list[EXC_CMDL], "pattern", + "skip files matching pattern", + 0, option_parse_exclude }, + { OPTION_CALLBACK, 'X', "exclude-from", &dir, "file", + "exclude patterns are read from ", + 0, option_parse_exclude_from }, + OPT_STRING(0, "exclude-per-directory", &dir.exclude_per_dir, "file", + "read additional per-directory exclude patterns in "), + { OPTION_CALLBACK, 0, "exclude-standard", &dir, NULL, + "add the standard git exclusions", + PARSE_OPT_NOARG, option_parse_exclude_standard }, + { OPTION_SET_INT, 0, "full-name", &prefix_offset, NULL, + "make the output relative to the project top directory", + PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL }, + OPT_BOOLEAN(0, "error-unmatch", &error_unmatch, + "if any is not in the index, treat this as an error"), + OPT_STRING(0, "with-tree", &with_tree, "tree-ish", + "pretend that paths removed since are still present"), + OPT__ABBREV(&abbrev), + OPT_END() + }; memset(&dir, 0, sizeof(dir)); if (prefix) prefix_offset = strlen(prefix); git_config(git_default_config, NULL); - for (i = 1; i < argc; i++) { - const char *arg = argv[i]; - - if (!strcmp(arg, "--")) { - i++; - break; - } - if (!strcmp(arg, "-z")) { - line_terminator = 0; - continue; - } - if (!strcmp(arg, "-t") || !strcmp(arg, "-v")) { - tag_cached = "H "; - tag_unmerged = "M "; - tag_removed = "R "; - tag_modified = "C "; - tag_other = "? "; - tag_killed = "K "; - if (arg[1] == 'v') - show_valid_bit = 1; - continue; - } - if (!strcmp(arg, "-c") || !strcmp(arg, "--cached")) { - show_cached = 1; - continue; - } - if (!strcmp(arg, "-d") || !strcmp(arg, "--deleted")) { - show_deleted = 1; - continue; - } - if (!strcmp(arg, "-m") || !strcmp(arg, "--modified")) { - show_modified = 1; - require_work_tree = 1; - continue; - } - if (!strcmp(arg, "-o") || !strcmp(arg, "--others")) { - show_others = 1; - require_work_tree = 1; - continue; - } - if (!strcmp(arg, "-i") || !strcmp(arg, "--ignored")) { - dir.flags |= DIR_SHOW_IGNORED; - require_work_tree = 1; - continue; - } - if (!strcmp(arg, "-s") || !strcmp(arg, "--stage")) { - show_stage = 1; - continue; - } - if (!strcmp(arg, "-k") || !strcmp(arg, "--killed")) { - show_killed = 1; - require_work_tree = 1; - continue; - } - if (!strcmp(arg, "--directory")) { - dir.flags |= DIR_SHOW_OTHER_DIRECTORIES; - continue; - } - if (!strcmp(arg, "--no-empty-directory")) { - dir.flags |= DIR_HIDE_EMPTY_DIRECTORIES; - continue; - } - if (!strcmp(arg, "-u") || !strcmp(arg, "--unmerged")) { - /* There's no point in showing unmerged unless - * you also show the stage information. - */ - show_stage = 1; - show_unmerged = 1; - continue; - } - if (!strcmp(arg, "-x") && i+1 < argc) { - exc_given = 1; - add_exclude(argv[++i], "", 0, &dir.exclude_list[EXC_CMDL]); - continue; - } - if (!prefixcmp(arg, "--exclude=")) { - exc_given = 1; - add_exclude(arg+10, "", 0, &dir.exclude_list[EXC_CMDL]); - continue; - } - if (!strcmp(arg, "-X") && i+1 < argc) { - exc_given = 1; - add_excludes_from_file(&dir, argv[++i]); - continue; - } - if (!prefixcmp(arg, "--exclude-from=")) { - exc_given = 1; - add_excludes_from_file(&dir, arg+15); - continue; - } - if (!prefixcmp(arg, "--exclude-per-directory=")) { - exc_given = 1; - dir.exclude_per_dir = arg + 24; - continue; - } - if (!strcmp(arg, "--exclude-standard")) { - exc_given = 1; - setup_standard_excludes(&dir); - continue; - } - if (!strcmp(arg, "--full-name")) { - prefix_offset = 0; - continue; - } - if (!strcmp(arg, "--error-unmatch")) { - error_unmatch = 1; - continue; - } - if (!prefixcmp(arg, "--with-tree=")) { - with_tree = arg + 12; - continue; - } - if (!prefixcmp(arg, "--abbrev=")) { - abbrev = strtoul(arg+9, NULL, 10); - if (abbrev && abbrev < MINIMUM_ABBREV) - abbrev = MINIMUM_ABBREV; - else if (abbrev > 40) - abbrev = 40; - continue; - } - if (!strcmp(arg, "--abbrev")) { - abbrev = DEFAULT_ABBREV; - continue; - } - if (*arg == '-') - usage(ls_files_usage); - break; + argc = parse_options(argc, argv, builtin_ls_files_options, + ls_files_usage, 0); + if (show_tag || show_valid_bit) { + tag_cached = "H "; + tag_unmerged = "M "; + tag_removed = "R "; + tag_modified = "C "; + tag_other = "? "; + tag_killed = "K "; } + if (show_modified || show_others || (dir.flags & DIR_SHOW_IGNORED) || show_killed) + require_work_tree = 1; + if (show_unmerged) + /* + * There's no point in showing unmerged unless + * you also show the stage information. + */ + show_stage = 1; + if (dir.exclude_per_dir) + exc_given = 1; if (require_work_tree && !is_inside_work_tree()) setup_work_tree(); - pathspec = get_pathspec(prefix, argv + i); + pathspec = get_pathspec(prefix, argv); /* be nice with submodule patsh ending in a slash */ read_cache(); -- cgit v0.10.2-6-g49f6 From 36419c8ee41cecadf67dfeab2808ff2e5025ca52 Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Wed, 18 Feb 2009 23:18:03 +0100 Subject: check_updates(): effective removal of cache entries marked CE_REMOVE Below is oprofile output from GIT command 'git chekcout -q my-v2.6.25' (move from tag v2.6.27 to tag v2.6.25 of the Linux kernel): CPU: Core 2, speed 1999.95 MHz (estimated) Counted CPU_CLK_UNHALTED events (Clock cycles when not halted) with a unit mask of 0x00 (Unhalted core cycles) count 20000 Counted INST_RETIRED_ANY_P events (number of instructions retired) with a unit mask of 0x00 (No unit mask) count 20000 CPU_CLK_UNHALT...|INST_RETIRED:2...| samples| %| samples| %| ------------------------------------ 409247 100.000 342878 100.000 git CPU_CLK_UNHALT...|INST_RETIRED:2...| samples| %| samples| %| ------------------------------------ 260476 63.6476 257843 75.1996 libz.so.1.2.3 100876 24.6492 64378 18.7758 kernel-2.6.28.4_2.vmlinux 30850 7.5382 7874 2.2964 libc-2.9.so 14775 3.6103 8390 2.4469 git 2020 0.4936 4325 1.2614 libcrypto.so.0.9.8 191 0.0467 32 0.0093 libpthread-2.9.so 58 0.0142 36 0.0105 ld-2.9.so 1 2.4e-04 0 0 libldap-2.3.so.0.2.31 Detail list of the top 20 function entries (libz counted in one blob): CPU_CLK_UNHALTED INST_RETIRED_ANY_P samples % samples % image name symbol name 260476 63.6862 257843 75.2725 libz.so.1.2.3 /lib/libz.so.1.2.3 16587 4.0555 3636 1.0615 libc-2.9.so memcpy 7710 1.8851 277 0.0809 libc-2.9.so memmove 3679 0.8995 1108 0.3235 kernel-2.6.28.4_2.vmlinux d_validate 3546 0.8670 2607 0.7611 kernel-2.6.28.4_2.vmlinux __getblk 3174 0.7760 1813 0.5293 libc-2.9.so _int_malloc 2396 0.5858 3681 1.0746 kernel-2.6.28.4_2.vmlinux copy_to_user 2270 0.5550 2528 0.7380 kernel-2.6.28.4_2.vmlinux __link_path_walk 2205 0.5391 1797 0.5246 kernel-2.6.28.4_2.vmlinux ext4_mark_iloc_dirty 2103 0.5142 1203 0.3512 kernel-2.6.28.4_2.vmlinux find_first_zero_bit 2077 0.5078 997 0.2911 kernel-2.6.28.4_2.vmlinux do_get_write_access 2070 0.5061 514 0.1501 git cache_name_compare 2043 0.4995 1501 0.4382 kernel-2.6.28.4_2.vmlinux rcu_irq_exit 2022 0.4944 1732 0.5056 kernel-2.6.28.4_2.vmlinux __ext4_get_inode_loc 2020 0.4939 4325 1.2626 libcrypto.so.0.9.8 /usr/lib/libcrypto.so.0.9.8 1965 0.4804 1384 0.4040 git patch_delta 1708 0.4176 984 0.2873 kernel-2.6.28.4_2.vmlinux rcu_sched_grace_period 1682 0.4112 727 0.2122 kernel-2.6.28.4_2.vmlinux sysfs_slab_alias 1659 0.4056 290 0.0847 git find_pack_entry_one 1480 0.3619 1307 0.3816 kernel-2.6.28.4_2.vmlinux ext4_writepage_trans_blocks Notice the memmove line, where the CPU did 7710 / 277 = 27.8 cycles per instruction, and compared to the total cycles spent inside the source code of GIT for this command, all the memmove() calls translates to (7710 * 100) / 14775 = 52.2% of this. Retesting with a GIT program compiled for gcov usage, I found out that the memmove() calls came from remove_index_entry_at() in read-cache.c, where we have: memmove(istate->cache + pos, istate->cache + pos + 1, (istate->cache_nr - pos) * sizeof(struct cache_entry *)); remove_index_entry_at() is called 4902 times from check_updates() in unpack-trees.c, and each time called we move each cache_entry pointers (from the removed one) one step to the left. Since we have 28828 entries in the cache this time, and if we on average move half of them each time, we in total move approximately 4902 * 0.5 * 28828 * 4 = 282 629 712 bytes, or twice this amount if each pointer is 8 bytes (64 bit). OK, is seems that the function check_updates() is called 28 times, so the estimated guess above had been more correct if check_updates() had been called only once, but the point is: we get lots of bytes moved. To fix this, and use an O(N) algorithm instead, where N is the number of cache_entries, we delete/remove all entries in one loop through all entries. From a retest, the new remove_marked_cache_entries() from the patch below, ended up with the following output line from oprofile: 46 0.0105 15 0.0041 git remove_marked_cache_entries If we can trust the numbers from oprofile in this case, we saved approximately ((7710 - 46) * 20000) / (2 * 1000 * 1000 * 1000) = 0.077 seconds CPU time with this fix for this particular test. And notice that now the CPU did only 46 / 15 = 3.1 cycles/instruction. Signed-off-by: Kjetil Barvik Acked-by: Linus Torvalds Signed-off-by: Junio C Hamano diff --git a/cache.h b/cache.h index 1bf2d4b..770d8bc 100644 --- a/cache.h +++ b/cache.h @@ -445,6 +445,7 @@ extern int add_index_entry(struct index_state *, struct cache_entry *ce, int opt extern struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really); extern void rename_index_entry_at(struct index_state *, int pos, const char *new_name); extern int remove_index_entry_at(struct index_state *, int pos); +extern void remove_marked_cache_entries(struct index_state *istate); extern int remove_file_from_index(struct index_state *, const char *path); #define ADD_CACHE_VERBOSE 1 #define ADD_CACHE_PRETEND 2 diff --git a/read-cache.c b/read-cache.c index 940ec76..59a274b 100644 --- a/read-cache.c +++ b/read-cache.c @@ -443,6 +443,26 @@ int remove_index_entry_at(struct index_state *istate, int pos) return 1; } +/* + * Remove all cache ententries marked for removal, that is where + * CE_REMOVE is set in ce_flags. This is much more effective than + * calling remove_index_entry_at() for each entry to be removed. + */ +void remove_marked_cache_entries(struct index_state *istate) +{ + struct cache_entry **ce_array = istate->cache; + unsigned int i, j; + + for (i = j = 0; i < istate->cache_nr; i++) { + if (ce_array[i]->ce_flags & CE_REMOVE) + remove_name_hash(ce_array[i]); + else + ce_array[j++] = ce_array[i]; + } + istate->cache_changed = 1; + istate->cache_nr = j; +} + int remove_file_from_index(struct index_state *istate, const char *path) { int pos = index_name_pos(istate, path, strlen(path)); diff --git a/unpack-trees.c b/unpack-trees.c index e3c3fa1..273b5da 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -93,11 +93,9 @@ static int check_updates(struct unpack_trees_options *o) display_progress(progress, ++cnt); if (o->update) unlink_entry(ce); - remove_index_entry_at(&o->result, i); - i--; - continue; } } + remove_marked_cache_entries(&o->result); remove_scheduled_dirs(); for (i = 0; i < index->cache_nr; i++) { -- cgit v0.10.2-6-g49f6 From 66648ad7fed840adef0343a1e0bf5188d32f5569 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 18 Feb 2009 22:35:45 -0500 Subject: branch: clean up repeated strlen Commit 45e2b61 fixed the initialization of a "len" struct parameter via strlen. We can use that to clean up what is now 3 strlens in a 6-line sequence. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/builtin-branch.c b/builtin-branch.c index 6106a1a..b15d351 100644 --- a/builtin-branch.c +++ b/builtin-branch.c @@ -436,8 +436,8 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str item.kind = REF_LOCAL_BRANCH; item.dest = NULL; item.commit = head_commit; - if (strlen(item.name) > ref_list.maxwidth) - ref_list.maxwidth = strlen(item.name); + if (item.len > ref_list.maxwidth) + ref_list.maxwidth = item.len; print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1, ""); free(item.name); } -- cgit v0.10.2-6-g49f6 From 0afc304406196e4470fd2a628c3733e966068d98 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 18 Feb 2009 22:34:44 -0500 Subject: add basic branch display tests We were not testing the output of "git branch" anywhere. Not only does this not protect us against regressions in the output, but we are not exercising code paths which may have bugs (such as the one fixed by 45e2b61). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh new file mode 100755 index 0000000..809d1c4 --- /dev/null +++ b/t/t3203-branch-output.sh @@ -0,0 +1,81 @@ +#!/bin/sh + +test_description='git branch display tests' +. ./test-lib.sh + +test_expect_success 'make commits' ' + echo content >file && + git add file && + git commit -m one && + echo content >>file && + git commit -a -m two +' + +test_expect_success 'make branches' ' + git branch branch-one + git branch branch-two HEAD^ +' + +test_expect_success 'make remote branches' ' + git update-ref refs/remotes/origin/branch-one branch-one + git update-ref refs/remotes/origin/branch-two branch-two + git symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/branch-one +' + +cat >expect <<'EOF' + branch-one + branch-two +* master +EOF +test_expect_success 'git branch shows local branches' ' + git branch >actual && + test_cmp expect actual +' + +cat >expect <<'EOF' + origin/HEAD -> origin/branch-one + origin/branch-one + origin/branch-two +EOF +test_expect_success 'git branch -r shows remote branches' ' + git branch -r >actual && + test_cmp expect actual +' + +cat >expect <<'EOF' + branch-one + branch-two +* master + remotes/origin/HEAD -> origin/branch-one + remotes/origin/branch-one + remotes/origin/branch-two +EOF +test_expect_success 'git branch -a shows local and remote branches' ' + git branch -a >actual && + test_cmp expect actual +' + +cat >expect <<'EOF' +two +one +two +EOF +test_expect_success 'git branch -v shows branch summaries' ' + git branch -v >tmp && + awk "{print \$NF}" actual && + test_cmp expect actual +' + +cat >expect <<'EOF' +* (no branch) + branch-one + branch-two + master +EOF +test_expect_success 'git branch shows detached HEAD properly' ' + git checkout HEAD^0 && + git branch >actual && + test_cmp expect actual +' + +test_done -- cgit v0.10.2-6-g49f6 From 8cd6192e16d8bf590afa1105c840b72106d72941 Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Thu, 19 Feb 2009 21:08:28 +0100 Subject: fix compile error when USE_NSEC is defined 'struct cache' does not have a 'usec' member, but a 'unsigned int nsec' member. Simmilar 'struct stat' does not have a 'st_mtim.usec' member, and we should instead use 'st_mtim.tv_nsec'. Signed-off-by: Kjetil Barvik Signed-off-by: Junio C Hamano diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c index 67fb80e..3b210c7 100644 --- a/builtin-fetch-pack.c +++ b/builtin-fetch-pack.c @@ -802,14 +802,14 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args, mtime.sec = st.st_mtime; #ifdef USE_NSEC - mtime.usec = st.st_mtim.usec; + mtime.nsec = st.st_mtim.tv_nsec; #endif if (stat(shallow, &st)) { if (mtime.sec) die("shallow file was removed during fetch"); } else if (st.st_mtime != mtime.sec #ifdef USE_NSEC - || st.st_mtim.usec != mtime.usec + || st.st_mtim.tv_nsec != mtime.nsec #endif ) die("shallow file was changed during fetch"); -- cgit v0.10.2-6-g49f6 From fba2f38a2c2cda458e490c18e0afbb12cbd37969 Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Thu, 19 Feb 2009 21:08:29 +0100 Subject: make USE_NSEC work as expected Since the filesystem ext4 is now defined as stable in Linux v2.6.28, and ext4 supports nanonsecond resolution timestamps natively, it is time to make USE_NSEC work as expected. This will make racy git situations less likely to happen. For 'git checkout' this means it will be less likely that we have to open, read the contents of the file into RAM, and check if file is really modified or not. The result sould be a litle less used CPU time, less pagefaults and a litle faster program, at least for 'git checkout'. Since the number of possible racy git situations would increase when disks gets faster, this patch would be more and more helpfull as times go by. For a fast Solid State Disk, this patch should be helpfull. Note that, when file operations starts to take less than 1 nanosecond, one would again start to get more racy git situations. For more info on racy git, see Documentation/technical/racy-git.txt For more info on ext4, see http://kernelnewbies.org/Ext4 Signed-off-by: Kjetil Barvik Signed-off-by: Junio C Hamano diff --git a/cache.h b/cache.h index 770d8bc..2badbfe 100644 --- a/cache.h +++ b/cache.h @@ -140,8 +140,8 @@ struct ondisk_cache_entry_extended { }; struct cache_entry { - unsigned int ce_ctime; - unsigned int ce_mtime; + struct cache_time ce_ctime; + struct cache_time ce_mtime; unsigned int ce_dev; unsigned int ce_ino; unsigned int ce_mode; @@ -282,7 +282,7 @@ struct index_state { struct cache_entry **cache; unsigned int cache_nr, cache_alloc, cache_changed; struct cache_tree *cache_tree; - time_t timestamp; + struct cache_time timestamp; void *alloc; unsigned name_hash_initialized : 1, initialized : 1; diff --git a/read-cache.c b/read-cache.c index 59a274b..bb07371 100644 --- a/read-cache.c +++ b/read-cache.c @@ -67,8 +67,15 @@ void rename_index_entry_at(struct index_state *istate, int nr, const char *new_n */ void fill_stat_cache_info(struct cache_entry *ce, struct stat *st) { - ce->ce_ctime = st->st_ctime; - ce->ce_mtime = st->st_mtime; + ce->ce_ctime.sec = (unsigned int)st->st_ctime; + ce->ce_mtime.sec = (unsigned int)st->st_mtime; +#ifdef USE_NSEC + ce->ce_ctime.nsec = (unsigned int)st->st_ctim.tv_nsec; + ce->ce_mtime.nsec = (unsigned int)st->st_mtim.tv_nsec; +#else + ce->ce_ctime.nsec = 0; + ce->ce_mtime.nsec = 0; +#endif ce->ce_dev = st->st_dev; ce->ce_ino = st->st_ino; ce->ce_uid = st->st_uid; @@ -196,11 +203,18 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st) default: die("internal error: ce_mode is %o", ce->ce_mode); } - if (ce->ce_mtime != (unsigned int) st->st_mtime) + if (ce->ce_mtime.sec != (unsigned int)st->st_mtime) changed |= MTIME_CHANGED; - if (trust_ctime && ce->ce_ctime != (unsigned int) st->st_ctime) + if (trust_ctime && ce->ce_ctime.sec != (unsigned int)st->st_ctime) changed |= CTIME_CHANGED; +#ifdef USE_NSEC + if (ce->ce_mtime.nsec != (unsigned int)st->st_mtim.tv_nsec) + changed |= MTIME_CHANGED; + if (trust_ctime && ce->ce_ctime.nsec != (unsigned int)st->st_ctim.tv_nsec) + changed |= CTIME_CHANGED; +#endif + if (ce->ce_uid != (unsigned int) st->st_uid || ce->ce_gid != (unsigned int) st->st_gid) changed |= OWNER_CHANGED; @@ -232,8 +246,16 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st) static int is_racy_timestamp(const struct index_state *istate, struct cache_entry *ce) { return (!S_ISGITLINK(ce->ce_mode) && - istate->timestamp && - ((unsigned int)istate->timestamp) <= ce->ce_mtime); + istate->timestamp.sec && +#ifdef USE_NSEC + /* nanosecond timestamped files can also be racy! */ + (istate->timestamp.sec < ce->ce_mtime.sec || + (istate->timestamp.sec == ce->ce_mtime.sec && + istate->timestamp.nsec <= ce->ce_mtime.nsec)) +#else + istate->timestamp.sec <= ce->ce_mtime.sec +#endif + ); } int ie_match_stat(const struct index_state *istate, @@ -1159,8 +1181,15 @@ static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_en size_t len; const char *name; - ce->ce_ctime = ntohl(ondisk->ctime.sec); - ce->ce_mtime = ntohl(ondisk->mtime.sec); + ce->ce_ctime.sec = ntohl(ondisk->ctime.sec); + ce->ce_mtime.sec = ntohl(ondisk->mtime.sec); +#ifdef USE_NSEC + ce->ce_ctime.nsec = ntohl(ondisk->ctime.nsec); + ce->ce_mtime.nsec = ntohl(ondisk->mtime.nsec); +#else + ce->ce_ctime.nsec = 0; + ce->ce_mtime.nsec = 0; +#endif ce->ce_dev = ntohl(ondisk->dev); ce->ce_ino = ntohl(ondisk->ino); ce->ce_mode = ntohl(ondisk->mode); @@ -1226,7 +1255,8 @@ int read_index_from(struct index_state *istate, const char *path) return istate->cache_nr; errno = ENOENT; - istate->timestamp = 0; + istate->timestamp.sec = 0; + istate->timestamp.nsec = 0; fd = open(path, O_RDONLY); if (fd < 0) { if (errno == ENOENT) @@ -1278,7 +1308,13 @@ int read_index_from(struct index_state *istate, const char *path) src_offset += ondisk_ce_size(ce); dst_offset += ce_size(ce); } - istate->timestamp = st.st_mtime; + istate->timestamp.sec = st.st_mtime; +#ifdef USE_NSEC + istate->timestamp.nsec = (unsigned int)st.st_mtim.tv_nsec; +#else + istate->timestamp.nsec = 0; +#endif + while (src_offset <= mmap_size - 20 - 8) { /* After an array of active_nr index entries, * there can be arbitrary number of extended @@ -1308,14 +1344,15 @@ unmap: int is_index_unborn(struct index_state *istate) { - return (!istate->cache_nr && !istate->alloc && !istate->timestamp); + return (!istate->cache_nr && !istate->alloc && !istate->timestamp.sec); } int discard_index(struct index_state *istate) { istate->cache_nr = 0; istate->cache_changed = 0; - istate->timestamp = 0; + istate->timestamp.sec = 0; + istate->timestamp.nsec = 0; istate->name_hash_initialized = 0; free_hash(&istate->name_hash); cache_tree_free(&(istate->cache_tree)); @@ -1461,10 +1498,15 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce) struct ondisk_cache_entry *ondisk = xcalloc(1, size); char *name; - ondisk->ctime.sec = htonl(ce->ce_ctime); + ondisk->ctime.sec = htonl(ce->ce_ctime.sec); + ondisk->mtime.sec = htonl(ce->ce_mtime.sec); +#ifdef USE_NSEC + ondisk->ctime.nsec = htonl(ce->ce_ctime.nsec); + ondisk->mtime.nsec = htonl(ce->ce_mtime.nsec); +#else ondisk->ctime.nsec = 0; - ondisk->mtime.sec = htonl(ce->ce_mtime); ondisk->mtime.nsec = 0; +#endif ondisk->dev = htonl(ce->ce_dev); ondisk->ino = htonl(ce->ce_ino); ondisk->mode = htonl(ce->ce_mode); diff --git a/unpack-trees.c b/unpack-trees.c index 273b5da..11902cd 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -360,8 +360,12 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options memset(&o->result, 0, sizeof(o->result)); o->result.initialized = 1; - if (o->src_index) - o->result.timestamp = o->src_index->timestamp; + if (o->src_index) { + o->result.timestamp.sec = o->src_index->timestamp.sec; +#ifdef USE_NSEC + o->result.timestamp.nsec = o->src_index->timestamp.nsec; +#endif + } o->merge_size = len; if (!dfc) -- cgit v0.10.2-6-g49f6 From 1dcafcc0e639ecc69b54421bda5f2270ed2601eb Mon Sep 17 00:00:00 2001 From: Kjetil Barvik Date: Thu, 19 Feb 2009 21:08:30 +0100 Subject: verify_uptodate(): add ce_uptodate(ce) test If we inside verify_uptodate() can already tell from the ce entry that it is already uptodate by testing it with ce_uptodate(ce), there is no need to call lstat(2) and ie_match_stat() afterwards. And, reading from the commit log message from: commit eadb5831342bb2e756fa05c03642c4aa1929d4f5 Author: Junio C Hamano Date: Fri Jan 18 23:45:24 2008 -0800 Avoid running lstat(2) on the same cache entry. this also seems to be correct usage of the ce_uptodate() macro introduced by that patch. This will avoid lots of lstat(2) calls in some cases, for example by running the 'git checkout' command. Signed-off-by: Kjetil Barvik Signed-off-by: Junio C Hamano diff --git a/unpack-trees.c b/unpack-trees.c index 11902cd..9fe0cd5 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -430,7 +430,7 @@ static int verify_uptodate(struct cache_entry *ce, { struct stat st; - if (o->index_only || o->reset) + if (o->index_only || o->reset || ce_uptodate(ce)) return 0; if (!lstat(ce->name, &st)) { -- cgit v0.10.2-6-g49f6 From 484cf6c3f1169786c45ccda54c9961ef66465c03 Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 19 Feb 2009 22:26:30 +0100 Subject: format-patch: threading test reactivation t4014 tests format-patch --thread since 7d812145, but the tests were ineffective right from the start at least for bash and dash. The loops of the form for ...; do something || break; done introduced by 7d812145 and 5d02294 always exit with status 0, even if 'something' failed, because 'break' returns 0 unless there was no loop to break. We take a rather different approach that uses an admittedly heinous inline Perl script to mangle all interesting information into a format that is invariant between runs. We can then test the full patch sequence in one go (with --stdout), doing away with the loop problem. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index f045898..345e6de 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -138,56 +138,120 @@ test_expect_success 'multiple files' ' ls patches/0001-Side-changes-1.patch patches/0002-Side-changes-2.patch patches/0003-Side-changes-3-with-n-backslash-n-in-it.patch ' -test_expect_success 'thread' ' +check_threading () { + expect="$1" && + shift && + (git format-patch --stdout "$@"; echo $? > status.out) | + # Prints everything between the Message-ID and In-Reply-To, + # and replaces all Message-ID-lookalikes by a sequence number + perl -ne ' + if (/^(message-id|references|in-reply-to)/i) { + $printing = 1; + } elsif (/^\S/) { + $printing = 0; + } + if ($printing) { + $h{$1}=$i++ if (/<([^>]+)>/ and !exists $h{$1}); + for $k (keys %h) {s/$k/$h{$k}/}; + print; + } + print "---\n" if /^From /i; + ' > actual && + test 0 = "$(cat status.out)" && + test_cmp "$expect" actual +} + +cat >> expect.no-threading <]*>\).*$/\1/") && - for i in patches/0002-* patches/0003-* - do - grep "References: $FIRST_MID" $i && - grep "In-Reply-To: $FIRST_MID" $i || break - done + check_threading expect.no-threading master ' -test_expect_success 'thread in-reply-to' ' +cat > expect.thread < +--- +Message-Id: <1> +In-Reply-To: <0> +References: <0> +--- +Message-Id: <2> +In-Reply-To: <0> +References: <0> +EOF - rm -rf patches/ && - git checkout side && - git format-patch --in-reply-to="" --thread -o patches/ master && - FIRST_MID="" && - for i in patches/* - do - grep "References: $FIRST_MID" $i && - grep "In-Reply-To: $FIRST_MID" $i || break - done +test_expect_success 'thread' ' + check_threading expect.thread --thread master ' -test_expect_success 'thread cover-letter' ' +cat > expect.in-reply-to < +In-Reply-To: <1> +References: <1> +--- +Message-Id: <2> +In-Reply-To: <1> +References: <1> +--- +Message-Id: <3> +In-Reply-To: <1> +References: <1> +EOF - rm -rf patches/ && - git checkout side && - git format-patch --cover-letter --thread -o patches/ master && - FIRST_MID=$(grep "Message-Id:" patches/0000-* | sed "s/^[^<]*\(<[^>]*>\).*$/\1/") && - for i in patches/0001-* patches/0002-* patches/0003-* - do - grep "References: $FIRST_MID" $i && - grep "In-Reply-To: $FIRST_MID" $i || break - done +test_expect_success 'thread in-reply-to' ' + check_threading expect.in-reply-to --in-reply-to="" \ + --thread master ' -test_expect_success 'thread cover-letter in-reply-to' ' +cat > expect.cover-letter < +--- +Message-Id: <1> +In-Reply-To: <0> +References: <0> +--- +Message-Id: <2> +In-Reply-To: <0> +References: <0> +--- +Message-Id: <3> +In-Reply-To: <0> +References: <0> +EOF - rm -rf patches/ && - git checkout side && - git format-patch --cover-letter --in-reply-to="" --thread -o patches/ master && - FIRST_MID="" && - for i in patches/* - do - grep "References: $FIRST_MID" $i && - grep "In-Reply-To: $FIRST_MID" $i || break - done +test_expect_success 'thread cover-letter' ' + check_threading expect.cover-letter --cover-letter --thread master +' + +cat > expect.cl-irt < +In-Reply-To: <1> +References: <1> +--- +Message-Id: <2> +In-Reply-To: <1> +References: <1> +--- +Message-Id: <3> +In-Reply-To: <1> +References: <1> +--- +Message-Id: <4> +In-Reply-To: <1> +References: <1> +EOF + +test_expect_success 'thread cover-letter in-reply-to' ' + check_threading expect.cl-irt --cover-letter \ + --in-reply-to="" --thread master ' test_expect_success 'excessive subject' ' -- cgit v0.10.2-6-g49f6 From 901c369af52ffcc8c08457fb5b330eab217a9cfb Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 19 Feb 2009 12:13:35 +0100 Subject: Support coverage testing with GCC/gcov With gcc's --coverage option, we can perform automatic coverage data collection for the test suite. Add a new Makefile target 'coverage' that scraps all previous coverage results, recompiles git with the required compiler/linker flags (in addition to any flags you specify manually), then runs the test suite and compiles a report. The compilation must be done with all optimizations disabled, since inlined functions (and for line-by-line coverage, also optimized branches/loops) break coverage tracking. The tests are run serially (with -j1). The coverage code should theoretically allow concurrent access to its data files, but the author saw random test failures. Obviously this could be improved. The report currently consists of a list of functions that were never executed during the tests, which is written to 'coverage-untested-functions'. Once this list becomes reasonably short, we would also want to look at branches that were never taken. Currently only toplevel *.c files are considered. It would be nice to at least include xdiff, but --coverage did not save data to subdirectories on the system used to write this (gcc 4.3.2). Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano diff --git a/Makefile b/Makefile index b040a96..c32881b 100644 --- a/Makefile +++ b/Makefile @@ -1640,3 +1640,27 @@ check-docs:: check-builtins:: ./check-builtins.sh +### Test suite coverage testing +# +.PHONY: coverage coverage-clean coverage-build coverage-report + +coverage: + $(MAKE) coverage-build + $(MAKE) coverage-report + +coverage-clean: + rm -f *.gcda *.gcno + +COVERAGE_CFLAGS = $(CFLAGS) -O0 -ftest-coverage -fprofile-arcs +COVERAGE_LDFLAGS = $(CFLAGS) -O0 -lgcov + +coverage-build: coverage-clean + $(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" all + $(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \ + -j1 test + +coverage-report: + gcov -b *.c + grep '^function.*called 0 ' *.c.gcov \ + | sed -e 's/\([^:]*\)\.gcov: *function \([^ ]*\) called.*/\1: \2/' \ + | tee coverage-untested-functions -- cgit v0.10.2-6-g49f6 From 85569d7498f3933d96a7604512f8832c73127067 Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 19 Feb 2009 12:13:36 +0100 Subject: Test that diff can read from stdin Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano diff --git a/t/t4002-diff-basic.sh b/t/t4002-diff-basic.sh index cc3681f..18695ce 100755 --- a/t/t4002-diff-basic.sh +++ b/t/t4002-diff-basic.sh @@ -258,4 +258,12 @@ test_expect_success \ git diff-tree -r -R $tree_A $tree_B >.test-b && cmp -s .test-a .test-b' +test_expect_success \ + 'diff can read from stdin' \ + 'test_must_fail git diff --no-index -- MN - < NN | + grep -v "^index" | sed "s#/-#/NN#" >.test-a && + test_must_fail git diff --no-index -- MN NN | + grep -v "^index" >.test-b && + test_cmp .test-a .test-b' + test_done -- cgit v0.10.2-6-g49f6 From f37bfb7a4d965e821591c98e66fe7a4e396377b5 Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 19 Feb 2009 12:13:37 +0100 Subject: Test diff --dirstat functionality This is only a very rudimentary test, but it was untested before. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index aba5320..f140b9c 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -263,6 +263,7 @@ diff --name-status dir2 dir diff --no-index --name-status dir2 dir diff --no-index --name-status -- dir2 dir diff master master^ side +diff --dirstat master~1 master~2 EOF test_done diff --git a/t/t4013/diff.diff_--dirstat_master~1_master~2 b/t/t4013/diff.diff_--dirstat_master~1_master~2 new file mode 100644 index 0000000..b672e1c --- /dev/null +++ b/t/t4013/diff.diff_--dirstat_master~1_master~2 @@ -0,0 +1,3 @@ +$ git diff --dirstat master~1 master~2 + 40.0% dir/ +$ -- cgit v0.10.2-6-g49f6 From 289e162318bbf4b0f90dd70371046e1b20d4c0be Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 19 Feb 2009 12:13:38 +0100 Subject: Test log --graph So far there were no tests checking that log --graph actually works. Note that the tests strip trailing whitespace, as the current --graph emits trailing whitespace on lines that do not contain anything but graph lines. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 7b976ee..93966f7 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -134,5 +134,153 @@ test_expect_success 'log --grep -i' ' test_cmp expect actual ' +cat > expect <actual && + test_cmp expect actual +' + +test_expect_success 'set up merge history' ' + git checkout -b side HEAD~4 && + test_commit side-1 1 1 && + test_commit side-2 2 2 && + git checkout master && + git merge side +' + +cat > expect <<\EOF +* Merge branch 'side' +|\ +| * side-2 +| * side-1 +* | Second +* | sixth +* | fifth +* | fourth +|/ +* third +* second +* initial +EOF + +test_expect_success 'log --graph with merge' ' + git log --graph --date-order --pretty=tformat:%s | + sed "s/ *$//" >actual && + test_cmp expect actual +' + +cat > expect <<\EOF +* commit master +|\ Merge: A B +| | Author: A U Thor +| | +| | Merge branch 'side' +| | +| * commit side +| | Author: A U Thor +| | +| | side-2 +| | +| * commit tags/side-1 +| | Author: A U Thor +| | +| | side-1 +| | +* | commit master~1 +| | Author: A U Thor +| | +| | Second +| | +* | commit master~2 +| | Author: A U Thor +| | +| | sixth +| | +* | commit master~3 +| | Author: A U Thor +| | +| | fifth +| | +* | commit master~4 +|/ Author: A U Thor +| +| fourth +| +* commit tags/side-1~1 +| Author: A U Thor +| +| third +| +* commit tags/side-1~2 +| Author: A U Thor +| +| second +| +* commit tags/side-1~3 + Author: A U Thor + + initial +EOF + +test_expect_success 'log --graph with full output' ' + git log --graph --date-order --pretty=short | + git name-rev --name-only --stdin | + sed "s/Merge:.*/Merge: A B/;s/ *$//" >actual && + test_cmp expect actual +' + +test_expect_success 'set up more tangled history' ' + git checkout -b tangle HEAD~6 && + test_commit tangle-a tangle-a a && + git merge master~3 && + git merge side~1 && + git checkout master && + git merge tangle +' + +cat > expect <<\EOF +* Merge branch 'tangle' +|\ +| * Merge branch 'side' (early part) into tangle +| |\ +| * \ Merge branch 'master' (early part) into tangle +| |\ \ +| * | | tangle-a +* | | | Merge branch 'side' +|\ \ \ \ +| * | | | side-2 +| | | |/ +| | |/| +| |/| | +| * | | side-1 +* | | | Second +* | | | sixth +| | |/ +| |/| +|/| | +* | | fifth +* | | fourth +|/ / +* | third +|/ +* second +* initial +EOF + +test_expect_success 'log --graph with merge' ' + git log --graph --date-order --pretty=tformat:%s | + sed "s/ *$//" >actual && + test_cmp expect actual +' + test_done -- cgit v0.10.2-6-g49f6 From 02a6552c28eabb524fcd23e8f5bd36e4f5afa63b Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 19 Feb 2009 12:13:39 +0100 Subject: Test fsck a bit harder git-fsck, of all tools, has very few tests. This adds some more: * a corrupted object; * a branch pointing to a non-commit; * a tag pointing to a nonexistent object; * and a tag pointing to an object of a type other than what the tag itself claims. Only the first two are caught. At least the third probably should, too, but currently slips through. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 4597af0..a22632f 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -28,4 +28,71 @@ test_expect_success 'loose objects borrowed from alternate are not missing' ' ) ' +# Corruption tests follow. Make sure to remove all traces of the +# specific corruption you test afterwards, lest a later test trip over +# it. + +test_expect_success 'object with bad sha1' ' + sha=$(echo blob | git hash-object -w --stdin) && + echo $sha && + old=$(echo $sha | sed "s+^..+&/+") && + new=$(dirname $old)/ffffffffffffffffffffffffffffffffffffff && + sha="$(dirname $new)$(basename $new)" + mv .git/objects/$old .git/objects/$new && + git update-index --add --cacheinfo 100644 $sha foo && + tree=$(git write-tree) && + cmt=$(echo bogus | git commit-tree $tree) && + git update-ref refs/heads/bogus $cmt && + (git fsck 2>out; true) && + grep "$sha.*corrupt" out && + rm -f .git/objects/$new && + git update-ref -d refs/heads/bogus && + git read-tree -u --reset HEAD +' + +test_expect_success 'branch pointing to non-commit' ' + git rev-parse HEAD^{tree} > .git/refs/heads/invalid && + git fsck 2>out && + grep "not a commit" out && + git update-ref -d refs/heads/invalid +' + +cat > invalid-tag < 1234567890 -0000 + +This is an invalid tag. +EOF + +test_expect_failure 'tag pointing to nonexistent' ' + tag=$(git hash-object -w --stdin < invalid-tag) && + echo $tag > .git/refs/tags/invalid && + git fsck --tags 2>out && + cat out && + grep "could not load tagged object" out && + rm .git/refs/tags/invalid +' + +cat > wrong-tag < 1234567890 -0000 + +This is an invalid tag. +EOF + +test_expect_failure 'tag pointing to something else than its type' ' + tag=$(git hash-object -w --stdin < wrong-tag) && + echo $tag > .git/refs/tags/wrong && + git fsck --tags 2>out && + cat out && + grep "some sane error message" out && + rm .git/refs/tags/wrong +' + + + test_done -- cgit v0.10.2-6-g49f6 From 28fd76bd0413e386ce5176cfac0fad7317145b91 Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 19 Feb 2009 12:13:40 +0100 Subject: Test log --decorate Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index f140b9c..e5715f3 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -203,6 +203,7 @@ log --root -c --patch-with-stat --summary master log --root --cc --patch-with-stat --summary master log -SF master log -SF -p master +log --decorate --all whatchanged master whatchanged -p master diff --git a/t/t4013/diff.log_--decorate_--all b/t/t4013/diff.log_--decorate_--all new file mode 100644 index 0000000..12da8ac --- /dev/null +++ b/t/t4013/diff.log_--decorate_--all @@ -0,0 +1,34 @@ +$ git log --decorate --all +commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (refs/heads/master) +Merge: 9a6d494 c7a2ab9 +Author: A U Thor +Date: Mon Jun 26 00:04:00 2006 +0000 + + Merge branch 'side' + +commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (refs/heads/side) +Author: A U Thor +Date: Mon Jun 26 00:03:00 2006 +0000 + + Side + +commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 +Author: A U Thor +Date: Mon Jun 26 00:02:00 2006 +0000 + + Third + +commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 +Author: A U Thor +Date: Mon Jun 26 00:01:00 2006 +0000 + + Second + + This is the second commit. + +commit 444ac553ac7612cc88969031b02b3767fb8a353a (refs/heads/initial) +Author: A U Thor +Date: Mon Jun 26 00:00:00 2006 +0000 + + Initial +$ -- cgit v0.10.2-6-g49f6 From fcbc6efc7c56973e0a308012d2ae1b42c6b11a33 Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 19 Feb 2009 12:13:41 +0100 Subject: Test rev-list --parents/--children Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index e5715f3..0f359ca 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -205,6 +205,9 @@ log -SF master log -SF -p master log --decorate --all +rev-list --parents HEAD +rev-list --children HEAD + whatchanged master whatchanged -p master whatchanged --root master diff --git a/t/t4013/diff.rev-list_--children_HEAD b/t/t4013/diff.rev-list_--children_HEAD new file mode 100644 index 0000000..e7f17d5 --- /dev/null +++ b/t/t4013/diff.rev-list_--children_HEAD @@ -0,0 +1,7 @@ +$ git rev-list --children HEAD +59d314ad6f356dd08601a4cd5e530381da3e3c64 +c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a 59d314ad6f356dd08601a4cd5e530381da3e3c64 +9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 59d314ad6f356dd08601a4cd5e530381da3e3c64 +1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 +444ac553ac7612cc88969031b02b3767fb8a353a 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a +$ diff --git a/t/t4013/diff.rev-list_--parents_HEAD b/t/t4013/diff.rev-list_--parents_HEAD new file mode 100644 index 0000000..65d2a80 --- /dev/null +++ b/t/t4013/diff.rev-list_--parents_HEAD @@ -0,0 +1,7 @@ +$ git rev-list --parents HEAD +59d314ad6f356dd08601a4cd5e530381da3e3c64 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a +c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a 444ac553ac7612cc88969031b02b3767fb8a353a +9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 +1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 444ac553ac7612cc88969031b02b3767fb8a353a +444ac553ac7612cc88969031b02b3767fb8a353a +$ -- cgit v0.10.2-6-g49f6 From b26d8d217d8d960cbd9ed1dbf6b3cccfd1a3a4db Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 19 Feb 2009 12:13:42 +0100 Subject: Test git-patch-id So far, git-patch-id was untested. Add some simple checks for output format and patch (in)equality. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano diff --git a/t/t4203-patch-id.sh b/t/t4203-patch-id.sh new file mode 100755 index 0000000..04f7bae --- /dev/null +++ b/t/t4203-patch-id.sh @@ -0,0 +1,38 @@ +#!/bin/sh + +test_description='git patch-id' + +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit initial foo a && + test_commit first foo b && + git checkout -b same HEAD^ && + test_commit same-msg foo b && + git checkout -b notsame HEAD^ && + test_commit notsame-msg foo c +' + +test_expect_success 'patch-id output is well-formed' ' + git log -p -1 | git patch-id > output && + grep "^[a-f0-9]\{40\} $(git rev-parse HEAD)$" output +' + +get_patch_id () { + git log -p -1 "$1" | git patch-id | + sed "s# .*##" > patch-id_"$1" +} + +test_expect_success 'patch-id detects equality' ' + get_patch_id master && + get_patch_id same && + test_cmp patch-id_master patch-id_same +' + +test_expect_success 'patch-id detects inequality' ' + get_patch_id master && + get_patch_id notsame && + ! test_cmp patch-id_master patch-id_notsame +' + +test_done -- cgit v0.10.2-6-g49f6 From b079c50e03a812f5c8197b8f38e0a5fe6dd31321 Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 19 Feb 2009 22:26:31 +0100 Subject: format-patch: track several references Currently, format-patch can only track a single reference (the In-Reply-To:) for each mail. To ensure proper threading, we should list all known references for every mail. Change the rev_info.ref_message_id field to a string_list, so that we can append references at will, and change the output formatting routines to print all of them in the References: header. The last entry in the list is implicitly assumed to be the In-Reply-To:, which gives output consistent with RFC 2822: The "References:" field will contain the contents of the parent's "References:" field (if any) followed by the contents of the parent's "Message-ID:" field (if any). Note that this is just preparatory work; nothing uses it yet, so all "References:" fields in the output are still only one deep. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano diff --git a/builtin-log.c b/builtin-log.c index 2ae39af..5967113 100644 --- a/builtin-log.c +++ b/builtin-log.c @@ -17,6 +17,7 @@ #include "run-command.h" #include "shortlog.h" #include "remote.h" +#include "string-list.h" /* Set a default date-time format for git log ("log.date" config variable) */ static const char *default_date_mode = NULL; @@ -1011,8 +1012,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) numbered = 1; if (numbered) rev.total = total + start_number - 1; - if (in_reply_to) - rev.ref_message_id = clean_message_id(in_reply_to); + if (in_reply_to || thread || cover_letter) + rev.ref_message_ids = xcalloc(1, sizeof(struct string_list)); + if (in_reply_to) { + const char *msgid = clean_message_id(in_reply_to); + string_list_append(msgid, rev.ref_message_ids); + } if (cover_letter) { if (thread) gen_message_id(&rev, "cover"); @@ -1036,10 +1041,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) * otherwise, make everything a reply * to that. */ - if (rev.ref_message_id) + if (rev.ref_message_ids->nr > 0) free(rev.message_id); else - rev.ref_message_id = rev.message_id; + string_list_append(rev.message_id, + rev.ref_message_ids); } gen_message_id(&rev, sha1_to_hex(commit->object.sha1)); } diff --git a/log-tree.c b/log-tree.c index 84a74e5..a315ebb 100644 --- a/log-tree.c +++ b/log-tree.c @@ -6,6 +6,7 @@ #include "log-tree.h" #include "reflog-walk.h" #include "refs.h" +#include "string-list.h" struct decoration name_decoration = { "object names" }; @@ -211,9 +212,13 @@ void log_write_email_headers(struct rev_info *opt, const char *name, printf("Message-Id: <%s>\n", opt->message_id); graph_show_oneline(opt->graph); } - if (opt->ref_message_id) { - printf("In-Reply-To: <%s>\nReferences: <%s>\n", - opt->ref_message_id, opt->ref_message_id); + if (opt->ref_message_ids && opt->ref_message_ids->nr > 0) { + int i, n; + n = opt->ref_message_ids->nr; + printf("In-Reply-To: <%s>\n", opt->ref_message_ids->items[n-1].string); + for (i = 0; i < n; i++) + printf("%s<%s>\n", (i > 0 ? "\t" : "References: "), + opt->ref_message_ids->items[i].string); graph_show_oneline(opt->graph); } if (opt->mime_boundary) { diff --git a/revision.h b/revision.h index 7cf8487..8c0a417 100644 --- a/revision.h +++ b/revision.h @@ -89,7 +89,7 @@ struct rev_info { int nr, total; const char *mime_boundary; char *message_id; - const char *ref_message_id; + struct string_list *ref_message_ids; const char *add_signoff; const char *extra_headers; const char *log_reencode; -- cgit v0.10.2-6-g49f6 From 2175c10d5ad2769936f5bf5bcca5ea32715a7307 Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 19 Feb 2009 22:26:32 +0100 Subject: format-patch: thread as reply to cover letter even with in-reply-to Currently, format-patch --thread --cover-letter --in-reply-to $parent makes all mails, including the cover letter, a reply to $parent. However, we would want the reader to consider the cover letter above all the patches. This changes the semantics so that only the cover letter is a reply to $parent, while all the patches are formatted as replies to the cover letter. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano diff --git a/builtin-log.c b/builtin-log.c index 5967113..1df38e1 100644 --- a/builtin-log.c +++ b/builtin-log.c @@ -1036,12 +1036,22 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) /* Have we already had a message ID? */ if (rev.message_id) { /* - * If we've got the ID to be a reply - * to, discard the current ID; - * otherwise, make everything a reply - * to that. + * Without --cover-letter and + * --in-reply-to, make every mail a + * reply to the one before. + * + * With --in-reply-to but no + * --cover-letter, make every mail a + * reply to the . + * + * With --cover-letter, make every + * mail but the cover letter a reply + * to the cover letter. The cover + * letter is a reply to the + * --in-reply-to, if specified. */ - if (rev.ref_message_ids->nr > 0) + if (rev.ref_message_ids->nr > 0 + && (!cover_letter || rev.nr > 1)) free(rev.message_id); else string_list_append(rev.message_id, diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 345e6de..8b970c3 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -237,16 +237,19 @@ In-Reply-To: <1> References: <1> --- Message-Id: <2> -In-Reply-To: <1> +In-Reply-To: <0> References: <1> + <0> --- Message-Id: <3> -In-Reply-To: <1> +In-Reply-To: <0> References: <1> + <0> --- Message-Id: <4> -In-Reply-To: <1> +In-Reply-To: <0> References: <1> + <0> EOF test_expect_success 'thread cover-letter in-reply-to' ' -- cgit v0.10.2-6-g49f6 From 30984ed2e92651962c6b8bdacf1f84da75d1da95 Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 19 Feb 2009 22:26:33 +0100 Subject: format-patch: support deep threading For deep threading mode, i.e., the mode that gives a thread structured like + [PATCH 0/n] Cover letter `-+ [PATCH 1/n] First patch `-+ [PATCH 2/n] Second patch `-+ ... we currently have to use 'git send-email --thread' (the default). On the other hand, format-patch also has a --thread option which gives shallow mode, i.e., + [PATCH 0/n] Cover letter |-+ [PATCH 1/n] First patch |-+ [PATCH 2/n] Second patch ... To reduce the confusion resulting from having two indentically named features in different tools giving different results, let format-patch take an optional argument '--thread=deep' that gives the same output as 'send-mail --thread'. With no argument, or 'shallow', behave as before. Also add a configuration variable format.thread with the same semantics. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano diff --git a/Documentation/config.txt b/Documentation/config.txt index f5152c5..300ab25 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -677,6 +677,16 @@ format.pretty:: See linkgit:git-log[1], linkgit:git-show[1], linkgit:git-whatchanged[1]. +format.thread:: + The default threading style for 'git-format-patch'. Can be + either a boolean value, `shallow` or `deep`. 'Shallow' + threading makes every mail a reply to the head of the series, + where the head is chosen from the cover letter, the + `\--in-reply-to`, and the first patch mail, in this order. + 'Deep' threading makes every mail a reply to the previous one. + A true boolean value is the same as `shallow`, and a false + value disables threading. + gc.aggressiveWindow:: The window size parameter used in the delta compression algorithm used by 'git-gc --aggressive'. This defaults diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 11a7d77..4302b14 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -122,10 +122,18 @@ include::diff-options.txt[] which is the commit message and the patch itself in the second part, with "Content-Disposition: inline". ---thread:: +--thread[=