* Re: git-send-email creates duplicate Message-Id's
From: Matti Aarnio @ 2007-09-17 20:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Adrian Bunk, Avi Kivity, linux-kernel, git
In-Reply-To: <7vejgxyrde.fsf@gitster.siamese.dyndns.org>
On Mon, Sep 17, 2007 at 01:22:05PM -0700, Junio C Hamano wrote:
> Adrian Bunk <bunk@kernel.org> writes:
>
> > The following might be a bug in git-send-email (git maintainers Cc'ed
> > and KVM list removed from Cc):
> >
> > Patch 54 got the same Message-Id as patch 61 and patch 89 got the same
> > Message-Id as patch 104.
> > ...
> > The emails are:
> > http://marc.info/?l=linux-kernel&m=119002061330270&w=2
> > http://marc.info/?l=linux-kernel&m=119002059626434&w=2
> > http://marc.info/?l=linux-kernel&m=119002060011801&w=2
> > http://marc.info/?l=linux-kernel&m=119002060318915&w=2
>
> The old code generated rand(4200) for each message and appended
> it to the timestamp. I do not know where the original author
> got 4200 from, but I think if you send many messages within a
> single second it is possible to get collisions.
>
> I guess something like this patch is an improvement? It
> generates a single prefix from timestamp and random, and appends
> a number that is incremented for each message.
Much better. You may also consider a possibility of
letting your local MTA do the Message-ID generation, unless
you are tracking something with it and thus need to know the
generated values. .. but apparently git much prefers sending
email by SMTP, where the message-id must be present, or one
really should block any such emails.. (except that systems
like qmail send error messages without message-id ...)
My own recipe is:
sprintf("%d-%d-%d", time , getpid, ++localsequence)
catenate on that your favourite domain name where that recipe
is likely to be valid, and you are all set.
/Matti Aarnio -- one of <postmaster@vger.kernel.org>
^ permalink raw reply
* [PATCH] git-commit.sh: Shell script cleanup
From: David Kastrup @ 2007-09-17 20:56 UTC (permalink / raw)
To: git
This moves "shift" out of the argument processing "case". It also
replaces quite a bit of expr calls with ${parameter#word} constructs,
and uses ${parameter:+word} for avoiding conditionals where possible.
Signed-off-by: David Kastrup <dak@gnu.org>
---
Actually, this is almost the same patch as the one posted during the
1.5.3 semi-freeze. I just checked that it still applies and works,
and that nothing has been changed incompatibly in the mean time.
Some might argue that git-commit.sh could be implemented in C soon,
anyway. Well, exactly that would be an excellent reason to do this
sort of cleanup, since then this file would get moved to
contrib/examples, and of course we want the examples to look as clean
and understandable as possible, don't we?
At more than 4 times as many deletions than insertions, this is a nice
cleanup, and quite more readable.
git-commit.sh | 72 +++++++++++---------------------------------------------
1 files changed, 14 insertions(+), 58 deletions(-)
diff --git a/git-commit.sh b/git-commit.sh
index bb113e8..3e46dbb 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -98,101 +98,71 @@ do
no_edit=t
log_given=t$log_given
logfile="$1"
- shift
;;
-F*|-f*)
no_edit=t
log_given=t$log_given
- logfile=`expr "z$1" : 'z-[Ff]\(.*\)'`
- shift
+ logfile="${1#-[Ff]}"
;;
--F=*|--f=*|--fi=*|--fil=*|--file=*)
no_edit=t
log_given=t$log_given
- logfile=`expr "z$1" : 'z-[^=]*=\(.*\)'`
- shift
+ logfile="${1#*=}"
;;
-a|--a|--al|--all)
all=t
- shift
;;
--au=*|--aut=*|--auth=*|--autho=*|--author=*)
- force_author=`expr "z$1" : 'z-[^=]*=\(.*\)'`
- shift
+ force_author="${1#*=}"
;;
--au|--aut|--auth|--autho|--author)
case "$#" in 1) usage ;; esac
shift
force_author="$1"
- shift
;;
-e|--e|--ed|--edi|--edit)
edit_flag=t
- shift
;;
-i|--i|--in|--inc|--incl|--inclu|--includ|--include)
also=t
- shift
;;
--int|--inte|--inter|--intera|--interac|--interact|--interacti|\
--interactiv|--interactive)
interactive=t
- shift
;;
-o|--o|--on|--onl|--only)
only=t
- shift
;;
-m|--m|--me|--mes|--mess|--messa|--messag|--message)
case "$#" in 1) usage ;; esac
shift
log_given=m$log_given
- if test "$log_message" = ''
- then
- log_message="$1"
- else
- log_message="$log_message
+ log_message="${log_message:+${log_message}
-$1"
- fi
+}$1"
no_edit=t
- shift
;;
-m*)
log_given=m$log_given
- if test "$log_message" = ''
- then
- log_message=`expr "z$1" : 'z-m\(.*\)'`
- else
- log_message="$log_message
+ log_message="${log_message:+${log_message}
-`expr "z$1" : 'z-m\(.*\)'`"
- fi
+}${1#-m}"
no_edit=t
- shift
;;
--m=*|--me=*|--mes=*|--mess=*|--messa=*|--messag=*|--message=*)
log_given=m$log_given
- if test "$log_message" = ''
- then
- log_message=`expr "z$1" : 'z-[^=]*=\(.*\)'`
- else
- log_message="$log_message
+ log_message="${log_message:+${log_message}
-`expr "z$1" : 'zq-[^=]*=\(.*\)'`"
- fi
+}${1#*=}"
no_edit=t
- shift
;;
-n|--n|--no|--no-|--no-v|--no-ve|--no-ver|--no-veri|--no-verif|\
--no-verify)
verify=
- shift
;;
--a|--am|--ame|--amen|--amend)
amend=t
use_commit=HEAD
- shift
;;
-c)
case "$#" in 1) usage ;; esac
@@ -200,15 +170,13 @@ $1"
log_given=t$log_given
use_commit="$1"
no_edit=
- shift
;;
--ree=*|--reed=*|--reedi=*|--reedit=*|--reedit-=*|--reedit-m=*|\
--reedit-me=*|--reedit-mes=*|--reedit-mess=*|--reedit-messa=*|\
--reedit-messag=*|--reedit-message=*)
log_given=t$log_given
- use_commit=`expr "z$1" : 'z-[^=]*=\(.*\)'`
+ use_commit="${1#*=}"
no_edit=
- shift
;;
--ree|--reed|--reedi|--reedit|--reedit-|--reedit-m|--reedit-me|\
--reedit-mes|--reedit-mess|--reedit-messa|--reedit-messag|\
@@ -218,7 +186,6 @@ $1"
log_given=t$log_given
use_commit="$1"
no_edit=
- shift
;;
-C)
case "$#" in 1) usage ;; esac
@@ -226,15 +193,13 @@ $1"
log_given=t$log_given
use_commit="$1"
no_edit=t
- shift
;;
--reu=*|--reus=*|--reuse=*|--reuse-=*|--reuse-m=*|--reuse-me=*|\
--reuse-mes=*|--reuse-mess=*|--reuse-messa=*|--reuse-messag=*|\
--reuse-message=*)
log_given=t$log_given
- use_commit=`expr "z$1" : 'z-[^=]*=\(.*\)'`
+ use_commit="${1#*=}"
no_edit=t
- shift
;;
--reu|--reus|--reuse|--reuse-|--reuse-m|--reuse-me|--reuse-mes|\
--reuse-mess|--reuse-messa|--reuse-messag|--reuse-message)
@@ -243,32 +208,26 @@ $1"
log_given=t$log_given
use_commit="$1"
no_edit=t
- shift
;;
-s|--s|--si|--sig|--sign|--signo|--signof|--signoff)
signoff=t
- shift
;;
-t|--t|--te|--tem|--temp|--templ|--templa|--templat|--template)
case "$#" in 1) usage ;; esac
shift
templatefile="$1"
no_edit=
- shift
;;
-q|--q|--qu|--qui|--quie|--quiet)
quiet=t
- shift
;;
-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
verbose=t
- shift
;;
-u|--u|--un|--unt|--untr|--untra|--untrac|--untrack|--untracke|\
--untracked|--untracked-|--untracked-f|--untracked-fi|--untracked-fil|\
--untracked-file|--untracked-files)
untracked_files=t
- shift
;;
--)
shift
@@ -281,6 +240,7 @@ $1"
break
;;
esac
+ shift
done
case "$edit_flag" in t) no_edit= ;; esac
@@ -441,12 +401,8 @@ esac
if test t = "$verify" && test -x "$GIT_DIR"/hooks/pre-commit
then
- if test "$TMP_INDEX"
- then
- GIT_INDEX_FILE="$TMP_INDEX" "$GIT_DIR"/hooks/pre-commit
- else
- GIT_INDEX_FILE="$USE_INDEX" "$GIT_DIR"/hooks/pre-commit
- fi || exit
+ GIT_INDEX_FILE="${TMP_INDEX:-${USE_INDEX}}" "$GIT_DIR"/hooks/pre-commit \
+ || exit
fi
if test "$log_message" != ''
--
1.5.3.1.96.g4568
^ permalink raw reply related
* testsuite problems
From: Miklos Vajna @ 2007-09-17 21:17 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1290 bytes --]
hello,
i was running 'make test' on v1.5.3.1-109-gacd6917 and i hit the
following issue:
* ok 6: validate file modification time
* ok 7: git get-tar-commit-id
* ok 8: extract tar archive
* ok 9: validate filenames
* ok 10: validate file contents
* ok 11: git tar-tree with prefix
* ok 12: extract tar archive with prefix
* ok 13: validate filenames with prefix
* ok 14: validate file contents with prefix
* ok 15: create an archive with a substfile
* ok 16: extract substfile
* ok 17: validate substfile contents
* ok 18: git archive --format=zip
* ok 19: extract ZIP archive
* ok 20: validate filenames
* FAIL 21: validate file contents
diff -r a d/a
* ok 22: git archive --format=zip with prefix
* ok 23: extract ZIP archive with prefix
* ok 24: validate filenames with prefix
* FAIL 25: validate file contents with prefix
diff -r a e/prefix/a
* ok 26: git archive --list outside of a git repo
* failed 2 among 26 test(s)
make[1]: *** [t5000-tar-tree.sh] Error 1
make[1]: Leaving directory `/home/vmiklos/git/git/t'
make: *** [test] Error 2
$ tar --version
tar (GNU tar) 1.18
$ diff --version
diff (GNU diffutils) 2.8.1
maybe these versions are too new or old? if i missed any needed info,
please let me know.
thanks,
- VMiklos
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: testsuite problems
From: Pierre Habouzit @ 2007-09-17 21:41 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
In-Reply-To: <20070917211742.GF19019@genesis.frugalware.org>
[-- Attachment #1: Type: text/plain, Size: 2049 bytes --]
On Mon, Sep 17, 2007 at 09:17:42PM +0000, Miklos Vajna wrote:
> hello,
>
> i was running 'make test' on v1.5.3.1-109-gacd6917 and i hit the
> following issue:
>
[...]
> * FAIL 21: validate file contents
> diff -r a d/a
> * ok 22: git archive --format=zip with prefix
> * ok 23: extract ZIP archive with prefix
> * ok 24: validate filenames with prefix
> * FAIL 25: validate file contents with prefix
> diff -r a e/prefix/a
> * ok 26: git archive --list outside of a git repo
> * failed 2 among 26 test(s)
> make[1]: *** [t5000-tar-tree.sh] Error 1
> make[1]: Leaving directory `/home/vmiklos/git/git/t'
> make: *** [test] Error 2
Well, it works for me:
$ git describe && git st
v1.5.3.1-109-gacd6917
# Not currently on any branch.
nothing to commit (working directory clean)
$ cd t && ./t5000-tar-tree.sh
* ok 1: populate workdir
* ok 2: add files to repository
* ok 3: git archive
* ok 4: git tar-tree
* ok 5: git archive vs. git tar-tree
* ok 6: validate file modification time
* ok 7: git get-tar-commit-id
* ok 8: extract tar archive
* ok 9: validate filenames
* ok 10: validate file contents
* ok 11: git tar-tree with prefix
* ok 12: extract tar archive with prefix
* ok 13: validate filenames with prefix
* ok 14: validate file contents with prefix
* ok 15: create an archive with a substfile
* ok 16: extract substfile
* ok 17: validate substfile contents
* ok 18: git archive --format=zip
* ok 19: extract ZIP archive
* ok 20: validate filenames
* ok 21: validate file contents
* ok 22: git archive --format=zip with prefix
* ok 23: extract ZIP archive with prefix
* ok 24: validate filenames with prefix
* ok 25: validate file contents with prefix
* ok 26: git archive --list outside of a git repo
* passed all 26 test(s)
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: testsuite problems
From: René Scharfe @ 2007-09-17 21:50 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
In-Reply-To: <20070917211742.GF19019@genesis.frugalware.org>
Miklos Vajna schrieb:
> * ok 18: git archive --format=zip
> * ok 19: extract ZIP archive
> * ok 20: validate filenames
> * FAIL 21: validate file contents
> diff -r a d/a
> * ok 22: git archive --format=zip with prefix
> * ok 23: extract ZIP archive with prefix
> * ok 24: validate filenames with prefix
> * FAIL 25: validate file contents with prefix
> diff -r a e/prefix/a
> * ok 26: git archive --list outside of a git repo
> * failed 2 among 26 test(s)
> make[1]: *** [t5000-tar-tree.sh] Error 1
> make[1]: Leaving directory `/home/vmiklos/git/git/t'
> make: *** [test] Error 2
>
> $ tar --version
> tar (GNU tar) 1.18
>
> $ diff --version
> diff (GNU diffutils) 2.8.1
>
> maybe these versions are too new or old? if i missed any needed info,
> please let me know.
The failing tests check ZIP file creation, not tar file creation.
Perhaps your unzip command works a bit differently from Info-ZIP's?
Also, what is the difference between t/trash/a and t/trash/d/a after
running the test script (that's what test 21 is comparing)?
René
^ permalink raw reply
* Re: testsuite problems
From: Miklos Vajna @ 2007-09-17 22:04 UTC (permalink / raw)
To: René Scharfe; +Cc: git
In-Reply-To: <46EEF6A8.1030308@lsrfire.ath.cx>
[-- Attachment #1: Type: text/plain, Size: 821 bytes --]
On Mon, Sep 17, 2007 at 11:50:32PM +0200, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> The failing tests check ZIP file creation, not tar file creation.
> Perhaps your unzip command works a bit differently from Info-ZIP's?
$ zip -v
Copyright (c) 1990-2006 Info-ZIP - Type 'zip "-L"' for software license.
This is Zip 2.32 (June 19th 2006), by Info-ZIP.
should this version be ok?
> Also, what is the difference between t/trash/a and t/trash/d/a after
> running the test script (that's what test 21 is comparing)?
$ diff -Naur t/trash/a t/trash/d/a
diff -Naur t/trash/a/l1 t/trash/d/a/l1
--- t/trash/a/l1 2007-09-17 23:10:03.000000000 +0200
+++ t/trash/d/a/l1 2007-09-17 23:10:03.000000000 +0200
@@ -1 +1 @@
-simple textfile
+a
\ No newline at end of file
thanks,
- VMiklos
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH] apply: get rid of --index-info in favor of --build-fake-ancestor
From: Johannes Schindelin @ 2007-09-17 22:34 UTC (permalink / raw)
To: gitster, git
git-am used "git apply -z --index-info" to find the original versions
of the files touched by the diff, to be able to do an inexpensive
three-way merge.
This operation makes only sense in a repository, since the index
information in the diff refers to blobs, which have to be present in
the current repository.
Therefore, teach "git apply" a mode to write out the result as an
index file to begin with, obviating the need for scripts to do it
themselves.
The primary user for --index-info is "git am", which is converted to
use --build-fake-ancestor in this patch.
Suggested by Junio.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Suggested by Junio, but the errors are all mine.
I am not quite certain, though, if there is really no porcelain
using that option. It has been around since Oct 7 2005 (!), so
there is a real chance that StGit, guilt or QGit use it. In that
case, this patch is obviously wrong.
Documentation/git-apply.txt | 11 +++++++----
builtin-apply.c | 35 ++++++++++++++++++++++-------------
git-am.sh | 6 ++----
3 files changed, 31 insertions(+), 21 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index 4c7e3a2..c1c54bf 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[verse]
'git-apply' [--stat] [--numstat] [--summary] [--check] [--index]
- [--apply] [--no-add] [--index-info] [-R | --reverse]
+ [--apply] [--no-add] [--build-fake-ancestor <file>] [-R | --reverse]
[--allow-binary-replacement | --binary] [--reject] [-z]
[-pNUM] [-CNUM] [--inaccurate-eof] [--cached]
[--whitespace=<nowarn|warn|error|error-all|strip>]
@@ -63,12 +63,15 @@ OPTIONS
cached data, apply the patch, and store the result in the index,
without using the working tree. This implies '--index'.
---index-info::
+--build-fake-ancestor <file>::
Newer git-diff output has embedded 'index information'
for each blob to help identify the original version that
the patch applies to. When this flag is given, and if
- the original version of the blob is available locally,
- outputs information about them to the standard output.
+ the original versions of the blobs is available locally,
+ builds a temporary index containing those blobs.
++
+When a pure mode change is encountered (which has no index information),
+the information is read from the current index instead.
-R, --reverse::
Apply the patch in reverse.
diff --git a/builtin-apply.c b/builtin-apply.c
index 938fd61..1372169 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -42,7 +42,7 @@ static int apply_in_reverse;
static int apply_with_reject;
static int apply_verbosely;
static int no_add;
-static int show_index_info;
+static const char *fake_ancestor;
static int line_termination = '\n';
static unsigned long p_context = ULONG_MAX;
static const char apply_usage[] =
@@ -2236,9 +2236,12 @@ static int get_current_sha1(const char *path, unsigned char *sha1)
return 0;
}
-static void show_index_list(struct patch *list)
+/* Build an index that contains the just the files needed for a 3way merge */
+static void build_fake_ancestor(struct patch *list, const char *filename)
{
struct patch *patch;
+ struct index_state result = { 0 };
+ int fd;
/* Once we start supporting the reverse patch, it may be
* worth showing the new sha1 prefix, but until then...
@@ -2246,11 +2249,12 @@ static void show_index_list(struct patch *list)
for (patch = list; patch; patch = patch->next) {
const unsigned char *sha1_ptr;
unsigned char sha1[20];
+ struct cache_entry *ce;
const char *name;
name = patch->old_name ? patch->old_name : patch->new_name;
if (0 < patch->is_new)
- sha1_ptr = null_sha1;
+ continue;
else if (get_sha1(patch->old_sha1_prefix, sha1))
/* git diff has no index line for mode/type changes */
if (!patch->lines_added && !patch->lines_deleted) {
@@ -2265,13 +2269,16 @@ static void show_index_list(struct patch *list)
else
sha1_ptr = sha1;
- printf("%06o %s ",patch->old_mode, sha1_to_hex(sha1_ptr));
- if (line_termination && quote_c_style(name, NULL, NULL, 0))
- quote_c_style(name, NULL, stdout, 0);
- else
- fputs(name, stdout);
- putchar(line_termination);
+ ce = make_cache_entry(patch->old_mode, sha1_ptr, name, 0, 0);
+ if (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD))
+ die ("Could not add %s to temporary index", name);
}
+
+ fd = open(filename, O_WRONLY | O_CREAT, 0666);
+ if (fd < 0 || write_index(&result, fd) || close(fd))
+ die ("Could not write temporary index to %s", filename);
+
+ discard_index(&result);
}
static void stat_patch_list(struct patch *patch)
@@ -2791,8 +2798,8 @@ static int apply_patch(int fd, const char *filename, int inaccurate_eof)
if (apply && write_out_results(list, skipped_patch))
exit(1);
- if (show_index_info)
- show_index_list(list);
+ if (fake_ancestor)
+ build_fake_ancestor(list, fake_ancestor);
if (diffstat)
stat_patch_list(list);
@@ -2900,9 +2907,11 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
apply = 1;
continue;
}
- if (!strcmp(arg, "--index-info")) {
+ if (!strcmp(arg, "--build-fake-ancestor")) {
apply = 0;
- show_index_info = 1;
+ if (++i >= argc)
+ die ("need a filename");
+ fake_ancestor = argv[i];
continue;
}
if (!strcmp(arg, "-z")) {
diff --git a/git-am.sh b/git-am.sh
index 4db4701..8340125 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -62,10 +62,8 @@ fall_back_3way () {
mkdir "$dotest/patch-merge-tmp-dir"
# First see if the patch records the index info that we can use.
- git apply -z --index-info "$dotest/patch" \
- >"$dotest/patch-merge-index-info" &&
- GIT_INDEX_FILE="$dotest/patch-merge-tmp-index" \
- git update-index -z --index-info <"$dotest/patch-merge-index-info" &&
+ git apply --build-fake-ancestor "$dotest/patch-merge-tmp-index" \
+ "$dotest/patch" &&
GIT_INDEX_FILE="$dotest/patch-merge-tmp-index" \
git write-tree >"$dotest/patch-merge-base+" ||
cannot_fallback "Repository lacks necessary blobs to fall back on 3-way merge."
--
1.5.3.1.989.g2059
^ permalink raw reply related
* Re: testsuite problems
From: Johannes Schindelin @ 2007-09-17 22:40 UTC (permalink / raw)
To: Miklos Vajna; +Cc: René Scharfe, git
In-Reply-To: <20070917220408.GG19019@genesis.frugalware.org>
Hi,
On Tue, 18 Sep 2007, Miklos Vajna wrote:
> On Mon, Sep 17, 2007 at 11:50:32PM +0200, Ren? Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> > The failing tests check ZIP file creation, not tar file creation.
> > Perhaps your unzip command works a bit differently from Info-ZIP's?
>
> $ zip -v
> Copyright (c) 1990-2006 Info-ZIP - Type 'zip "-L"' for software license.
> This is Zip 2.32 (June 19th 2006), by Info-ZIP.
>
> should this version be ok?
>
> > Also, what is the difference between t/trash/a and t/trash/d/a after
> > running the test script (that's what test 21 is comparing)?
>
> $ diff -Naur t/trash/a t/trash/d/a
> diff -Naur t/trash/a/l1 t/trash/d/a/l1
> --- t/trash/a/l1 2007-09-17 23:10:03.000000000 +0200
> +++ t/trash/d/a/l1 2007-09-17 23:10:03.000000000 +0200
> @@ -1 +1 @@
> -simple textfile
> +a
> \ No newline at end of file
Ah, that's my friend GNU diff again... Try running the test with "-i -v",
and it will pass, if I am right.
I actually made a patch for GNU diff back when this hit me, only to find
out that it was fixed in CVS. Apparently, for a short period, during
which 2.8.1 was released, there was a bug that prevented GNU diff from
working properly when the output was redirected to a non-tty.
If I'm right, and it is that bug, it might be worthwhile to convert the
calls to "diff -u" in that test script with calls to "git diff" (possibly
with --no-index).
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] apply: get rid of --index-info in favor of --build-fake-ancestor
From: Junio C Hamano @ 2007-09-17 22:42 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: gitster, git
In-Reply-To: <Pine.LNX.4.64.0709172330400.28586@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Suggested by Junio, but the errors are all mine.
>
> I am not quite certain, though, if there is really no porcelain
> using that option. It has been around since Oct 7 2005 (!), so
> there is a real chance that StGit, guilt or QGit use it. In that
> case, this patch is obviously wrong.
You do not have to do the deprecating/removing part if that is
the issue.
^ permalink raw reply
* Re: testsuite problems
From: Miklos Vajna @ 2007-09-17 22:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: René Scharfe, git
In-Reply-To: <Pine.LNX.4.64.0709172337470.28586@racer.site>
[-- Attachment #1: Type: text/plain, Size: 461 bytes --]
On Mon, Sep 17, 2007 at 11:40:27PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Ah, that's my friend GNU diff again... Try running the test with "-i -v",
> and it will pass, if I am right.
* expecting success: diff -r a d/a
diff -r a/l1 d/a/l1
1c1
< simple textfile
---
> a
\ No newline at end of file
* FAIL 21: validate file contents
diff -r a d/a
it still fails, so i don't think this is that issue
- VMiklos
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: testsuite problems
From: Johannes Schindelin @ 2007-09-17 22:50 UTC (permalink / raw)
To: Miklos Vajna; +Cc: René Scharfe, git
In-Reply-To: <20070917224828.GI19019@genesis.frugalware.org>
Hi,
On Tue, 18 Sep 2007, Miklos Vajna wrote:
> On Mon, Sep 17, 2007 at 11:40:27PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > Ah, that's my friend GNU diff again... Try running the test with "-i -v",
> > and it will pass, if I am right.
>
> * expecting success: diff -r a d/a
> diff -r a/l1 d/a/l1
> 1c1
> < simple textfile
> ---
> > a
> \ No newline at end of file
> * FAIL 21: validate file contents
> diff -r a d/a
>
> it still fails, so i don't think this is that issue
Oops. I blame it on tiredness...
Sorry for the noise,
Dscho
^ permalink raw reply
* List of bugs and suggestions
From: Yann Dirson @ 2007-09-17 22:54 UTC (permalink / raw)
To: GIT list
Here are a list of issues I wrote down while using git and friends at
work, but did not take time to report of fix myself before. They are
relative to git 1.5.3.
BUGS
gitk:
- when selecting again a "local changes" pseudo-patch, gitk re-reads
the diff, and the results can be confusing if HEAD changed since last
full reload (eg. git-commit or stg/guilt push/pop). Should detect the
HEAD change and propose a reload.
- clicking in diff-contents or file-list panes does not move focus out
of text fields (must click eg. between fields instead). I remember
routinely click in diff-contents to focus out of the search field.
- if a file that is modified and copied in the same commit, clicking
on the original in file-list pane jumps to the "copy" entry. This can
easily cause overlooking of changes to the original file.
stash:
- "git stash show junk" acts like "git stash show"
- changes stashed in the index are unstashed only in workdir. If
that's intended, a note in the "stash apply" doc would be useful; but
then I am left puzzled as to the usefulness of storing the index state
in the stash branch: it could surely be applied by a variant of "stash
apply", but such a command does not seem to exist yet ?
misc git:
- git has problems with cascaded alternates that use relative paths.
"git clone -s A B && git clone -s B C" puts absolute paths in
info/alternates - there is probably a good reason, but it will
obviously hurt when moving the clone. Further more,
Documentation/repository-layout.txt says scary things about
using absolute paths there. But if one manually sets up
alternates in a set of repositories (easily reproduced by
hand-editing the alternates files in the B clone
described above), one ends with commits from A not being seen,
and the following error message on each command run with C,
even when no object lookup from A is required:
| $ git fsck
| error: .git/objects/../../../B/.git/objects: ignoring relative alternate object store ../../../A/.git/objects
- repack -a -l (after transitionning a repo to alternates) does not
detect that old packs including objects now in alternate.
I had 2 branches in a single repo and changed the setup to 2
repos (initialize by "cp -al" and removing unwanted refs from
each repo), A finally referencing B as remote and alternate.
"git repack -a -l" in A seems to correctly only include only
local objects in the pack, but all the old packs containing
objects now in B are kept.
SUGGESTIONS
git-diff*:
- could allow to declare word separators for --color-words
- could print hunk number in header (a la filterdiff --annotate)
gitk:
- on-demand loading of additional revs (eg. ancestors from reflogs)
- non-printable keystrokes could be made available from within text
fields (F5, PgUp, etc)
- large commits would be made easier to navigate with some additionnal
highlighting/coloring. Eg:
- coloring of add/remove (and copy/move ?) in patch file list
- move selection in patch file list according to currently selected
search match when there is one, or according to the file(s) whose
diff currently appears
- highlight search matches in patch file list
- highlight search matches, and the file selected from the patch file
list, in/beside the patch display slider
- file list could have its own focus history
- for large commits, it takes time to get the full diff. Showing the
progress (nfiles processed / nfiles in commit) would be nice
- a "goto prev file" binding to reverse 'f' key (or quickly find in
which file a search match occured) would be great
- a "scroll diff view to next hunk" key binding ('h' ?)
- search backwards, and case-insensitively
- a list of known heads to jump to would be useful to quickly navigate
in multiple-head display mode.
- selecting a file in file list xould put filename in paste buffer,
like what's done for commit ids in history pane.
- "lock selected diff" toggle, to avoid losing a particular diff by
error (esp. useful when getting an expensive diff relatively to
selected commit)
stgit:
- derive a stack-log from patch logs
The ordering information provided by patchlogs and starck
reflog could be used to present a history of what happenned in
a stack (eg. when coming back from holiday wondering what one
was doing before leaving).
stgit contrib scripts (mostly reminders to myself ;):
- stg-whatchanged does not identify conflicts caused by "stg pick --fold"
- stg-fold-files-from cannot fold binary files (filterdiff limitation)
=> the following only works for filenames with no special char
(would need --zero):
git show --binary $(stg id occ53) -- $(stg files --bare occ53|grep '^doc/') | stg fold
=> the following does not work either:
git show --binary $(stg id occ53) -- $(git-ls-tree --name-only $(stg id occ53) doc/)
- stg-fold-files-from using non-git-aware filterdiff causes git-apply to
mistake a "add content to an empty file" hunk for a "create file" hunk
^ permalink raw reply
* Re: [PATCH 9/9] Implement git commit as a builtin command.
From: Kristian Høgsberg @ 2007-09-17 22:58 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709061741370.28586@racer.site>
On Thu, 2007-09-06 at 17:59 +0100, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
>
> > contrib/examples/git-commit.sh | 665 +++++++++++++++++++++++++++++++++++
> > git-commit.sh | 665 -----------------------------------
>
> You might want to use "git format-patch -M" next time ;-)
>
> > @@ -357,7 +358,6 @@ BUILTIN_OBJS = \
> > builtin-rev-parse.o \
> > builtin-revert.o \
> > builtin-rm.o \
> > - builtin-runstatus.o \
>
> Better keep it; some people's scripts could depend on it.
Seriously? Why don't we remove it and see if somebody yells? It's more
of an implementation detail than most other git commands; if you need
status output in your script why wouldn't you just run git status?
> > +struct option {
> > + enum option_type type;
> > + const char *long_name;
> > + char short_name;
> > + void *value;
> > +};
> > +
> > +static int scan_options(const char ***argv, struct option *options)
> > +{
>
> I would not (no longer, anyway) be opposed to replacing the option parsing
> in git with getopt(); I hear that it is small enough to keep a copy in
> compat/getopt.c.
>
> But let's go forward with builtin-commit; getopt() can come later.
I don't know. I think it's a situation much like the string library
discussion. It's a small enough dependency (70 lines!) that there's no
gain in depending on an external implementation, and we can tailor it to
gits needs as we extend the use within git. And we can call it gitopt!
> > +static char *
> > +prepare_index(const char **files, const char *prefix)
> > +{
> > + int fd;
> > + struct tree *tree;
> > + struct lock_file *next_index_lock;
> > +
> > + fd = hold_locked_index(&lock_file, 1);
> > + if (read_cache() < 0)
> > + die("index file corrupt");
> > +
> > + if (all) {
> > + add_files_to_cache(fd, files, NULL);
> > + return lock_file.filename;
> > + } else if (also) {
> > + add_files_to_cache(fd, files, prefix);
> > + return lock_file.filename;
> > + }
> > +
> > + if (interactive)
> > + interactive_add();
> > +
> > + if (*files == NULL) {
> > + /* Commit index as-is. */
> > + rollback_lock_file(&lock_file);
> > + return get_index_file();
> > + }
> > +
> > + /*
> > + * FIXME: Warn on unknown files. Shell script does
> > + *
> > + * commit_only=`git-ls-files --error-unmatch -- "$@"`
> > + */
> > +
> > + /*
> > + * FIXME: shell script does
> > + *
> > + * git-read-tree --index-output="$TMP_INDEX" -i -m HEAD
> > + *
> > + * which warns about unmerged files in the index.
> > + */
> > +
> > + /* update the user index file */
> > + add_files_to_cache(fd, files, prefix);
>
> I suspect this, or ...
>
> > +
> > + if (!initial_commit) {
> > + tree = parse_tree_indirect(head_sha1);
> > + if (!tree)
> > + die("failed to unpack HEAD tree object");
> > + if (read_tree(tree, 0, NULL))
> > + die("failed to read HEAD tree object");
> > + }
> > +
> > + /* Uh oh, abusing lock_file to create a garbage collected file */
> > + next_index_lock = xmalloc(sizeof(*next_index_lock));
> > + fd = hold_lock_file_for_update(next_index_lock,
> > + git_path("next-index-%d", getpid()), 1);
> > + add_files_to_cache(fd, files, prefix);
>
> ... this, but not both.
No, this needs both. The first add_files_to_cache() call updates the
user index (.git/index) by adding the given files, then we build a
temporary index from a tree and add the files to that index.
>
> > +/* Find out if the message starting at position 'start' in the strbuf
> > + * contains only whitespace and Signed-off-by lines. */
> > +static int message_is_empty(struct strbuf *sb, int start)
> > +{
> > + static const char signed_off_by[] = "Signed-off-by: ";
>
> I think you already defined that globally earlier.
Ah, yes, fixed.
> In the function message_is_empty() you write:
>
> > + /* See if the template is just a prefix of the message. */
> > + strbuf_init(&tmpl);
> > + if (template_file && strbuf_read_path(&tmpl, template_file) > 0) {
> > + stripspace(&tmpl, 1);
> > + if (start + tmpl.len <= sb->len &&
> > + memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)
> > + start += tmpl.len;
>
> Could we not bail out here, if there is no match? In that case, the
> message is clearly not empty...
The contents could be just sign-off-by's.
> > + /* Check if the rest is just whitespace and Signed-of-by's. */
> > + for (i = start; i < sb->len; i++) {
> > + nl = memchr(sb->buf + i, '\n', sb->len - i);
> > + if (nl)
> > + eol = nl - sb->buf;
> > + else
> > + eol = sb->len;
>
> Why not just "if (isspace(sb->buf[i]) || sb->buf[i] == '\n') continue;"?
> This would also catch the cases where people indent their S-O-Bs.
>
> > +
> > + if (strlen(signed_off_by) <= eol - i &&
> > + !prefixcmp(sb->buf + i, signed_off_by)) {
> > + i = eol;
> > + continue;
> > + }
> > + while (i < eol)
> > + if (!isspace(sb->buf[i++]))
> > + return 0;
> > + }
> > +
> > + return 1;
> > +}
>
> I did not review the rest of the code closely yet...
I'm sending an updated version against Pierre's strbuf changes now.
It's a smaller patch set, so hopefully we can get it in soon.
Kristian
^ permalink raw reply
* Re: [PATCH 6/9] Rewrite launch_editor, create_tag and stripspace to use strbufs.
From: Kristian Høgsberg @ 2007-09-17 22:59 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709061732480.28586@racer.site>
On Thu, 2007-09-06 at 17:38 +0100, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
>
> > diff --git a/strbuf.c b/strbuf.c
> > index fcfc05e..ed2afea 100644
> > --- a/strbuf.c
> > +++ b/strbuf.c
> > @@ -73,43 +74,15 @@ void strbuf_printf(struct strbuf *sb, const char *fmt, ...)
> > {
> > char buffer[2048];
> > va_list args;
> > - int len, size = 2 * sizeof buffer;
> > + int len;
> >
> > va_start(args, fmt);
> > len = vsnprintf(buffer, sizeof(buffer), fmt, args);
> > va_end(args);
> >
> > - if (len > sizeof(buffer)) {
> > - /*
> > - * Didn't fit in the buffer, but this vsnprintf at
> > - * least gives us the required length back. Grow the
> > - * buffer acccordingly and try again.
> > - */
> > - strbuf_grow(sb, len);
> > - va_start(args, fmt);
> > - len = vsnprintf(sb->buf + sb->len,
> > - sb->alloc - sb->len, fmt, args);
> > - va_end(args);
> > - } else if (len >= 0) {
> > - /*
> > - * The initial vsnprintf fit in the temp buffer, just
> > - * copy it to the strbuf.
> > - */
> > - strbuf_add(sb, buffer, len);
> > - } else {
> > - /*
> > - * This vnsprintf sucks and just returns -1 when the
> > - * buffer is too small. Keep doubling the size until
> > - * it fits.
> > - */
> > - while (len < 0) {
> > - strbuf_grow(sb, size);
> > - va_start(args, fmt);
> > - len = vsnprintf(sb->buf + sb->len,
> > - sb->alloc - sb->len, fmt, args);
> > - va_end(args);
> > - size *= 2;
> > - }
> > - }
> > + if (len > sizeof(buffer) || len < 0)
> > + die("out of buffer space\n");
> > +
> > + strbuf_add(sb, buffer, len);
> > }
>
> Really?
>
> (If you find the time, it would be really nice to rebase that patch series
> on top of Pierre's strbuf work...)
Argh, this was a screwup when I edited the patch series. The next
series is based on Pierres changes.
Kristian
^ permalink raw reply
* Re: [PATCH] apply: get rid of --index-info in favor of --build-fake-ancestor
From: Johannes Schindelin @ 2007-09-17 23:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7imozzgm.fsf@gitster.siamese.dyndns.org>
Hi,
On Mon, 17 Sep 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Suggested by Junio, but the errors are all mine.
> >
> > I am not quite certain, though, if there is really no porcelain
> > using that option. It has been around since Oct 7 2005 (!), so
> > there is a real chance that StGit, guilt or QGit use it. In that
> > case, this patch is obviously wrong.
>
> You do not have to do the deprecating/removing part if that is
> the issue.
I'd rather avoid keeping this code, if nobody uses it anyway...
So I cloned StGit and guilt (the two porcelains that I feel are most used,
apart from what is in git.git, and I'm too lazy to find that mail
analysing the recent Git survey). Neither of them has any --index-info in
their complete history.
So I'm more confident now that there is no need to keep --index-info.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 4/9] Introduce entry point for launching add--interactive.
From: Kristian Høgsberg @ 2007-09-17 23:13 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709061729270.28586@racer.site>
On Thu, 2007-09-06 at 17:31 +0100, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
>
> > diff --git a/builtin-add.c b/builtin-add.c
> > index 3dd4ded..e79e8f7 100644
> > --- a/builtin-add.c
> > +++ b/builtin-add.c
> > @@ -153,6 +154,13 @@ static int git_add_config(const char *var, const char *value)
> > return git_default_config(var, value);
> > }
> >
> > +int interactive_add(void)
> > +{
> > + const char *argv[2] = { "add--interactive", NULL };
> > +
> > + return run_command_v_opt(argv, RUN_GIT_CMD);
> > +}
>
> I'd rather have this in builtin-commit.c, since it is quite funny if
> builtin-add.c has code to fork() and exec() itself (eventually, that
> is) ;-)
Huh... it ends up in the same binary, and interactive_add() sounds like
it should live in builtin-add.c rather than builtin-commit.c. Either
way, I don't care too much, but can we fix it up later?
> > diff --git a/commit.h b/commit.h
> > index 467872e..64e1d4b 100644
> > --- a/commit.h
> > +++ b/commit.h
> > @@ -122,4 +122,13 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
> > int depth, int shallow_flag, int not_shallow_flag);
> >
> > int in_merge_bases(struct commit *, struct commit **, int);
> > +
> > +extern const unsigned char *
> > +create_commit(const unsigned char *tree_sha1,
> > + unsigned char parent_sha1[][20], int parents,
> > + const char *author_info, const char *committer_info,
> > + const char *message, int length);
> > +
> > +extern int interactive_add(void);
> > +
>
> Just a guess: you did not want create_commit() to creep in here, right?
Yeah, that was another oversight, fixed in the next series.
Kristian
^ permalink raw reply
* Re: [PATCH 9/9] Implement git commit as a builtin command.
From: Johannes Schindelin @ 2007-09-17 23:16 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <1190069881.10112.10.camel@hinata.boston.redhat.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1800 bytes --]
Hi,
On Mon, 17 Sep 2007, Kristian H?gsberg wrote:
> On Thu, 2007-09-06 at 17:59 +0100, Johannes Schindelin wrote:
> >
> > On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
> >
> > > - builtin-runstatus.o \
> >
> > Better keep it; some people's scripts could depend on it.
>
> Seriously? Why don't we remove it and see if somebody yells? It's more
> of an implementation detail than most other git commands; if you need
> status output in your script why wouldn't you just run git status?
git status is deemed porcelain.
Yes, we recently converted a few things to use "git log", which is
porcelain, too, and I was not happy...
But then, you're right, we could just go and break peoples' scripts, if
they indeed did not use "git diff" directly.
> > > +struct option {
> > > + enum option_type type;
> > > + const char *long_name;
> > > + char short_name;
> > > + void *value;
> > > +};
> > > +
> > > +static int scan_options(const char ***argv, struct option *options)
> > > +{
> >
> > I would not (no longer, anyway) be opposed to replacing the option parsing
> > in git with getopt(); I hear that it is small enough to keep a copy in
> > compat/getopt.c.
> >
> > But let's go forward with builtin-commit; getopt() can come later.
>
> I don't know. I think it's a situation much like the string library
> discussion. It's a small enough dependency (70 lines!) that there's no
> gain in depending on an external implementation, and we can tailor it to
> gits needs as we extend the use within git. And we can call it gitopt!
Hm. I liked the semantics of getopt better, but what the heck.
> I'm sending an updated version against Pierre's strbuf changes now. It's
> a smaller patch set, so hopefully we can get it in soon.
Yes, that would be good.
Thanks,
Dscho
^ permalink raw reply
* Re: List of bugs and suggestions
From: Johannes Schindelin @ 2007-09-17 23:26 UTC (permalink / raw)
To: Yann Dirson; +Cc: GIT list
In-Reply-To: <20070917225441.GA32647@nan92-1-81-57-214-146.fbx.proxad.net>
Hi,
On Tue, 18 Sep 2007, Yann Dirson wrote:
> stash:
>
> - "git stash show junk" acts like "git stash show"
>
> - changes stashed in the index are unstashed only in workdir. If that's
> intended, a note in the "stash apply" doc would be useful; but then I am
> left puzzled as to the usefulness of storing the index state in the
> stash branch: it could surely be applied by a variant of "stash apply",
> but such a command does not seem to exist yet ?
It does, but it is not documented: "git stash apply --index".
> misc git:
>
> - git has problems with cascaded alternates that use relative paths.
Known error, Pasky has lamented about that already. I think we could get
away with using make_absolute_path() cleverly.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 4/9] Introduce entry point for launching add--interactive.
From: Johannes Schindelin @ 2007-09-17 23:27 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <1190070792.10112.15.camel@hinata.boston.redhat.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1120 bytes --]
Hi,
On Mon, 17 Sep 2007, Kristian H?gsberg wrote:
> On Thu, 2007-09-06 at 17:31 +0100, Johannes Schindelin wrote:
> >
> > On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
> >
> > > diff --git a/builtin-add.c b/builtin-add.c
> > > index 3dd4ded..e79e8f7 100644
> > > --- a/builtin-add.c
> > > +++ b/builtin-add.c
> > > @@ -153,6 +154,13 @@ static int git_add_config(const char *var, const char *value)
> > > return git_default_config(var, value);
> > > }
> > >
> > > +int interactive_add(void)
> > > +{
> > > + const char *argv[2] = { "add--interactive", NULL };
> > > +
> > > + return run_command_v_opt(argv, RUN_GIT_CMD);
> > > +}
> >
> > I'd rather have this in builtin-commit.c, since it is quite funny if
> > builtin-add.c has code to fork() and exec() itself (eventually, that
> > is) ;-)
>
> Huh... it ends up in the same binary, and interactive_add() sounds like
> it should live in builtin-add.c rather than builtin-commit.c. Either
> way, I don't care too much, but can we fix it up later?
Well, you are probably right on the former, and therefore we do not have
to do the latter.
Thanks,
Dscho
^ permalink raw reply
* Re: [PATCH 1/9] Enable wt-status output to a given FILE pointer.
From: Kristian Høgsberg @ 2007-09-17 23:30 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709061726010.28586@racer.site>
On Thu, 2007-09-06 at 17:27 +0100, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
>
> > Still defaults to stdout, but you can now override wt_status.fp after
> > calling wt_status_prepare().
>
> Would it not be easier to freopen(filename, "a", stdout)?
It's probably easier, but I think this is cleaner and the patch isn't
that big. I don't want to worry about the side effects of freopening
stdout...
Kristian
^ permalink raw reply
* Re: [PATCH 3/3] git-apply: add tests for stripping of leading and trailing whitespace
From: Krzysztof Halasa @ 2007-09-17 23:44 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Junio C Hamano, git
In-Reply-To: <20070917150213.GB4957@fieldses.org>
"J. Bruce Fields" <bfields@fieldses.org> writes:
>> It may be valid, some projects use tabs for indentation and spaces
>> for alignment, e.g.:
>
> Yeah, I know. I was hoping that the stripspace behavior was already
> specific enough to the linux-kernel style that we could just assume that
> it was only used by developers on projects with the same style.
Actually I would consider linux kernel an example of such project
with spaces for alignment. Except that current tools are not to
the task. Someday...
Obviously other cases are valid, especially 'spaces before tabs'
which IMHO includes '8 or more non-consecutive initial spaces'.
And all trailing whitespace of course.
--
Krzysztof Halasa
^ permalink raw reply
* Re: commit summary, --pretty=short and other tools
From: Benoit SIGOURE @ 2007-09-17 23:52 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <20070917112136.GA30201@glandium.org>
[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]
On Sep 17, 2007, at 1:21 PM, Mike Hommey wrote:
> Hi,
>
> I kind of shot myself in the foot with how to type proper commit
> messages.
>
> The git-commit manual page reads:
> Though not required, it´s a good idea to begin the commit message
> with a
> single short (less than 50 character) line summarizing the change,
> followed by a blank line and then a more thorough description.
>
> ... and I happen to not have done the "followed by a blank line" part.
>
[...]
> What do you think ?
I started using Git as a "better SVN client" and didn't follow this
"good idea". The thing, as I already pointed out on IRC, these a are
more rules than just guidelines. Some tools (such as rebase) enforce
them. That is, they rewrite commit messages. I found this extremely
annoying (Junio provided a patch but I don't know whether it's been
applied, I personally use it in my Git).
See this thread: http://marc.info/?t=118561729500001&r=1&w=2
My opinion is that it would be better to keep the first line and
never ever rewrite the commit messages.
Cheers,
--
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]
^ permalink raw reply
* Re: [PATCH 9/9] Implement git commit as a builtin command.
From: Jeff King @ 2007-09-17 23:56 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: Johannes Schindelin, git
In-Reply-To: <1190069881.10112.10.camel@hinata.boston.redhat.com>
On Mon, Sep 17, 2007 at 06:58:01PM -0400, Kristian Høgsberg wrote:
> > > - builtin-runstatus.o \
> >
> > Better keep it; some people's scripts could depend on it.
>
> Seriously? Why don't we remove it and see if somebody yells? It's more
> of an implementation detail than most other git commands; if you need
> status output in your script why wouldn't you just run git status?
As the author of builtin-runstatus, I had always intended that it was a
temporary part of the transition to a C git-commit, and would go away
then. But I see in the interim somebody documented it. It should
perhaps have been called git-status--helper, and the documentation
should have read "DO NOT USE THIS."
So certainly my intent was for it to go away, but whether it has become
something else is perhaps up to others to judge.
-Peff
^ permalink raw reply
* [PATCH 2/7] Enable wt-status to run against non-standard index file.
From: Kristian Høgsberg @ 2007-09-18 0:06 UTC (permalink / raw)
To: git; +Cc: Kristian Høgsberg
In-Reply-To: <1190074008617-git-send-email-krh@redhat.com>
We still default to get_index_file(), but this can be overridden
by setting wt_status.index_file after calling wt_status_prepare().
Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
wt-status.c | 3 ++-
wt-status.h | 1 +
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index eeb1691..03b5ec4 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -53,6 +53,7 @@ void wt_status_prepare(struct wt_status *s)
s->branch = head ? xstrdup(head) : NULL;
s->reference = "HEAD";
s->fp = stdout;
+ s->index_file = get_index_file();
}
static void wt_status_print_cached_header(struct wt_status *s)
@@ -198,7 +199,7 @@ static void wt_status_print_changed_cb(struct diff_queue_struct *q,
static void wt_read_cache(struct wt_status *s)
{
discard_cache();
- read_cache();
+ read_cache_from(s->index_file);
}
static void wt_status_print_initial(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 4f3a615..7744932 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -21,6 +21,7 @@ struct wt_status {
int commitable;
int workdir_dirty;
int workdir_untracked;
+ const char *index_file;
FILE *fp;
};
--
1.5.3.1.993.gbf388-dirty
^ permalink raw reply related
* [PATCH 1/7] Enable wt-status output to a given FILE pointer.
From: Kristian Høgsberg @ 2007-09-18 0:06 UTC (permalink / raw)
To: git; +Cc: Kristian Høgsberg
Still defaults to stdout, but you can now override wt_status.fp after
calling wt_status_prepare().
Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
color.c | 18 ++++++------
color.h | 4 +-
wt-status.c | 85 ++++++++++++++++++++++++++++++----------------------------
wt-status.h | 3 ++
4 files changed, 58 insertions(+), 52 deletions(-)
diff --git a/color.c b/color.c
index 09d82ee..124ba33 100644
--- a/color.c
+++ b/color.c
@@ -135,39 +135,39 @@ int git_config_colorbool(const char *var, const char *value)
return git_config_bool(var, value);
}
-static int color_vprintf(const char *color, const char *fmt,
+static int color_vfprintf(FILE *fp, const char *color, const char *fmt,
va_list args, const char *trail)
{
int r = 0;
if (*color)
- r += printf("%s", color);
- r += vprintf(fmt, args);
+ r += fprintf(fp, "%s", color);
+ r += vfprintf(fp, fmt, args);
if (*color)
- r += printf("%s", COLOR_RESET);
+ r += fprintf(fp, "%s", COLOR_RESET);
if (trail)
- r += printf("%s", trail);
+ r += fprintf(fp, "%s", trail);
return r;
}
-int color_printf(const char *color, const char *fmt, ...)
+int color_fprintf(FILE *fp, const char *color, const char *fmt, ...)
{
va_list args;
int r;
va_start(args, fmt);
- r = color_vprintf(color, fmt, args, NULL);
+ r = color_vfprintf(fp, color, fmt, args, NULL);
va_end(args);
return r;
}
-int color_printf_ln(const char *color, const char *fmt, ...)
+int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...)
{
va_list args;
int r;
va_start(args, fmt);
- r = color_vprintf(color, fmt, args, "\n");
+ r = color_vfprintf(fp, color, fmt, args, "\n");
va_end(args);
return r;
}
diff --git a/color.h b/color.h
index 88bb8ff..6809800 100644
--- a/color.h
+++ b/color.h
@@ -6,7 +6,7 @@
int git_config_colorbool(const char *var, const char *value);
void color_parse(const char *var, const char *value, char *dst);
-int color_printf(const char *color, const char *fmt, ...);
-int color_printf_ln(const char *color, const char *fmt, ...);
+int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
+int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
#endif /* COLOR_H */
diff --git a/wt-status.c b/wt-status.c
index 10ce6ee..eeb1691 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -52,31 +52,33 @@ void wt_status_prepare(struct wt_status *s)
head = resolve_ref("HEAD", sha1, 0, NULL);
s->branch = head ? xstrdup(head) : NULL;
s->reference = "HEAD";
+ s->fp = stdout;
}
-static void wt_status_print_cached_header(const char *reference)
+static void wt_status_print_cached_header(struct wt_status *s)
{
const char *c = color(WT_STATUS_HEADER);
- color_printf_ln(c, "# Changes to be committed:");
- if (reference) {
- color_printf_ln(c, "# (use \"git reset %s <file>...\" to unstage)", reference);
+ color_fprintf_ln(s->fp, c, "# Changes to be committed:");
+ if (s->reference) {
+ color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
} else {
- color_printf_ln(c, "# (use \"git rm --cached <file>...\" to unstage)");
+ color_fprintf_ln(s->fp, c, "# (use \"git rm --cached <file>...\" to unstage)");
}
- color_printf_ln(c, "#");
+ color_fprintf_ln(s->fp, c, "#");
}
-static void wt_status_print_header(const char *main, const char *sub)
+static void wt_status_print_header(struct wt_status *s,
+ const char *main, const char *sub)
{
const char *c = color(WT_STATUS_HEADER);
- color_printf_ln(c, "# %s:", main);
- color_printf_ln(c, "# (%s)", sub);
- color_printf_ln(c, "#");
+ color_fprintf_ln(s->fp, c, "# %s:", main);
+ color_fprintf_ln(s->fp, c, "# (%s)", sub);
+ color_fprintf_ln(s->fp, c, "#");
}
-static void wt_status_print_trailer(void)
+static void wt_status_print_trailer(struct wt_status *s)
{
- color_printf_ln(color(WT_STATUS_HEADER), "#");
+ color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "#");
}
static const char *quote_crlf(const char *in, char *buf, size_t sz)
@@ -108,7 +110,8 @@ static const char *quote_crlf(const char *in, char *buf, size_t sz)
return ret;
}
-static void wt_status_print_filepair(int t, struct diff_filepair *p)
+static void wt_status_print_filepair(struct wt_status *s,
+ int t, struct diff_filepair *p)
{
const char *c = color(t);
const char *one, *two;
@@ -117,36 +120,36 @@ static void wt_status_print_filepair(int t, struct diff_filepair *p)
one = quote_crlf(p->one->path, onebuf, sizeof(onebuf));
two = quote_crlf(p->two->path, twobuf, sizeof(twobuf));
- color_printf(color(WT_STATUS_HEADER), "#\t");
+ color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
switch (p->status) {
case DIFF_STATUS_ADDED:
- color_printf(c, "new file: %s", one);
+ color_fprintf(s->fp, c, "new file: %s", one);
break;
case DIFF_STATUS_COPIED:
- color_printf(c, "copied: %s -> %s", one, two);
+ color_fprintf(s->fp, c, "copied: %s -> %s", one, two);
break;
case DIFF_STATUS_DELETED:
- color_printf(c, "deleted: %s", one);
+ color_fprintf(s->fp, c, "deleted: %s", one);
break;
case DIFF_STATUS_MODIFIED:
- color_printf(c, "modified: %s", one);
+ color_fprintf(s->fp, c, "modified: %s", one);
break;
case DIFF_STATUS_RENAMED:
- color_printf(c, "renamed: %s -> %s", one, two);
+ color_fprintf(s->fp, c, "renamed: %s -> %s", one, two);
break;
case DIFF_STATUS_TYPE_CHANGED:
- color_printf(c, "typechange: %s", one);
+ color_fprintf(s->fp, c, "typechange: %s", one);
break;
case DIFF_STATUS_UNKNOWN:
- color_printf(c, "unknown: %s", one);
+ color_fprintf(s->fp, c, "unknown: %s", one);
break;
case DIFF_STATUS_UNMERGED:
- color_printf(c, "unmerged: %s", one);
+ color_fprintf(s->fp, c, "unmerged: %s", one);
break;
default:
die("bug: unhandled diff status %c", p->status);
}
- printf("\n");
+ fprintf(s->fp, "\n");
}
static void wt_status_print_updated_cb(struct diff_queue_struct *q,
@@ -160,14 +163,14 @@ static void wt_status_print_updated_cb(struct diff_queue_struct *q,
if (q->queue[i]->status == 'U')
continue;
if (!shown_header) {
- wt_status_print_cached_header(s->reference);
+ wt_status_print_cached_header(s);
s->commitable = 1;
shown_header = 1;
}
- wt_status_print_filepair(WT_STATUS_UPDATED, q->queue[i]);
+ wt_status_print_filepair(s, WT_STATUS_UPDATED, q->queue[i]);
}
if (shown_header)
- wt_status_print_trailer();
+ wt_status_print_trailer(s);
}
static void wt_status_print_changed_cb(struct diff_queue_struct *q,
@@ -184,12 +187,12 @@ static void wt_status_print_changed_cb(struct diff_queue_struct *q,
msg = use_add_rm_msg;
break;
}
- wt_status_print_header("Changed but not updated", msg);
+ wt_status_print_header(s, "Changed but not updated", msg);
}
for (i = 0; i < q->nr; i++)
- wt_status_print_filepair(WT_STATUS_CHANGED, q->queue[i]);
+ wt_status_print_filepair(s, WT_STATUS_CHANGED, q->queue[i]);
if (q->nr)
- wt_status_print_trailer();
+ wt_status_print_trailer(s);
}
static void wt_read_cache(struct wt_status *s)
@@ -206,16 +209,16 @@ static void wt_status_print_initial(struct wt_status *s)
wt_read_cache(s);
if (active_nr) {
s->commitable = 1;
- wt_status_print_cached_header(NULL);
+ wt_status_print_cached_header(s);
}
for (i = 0; i < active_nr; i++) {
- color_printf(color(WT_STATUS_HEADER), "#\t");
- color_printf_ln(color(WT_STATUS_UPDATED), "new file: %s",
+ color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
+ color_fprintf_ln(s->fp, color(WT_STATUS_UPDATED), "new file: %s",
quote_crlf(active_cache[i]->name,
buf, sizeof(buf)));
}
if (active_nr)
- wt_status_print_trailer();
+ wt_status_print_trailer(s);
}
static void wt_status_print_updated(struct wt_status *s)
@@ -282,12 +285,12 @@ static void wt_status_print_untracked(struct wt_status *s)
}
if (!shown_header) {
s->workdir_untracked = 1;
- wt_status_print_header("Untracked files",
+ wt_status_print_header(s, "Untracked files",
use_add_to_include_msg);
shown_header = 1;
}
- color_printf(color(WT_STATUS_HEADER), "#\t");
- color_printf_ln(color(WT_STATUS_UNTRACKED), "%.*s",
+ color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
+ color_fprintf_ln(s->fp, color(WT_STATUS_UNTRACKED), "%.*s",
ent->len, ent->name);
}
}
@@ -317,14 +320,14 @@ void wt_status_print(struct wt_status *s)
branch_name = "";
on_what = "Not currently on any branch.";
}
- color_printf_ln(color(WT_STATUS_HEADER),
+ color_fprintf_ln(s->fp, color(WT_STATUS_HEADER),
"# %s%s", on_what, branch_name);
}
if (s->is_initial) {
- color_printf_ln(color(WT_STATUS_HEADER), "#");
- color_printf_ln(color(WT_STATUS_HEADER), "# Initial commit");
- color_printf_ln(color(WT_STATUS_HEADER), "#");
+ color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "#");
+ color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "# Initial commit");
+ color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "#");
wt_status_print_initial(s);
}
else {
@@ -338,7 +341,7 @@ void wt_status_print(struct wt_status *s)
wt_status_print_verbose(s);
if (!s->commitable) {
if (s->amend)
- printf("# No changes\n");
+ fprintf(s->fp, "# No changes\n");
else if (s->workdir_dirty)
printf("no changes added to commit (use \"git add\" and/or \"git commit -a\")\n");
else if (s->workdir_untracked)
diff --git a/wt-status.h b/wt-status.h
index cfea4ae..4f3a615 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -1,6 +1,8 @@
#ifndef STATUS_H
#define STATUS_H
+#include <stdio.h>
+
enum color_wt_status {
WT_STATUS_HEADER,
WT_STATUS_UPDATED,
@@ -19,6 +21,7 @@ struct wt_status {
int commitable;
int workdir_dirty;
int workdir_untracked;
+ FILE *fp;
};
int git_status_config(const char *var, const char *value);
--
1.5.3.1.993.gbf388-dirty
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox