From 0cfe1d303ab1cab13315c1d42df26ab93bf09b86 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 12 Aug 2005 23:39:15 -0700 Subject: [PATCH 1/7] Add --signoff and --verify option to git commit. As brought up in the discussion which followed a patch to add a signed-off-by line with the --sign flag to format-patch from Johannes Schindelin, add --signoff to the git commit command. Also add --verify to make sure the lines you introduced are clean, which is more useful in commit but not very much in format-patch as it was originally implemented, because finding botches at format-patch time is too late. Signed-off-by: Junio C Hamano --- git-commit-script | 76 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 72 insertions(+), 4 deletions(-) diff --git a/git-commit-script b/git-commit-script index 24ec446a9d..5e6b877d1b 100755 --- a/git-commit-script +++ b/git-commit-script @@ -9,7 +9,7 @@ usage () { die 'git commit [-a] [-m ] [-F ] [(-C|-c) ] [...]' } -all= logfile= use_commit= no_edit= log_given= log_message= +all= logfile= use_commit= no_edit= log_given= log_message= verify= signoff= while case "$#" in 0) break;; esac do case "$1" in @@ -64,6 +64,12 @@ do use_commit="$1" no_edit=t shift ;; + -s|--s|--si|--sig|--sign|--signo|--signof|--signoff) + signoff=t + shift ;; + -v|--v|--ve|--ver|--veri|--verif|--verify) + verify=t + shift ;; --) shift break ;; @@ -86,6 +92,60 @@ t) esac git-update-cache -q --refresh -- "$@" || exit 1 +case "$verify" in +t) + # This is slightly modified from Andrew Morton's Perfect Patch. + # Lines you introduce should not have trailing whitespace. + # Also check for an indentation that has SP before a TAB. + perl -e ' + my $fh; + my $found_bad = 0; + my $filename; + my $reported_filename = ""; + my $lineno; + sub bad_line { + my ($why, $line) = @_; + if (!$found_bad) { + print "*\n"; + print "* You have some suspicious patch lines:\n"; + print "*\n"; + $found_bad = 1; + } + if ($reported_filename ne $filename) { + print "* In $filename\n"; + $reported_filename = $filename; + } + print "* $why (line $lineno)\n$line\n"; + } + open $fh, "-|", qw(git-diff-cache -p -M --cached HEAD); + while (<$fh>) { + if (m|^diff --git a/(.*) b/\1$|) { + $filename = $1; + next; + } + if (/^@@ -\S+ \+(\d+)/) { + $lineno = $1 - 1; + next; + } + if (/^ /) { + $lineno++; + next; + } + if (s/^\+//) { + $lineno++; + chomp; + if (/\s$/) { + bad_line("trailing whitespace", $_); + } + if (/^\s* /) { + bad_line("indent SP followed by a TAB", $_); + } + } + } + exit($found_bad); + ' || exit ;; +esac + PARENTS="-p HEAD" if [ ! -r "$GIT_DIR/HEAD" ]; then if [ -z "$(git-ls-files)" ]; then @@ -161,7 +221,14 @@ else export GIT_AUTHOR_DATE git-cat-file commit "$use_commit" | sed -e '1,/^$/d' - fi >.editmsg + fi | + git-stripspace >.editmsg + case "$signoff" in + t) + git-var GIT_COMMITTER_IDENT | sed -e ' + s/>.*/>/ + s/^/Signed-off-by: /' >>.editmsg ;; + esac git-status-script >>.editmsg fi if [ "$?" != "0" -a ! -f $GIT_DIR/MERGE_HEAD ] @@ -176,7 +243,8 @@ case "$no_edit" in ;; esac grep -v '^#' < .editmsg | git-stripspace > .cmitmsg -if test -s .cmitmsg +grep -v -i '^Signed-off-by' .cmitmsg >.cmitchk +if test -s .cmitchk then tree=$(git-write-tree) && commit=$(cat .cmitmsg | git-commit-tree $tree $PARENTS) && @@ -187,5 +255,5 @@ else false fi ret="$?" -rm -f .cmitmsg .editmsg +rm -f .cmitmsg .editmsg .cmitchk exit "$ret" From 314082512403f7f6969cc6d5ded4a48c68a9962e Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 12 Aug 2005 23:48:09 -0700 Subject: [PATCH 2/7] Add SubmittingPatches Not that I have stricter patch submission standard than ordinary projects, I wanted to have it to make sure people understand what they are doing when they add their own Signed-off-by line. Signed-off-by: Junio C Hamano --- Documentation/SubmittingPatches | 130 ++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 Documentation/SubmittingPatches diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches new file mode 100644 index 0000000000..6121e902ae --- /dev/null +++ b/Documentation/SubmittingPatches @@ -0,0 +1,130 @@ +I started reading over the SubmittingPatches document for Linux +kernel, primarily because I wanted to have a document similar to +it for the core GIT to make sure people understand what they are +doing when they write "Signed-off-by" line. + +But the patch submission requirements are a lot more relaxed +here, because the core GIT is thousand times smaller ;-). So +here is only the relevant bits. + + +(1) Make separate commits for logically separate changes. + +Unless your patch is really trivial, you should not be sending +out a patch that was generated between your working tree and +your commit head. Instead, always make a commit with complete +commit message and generate a series of patches from your +repository. It is a good discipline. + +Describe the technical detail of the change(s). + +If your description starts to get long, that's a sign that you +probably need to split up your commit to finer grained pieces. + + +(2) Generate your patch using git/cogito out of your commits. + +git diff tools generate unidiff which is the preferred format. +You do not have to be afraid to use -M option to "git diff" or +"git format-patch", if your patch involves file renames. The +receiving end can handle them just fine. + +Please make sure your patch does not include any extra files +which do not belong in a patch submission. Make sure to review +your patch after generating it, to ensure accuracy. Before +sending out, please make sure it cleanly applies to the "master" +branch head. + + +(3) Sending your patches. + +People on the git mailing list needs to be able to read and +comment on the changes you are submitting. It is important for +a developer to be able to "quote" your changes, using standard +e-mail tools, so that they may comment on specific portions of +your code. For this reason, all patches should be submitting +e-mail "inline". WARNING: Be wary of your MUAs word-wrap +corrupting your patch. Do not cut-n-paste your patch. + +It is common convention to prefix your subject line with +[PATCH]. This lets people easily distinguish patches from other +e-mail discussions. + +"git format-patch" command follows the best current practice to +format the body of an e-mail message. At the beginning of the +patch should come your commit message, ending with the +Signed-off-by: lines, and a line that consists of three dashes, +followed by the diffstat information and the patch itself. If +you are forwarding a patch from somebody else, optionally, at +the beginning of the e-mail message just before the commit +message starts, you can put a "From: " line to name that person. + +You often want to add additional explanation about the patch, +other than the commit message itself. Place such "cover letter" +material between the three dash lines and the diffstat. + +Do not attach the patch as a MIME attachment, compressed or not. +Do not let your e-mail client send quoted-printable. Many +popular e-mail applications will not always transmit a MIME +attachment as plain text, making it impossible to comment on +your code. A MIME attachment also takes a bit more time to +process. This does not decrease the likelihood of your +MIME-attached change being accepted, but it makes it more likely +that it will be postponed. + +Exception: If your mailer is mangling patches then someone may ask +you to re-send them using MIME. + +Note that your maintainer does not subscribe to the git mailing +list (he reads it via mail-to-news gateway). If your patch is +for discussion first, send it "To:" the mailing list, and +optoinally "cc:" him. If it is trivially correct or after list +discussion reached consensus, send it "To:" the maintainer and +optionally "cc:" the list. + + +(6) Sign your work + +To improve tracking of who did what, we've borrowed the +"sign-off" procedure from the Linux kernel project on patches +that are being emailed around. Although core GIT is a lot +smaller project it is a good discipline to follow it. + +The sign-off is a simple line at the end of the explanation for +the patch, which certifies that you wrote it or otherwise have +the right to pass it on as a open-source patch. The rules are +pretty simple: if you can certify the below: + + Developer's Certificate of Origin 1.1 + + By making a contribution to this project, I certify that: + + (a) The contribution was created in whole or in part by me and I + have the right to submit it under the open source license + indicated in the file; or + + (b) The contribution is based upon previous work that, to the best + of my knowledge, is covered under an appropriate open source + license and I have the right under that license to submit that + work with modifications, whether created in whole or in part + by me, under the same open source license (unless I am + permitted to submit under a different license), as indicated + in the file; or + + (c) The contribution was provided directly to me by some other + person who certified (a), (b) or (c) and I have not modified + it. + + (d) I understand and agree that this project and the contribution + are public and that a record of the contribution (including all + personal information I submit with it, including my sign-off) is + maintained indefinitely and may be redistributed consistent with + this project or the open source license(s) involved. + +then you just add a line saying + + Signed-off-by: Random J Developer + +Some people also put extra tags at the end. They'll just be ignored for +now, but you can do this to mark internal company procedures or just +point out some special detail about the sign-off. From 02a4a32c2da9a89eb9ebc58325adcb47775c9ed5 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sat, 13 Aug 2005 10:50:56 -0700 Subject: [PATCH 3/7] [PATCH] Make sure git-resolve-script always works on commits You can resolve a tag, and it does the right thing except that it might end up writing the tag itself into the resulting HEAD, which will confuse subsequent operations no end. This makes sure that when we resolve two heads, we will have turned them into proper commits before we start acting on them. This also fixes the parsing of "treeish^0", which would incorrectly resolve to "treeish" instead of causing an error. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- git-resolve-script | 4 ++-- sha1_name.c | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/git-resolve-script b/git-resolve-script index 52dd83bae0..4641119e0f 100755 --- a/git-resolve-script +++ b/git-resolve-script @@ -6,8 +6,8 @@ # . git-sh-setup-script || die "Not a git archive" -head=$(git-rev-parse --verify "$1") -merge=$(git-rev-parse --verify "$2") +head=$(git-rev-parse --verify "$1"^0) || exit +merge=$(git-rev-parse --verify "$2"^0) || exit merge_msg="$3" dropheads() { diff --git a/sha1_name.c b/sha1_name.c index 5d1e441e5a..df45b172a3 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -208,13 +208,9 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1) } else parent = -1; - if (0 <= parent) { - ret = get_parent(name, len, sha1, parent); - if (!ret) - return 0; - else if(parent>0) - return ret; - } + if (parent >= 0) + return get_parent(name, len, sha1, parent); + ret = get_sha1_basic(name, len, sha1); if (!ret) return 0; From 3c3852e33b3e40ad64885d845cb92a52c7b15884 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sat, 13 Aug 2005 11:05:25 -0700 Subject: [PATCH 4/7] [PATCH] Make get_sha1_basic() more careful The "get_sha1_hex()" function is designed to work with SHA1 hex strings that may be followed by arbitrary crud. However, that's not acceptable for "get_sha1()" which is used for command line arguments etc: we don't want to silently allow random characters after the end of the SHA1. So verify that the hex string is all we have. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- sha1_name.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sha1_name.c b/sha1_name.c index df45b172a3..fdd321448c 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -148,7 +148,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) }; const char **p; - if (!get_sha1_hex(str, sha1)) + if (len == 40 && !get_sha1_hex(str, sha1)) return 0; for (p = prefix; *p; p++) { From 31f584c242e7af28018ff920b6c8d1952beadbd4 Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Sat, 13 Aug 2005 10:29:32 -0500 Subject: [PATCH 5/7] [PATCH] fix git-checkout-cache segfault on parisc The index cleanup code is executed via atexit() which is *after* main has completed, so the stack allocated cache_file has gone out of scope. Parisc seems to use stack in the destructor functions, so cache_file gets partially overwritten leading to the predictable disastrous consequences. [jc: Just to make sure, I audited other users of the function hold_index_file_for_update() to make sure they do not have this problem; everybody else uses non-stack cache_file structure and is fine. Thanks, James.] Signed-off-by: James Bottomley Signed-off-by: Junio C Hamano --- checkout-cache.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/checkout-cache.c b/checkout-cache.c index 8d67403ae8..31ff577500 100644 --- a/checkout-cache.c +++ b/checkout-cache.c @@ -78,10 +78,11 @@ static int checkout_all(void) static const char checkout_cache_usage[] = "git-checkout-cache [-u] [-q] [-a] [-f] [-n] [--prefix=] [--] ..."; +static struct cache_file cache_file; + int main(int argc, char **argv) { int i, force_filename = 0; - struct cache_file cache_file; int newfd = -1; if (read_cache() < 0) { From e54c5ea93e0785dfd37fd9201797977a02ca0ddb Mon Sep 17 00:00:00 2001 From: Brad Roberts Date: Sat, 13 Aug 2005 03:22:46 -0700 Subject: [PATCH 6/7] [PATCH] fix null TZ problems on os/x It seems that the localtime() libc routine doesn't care for finding a TZ that's empty. It's ok with TZ not being set. Setting the TZ to GMT allowed these tests to pass. $ uname -v Darwin Kernel Version 7.9.0: Wed Mar 30 20:11:17 PST 2005; root:xnu/xnu-517.12.7.obj~1/RELEASE_PPC Signed-off-by: Brad Roberts Signed-off-by: Junio C Hamano --- t/t1100-commit-tree-options.sh | 2 +- t/t5000-tar-tree.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t1100-commit-tree-options.sh b/t/t1100-commit-tree-options.sh index e59f724f2a..19a0ed4d20 100755 --- a/t/t1100-commit-tree-options.sh +++ b/t/t1100-commit-tree-options.sh @@ -32,7 +32,7 @@ test_expect_success \ GIT_COMMITTER_NAME="Committer Name" \ GIT_COMMITTER_EMAIL="committer@email" \ GIT_COMMITTER_DATE="2005-05-26 23:30" \ - TZ= git-commit-tree `cat treeid` >commitid 2>/dev/null' + TZ=GMT git-commit-tree `cat treeid` >commitid 2>/dev/null' test_expect_success \ 'read commit' \ diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh index 6d72ed34c1..abce66921c 100755 --- a/t/t5000-tar-tree.sh +++ b/t/t5000-tar-tree.sh @@ -41,7 +41,7 @@ test_expect_success \ find a -type l | xargs git-update-cache --add && treeid=`git-write-tree` && echo $treeid >treeid && - TZ= GIT_COMMITTER_DATE="2005-05-27 22:00:00" \ + TZ=GMT GIT_COMMITTER_DATE="2005-05-27 22:00:00" \ git-commit-tree $treeid .git/HEAD' test_expect_success \ @@ -50,7 +50,7 @@ test_expect_success \ test_expect_success \ 'validate file modification time' \ - 'TZ= tar tvf b.tar a/a | + 'TZ=GMT tar tvf b.tar a/a | awk \{print\ \$4,\ \(length\(\$5\)\<7\)\ ?\ \$5\":00\"\ :\ \$5\} \ >b.mtime && echo "2005-05-27 22:00:00" >expected.mtime && From 068eac91ce04b9aca163acb1927c3878c45d1a07 Mon Sep 17 00:00:00 2001 From: Yasushi SHOJI Date: Sat, 13 Aug 2005 19:58:56 +0900 Subject: [PATCH 7/7] [PATCH] plug memory leak in diff.c::diff_free_filepair() When I run git-diff-tree on big change, it seems the command eats so much memory. so I just put git under valgrind to see what's going on. diff_free_filespec_data() doesn't free diff_filespec itself. [jc: I ended up doing things slightly differently from Yasushi's patch. The original idea was to use free_filespec_data() only to free the data portion and keep useing the filespec itself, but no existing code seems to do things that way, so I just yanked that part out.] Signed-off-by: Yasushi SHOJI Signed-off-by: Junio C Hamano --- diff.c | 9 ++++----- diffcore-break.c | 4 ++-- diffcore.h | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/diff.c b/diff.c index 3e52fec187..bb2a43b5b0 100644 --- a/diff.c +++ b/diff.c @@ -405,14 +405,13 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only) return 0; } -void diff_free_filespec_data(struct diff_filespec *s) +void diff_free_filespec(struct diff_filespec *s) { if (s->should_free) free(s->data); else if (s->should_munmap) munmap(s->data, s->size); - s->should_free = s->should_munmap = 0; - s->data = NULL; + free(s); } static void prep_temp_blob(struct diff_tempfile *temp, @@ -769,8 +768,8 @@ struct diff_filepair *diff_queue(struct diff_queue_struct *queue, void diff_free_filepair(struct diff_filepair *p) { - diff_free_filespec_data(p->one); - diff_free_filespec_data(p->two); + diff_free_filespec(p->one); + diff_free_filespec(p->two); free(p); } diff --git a/diffcore-break.c b/diffcore-break.c index 06f9a7f0ee..b0c8461e12 100644 --- a/diffcore-break.c +++ b/diffcore-break.c @@ -231,8 +231,8 @@ static void merge_broken(struct diff_filepair *p, dp = diff_queue(outq, d->one, c->two); dp->score = p->score; - diff_free_filespec_data(d->two); - diff_free_filespec_data(c->one); + diff_free_filespec(d->two); + diff_free_filespec(c->one); free(d); free(c); } diff --git a/diffcore.h b/diffcore.h index f1b5ca748c..633d1ae5cf 100644 --- a/diffcore.h +++ b/diffcore.h @@ -43,7 +43,7 @@ extern void fill_filespec(struct diff_filespec *, const unsigned char *, unsigned short); extern int diff_populate_filespec(struct diff_filespec *, int); -extern void diff_free_filespec_data(struct diff_filespec *); +extern void diff_free_filespec(struct diff_filespec *); struct diff_filepair { struct diff_filespec *one;