* [PATCH 4/5] commit: write cache-tree data when writing index anyway
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
In-Reply-To: <cover.1323191497.git.trast@student.ethz.ch>
In prepare_index(), we refresh the index, and then write it to disk if
this changed the index data. After running hooks we re-read the index
and compute the root tree sha1 with the cache-tree machinery.
This gives us a mostly free opportunity to write up-to-date cache-tree
data: we can compute it in prepare_index() immediately before writing
the index to disk.
If we do this, we were going to write the index anyway, and the later
cache-tree update has no further work to do. If we don't do it, we
don't do any extra work, though we still don't have have cache-tree
data after the commit.
The only case that suffers badly is when the pre-commit hook changes
many trees in the index. I'm writing this off as highly unusual.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Don't ask me why git thinks the index has been changed only when
already building upon an earlier commit. I don't know. I suspect
it's some raciness issue though.
builtin/commit.c | 2 ++
t/t0090-cache-tree.sh | 2 +-
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 0163086..57d028e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -394,6 +394,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
fd = hold_locked_index(&index_lock, 1);
add_files_to_cache(also ? prefix : NULL, pathspec, 0);
refresh_cache_or_die(refresh_flags);
+ update_main_cache_tree(1);
if (write_cache(fd, active_cache, active_nr) ||
close_lock_file(&index_lock))
die(_("unable to write new_index file"));
@@ -414,6 +415,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
if (active_cache_changed) {
+ update_main_cache_tree(1);
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(&index_lock))
die(_("unable to write new_index file"));
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 3d0702a..a3527a5 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -70,7 +70,7 @@ test_expect_success 'test-scrap-cache-tree works' '
test_no_cache_tree
'
-test_expect_failure 'second commit has cache-tree' '
+test_expect_success 'second commit has cache-tree' '
test_commit bar &&
test_shallow_cache_tree
'
--
1.7.8.425.ga639d3
^ permalink raw reply related
* [PATCH 5/5] reset: update cache-tree data when appropriate
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
In-Reply-To: <cover.1323191497.git.trast@student.ethz.ch>
In the case of --mixed and --hard, we throw away the old index and
rebuild everything from the tree argument (or HEAD). So we have an
opportunity here to fill in the cache-tree data, just as read-tree
did.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
builtin/reset.c | 7 +++++++
t/t0090-cache-tree.sh | 4 ++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 811e8e2..8c2c1d5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -43,6 +43,7 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
int nr = 1;
int newfd;
struct tree_desc desc[2];
+ struct tree *tree;
struct unpack_trees_options opts;
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
@@ -84,6 +85,12 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
return error(_("Failed to find tree of %s."), sha1_to_hex(sha1));
if (unpack_trees(nr, desc, &opts))
return -1;
+
+ if (reset_type == MIXED || reset_type == HARD) {
+ tree = parse_tree_indirect(sha1);
+ prime_cache_tree(&active_cache_tree, tree);
+ }
+
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
return error(_("Could not write new index file."));
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index a3527a5..f972562 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -75,13 +75,13 @@ test_expect_success 'second commit has cache-tree' '
test_shallow_cache_tree
'
-test_expect_failure 'reset --hard gives cache-tree' '
+test_expect_success 'reset --hard gives cache-tree' '
test-scrap-cache-tree &&
git reset --hard &&
test_shallow_cache_tree
'
-test_expect_failure 'reset --hard without index gives cache-tree' '
+test_expect_success 'reset --hard without index gives cache-tree' '
rm -f .git/index &&
git reset --hard &&
test_shallow_cache_tree
--
1.7.8.425.ga639d3
^ permalink raw reply related
* [PATCH 2/5] Test the current state of the cache-tree optimization
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
In-Reply-To: <cover.1323191497.git.trast@student.ethz.ch>
The cache-tree optimization originally helped speed up write-tree
operation. However, many commands no longer properly maintain -- or
use an opportunity to cheaply generate -- the cache-tree data. In
particular, this affects commit, checkout and reset. The notable
examples that *do* write cache-tree data are read-tree and write-tree.
This sadly means most people no longer benefit from the optimization,
as they would not normally use the plumbing commands.
Document the current state of affairs in a test file, in preparation
for improvements in the area.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
t/t0090-cache-tree.sh | 95 +++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 95 insertions(+), 0 deletions(-)
create mode 100755 t/t0090-cache-tree.sh
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
new file mode 100755
index 0000000..3d0702a
--- /dev/null
+++ b/t/t0090-cache-tree.sh
@@ -0,0 +1,95 @@
+#!/bin/sh
+
+test_description="Test whether cache-tree is properly updated
+
+Tests whether various commands properly update and/or rewrite the
+cache-tree extension.
+"
+ . ./test-lib.sh
+
+cmp_cache_tree () {
+ test-dump-cache-tree >actual &&
+ sed "s/$_x40/SHA/" <actual >filtered &&
+ test_cmp "$1" filtered
+}
+
+# We don't bother with actually checking the SHA1:
+# test-dump-cache-tree already verifies that all existing data is
+# correct.
+test_shallow_cache_tree () {
+ echo "SHA " \
+ "($(git ls-files|wc -l) entries, 0 subtrees)" >expect &&
+ cmp_cache_tree expect
+}
+
+test_invalid_cache_tree () {
+ echo "invalid (0 subtrees)" >expect &&
+ echo "SHA #(ref) " \
+ "($(git ls-files|wc -l) entries, 0 subtrees)" >>expect &&
+ cmp_cache_tree expect
+}
+
+test_no_cache_tree () {
+ : >expect &&
+ cmp_cache_tree expect
+}
+
+test_expect_failure 'initial commit has cache-tree' '
+ test_commit foo &&
+ test_shallow_cache_tree
+'
+
+test_expect_success 'read-tree HEAD establishes cache-tree' '
+ git read-tree HEAD &&
+ test_shallow_cache_tree
+'
+
+test_expect_success 'git-add invalidates cache-tree' '
+ test_when_finished "git reset --hard; git read-tree HEAD" &&
+ echo "I changed this file" > foo &&
+ git add foo &&
+ test_invalid_cache_tree
+'
+
+test_expect_success 'update-index invalidates cache-tree' '
+ test_when_finished "git reset --hard; git read-tree HEAD" &&
+ echo "I changed this file" > foo &&
+ git update-index --add foo &&
+ test_invalid_cache_tree
+'
+
+test_expect_success 'write-tree establishes cache-tree' '
+ test-scrap-cache-tree &&
+ git write-tree &&
+ test_shallow_cache_tree
+'
+
+test_expect_success 'test-scrap-cache-tree works' '
+ git read-tree HEAD &&
+ test-scrap-cache-tree &&
+ test_no_cache_tree
+'
+
+test_expect_failure 'second commit has cache-tree' '
+ test_commit bar &&
+ test_shallow_cache_tree
+'
+
+test_expect_failure 'reset --hard gives cache-tree' '
+ test-scrap-cache-tree &&
+ git reset --hard &&
+ test_shallow_cache_tree
+'
+
+test_expect_failure 'reset --hard without index gives cache-tree' '
+ rm -f .git/index &&
+ git reset --hard &&
+ test_shallow_cache_tree
+'
+
+test_expect_failure 'checkout gives cache-tree' '
+ git checkout HEAD^ &&
+ test_shallow_cache_tree
+'
+
+test_done
--
1.7.8.425.ga639d3
^ permalink raw reply related
* [PATCH 3/5] Refactor cache_tree_update idiom from commit
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
In-Reply-To: <cover.1323191497.git.trast@student.ethz.ch>
We'll need to safely create or update the cache-tree data of the_index
from other places. While at it, give it an argument that lets us
silence the messages produced by unmerged entries (which prevent it
from working).
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
builtin/commit.c | 5 +----
cache-tree.c | 19 +++++++++++++++----
cache-tree.h | 4 +++-
merge-recursive.c | 2 +-
test-dump-cache-tree.c | 2 +-
5 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index e36e9ad..0163086 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -862,10 +862,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
*/
discard_cache();
read_cache_from(index_file);
- if (!active_cache_tree)
- active_cache_tree = cache_tree();
- if (cache_tree_update(active_cache_tree,
- active_cache, active_nr, 0, 0) < 0) {
+ if (update_main_cache_tree(0)) {
error(_("Error building trees"));
return 0;
}
diff --git a/cache-tree.c b/cache-tree.c
index f755590..8de3959 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -150,7 +150,7 @@ void cache_tree_invalidate_path(struct cache_tree *it, const char *path)
}
static int verify_cache(struct cache_entry **cache,
- int entries)
+ int entries, int silent)
{
int i, funny;
@@ -159,6 +159,8 @@ static int verify_cache(struct cache_entry **cache,
for (i = 0; i < entries; i++) {
struct cache_entry *ce = cache[i];
if (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {
+ if (silent)
+ return -1;
if (10 < ++funny) {
fprintf(stderr, "...\n");
break;
@@ -370,10 +372,11 @@ int cache_tree_update(struct cache_tree *it,
struct cache_entry **cache,
int entries,
int missing_ok,
- int dryrun)
+ int dryrun,
+ int silent)
{
int i;
- i = verify_cache(cache, entries);
+ i = verify_cache(cache, entries, silent);
if (i)
return i;
i = update_one(it, cache, entries, "", 0, missing_ok, dryrun);
@@ -573,7 +576,7 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)
if (cache_tree_update(active_cache_tree,
active_cache, active_nr,
- missing_ok, 0) < 0)
+ missing_ok, 0, 0) < 0)
return WRITE_TREE_UNMERGED_INDEX;
if (0 <= newfd) {
if (!write_cache(newfd, active_cache, active_nr) &&
@@ -668,3 +671,11 @@ int cache_tree_matches_traversal(struct cache_tree *root,
return it->entry_count;
return 0;
}
+
+int update_main_cache_tree (int silent)
+{
+ if (!the_index.cache_tree)
+ the_index.cache_tree = cache_tree();
+ return cache_tree_update(the_index.cache_tree,
+ the_index.cache, the_index.cache_nr, 0, 0, silent);
+}
diff --git a/cache-tree.h b/cache-tree.h
index 3df641f..0ec0b2a 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -29,7 +29,9 @@ struct cache_tree {
struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
int cache_tree_fully_valid(struct cache_tree *);
-int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int);
+int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int, int);
+
+int update_main_cache_tree(int);
/* bitmasks to write_cache_as_tree flags */
#define WRITE_TREE_MISSING_OK 1
diff --git a/merge-recursive.c b/merge-recursive.c
index 5a2db29..d83cd6c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -264,7 +264,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)
if (!cache_tree_fully_valid(active_cache_tree) &&
cache_tree_update(active_cache_tree,
- active_cache, active_nr, 0, 0) < 0)
+ active_cache, active_nr, 0, 0, 0) < 0)
die("error building trees");
result = lookup_tree(active_cache_tree->sha1);
diff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c
index 1f73f1e..e6c2923 100644
--- a/test-dump-cache-tree.c
+++ b/test-dump-cache-tree.c
@@ -59,6 +59,6 @@ int main(int ac, char **av)
struct cache_tree *another = cache_tree();
if (read_cache() < 0)
die("unable to read index file");
- cache_tree_update(another, active_cache, active_nr, 0, 1);
+ cache_tree_update(another, active_cache, active_nr, 0, 1, 0);
return dump_cache_tree(active_cache_tree, another, "");
}
--
1.7.8.425.ga639d3
^ permalink raw reply related
* [PATCH 1/5] Add test-scrap-cache-tree
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
In-Reply-To: <cover.1323191497.git.trast@student.ethz.ch>
A simple utility that invalidates all existing cache-tree data. We
need this for tests. (We don't need a tool to rebuild the cache-tree
data; git read-tree HEAD works for that.)
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
.gitignore | 1 +
Makefile | 1 +
test-scrap-cache-tree.c | 17 +++++++++++++++++
3 files changed, 19 insertions(+), 0 deletions(-)
create mode 100644 test-scrap-cache-tree.c
diff --git a/.gitignore b/.gitignore
index 8572c8c..122336c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -171,6 +171,7 @@
/test-date
/test-delta
/test-dump-cache-tree
+/test-scrap-cache-tree
/test-genrandom
/test-index-version
/test-line-buffer
diff --git a/Makefile b/Makefile
index cbb5601..6bfa10b 100644
--- a/Makefile
+++ b/Makefile
@@ -436,6 +436,7 @@ TEST_PROGRAMS_NEED_X += test-ctype
TEST_PROGRAMS_NEED_X += test-date
TEST_PROGRAMS_NEED_X += test-delta
TEST_PROGRAMS_NEED_X += test-dump-cache-tree
+TEST_PROGRAMS_NEED_X += test-scrap-cache-tree
TEST_PROGRAMS_NEED_X += test-genrandom
TEST_PROGRAMS_NEED_X += test-index-version
TEST_PROGRAMS_NEED_X += test-line-buffer
diff --git a/test-scrap-cache-tree.c b/test-scrap-cache-tree.c
new file mode 100644
index 0000000..4728013
--- /dev/null
+++ b/test-scrap-cache-tree.c
@@ -0,0 +1,17 @@
+#include "cache.h"
+#include "tree.h"
+#include "cache-tree.h"
+
+static struct lock_file index_lock;
+
+int main(int ac, char **av)
+{
+ int fd = hold_locked_index(&index_lock, 1);
+ if (read_cache() < 0)
+ die("unable to read index file");
+ active_cache_tree = NULL;
+ if (write_cache(fd, active_cache, active_nr)
+ || commit_lock_file(&index_lock))
+ die("unable to write index file");
+ return 0;
+}
--
1.7.8.425.ga639d3
^ permalink raw reply related
* [PATCH 0/5] cache-tree revisited
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
Junio C Hamano wrote:
> Ahh, I forgot all about that exchange.
>
> http://thread.gmane.org/gmane.comp.version-control.git/178480/focus=178515
>
> The cache-tree mechanism has traditionally been one of the more important
> optimizations and it would be very nice if we can resurrect the behaviour
> for "git commit" too.
Oh, I buried that. Let's try something other than the aggressive
strategy I had there: only compute cache-tree if
* we know we're going to need it soon, and we're about to write out
the index anyway (as in git-commit)
* we know we can rebuild it from a tree, and we're about to write out
the index anyway (as in git-reset)
Both of these are essentially free, so I'm not sure I should even
bother with timings, but I ran the silly script at the end on my
favourite linux v2.6.37-rc2 repo, and it takes
before-: real 0m29.103s user 0m17.335s sys 0m8.415s
before+: real 0m30.860s user 0m9.741s sys 0m7.287s
after-: real 0m28.798s user 0m17.367s sys 0m8.435s
after+: real 0m17.563s user 0m8.246s sys 0m7.122s
where the + signifies starting out with fully valid cache-tree data,
and - means starting out with none at all.
So it's really free when it's starting out in bad shape, and it's
much faster when it starts out with valid data.
If you insist on writing cache-tree at *every* (except --only) commit,
then we might make it so that it writes out the index again at the
very end if it didn't already update it earlier. It would (and does,
I've tried... ok ok, patch is at the end) of course give the +
performance in the - case, but it's not free so other operations may
be affected.
---- 8< ---- test script for the above timings ---- 8< ----
#!/bin/sh
for i in $(seq 1 100); do
echo $i > arch/alpha/boot/foo
git add arch/alpha/boot/foo
git commit -m$i
done
---- >8 ----
---- 8< ---- patch to let git-commit always write the index ---- 8< ----
diff --git i/builtin/commit.c w/builtin/commit.c
index 57d028e..8e0c773 100644
--- i/builtin/commit.c
+++ w/builtin/commit.c
@@ -333,6 +333,8 @@ static void refresh_cache_or_die(int refresh_flags)
die_resolve_conflict("commit");
}
+static int wrote_index_already = 0;
+
static char *prepare_index(int argc, const char **argv, const char *prefix,
const struct commit *current_head, int is_status)
{
@@ -395,6 +397,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
add_files_to_cache(also ? prefix : NULL, pathspec, 0);
refresh_cache_or_die(refresh_flags);
update_main_cache_tree(1);
+ wrote_index_already = 1;
if (write_cache(fd, active_cache, active_nr) ||
close_lock_file(&index_lock))
die(_("unable to write new_index file"));
@@ -415,6 +418,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
if (active_cache_changed) {
+ wrote_index_already = 1;
update_main_cache_tree(1);
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(&index_lock))
@@ -639,6 +643,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
const char *hook_arg2 = NULL;
int ident_shown = 0;
int clean_message_contents = (cleanup_mode != CLEANUP_NONE);
+ int fd;
if (!no_verify && run_hook(index_file, "pre-commit", NULL))
return 0;
@@ -863,11 +868,17 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
* the editor and after we invoke run_status above.
*/
discard_cache();
+ if (!wrote_index_already)
+ fd = hold_locked_index(&index_lock, 1);
read_cache_from(index_file);
if (update_main_cache_tree(0)) {
error(_("Error building trees"));
return 0;
}
+ if (!wrote_index_already && commit_style != COMMIT_PARTIAL)
+ if (write_cache(fd, active_cache, active_nr) ||
+ close_lock_file(&index_lock))
+ die(_("unable to write new_index file"));
if (run_hook(index_file, "prepare-commit-msg",
git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
---- >8 ----
Thomas Rast (5):
Add test-scrap-cache-tree
Test the current state of the cache-tree optimization
Refactor cache_tree_update idiom from commit
commit: write cache-tree data when writing index anyway
reset: update cache-tree data when appropriate
.gitignore | 1 +
Makefile | 1 +
builtin/commit.c | 7 +--
builtin/reset.c | 7 +++
cache-tree.c | 19 +++++++--
cache-tree.h | 4 +-
merge-recursive.c | 2 +-
t/t0090-cache-tree.sh | 95 +++++++++++++++++++++++++++++++++++++++++++++++
test-dump-cache-tree.c | 2 +-
test-scrap-cache-tree.c | 17 ++++++++
10 files changed, 144 insertions(+), 11 deletions(-)
create mode 100755 t/t0090-cache-tree.sh
create mode 100644 test-scrap-cache-tree.c
--
1.7.8.425.ga639d3
^ permalink raw reply related
* git svn rebase “out of memory” error after merging two tracking branches
From: Onsager @ 2011-12-06 16:40 UTC (permalink / raw)
To: git
@list
... more detailed explanation on stackoverflow:
http://stackoverflow.com/questions/8398405/git-svn-rebase-out-of-memory-error-after-merging-two-tracking-branches
Is there something, I can do about this issue as a common user?
Michael
^ permalink raw reply
* [PATCH] userdiff: allow * between cpp funcname words
From: Thomas Rast @ 2011-12-06 16:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
The cpp pattern, used for C and C++, would not match the start of a
declaration such as
static char *prepare_index(int argc,
because it did not allow for * anywhere between the various words that
constitute the modifiers, type and function name. Fix it.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
This is a really sneaky one-character bug that I cannot believe went
unnoticed for so long, seeing as there are plenty of instances within
git itself where it matters.
userdiff.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/userdiff.c b/userdiff.c
index 7c983c1..76109da 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -118,7 +118,7 @@
/* Jump targets or access declarations */
"!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n"
/* C/++ functions/methods at top level */
- "^([A-Za-z_][A-Za-z_0-9]*([ \t]+[A-Za-z_][A-Za-z_0-9]*([ \t]*::[ \t]*[^[:space:]]+)?){1,}[ \t]*\\([^;]*)$\n"
+ "^([A-Za-z_][A-Za-z_0-9]*([ \t*]+[A-Za-z_][A-Za-z_0-9]*([ \t]*::[ \t]*[^[:space:]]+)?){1,}[ \t]*\\([^;]*)$\n"
/* compound type at top level */
"^((struct|class|enum)[^;]*)$",
/* -- */
--
1.7.8.424.gcb840
^ permalink raw reply related
* Re: Query on git commit amend
From: Vijay Lakshminarayanan @ 2011-12-06 15:46 UTC (permalink / raw)
To: Viresh Kumar; +Cc: git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <4EDDD0E4.6040003@st.com>
Viresh Kumar <viresh.kumar@st.com> writes:
> Hello,
>
> Suppose i want to add few new changes to my last commit (HEAD).
> The way i do it is
> $ git add all_changed_files
> $ git commit --amend
>
> OR
> $ git commit --amend -a
>
> With both these ways, i get a screen to edit the message too.
>
> I want to know if there is a way to skip this screen.
>
> i.e.
> $ git commit --amend -a -some_other_option
>
> which simply adds new changes to existing commit, without asking to change
> message.
>
> If there is no such way, then can we add a patch for this, if it looks a valid
> case.
I've found
$ GIT_EDITOR=cat git commit --amend
useful.
The benefit of this technique is that it even works for git-rebase -i.
In my typical git usage, I do a lot of git-commit --fixup's. After
reaching a level of stability, I change the history with:
GIT_EDITOR=cat git rebase -i --autosquash
and my history is adjusted without requiring manual intervention.
--
Cheers
~vijay
Gnus should be more complicated.
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Nguyen Thai Ngoc Duy @ 2011-12-06 15:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcpthh97.fsf@alter.siamese.dyndns.org>
2011/12/6 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> Too deep delta chains can cause stack overflow in get_base_data(). Set
>> a hard limit so that index-pack does not run out of stack. Also stop
>> people from producing such a long delta chains using "pack-object
>> --depth=<too large>"
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> I used to make very long delta chains and triggered this in index-pack.
>> I did not care reporting because it's my fault anyway. Think again,
>> index-pack is called at server side and a malicious client can
>> trigger this. This patch does not improve the situation much, but at
>> least we won't get sigsegv at server side.
>
> Why should we treat this condition any differently from the case where the
> sender of a pack used beefier machine than you have and stuffed a huge
> object that the index-pack running on your box cannot hold in core,
> causing xmalloc() to die on your machine?
That's interesting. First of all xmalloc() is controlled by us while
index-pack code might lead to stack overflow exploit (never done it,
not sure if it's really pratical to do in this case).
But can I really use up all memory at server side by sending a huge pack?
> I do not think this is the right way to handle the issue. Your other patch
> to flatten the recursion to iteration looked a lot saner approach.
It may take me some time as I'm not really familar with this code.
Anybody is welcome to step up and flatten the function.
--
Duy
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Nguyen Thai Ngoc Duy @ 2011-12-06 15:30 UTC (permalink / raw)
To: Michael Haggerty; +Cc: kusmabite, git, Junio C Hamano
In-Reply-To: <4EDE2C95.5040804@alum.mit.edu>
2011/12/6 Michael Haggerty <mhagger@alum.mit.edu>:
> On 12/06/2011 01:17 PM, Erik Faye-Lund wrote:
>> 2011/12/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>>> Too deep delta chains can cause stack overflow in get_base_data(). Set
>>> a hard limit so that index-pack does not run out of stack. Also stop
>>> people from producing such a long delta chains using "pack-object
>>> --depth=<too large>"
>>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>> ---
>>> I used to make very long delta chains and triggered this in index-pack.
>>> I did not care reporting because it's my fault anyway. Think again,
>>> index-pack is called at server side and a malicious client can
>>> trigger this. This patch does not improve the situation much, but at
>>> least we won't get sigsegv at server side.
>>
>> Wouldn't it make more sense to make the limit a config option rather
>> than a hard-coded value of 128 (which seems arbitrary to me)? After
>> all, different platforms have different stack-limitations...
>
> I'm confused: is the data only ever read by the same host that generated
> it?
index-pack is called at server side as part of a push. So in theory
the sending side can generate very long delta chains and bring down
the server side.
> Because if not, then the "creator" had better never be configured
> to use a chain depth that the "reader" cannot handle.
Normal creators (i.e. C Git) use default depth 50 so we should be safe.
> This in turn
> imply that there should be a common limit that is supported by all git
> clients and is a documented part of the protocol. (Or the code has to
> be rewritten to use an explicit stack instead of recursion.)
It's the implementation limitation, not the protocol. If the server
propagates error messages from index-pack back to client (I'm not
sure), then users can adjust --depth to be accepted by server. We
could negotiate the limit over the protocol but not sure we would want
to go that route.
The troubled code could be rewritten to avoid recursion. However long
delta chains may degrade performance, I'd rather have support to split
long chains (which can be done with --depth at client already) instead
of just recursion elimination. This patch is simpler, so it would be
easier to back port it if someone wants to do so.
--
Duy
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Junio C Hamano @ 2011-12-06 15:06 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1323068688-31481-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Too deep delta chains can cause stack overflow in get_base_data(). Set
> a hard limit so that index-pack does not run out of stack. Also stop
> people from producing such a long delta chains using "pack-object
> --depth=<too large>"
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> I used to make very long delta chains and triggered this in index-pack.
> I did not care reporting because it's my fault anyway. Think again,
> index-pack is called at server side and a malicious client can
> trigger this. This patch does not improve the situation much, but at
> least we won't get sigsegv at server side.
Why should we treat this condition any differently from the case where the
sender of a pack used beefier machine than you have and stuffed a huge
object that the index-pack running on your box cannot hold in core,
causing xmalloc() to die on your machine?
I do not think this is the right way to handle the issue. Your other patch
to flatten the recursion to iteration looked a lot saner approach.
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Michael Haggerty @ 2011-12-06 14:54 UTC (permalink / raw)
To: kusmabite; +Cc: Nguyễn Thái Ngọc Duy, git, Junio C Hamano
In-Reply-To: <CABPQNSaE=TWGbBRMnthEuT181=XbOafPfgx88_JQnnQ6TiYyqw@mail.gmail.com>
On 12/06/2011 01:17 PM, Erik Faye-Lund wrote:
> 2011/12/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>> Too deep delta chains can cause stack overflow in get_base_data(). Set
>> a hard limit so that index-pack does not run out of stack. Also stop
>> people from producing such a long delta chains using "pack-object
>> --depth=<too large>"
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> I used to make very long delta chains and triggered this in index-pack.
>> I did not care reporting because it's my fault anyway. Think again,
>> index-pack is called at server side and a malicious client can
>> trigger this. This patch does not improve the situation much, but at
>> least we won't get sigsegv at server side.
>
> Wouldn't it make more sense to make the limit a config option rather
> than a hard-coded value of 128 (which seems arbitrary to me)? After
> all, different platforms have different stack-limitations...
I'm confused: is the data only ever read by the same host that generated
it? Because if not, then the "creator" had better never be configured
to use a chain depth that the "reader" cannot handle. This in turn
imply that there should be a common limit that is supported by all git
clients and is a documented part of the protocol. (Or the code has to
be rewritten to use an explicit stack instead of recursion.)
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH 0/8] nd/resolve-ref v2
From: Nguyen Thai Ngoc Duy @ 2011-12-06 14:07 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, Junio C Hamano, Jeff King, Michael Haggerty,
Ramkumar Ramachandra
In-Reply-To: <20111117103924.GA5277@elie.hsd1.il.comcast.net>
2011/11/17 Jonathan Nieder <jrnieder@gmail.com>:
> Hi,
>
> Nguyễn Thái Ngọc Duy wrote:
>
>> Nguyễn Thái Ngọc Duy (8):
>
> Here are some comments from a quick glance over the series, to avoid
> too much noise that would distract from reports about the upcoming
> release.
Sorry for the late reply. Somehow I managed to miss your mail.
>> Re-add resolve_ref() that always returns an allocated buffer
>
> Makes me nervous, since it would introduce memory leaks if some other
> patch in flight calls resolve_ref(). Why not call it ref_resolve() or
> something?
Yeah, I made the same mistake before and made it again.
>> Enable GIT_DEBUG_MEMCHECK on git_pathname()
>
> __VA_ARGS__ was introduced in C99. I suspect some compilers that
> currently can compile git don't support it. But if you need to use
> it, that wouldn't rule out doing so in some corner guarded with an
> #ifdef.
>
> Looks pleasant overall. I look forward to looking more closely at
> this later.
This patch is bonus anyway and I think I'll drop it. Keeping a bunch
of ifdefs looks really ugly. I think having git_pathname()'s static
buffer per callsite file would be a good thing to do instead.
--
Duy
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Nguyen Thai Ngoc Duy @ 2011-12-06 14:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8vmqi98f.fsf@alter.siamese.dyndns.org>
On Tue, Dec 6, 2011 at 12:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> * nd/resolve-ref (2011-12-05) 2 commits
> (merged to 'next' on 2011-12-05 at cc79e86)
> + Copy resolve_ref() return value for longer use
> + Convert many resolve_ref() calls to read_ref*() and ref_exists()
>
> Will merge to 'master'.
I thought this would be replaced by a new version [1] I posted a while
ago (and forgot to address comments). Do you want me to rebase that on
top of this or replace it?
[1] http://article.gmane.org/gmane.comp.version-control.git/185580
--
Duy
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Nguyen Thai Ngoc Duy @ 2011-12-06 12:48 UTC (permalink / raw)
To: kusmabite; +Cc: git, Junio C Hamano
In-Reply-To: <CABPQNSbOvVkSFAE32hXoZEZeHqg-+nHc93uGytfCPQMicu0uVg@mail.gmail.com>
On Tue, Dec 6, 2011 at 7:41 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>> Wouldn't it make more sense to make the limit a config option rather
>>> than a hard-coded value of 128 (which seems arbitrary to me)? After
>>> all, different platforms have different stack-limitations...
>>
>> Then it'd make more sense to make a compile time config based on
>> platform.
>
> Can how much stack each recursion use be calculated at compile-time?
> If so, I agree with you.
No, but at least we know default stack size of each platform and can
make pretty good limit based on that.
>> We could have a config option that can override the default,
>> but I really don't see the point of making long delta chains.
>
> Aha, I figured you _did_ see a point in this, because 128 seemed
> excessive to me already. I was thinking more that some platforms can
> have a much smaller stack than (I would expect to) fit in 128
> recursions (I've worked relatively recently with platforms with as
> small as a static 2k stack per process), so you might not be fixing
> the issue for such platforms. But that's not really your
> responsibility either ;)
Ah, I was thinking of an option that extends the limit, not shortens
it. Yes it makes sense in this case.
--
Duy
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Erik Faye-Lund @ 2011-12-06 12:41 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <CACsJy8BuUn4htSR6jAJfbueOshE6-YVV5dZGSWTGXbkuA0HO=A@mail.gmail.com>
On Tue, Dec 6, 2011 at 1:32 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> 2011/12/6 Erik Faye-Lund <kusmabite@gmail.com>:
>> 2011/12/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>>> Too deep delta chains can cause stack overflow in get_base_data(). Set
>>> a hard limit so that index-pack does not run out of stack. Also stop
>>> people from producing such a long delta chains using "pack-object
>>> --depth=<too large>"
>>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>> ---
>>> I used to make very long delta chains and triggered this in index-pack.
>>> I did not care reporting because it's my fault anyway. Think again,
>>> index-pack is called at server side and a malicious client can
>>> trigger this. This patch does not improve the situation much, but at
>>> least we won't get sigsegv at server side.
>>>
>>
>> Wouldn't it make more sense to make the limit a config option rather
>> than a hard-coded value of 128 (which seems arbitrary to me)? After
>> all, different platforms have different stack-limitations...
>
> Then it'd make more sense to make a compile time config based on
> platform.
Can how much stack each recursion use be calculated at compile-time?
If so, I agree with you.
> We could have a config option that can override the default,
> but I really don't see the point of making long delta chains.
Aha, I figured you _did_ see a point in this, because 128 seemed
excessive to me already. I was thinking more that some platforms can
have a much smaller stack than (I would expect to) fit in 128
recursions (I've worked relatively recently with platforms with as
small as a static 2k stack per process), so you might not be fixing
the issue for such platforms. But that's not really your
responsibility either ;)
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Nguyen Thai Ngoc Duy @ 2011-12-06 12:32 UTC (permalink / raw)
To: kusmabite; +Cc: git, Junio C Hamano
In-Reply-To: <CABPQNSaE=TWGbBRMnthEuT181=XbOafPfgx88_JQnnQ6TiYyqw@mail.gmail.com>
2011/12/6 Erik Faye-Lund <kusmabite@gmail.com>:
> 2011/12/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>> Too deep delta chains can cause stack overflow in get_base_data(). Set
>> a hard limit so that index-pack does not run out of stack. Also stop
>> people from producing such a long delta chains using "pack-object
>> --depth=<too large>"
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> I used to make very long delta chains and triggered this in index-pack.
>> I did not care reporting because it's my fault anyway. Think again,
>> index-pack is called at server side and a malicious client can
>> trigger this. This patch does not improve the situation much, but at
>> least we won't get sigsegv at server side.
>>
>
> Wouldn't it make more sense to make the limit a config option rather
> than a hard-coded value of 128 (which seems arbitrary to me)? After
> all, different platforms have different stack-limitations...
Then it'd make more sense to make a compile time config based on
platform. We could have a config option that can override the default,
but I really don't see the point of making long delta chains.
--
Duy
^ permalink raw reply
* Re: [PATCH] Set hard limit on delta chain depth
From: Erik Faye-Lund @ 2011-12-06 12:17 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <1323068688-31481-1-git-send-email-pclouds@gmail.com>
2011/12/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
> Too deep delta chains can cause stack overflow in get_base_data(). Set
> a hard limit so that index-pack does not run out of stack. Also stop
> people from producing such a long delta chains using "pack-object
> --depth=<too large>"
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> I used to make very long delta chains and triggered this in index-pack.
> I did not care reporting because it's my fault anyway. Think again,
> index-pack is called at server side and a malicious client can
> trigger this. This patch does not improve the situation much, but at
> least we won't get sigsegv at server side.
>
Wouldn't it make more sense to make the limit a config option rather
than a hard-coded value of 128 (which seems arbitrary to me)? After
all, different platforms have different stack-limitations...
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Erik Faye-Lund @ 2011-12-06 11:22 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Git Mailing List, Johannes Sixt, Stephan Beyer
In-Reply-To: <20111206055239.GA20671@sigill.intra.peff.net>
On Tue, Dec 6, 2011 at 6:52 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Dec 05, 2011 at 09:01:52PM -0800, Junio C Hamano wrote:
>
>> * jk/credentials (2011-11-28) 20 commits
>> - fixup! 034c066e
>> - compat/getpass: add a /dev/tty implementation
>> - credential: use git_prompt instead of git_getpass
>> - prompt: add PROMPT_ECHO flag
>> - stub out getpass_echo function
>> - refactor git_getpass into generic prompt function
>> - move git_getpass to its own source file
>> - t: add test harness for external credential helpers
>> - credentials: add "store" helper
>> - strbuf: add strbuf_add*_urlencode
>> - credentials: add "cache" helper
>> - docs: end-user documentation for the credential subsystem
>> - credential: make relevance of http path configurable
>> - credential: add credential.*.username
>> - credential: apply helper config
>> - http: use credential API to get passwords
>> - credential: add function for parsing url components
>> - introduce credentials API
>> - t5550: fix typo
>> - test-lib: add test_config_global variant
>>
>> Expecting a reroll?
>
> Yes, I have a re-roll ready. I was holding back to see if some of the
> helper authors might comment, but got nothing. Perhaps the new version
> will spur some interest.
>
> Also, let's drop the top git_getpass bits from the topic for now (they
> will not be part of my rebase). They are a separate topic that can go on
> top, but I think there was some question from Erik of whether we should
> simply roll our own getpass().
>
>> * jk/upload-archive-use-start-command (2011-11-21) 1 commit
>> - upload-archive: use start_command instead of fork
>>
>> What's the status of this one?
>
> I think what you have in pu is good, but of course I didn't actually
> test it on Windows. Erik?
>
I tried to check out ee27ca4 and build, and got hit by a wall of warnings:
compat/mingw.h: In function 'waitpid':
compat/mingw.h:117: warning: pointer targets in passing argument 1 of
'_cwait' differ in signedness
d:\msysgit\mingw\bin\../lib/gcc/mingw32/4.4.0/../../../../include/process.h:60:
note: expected 'int *' but argument is of type 'unsigned int *'
This seems to be an issue that has been around since the birth of the
windows port, and can be fixed by making our waitpid signature
identical to what POSIX expects
(http://pubs.opengroup.org/onlinepubs/7908799/xsh/waitpid.html):
---8<---
diff --git a/compat/mingw.h b/compat/mingw.h
index a255898..2036013 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -111,7 +111,7 @@ static inline int mingw_unlink(const char *pathname)
}
#define unlink mingw_unlink
-static inline int waitpid(pid_t pid, unsigned *status, unsigned options)
+static inline int waitpid(pid_t pid, int *status, int options)
{
if (options == 0)
return _cwait(status, pid, 0);
---8<---
But then, there's a couple of much more scary warnings:
decorate.c: In function 'hash_obj':
decorate.c:11: warning: dereferencing type-punned pointer will break
strict-aliasing rules
<snip>
object.c: In function 'hash_obj':
object.c:48: warning: dereferencing type-punned pointer will break
strict-aliasing rules
<snip>
builtin-merge-recursive.c: In function 'cmd_merge_recursive':
builtin-merge-recursive.c:49: warning: unknown conversion type character 'z' in
format
builtin-merge-recursive.c:49: warning: format '%s' expects type 'char *', but ar
gument 2 has type 'unsigned int'
builtin-merge-recursive.c:49: warning: too many arguments for format
Looking at the code, the first two looks like the real thing, and not
just false positives.
IIRC, the strict aliasing rule is not a part of C89, but is in C99.
And GCC starts to generate code that depends on strict-aliasing rules
with -O3. We don't use -O3, so this might be theoretical, at least on
my machine at this time.
The last one is a usage if "%zu", which our printf doesn't support.
This comes from commit 73118f89 ("merge-recursive.c: Add more generic
merge_recursive_generic()"), and should probably be fixed by doing:
---8<---
diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
index 703045b..02242cc 100644
--- a/builtin-merge-recursive.c
+++ b/builtin-merge-recursive.c
@@ -45,8 +45,9 @@ int cmd_merge_recursive(int argc, const char **argv,
const char *prefix)
bases[bases_count++] = sha;
}
else
- warning("Cannot handle more than %zu bases. "
- "Ignoring %s.", ARRAY_SIZE(bases)-1, argv[i]);
+ warning("Cannot handle more than %"PRIuMAX" bases. "
+ "Ignoring %s.",
+ (uintmax_t)ARRAY_SIZE(bases)-1, argv[i]);
}
if (argc - i != 3) /* "--" "<head>" "<remote>" */
die("Not handling anything other than two heads merge.");
---8<---
But no matter what, something has clearly happened to our warning
level, and that should probably be investigated.
Back to topic: after fixing these (apart from the strict aliasing
issues), t5000-tar-tree pass fine here. So I guess that's at least
something. I haven't tested from git-daemon yet, but I don't have time
for that right now...
I'll submit the first fix later tonight, as it's clearly an
improvement IMO... but perhaps it's better if Stephan just squash in
the latter in the next round of the series, since it seems to be in a
pu-only series so far?
^ permalink raw reply related
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Nguyen Thai Ngoc Duy @ 2011-12-06 11:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8vmqi98f.fsf@alter.siamese.dyndns.org>
On Tue, Dec 6, 2011 at 12:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Here are the topics that have been cooking. Commits prefixed with '-' are
> only in 'pu' (proposed updates) while commits prefixed with '+' are in
> 'next'.
What can I do to get "build in repack" [1] series in or moved forward?
[1] http://thread.gmane.org/gmane.comp.version-control.git/185190
--
Duy
^ permalink raw reply
* Re: Roadmap for 1.7.9
From: Nguyen Thai Ngoc Duy @ 2011-12-06 11:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd3c2lr36.fsf@alter.siamese.dyndns.org>
On Tue, Dec 6, 2011 at 3:07 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I expect the following will not make much progress without further
> discussion:
>
> * Ignored vs Precious (Nguyen Thai Ngoc Duy)
I'm working on adding --exclude-standard and --exclude-from to
read-tree. Apart from that I don't plan on supporting precious files
because I don't use them.
--
Duy
^ permalink raw reply
* Re: Query on git commit amend
From: Viresh Kumar @ 2011-12-06 9:22 UTC (permalink / raw)
To: Johannes Sixt; +Cc: flatworm, git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <4EDDDE5C.6040200@st.com>
On 12/6/2011 2:50 PM, Viresh Kumar wrote:
> On 12/6/2011 2:41 PM, Johannes Sixt wrote:
>> Am 12/6/2011 9:23, schrieb Viresh Kumar:
>> $ git commit --amend -a -C HEAD
>>
>> But let's count keystrokes (after -a):
>>
>> <BLANK>-<SHIFT>C<BLANK>HEAD<ENTER>
>> 10 keystrokes (more if you release SHIFT before D)
>>
>> But if vi pops up you have:
>>
>> <ENTER><SHIFT>ZZ
>> 4 keystrokes
>>
>> Where is the advantage of the option?
>>
>
> Thanks guys.
>
> @Johannes: You are right but, i will make an alias for the entire command.
> So keystrokes are same for me. :)
>
There is one more benefit, we don't have to wait for git. We can simply switch to
some other window and work. And can write further command on the same window, in
the time git processes commit --amend.
--
viresh
^ permalink raw reply
* Re: Query on git commit amend
From: Viresh Kumar @ 2011-12-06 9:20 UTC (permalink / raw)
To: Johannes Sixt, flatworm; +Cc: git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <4EDDDC38.8080108@viscovery.net>
On 12/6/2011 2:41 PM, Johannes Sixt wrote:
> Am 12/6/2011 9:23, schrieb Viresh Kumar:
> $ git commit --amend -a -C HEAD
>
> But let's count keystrokes (after -a):
>
> <BLANK>-<SHIFT>C<BLANK>HEAD<ENTER>
> 10 keystrokes (more if you release SHIFT before D)
>
> But if vi pops up you have:
>
> <ENTER><SHIFT>ZZ
> 4 keystrokes
>
> Where is the advantage of the option?
>
Thanks guys.
@Johannes: You are right but, i will make an alias for the entire command.
So keystrokes are same for me. :)
--
viresh
^ permalink raw reply
* Re: Query on git commit amend
From: Johannes Sixt @ 2011-12-06 9:11 UTC (permalink / raw)
To: Viresh Kumar; +Cc: git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <4EDDD0E4.6040003@st.com>
Am 12/6/2011 9:23, schrieb Viresh Kumar:
>
> Hello,
>
> Suppose i want to add few new changes to my last commit (HEAD).
> The way i do it is
> $ git add all_changed_files
> $ git commit --amend
>
> OR
> $ git commit --amend -a
>
> With both these ways, i get a screen to edit the message too.
>
> I want to know if there is a way to skip this screen.
>
> i.e.
> $ git commit --amend -a -some_other_option
>
> which simply adds new changes to existing commit, without asking to change
> message.
$ git commit --amend -a -C HEAD
But let's count keystrokes (after -a):
<BLANK>-<SHIFT>C<BLANK>HEAD<ENTER>
10 keystrokes (more if you release SHIFT before D)
But if vi pops up you have:
<ENTER><SHIFT>ZZ
4 keystrokes
Where is the advantage of the option?
-- Hannes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox