* [PATCH 2/3] commit/status: "git add <path>" is not necessarily how to resolve
From: Junio C Hamano @ 2009-12-12 9:02 UTC (permalink / raw)
To: git
In-Reply-To: <1260608523-15579-2-git-send-email-gitster@pobox.com>
When the desired resolution is to remove the path, "git rm <path>" is the
command the user needs to use. Just like in "Changed but not updated"
section, suggest to use "git add/rm" as appropriate.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t7060-wtstatus.sh | 2 +-
wt-status.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
index 7b5db80..6c1af26 100755
--- a/t/t7060-wtstatus.sh
+++ b/t/t7060-wtstatus.sh
@@ -32,7 +32,7 @@ cat >expect <<EOF
# On branch side
# Unmerged paths:
# (use "git reset HEAD <file>..." to unstage)
-# (use "git add <file>..." to mark resolution)
+# (use "git add/rm <file>..." as appropriately to mark resolution)
#
# deleted by us: foo
#
diff --git a/wt-status.c b/wt-status.c
index 3fdcf97..5271b6a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -52,7 +52,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
else
color_fprintf_ln(s->fp, c, "# (use \"git rm --cached <file>...\" to unstage)");
- color_fprintf_ln(s->fp, c, "# (use \"git add <file>...\" to mark resolution)");
+ color_fprintf_ln(s->fp, c, "# (use \"git add/rm <file>...\" as appropriately to mark resolution)");
color_fprintf_ln(s->fp, c, "#");
}
--
1.6.6.rc2.5.g49666
^ permalink raw reply related
* [PATCH 3/3] status/commit: do not suggest "reset HEAD <path>" while merging
From: Junio C Hamano @ 2009-12-12 9:02 UTC (permalink / raw)
To: git
In-Reply-To: <1260608523-15579-3-git-send-email-gitster@pobox.com>
Suggesting "'reset HEAD <path>' to unstage" is dead wrong if we are about
to record a merge commit. For either an unmerged path (i.e. with
unresolved conflicts), or an updated path, it would result in discarding
what the other branch did.
Note that we do not do anything special in a case where we are amending a
merge. The user is making an evil merge starting from an already
committed merge, and running "reset HEAD <path>" is the right way to get
rid of the local edit that has been added to the index.
Once "reset --unresolve <path>" becomes available, we might want to
suggest it for a merged path that has unresolve information, but until
then, just remove the incorrect advice.
We might also want to suggest "checkout --conflict <path>" to revert the
file in the work tree to the state of failed automerge for an unmerged
path, but we never did that, and this commit does not change that.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-commit.c | 2 ++
t/t7060-wtstatus.sh | 1 -
wt-status.c | 14 ++++++++++----
wt-status.h | 1 +
4 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 17dd462..7218454 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -960,6 +960,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
read_cache();
refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
+ s.in_merge = in_merge;
wt_status_collect(&s);
if (s.relative_paths)
@@ -1056,6 +1057,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
wt_status_prepare(&s);
git_config(git_commit_config, &s);
in_merge = file_exists(git_path("MERGE_HEAD"));
+ s.in_merge = in_merge;
if (s.use_color == -1)
s.use_color = git_use_color_default;
diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
index 6c1af26..6dd7077 100755
--- a/t/t7060-wtstatus.sh
+++ b/t/t7060-wtstatus.sh
@@ -31,7 +31,6 @@ test_expect_success 'Report new path with conflict' '
cat >expect <<EOF
# On branch side
# Unmerged paths:
-# (use "git reset HEAD <file>..." to unstage)
# (use "git add/rm <file>..." as appropriately to mark resolution)
#
# deleted by us: foo
diff --git a/wt-status.c b/wt-status.c
index 5271b6a..3f62c44 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -47,8 +47,11 @@ void wt_status_prepare(struct wt_status *s)
static void wt_status_print_unmerged_header(struct wt_status *s)
{
const char *c = color(WT_STATUS_HEADER, s);
+
color_fprintf_ln(s->fp, c, "# Unmerged paths:");
- if (!s->is_initial)
+ if (s->in_merge)
+ ;
+ else if (!s->is_initial)
color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
else
color_fprintf_ln(s->fp, c, "# (use \"git rm --cached <file>...\" to unstage)");
@@ -59,12 +62,14 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
static void wt_status_print_cached_header(struct wt_status *s)
{
const char *c = color(WT_STATUS_HEADER, s);
+
color_fprintf_ln(s->fp, c, "# Changes to be committed:");
- if (!s->is_initial) {
+ if (s->in_merge)
+ ; /* NEEDSWORK: use "git reset --unresolve"??? */
+ else if (!s->is_initial)
color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
- } else {
+ else
color_fprintf_ln(s->fp, c, "# (use \"git rm --cached <file>...\" to unstage)");
- }
color_fprintf_ln(s->fp, c, "#");
}
@@ -72,6 +77,7 @@ static void wt_status_print_dirty_header(struct wt_status *s,
int has_deleted)
{
const char *c = color(WT_STATUS_HEADER, s);
+
color_fprintf_ln(s->fp, c, "# Changed but not updated:");
if (!has_deleted)
color_fprintf_ln(s->fp, c, "# (use \"git add <file>...\" to update what will be committed)");
diff --git a/wt-status.h b/wt-status.h
index a4bddcf..c60f40a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -34,6 +34,7 @@ struct wt_status {
const char **pathspec;
int verbose;
int amend;
+ int in_merge;
int nowarn;
int use_color;
int relative_paths;
--
1.6.6.rc2.5.g49666
^ permalink raw reply related
* [PATCH 0/3] Update advice in commit/status output
From: Junio C Hamano @ 2009-12-12 9:02 UTC (permalink / raw)
To: git
In-Reply-To: <7vk4wtysyu.fsf@alter.siamese.dyndns.org>
Jay Soffian noticed that we give "git reset HEAD <path>" as an instruction
to get rid of the local change that has already been added to the index
even when <path> is unmerged, or it is merged and we are about to commit a
merge.
In neither case, "git reset HEAD <path>" is absolutely a wrong thing to do
while merging.
This miniseries updates the advices given in status/commit. It applies on
top of the jk/1.7.0-status topic, and has trivial conflicts in wt-status.c
with the jk/unwanted-advices topic that has already graduated to 'maint'.
Junio C Hamano (3):
commit/status: check $GIT_DIR/MERGE_HEAD only once
commit/status: "git add <path>" is not necessarily how to resolve
status/commit: do not suggest "reset HEAD <path>" while merging
builtin-commit.c | 12 ++++++------
t/t7060-wtstatus.sh | 3 +--
wt-status.c | 16 +++++++++++-----
wt-status.h | 1 +
4 files changed, 19 insertions(+), 13 deletions(-)
^ permalink raw reply
* [PATCH 1/3] commit/status: check $GIT_DIR/MERGE_HEAD only once
From: Junio C Hamano @ 2009-12-12 9:02 UTC (permalink / raw)
To: git
In-Reply-To: <1260608523-15579-1-git-send-email-gitster@pobox.com>
The code checked for the MERGE_HEAD file to see if we were about
to commit a merge twice in the codepath; also one of them used a
variable merge_head_sha1[] which was set but was never used.
Just check it once, but do so also in "git status", too, as
we will be using this for status generation in the next patch.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-commit.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index b39295f..17dd462 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -36,7 +36,7 @@ static const char * const builtin_status_usage[] = {
NULL
};
-static unsigned char head_sha1[20], merge_head_sha1[20];
+static unsigned char head_sha1[20];
static char *use_message_buffer;
static const char commit_editmsg[] = "COMMIT_EDITMSG";
static struct lock_file index_lock; /* real index */
@@ -319,7 +319,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
*/
commit_style = COMMIT_PARTIAL;
- if (file_exists(git_path("MERGE_HEAD")))
+ if (in_merge)
die("cannot do a partial commit during a merge.");
memset(&partial, 0, sizeof(partial));
@@ -758,9 +758,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
if (get_sha1("HEAD", head_sha1))
initial_commit = 1;
- if (!get_sha1("MERGE_HEAD", merge_head_sha1))
- in_merge = 1;
-
/* Sanity check options */
if (amend && initial_commit)
die("You have nothing to amend.");
@@ -951,6 +948,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
wt_status_prepare(&s);
git_config(git_status_config, &s);
+ in_merge = file_exists(git_path("MERGE_HEAD"));
argc = parse_options(argc, argv, prefix,
builtin_status_options,
builtin_status_usage, 0);
@@ -1057,10 +1055,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
wt_status_prepare(&s);
git_config(git_commit_config, &s);
+ in_merge = file_exists(git_path("MERGE_HEAD"));
if (s.use_color == -1)
s.use_color = git_use_color_default;
-
argc = parse_and_validate_options(argc, argv, builtin_commit_usage,
prefix, &s);
if (dry_run) {
--
1.6.6.rc2.5.g49666
^ permalink raw reply related
* Re: [PATCH 1/3] octopus: make merge process simpler to follow
From: Junio C Hamano @ 2009-12-12 8:53 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git, Jari Aalto
In-Reply-To: <1260578339-30750-1-git-send-email-bebarino@gmail.com>
Stephen Boyd <bebarino@gmail.com> writes:
> Its not very easy to understand what heads are being merged given
> the current output of an octopus merge. Fix this by replacing the
> sha1 with the (usually) better description in GITHEAD_<SHA1>.
>
> Suggested-by: Jari Aalto <jari.aalto@cante.net>
> Signed-off-by: Stephen Boyd <bebarino@gmail.com>
> ---
>
> Maybe this will work? At least it will replace the sha1 with
> whatever is given on the command line.
This shows what was given from the command line instead of the
commit object name. I think it makes sense to not even make this
dependent on any option. Also I don't think anybody misses the
commit object name in the output---after all, that is all internal
to git and is different from what the user gave us anyway (unless
the user did give us the full 40-char object name, in which case
the value of GITHEAD_<SHA1> would be that name, and we will show it,
which is also fine).
No, I haven't applied nor run it yet. The above is purely from my cursory
reading of the code.
Anyway, I'll keep this in my inbox or queue it on 'pu' if I had time.
^ permalink raw reply
* Re: FEATURE REQUEST: Announce branch name with merge comamnd
From: Jari Aalto @ 2009-12-12 8:35 UTC (permalink / raw)
To: Alex Riesen; +Cc: git
In-Reply-To: <81b0412b0912111735v474a9d3k5a24024c2d51587b@mail.gmail.com>
Alex Riesen <raa.lkml@gmail.com> writes:
> On Fri, Dec 11, 2009 at 19:55, Jari Aalto <jari.aalto@cante.net> wrote:
>
>> Please announce the branch name being merged so that the listing is
>> easier to follow (possibly only with --verbose, -v option). Add
>> "Branch: <name>" just before the merge is attempted. somethiglike this
>>
>> Branch: bug--manpage-fix-hyphen
>> Trying simple merge with c87c49b1e413e5dc378d7e6b16951761a1e82f6d
>
> It is not exactly "easier" to follow in your case. It is more
> text and there is no immediately visible cue that the two
> lines are related. You have to give the observer this information.
> Put reference name and SHA-1 on the same line?
That would work too. As long as the branch name appears there
(optionally only with -v in effect).
Jari
^ permalink raw reply
* [PATCH RESEND] gitk: add "--no-replace-objects" option
From: Christian Couder @ 2009-12-12 4:52 UTC (permalink / raw)
To: Paul Mackerras
Cc: git, Michael J Gruber, Jakub Narebski, Johannes Sixt, bill lam,
Andreas Schwab, Junio C Hamano
Replace refs are useful to change some git objects after they
have started to be shared between different repositories. One
might want to ignore them to see the original state, and
"--no-replace-objects" option can be used from the command
line to do so.
This option simply sets the GIT_NO_REPLACE_OBJECTS environment
variable, and that is enough to make gitk ignore replace refs.
The GIT_NO_REPLACE_OBJECTS is set to "1" instead of "" as it is
safer on some platforms, thanks to Johannes Sixt and Michael J
Gruber.
Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
gitk-git/gitk | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
I previously sent this patch as part of a series:
http://thread.gmane.org/gmane.comp.version-control.git/133423/focus=133427
and it looks like it has been lost.
Thanks in advance.
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 364c7a8..86dff0f 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -130,7 +130,7 @@ proc unmerged_files {files} {
}
proc parseviewargs {n arglist} {
- global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs
+ global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env
set vdatemode($n) 0
set vmergeonly($n) 0
@@ -210,6 +210,9 @@ proc parseviewargs {n arglist} {
# git rev-parse doesn't understand --merge
lappend revargs --gitk-symmetric-diff-marker MERGE_HEAD...HEAD
}
+ "--no-replace-objects" {
+ set env(GIT_NO_REPLACE_OBJECTS) "1"
+ }
"-*" {
# Other flag arguments including -<n>
if {[string is digit -strict [string range $arg 1 end]]} {
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 2/7] reset: add a few tests for "git reset --merge"
From: Christian Couder @ 2009-12-12 4:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20091212042042.3930.54783.chriscool@tuxfamily.org>
Commit 9e8eceab ("Add 'merge' mode to 'git reset'", 2008-12-01),
added the --merge option to git reset, but there were no test cases
for it.
This was not a big problem because "git reset" was just forking and
execing "git read-tree", but this will change in a following patch.
So let's add a few test cases to make sure that there will be no
regression.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t7110-reset-merge.sh | 94 ++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 94 insertions(+), 0 deletions(-)
create mode 100755 t/t7110-reset-merge.sh
diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
new file mode 100755
index 0000000..8190da1
--- /dev/null
+++ b/t/t7110-reset-merge.sh
@@ -0,0 +1,94 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Christian Couder
+#
+
+test_description='Tests for "git reset --merge"'
+
+. ./test-lib.sh
+
+test_expect_success 'creating initial files' '
+ echo "line 1" >> file1 &&
+ echo "line 2" >> file1 &&
+ echo "line 3" >> file1 &&
+ cp file1 file2 &&
+ git add file1 file2 &&
+ test_tick &&
+ git commit -m "Initial commit"
+'
+
+test_expect_success 'reset --merge is ok with changes in file it does not touch' '
+ echo "line 4" >> file1 &&
+ echo "line 4" >> file2 &&
+ test_tick &&
+ git commit -m "add line 4" file1 &&
+ git reset --merge HEAD^ &&
+ ! grep 4 file1 &&
+ grep 4 file2 &&
+ git reset --merge HEAD@{1} &&
+ grep 4 file1 &&
+ grep 4 file2
+'
+
+test_expect_success 'reset --merge discards changes added to index (1)' '
+ echo "line 5" >> file1 &&
+ git add file1 &&
+ git reset --merge HEAD^ &&
+ ! grep 4 file1 &&
+ ! grep 5 file1 &&
+ grep 4 file2 &&
+ echo "line 5" >> file2 &&
+ git add file2 &&
+ git reset --merge HEAD@{1} &&
+ ! grep 4 file2 &&
+ ! grep 5 file1 &&
+ grep 4 file1
+'
+
+test_expect_success 'reset --merge discards changes added to index (2)' '
+ echo "line 4" >> file2 &&
+ git add file2 &&
+ git reset --merge HEAD^ &&
+ ! grep 4 file2 &&
+ git reset --merge HEAD@{1} &&
+ ! grep 4 file2 &&
+ grep 4 file1
+'
+
+test_expect_success 'reset --merge fails with changes in file it touches' '
+ echo "line 5" >> file1 &&
+ test_tick &&
+ git commit -m "add line 5" file1 &&
+ sed -e "s/line 1/changed line 1/" <file1 >file3 &&
+ mv file3 file1 &&
+ test_must_fail git reset --merge HEAD^ 2>err.log &&
+ grep file1 err.log | grep "not uptodate" &&
+ git reset --hard HEAD^
+'
+
+test_expect_success 'setup 2 different branches' '
+ git branch branch1 &&
+ git branch branch2 &&
+ git checkout branch1 &&
+ echo "line 5 in branch1" >> file1 &&
+ test_tick &&
+ git commit -a -m "change in branch1" &&
+ git checkout branch2 &&
+ echo "line 5 in branch2" >> file1 &&
+ test_tick &&
+ git commit -a -m "change in branch2"
+'
+
+test_expect_success '"reset --merge HEAD^" fails with pending merge' '
+ test_must_fail git merge branch1 &&
+ test_must_fail git reset --merge HEAD^ &&
+ git reset --hard HEAD
+'
+
+test_expect_success '"reset --merge HEAD" fails with pending merge' '
+ test_must_fail git merge branch1 &&
+ test_must_fail git reset --merge HEAD &&
+ git reset --hard HEAD
+'
+
+test_done
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 5/7] reset: add test cases for "--keep" option
From: Christian Couder @ 2009-12-12 4:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20091212042042.3930.54783.chriscool@tuxfamily.org>
This shows that with the "--keep" option, changes that are both in
the work tree and the index are kept in the work tree after the
reset (but discarded in the index). As with the "--merge" option,
changes that are in both the work tree and the index are discarded
after the reset.
In the case of unmerged entries, we can see that
"git reset --merge HEAD" is disallowed while it works using
"--keep" instead.
And this shows that otherwise "--merge" and "--keep" have the same
behavior.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t7110-reset-merge.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 61 insertions(+), 1 deletions(-)
diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index 6afaf73..bd2ef94 100755
--- a/t/t7110-reset-merge.sh
+++ b/t/t7110-reset-merge.sh
@@ -3,7 +3,7 @@
# Copyright (c) 2009 Christian Couder
#
-test_description='Tests for "git reset --merge"'
+test_description='Tests for "git reset" with --merge and --keep'
. ./test-lib.sh
@@ -30,6 +30,20 @@ test_expect_success 'reset --merge is ok with changes in file it does not touch'
grep 4 file2
'
+test_expect_success 'reset --keep is ok with changes in file it does not touch' '
+ git reset --hard HEAD^ &&
+ echo "line 4" >> file1 &&
+ echo "line 4" >> file2 &&
+ test_tick &&
+ git commit -m "add line 4" file1 &&
+ git reset --keep HEAD^ &&
+ ! grep 4 file1 &&
+ grep 4 file2 &&
+ git reset --keep HEAD@{1} &&
+ grep 4 file1 &&
+ grep 4 file2
+'
+
test_expect_success 'reset --merge discards changes added to index (1)' '
echo "line 5" >> file1 &&
git add file1 &&
@@ -55,6 +69,25 @@ test_expect_success 'reset --merge discards changes added to index (2)' '
grep 4 file1
'
+test_expect_success 'reset --keep fails with changes in index in files it touches' '
+ echo "line 4" >> file2 &&
+ echo "line 5" >> file1 &&
+ git add file1 &&
+ test_must_fail git reset --keep HEAD^ &&
+ git reset --hard HEAD
+'
+
+test_expect_success 'reset --keep keeps changes it does not touch' '
+ echo "line 4" >> file2 &&
+ git add file2 &&
+ git reset --keep HEAD^ &&
+ grep 4 file2 &&
+ git reset --keep HEAD@{1} &&
+ grep 4 file2 &&
+ grep 4 file1 &&
+ git reset --hard HEAD
+'
+
test_expect_success 'reset --merge fails with changes in file it touches' '
echo "line 5" >> file1 &&
test_tick &&
@@ -66,6 +99,17 @@ test_expect_success 'reset --merge fails with changes in file it touches' '
git reset --hard HEAD^
'
+test_expect_success 'reset --keep fails with changes in file it touches' '
+ echo "line 5" >> file1 &&
+ test_tick &&
+ git commit -m "add line 5" file1 &&
+ sed -e "s/line 1/changed line 1/" <file1 >file3 &&
+ mv file3 file1 &&
+ test_must_fail git reset --keep HEAD^ 2>err.log &&
+ grep file1 err.log | grep "not uptodate" &&
+ git reset --hard HEAD^
+'
+
test_expect_success 'setup 2 different branches' '
git branch branch1 &&
git branch branch2 &&
@@ -87,10 +131,26 @@ test_expect_success '"reset --merge HEAD^" is ok with pending merge' '
git reset --hard HEAD@{1}
'
+test_expect_success '"reset --keep HEAD^" is ok with pending merge' '
+ test_must_fail git merge branch1 &&
+ git reset --keep HEAD^ &&
+ test -z "$(git diff --cached)" &&
+ test -n "$(git diff)" &&
+ git reset --hard HEAD@{1}
+'
+
test_expect_success '"reset --merge HEAD" fails with pending merge' '
test_must_fail git merge branch1 &&
test_must_fail git reset --merge HEAD &&
git reset --hard HEAD
'
+test_expect_success '"reset --keep HEAD" is ok with pending merge' '
+ test_must_fail git merge branch1 &&
+ git reset --keep HEAD &&
+ test -z "$(git diff --cached)" &&
+ test -n "$(git diff)" &&
+ git reset --hard HEAD
+'
+
test_done
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 7/7] Documentation: reset: add some tables to describe the different options
From: Christian Couder @ 2009-12-12 4:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20091212042042.3930.54783.chriscool@tuxfamily.org>
This patch adds a DISCUSSION section that contains some tables to
show how the different "git reset" options work depending on the
states of the files in the working tree, the index, HEAD and the
target commit.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-reset.txt | 72 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 72 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index f8724d0..4ded9e3 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -72,6 +72,78 @@ linkgit:git-add[1]).
<commit>::
Commit to make the current HEAD. If not given defaults to HEAD.
+DISCUSSION
+----------
+
+The tables below show what happens when running:
+
+----------
+git reset --option target
+----------
+
+to reset the HEAD to another commit (`target`) with the different
+reset options depending on the state of the files.
+
+ working index HEAD target working index HEAD
+ ----------------------------------------------------
+ A B C D --soft A B D
+ --mixed A D D
+ --hard D D D
+ --merge (disallowed)
+ --keep (disallowed)
+
+ working index HEAD target working index HEAD
+ ----------------------------------------------------
+ A B C C --soft A B C
+ --mixed A C C
+ --hard C C C
+ --merge (disallowed)
+ --keep A C C
+
+ working index HEAD target working index HEAD
+ ----------------------------------------------------
+ B B C D --soft B B D
+ --mixed B D D
+ --hard D D D
+ --merge D D D
+ --keep (disallowed)
+
+ working index HEAD target working index HEAD
+ ----------------------------------------------------
+ B B C C --soft B B C
+ --mixed B C C
+ --hard C C C
+ --merge C C C
+ --keep B C C
+
+In these tables, A, B, C and D are some different states of a
+file. For example, the last line of the last table means that if a
+file is in state B in the working tree and the index, and in a
+different state C in HEAD and in the target, then "git reset
+--keep target" will put the file in state B in the working tree
+and in state C in the index and HEAD.
+
+The following tables show what happens when there are unmerged
+entries:
+
+ working index HEAD target working index HEAD
+ ----------------------------------------------------
+ X U A B --soft (disallowed)
+ --mixed X B B
+ --hard B B B
+ --merge X B B
+ --keep X B B
+
+ working index HEAD target working index HEAD
+ ----------------------------------------------------
+ X U A A --soft (disallowed)
+ --mixed X A A
+ --hard A A A
+ --merge (disallowed)
+ --keep X A A
+
+X means any state and U means an unmerged index.
+
Examples
--------
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 3/7] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Christian Couder @ 2009-12-12 4:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20091212042042.3930.54783.chriscool@tuxfamily.org>
From: Stephan Beyer <s-beyer@gmx.net>
This patch makes "reset_index_file()" call "unpack_trees()" directly
instead of forking and execing "git read-tree". So the code is more
efficient.
And it's also easier to see which unpack_tree() options will be used,
as we don't need to follow "git read-tree"'s command line parsing
which is quite complex.
As Daniel Barkalow found, there is a difference between this new
version and the old one. The old version gives an error for
"git reset --merge" with unmerged entries and the new version does
not. But this can be seen as a bug fix, because "--merge" was the
only "git reset" option with this behavior and this behavior was
not documented.
In fact there is still an error with unmerge entries if we reset
the unmerge entries to the same state as HEAD. So the bug is not
completely fixed.
The code comes from the sequencer GSoC project:
git://repo.or.cz/git/sbeyer.git
(at commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)
Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-reset.c | 41 ++++++++++++++++++++++++++++++-----------
t/t7110-reset-merge.sh | 8 +++++---
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/builtin-reset.c b/builtin-reset.c
index 14bdb03..ac38aaa 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -18,6 +18,8 @@
#include "tree.h"
#include "branch.h"
#include "parse-options.h"
+#include "unpack-trees.h"
+#include "cache-tree.h"
static const char * const git_reset_usage[] = {
"git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]",
@@ -54,27 +56,44 @@ static inline int is_merge(void)
static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet)
{
- int i = 0;
- const char *args[6];
+ int nr = 1;
+ int newfd;
+ struct tree_desc desc[2];
+ struct unpack_trees_options opts;
+ struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- args[i++] = "read-tree";
+ memset(&opts, 0, sizeof(opts));
+ opts.head_idx = 1;
+ opts.src_index = &the_index;
+ opts.dst_index = &the_index;
+ opts.fn = oneway_merge;
+ opts.merge = 1;
if (!quiet)
- args[i++] = "-v";
+ opts.verbose_update = 1;
switch (reset_type) {
case MERGE:
- args[i++] = "-u";
- args[i++] = "-m";
+ opts.update = 1;
break;
case HARD:
- args[i++] = "-u";
+ opts.update = 1;
/* fallthrough */
default:
- args[i++] = "--reset";
+ opts.reset = 1;
}
- args[i++] = sha1_to_hex(sha1);
- args[i] = NULL;
- return run_command_v_opt(args, RUN_GIT_CMD);
+ newfd = hold_locked_index(lock, 1);
+
+ read_cache_unmerged();
+
+ if (!fill_tree_descriptor(desc + nr - 1, sha1))
+ return error("Failed to find tree of %s.", sha1_to_hex(sha1));
+ if (unpack_trees(nr, desc, &opts))
+ return -1;
+ if (write_cache(newfd, active_cache, active_nr) ||
+ commit_locked_index(lock))
+ return error("Could not write new index file.");
+
+ return 0;
}
static void print_new_head_line(struct commit *commit)
diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index 8190da1..6afaf73 100755
--- a/t/t7110-reset-merge.sh
+++ b/t/t7110-reset-merge.sh
@@ -79,10 +79,12 @@ test_expect_success 'setup 2 different branches' '
git commit -a -m "change in branch2"
'
-test_expect_success '"reset --merge HEAD^" fails with pending merge' '
+test_expect_success '"reset --merge HEAD^" is ok with pending merge' '
test_must_fail git merge branch1 &&
- test_must_fail git reset --merge HEAD^ &&
- git reset --hard HEAD
+ git reset --merge HEAD^ &&
+ test -z "$(git diff --cached)" &&
+ test -n "$(git diff)" &&
+ git reset --hard HEAD@{1}
'
test_expect_success '"reset --merge HEAD" fails with pending merge' '
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 6/7] Documentation: reset: describe new "--keep" option
From: Christian Couder @ 2009-12-12 4:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20091212042042.3930.54783.chriscool@tuxfamily.org>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-reset.txt | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 2d27e40..f8724d0 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -8,7 +8,7 @@ git-reset - Reset current HEAD to the specified state
SYNOPSIS
--------
[verse]
-'git reset' [--mixed | --soft | --hard | --merge] [-q] [<commit>]
+'git reset' [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]
'git reset' [-q] [<commit>] [--] <paths>...
'git reset' --patch [<commit>] [--] [<paths>...]
@@ -52,6 +52,11 @@ OPTIONS
and updates the files that are different between the named commit
and the current commit in the working tree.
+--keep::
+ This does the same thing as --merge, but it keeps changes in
+ the working tree and the index or it aborts if it is not
+ possible.
+
-p::
--patch::
Interactively select hunks in the difference between the index
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 4/7] reset: add option "--keep" to "git reset"
From: Christian Couder @ 2009-12-12 4:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20091212042042.3930.54783.chriscool@tuxfamily.org>
The purpose of this new option is to discard some of the
last commits but to keep current changes in the work tree.
The use case is when you work on something and commit
that work. And then you work on something else that touches
other files, but you don't commit it yet. Then you realize
that what you commited when you worked on the first thing
is not good or belongs to another branch.
So you want to get rid of the previous commits (at least in
the current branch) but you want to make sure that you keep
the changes you have in the work tree. And you are pretty
sure that your changes are independent from what you
previously commited, so you don't want the reset to succeed
if the previous commits changed a file that you also
changed in your work tree.
The "--keep" option will do what you want.
The table below shows what happens when running
"git reset --option target" to reset the HEAD to another
commit (as a special case "target" could be the same as
HEAD) in the cases where "--merge" and "--keep" behave
differently.
working index HEAD target working index HEAD
----------------------------------------------------
A B C D --keep (disallowed)
--merge (disallowed)
A B C C --keep A C C
--merge (disallowed)
B B C D --keep (disallowed)
--merge C C C
B B C C --keep B C C
--merge C C C
In this table, A, B and C are some different states of
a file. For example the last line of the table means
that if a file is in state B in the working tree and
the index, and in a different state C in HEAD and in
the target, then "git reset --merge target" will put
the file in state C in the working tree, in the index
and in HEAD.
So as can be seen in the table, "--merge" can discard
changes in the working tree, while "--keep" does not.
So if one wants to avoid using "git stash" before and
after using "git reset" to save current changes, it is
better to use "--keep" rather than "--merge".
The following table shows what happens on unmerged entries:
working index HEAD target working index HEAD
----------------------------------------------------
X U A B --keep X B B
--merge X B B
X U A A --keep X A A
--merge (disallowed)
In this table X can be any state and U means an unmerged
entry.
A following patch will add some test cases for
"--keep", where the differences between "--merge" and
"--keep" can also be seen.
The "--keep" option is implemented by doing a 2 way merge
between HEAD and the reset target, and if this succeeds
by doing a mixed reset to the target.
The code comes from the sequencer GSoC project, where
such an option was developed by Stephan Beyer:
git://repo.or.cz/git/sbeyer.git
(at commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)
But in the sequencer project the "reset" flag was set
in the "struct unpack_trees_options" passed to
"unpack_trees()". With this flag the changes in the
working tree were discarded if the file was different
between HEAD and the reset target.
Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-reset.c | 30 +++++++++++++++++++++++++-----
1 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/builtin-reset.c b/builtin-reset.c
index ac38aaa..859e0d1 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -22,13 +22,15 @@
#include "cache-tree.h"
static const char * const git_reset_usage[] = {
- "git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]",
+ "git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]",
"git reset [--mixed] <commit> [--] <paths>...",
NULL
};
-enum reset_type { MIXED, SOFT, HARD, MERGE, NONE };
-static const char *reset_type_names[] = { "mixed", "soft", "hard", "merge", NULL };
+enum reset_type { MIXED, SOFT, HARD, MERGE, KEEP, NONE };
+static const char *reset_type_names[] = {
+ "mixed", "soft", "hard", "merge", "keep", NULL
+};
static char *args_to_str(const char **argv)
{
@@ -71,6 +73,7 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
if (!quiet)
opts.verbose_update = 1;
switch (reset_type) {
+ case KEEP:
case MERGE:
opts.update = 1;
break;
@@ -85,6 +88,16 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
read_cache_unmerged();
+ if (reset_type == KEEP) {
+ unsigned char head_sha1[20];
+ if (get_sha1("HEAD", head_sha1))
+ return error("You do not have a valid HEAD.");
+ if (!fill_tree_descriptor(desc, head_sha1))
+ return error("Failed to find tree of HEAD.");
+ nr++;
+ opts.fn = twoway_merge;
+ }
+
if (!fill_tree_descriptor(desc + nr - 1, sha1))
return error("Failed to find tree of %s.", sha1_to_hex(sha1));
if (unpack_trees(nr, desc, &opts))
@@ -228,6 +241,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
"reset HEAD, index and working tree", HARD),
OPT_SET_INT(0, "merge", &reset_type,
"reset HEAD, index and working tree", MERGE),
+ OPT_SET_INT(0, "keep", &reset_type,
+ "reset HEAD but keep local changes",
+ KEEP),
OPT_BOOLEAN('q', NULL, &quiet,
"disable showing new HEAD in hard reset and progress message"),
OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
@@ -315,9 +331,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (reset_type == SOFT) {
if (is_merge() || read_cache() < 0 || unmerged_cache())
die("Cannot do a soft reset in the middle of a merge.");
+ } else {
+ int err = reset_index_file(sha1, reset_type, quiet);
+ if (reset_type == KEEP)
+ err = err || reset_index_file(sha1, MIXED, quiet);
+ if (err)
+ die("Could not reset index file to revision '%s'.", rev);
}
- else if (reset_index_file(sha1, reset_type, quiet))
- die("Could not reset index file to revision '%s'.", rev);
/* Any resets update HEAD to the head being switched to,
* saving the previous head in ORIG_HEAD before. */
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 1/7] reset: do not accept a mixed reset in a .git dir
From: Christian Couder @ 2009-12-12 4:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <20091212042042.3930.54783.chriscool@tuxfamily.org>
It is strange and fragile that a mixed reset is disallowed in a bare
repo but is allowed in a .git dir. So this patch simplifies things
by only allowing soft resets when not in a working tree.
This patch is also needed to speed up "git reset" by using
unpack_tree() directly (instead of execing "git read-tree"). A
following patch will do just that.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-reset.c | 3 +--
t/t7103-reset-bare.sh | 4 ++--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/builtin-reset.c b/builtin-reset.c
index 11d1c6e..14bdb03 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -286,8 +286,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (reset_type == NONE)
reset_type = MIXED; /* by default */
- if ((reset_type == HARD || reset_type == MERGE)
- && !is_inside_work_tree())
+ if (reset_type != SOFT && !is_inside_work_tree())
die("%s reset requires a work tree",
reset_type_names[reset_type]);
diff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh
index 68041df..13344b2 100755
--- a/t/t7103-reset-bare.sh
+++ b/t/t7103-reset-bare.sh
@@ -21,8 +21,8 @@ test_expect_success 'merge reset requires a worktree' '
test_must_fail git reset --merge)
'
-test_expect_success 'mixed reset is ok' '
- (cd .git && git reset)
+test_expect_success 'mixed reset requires a worktree' '
+ (cd .git && test_must_fail git reset)
'
test_expect_success 'soft reset is ok' '
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 0/7] "git reset --merge" related improvements
From: Christian Couder @ 2009-12-12 4:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
Another reroll with the following changes:
- the name of the new option is now "--keep" instead of "--keep-local-changes",
- the fill_tree_descriptor() function is used instead of adding a new
parse_and_init_tree_desc() function (thanks to Stephen Boyd),
- patch 1/7 was added to only accept soft reset when not in a working tree;
this makes the test suite pass,
- the commit message of patch 4/7 that adds the --keep option has been
improved; it talks more about the use case of this new option.
Christian Couder (6):
reset: do not accept a mixed reset in a .git dir
reset: add a few tests for "git reset --merge"
reset: add option "--keep" to "git reset"
reset: add test cases for "--keep" option
Documentation: reset: describe new "--keep" option
Documentation: reset: add some tables to describe the different
options
Stephan Beyer (1):
reset: use "unpack_trees()" directly instead of "git read-tree"
Documentation/git-reset.txt | 79 +++++++++++++++++++++-
builtin-reset.c | 74 +++++++++++++++-----
t/t7103-reset-bare.sh | 4 +-
t/t7110-reset-merge.sh | 156 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 292 insertions(+), 21 deletions(-)
create mode 100755 t/t7110-reset-merge.sh
^ permalink raw reply
* Re: [PATCH v4 2/6] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Christian Couder @ 2009-12-12 4:19 UTC (permalink / raw)
To: Stephen Boyd
Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
Stephan Beyer, Daniel Barkalow, Jakub Narebski, Paolo Bonzini,
Johannes Sixt
In-Reply-To: <1260261919.1554.11.camel@swboyd-laptop>
On mardi 08 décembre 2009, Stephen Boyd wrote:
> On Tue, 2009-12-08 at 08:56 +0100, Christian Couder wrote:
> > +static int parse_and_init_tree_desc(const unsigned char *sha1,
> > + struct tree_desc *desc)
> > +{
> > + struct tree *tree = parse_tree_indirect(sha1);
> > + if (!tree)
> > + return 1;
> > + init_tree_desc(desc, tree->buffer, tree->size);
> > + return 0;
> > +}
> > +
>
> Is there a reason why you use this function instead of
> fill_tree_descriptor()?
I didn't know about fill_tree_descriptor(). I will use it in the next
series.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH 0/6] Gitweb caching changes v2
From: Jakub Narebski @ 2009-12-12 1:37 UTC (permalink / raw)
To: J.H.; +Cc: git
In-Reply-To: <4B228ED3.3030901@kernel.org>
On Fri, 11 Dec 2009, J.H. wrote:
> Jakub Narebski wrote:
>> On Fri, 11 Dec 2009, J.H. (John 'Warthog9' Hawley) wrote:
>>> Jakub Narebski wrote:
>>>> "John 'Warthog9' Hawley" <warthog9@kernel.org> writes:
>>>>>
>>>>> GITWEB - File based caching layer
>>>>>
>>>> This patch didn't made it to git mailing list. I suspect that you ran
>>>> afoul vger anti-SPAM filter.
>>>
>>> Yeah, it does seem that way (like I said eaten by a grue),
It _might_ be caused by the fact that you used attachement. But it might
not; you can always use vger-taboo.perl script to check.
>>>> Does this "File based caching layer" have anything common with GSoC
>>>> 2008 project, available at git://repo.or.cz/git/gitweb-caching.git ?
>>>
>>> It *currently* has nothing to do with Lea's GSoC code but it is still my
>>> intention, in long term, to integrate the two.
The question would be then whether it makes sense to have two caches at
different levels in the stack (see also discussion below about Lea
approach).
>>> The patch, in all it's glory can be viewed at:
>>> http://git.kernel.org/?p=git/warthog9/gitweb.git;a=commitdiff;h=42641b1e3bfae14d5cc2e0150355e89cb87951db
>>>
>>> It is anything but a small patch to gitweb, the patch is 117K and
>>> comprises 3539 lines (including git header commit information). There's
>>> not any real good way to break it up as it's a bit of an all or nothing
>>> patch.
>>
>> First, why do you reinvent the wheel instead of using one of existing
>> caching interfaces like CHI or Cache::Cache (perhaps creating a custom
>> backend or middle layer which incorporates required features, like being
>> load-aware)?
>
> Well for starters this isn't exactly a reinvention of the wheel, and
> this isn't something "new" per-se. This code has been actively running
> on git.kernel.org for something like 3 - 4 years so there's something to
> be said for the devil we know and understand.
Well, if it is not reinventing the wheel, then it is yak shaving (yet
another ...) ;-)
The fact that the code was used and tested at one single installation
doesn't mean that it doesn't have warts that could be avoided by using
ready solution (at least for parts of it).
> As well using the other
> caching strategies involves adding dramatically more complex
> interactions with caching layer.
I am hoping that if it was done right, by using CHI or Cache::Cache
caching interface, then choosing alternate caching engine, or adding
extra level of cache would be simple and decoupled from issues specific
to web app or gitweb in particular.
> The caching layer is actually quite
> specific to how git + gitweb works and solves more than just "caching"
> on the surface.
The idea is for gitweb/cache.pm (or gitweb/Gitweb-Cache.pm, or
gitweb/Gitweb/Cache.pm) is to encapsulate issues specific to gitweb,
like generating cache key, or printing "Generating...", etc.
Perhaps also the idea of filling cache in background (but see discussion
below about capturing STDOUT) could be put there.
> Specifically it solves the stampeding herd problem
> which would have to be solved either way even if I didn't implement my
> own caching, and since I had to do that caching was barely a step beyond
> that to implement.
CHI tries to solve cache miss stampedes via expires_variance mechanism.
There is Cache::Adaptive (and its subclass Cache::Adaptive::ByLoad)
which does adaptive lifetime control (it accepts any Cache::Cache
compatible cache, so I think it also accepts CHI compatible cache).
Those problems _were_ solved.
>> This way changing from file-based cache to e.g. mmap based
>> one or to memcached would be very simple.
>
> True but these are *VERY* different caching strategies than the one I've
> got here, yes it's using files as a backend but it's doing so with
> specific goals in mind. As I've said I plan to integrate Lea's
> memcached based caching into this in the future and that has different
> advantages and disadvantages.
Errr... besides using Cache::Cache compatible cache (see!!!), for example
Cache::Memcached, Lea Wiemann's gitweb caching did caching at entirely
different level than original kernel.org's gitweb.
The stack for gitweb looks somewhat like this:
git commands output open my $fd, '-|, git_cmd(), ...
|
v
parsed output data parse_ls_tree_line, parse_commit, ...
|
v
generated HTML etc. print ...
:
V
caching optional
reverse proxy Varnish, Squid
If I understand correctly Lea Wiemann code cache git command output.
The fork of gitweb used at repo.or.cz does caching of parsed data at
least for most intensive projects list page. This patch was about caching
generated output. HTTP caching requires that gitweb can respond to
If-Modified-Since (and generate Last-Modified) and If-None-Match (and
generate ETag) in a time that is much faster than generating full response.
There are advantages and disadvantages for each method of caching; also
the balance might depend on the view used. For example 'snapshot' view
is best cached via output caching with file-based cache, while for
'blame_incremental' view straight caching of output doesn't make much
sense while caching command output should give good behaviour.
> At the end of the day the "normal" caching engines aren't as efficient
> as mine and there is the case the very high performance sites are going
> to have to investigate a number of different solutions to see what works
> best for them. Mine is also *dramatically* simpler to setup as well,
> turn it on, point it at a directory and you're done.
Do you have any benchmarks?
>> And you would avoid pitfals
>> in doing your own cache management. perl-Cache-Cache should be available
>> package in extras repositories.
>
> There's pitfalls if I do it myself, or I use one of the other "common"
> perl modules. I did it this way years ago, I've maintained it and it
> works pretty well. I won't admit that it's the smartest caching engine
> on the planet, far from it, but it has evolved specifically for gitweb
> and that itself saves me a lot of pitfalls from cache engine + gitweb
> integration.
If I remember correctly the solution presented here has serious
disadvantage of not having any cache expire policy, and not being
size-aware.
>> If module is no available this would simply mean no caching, like in many
>> (or not so many) other cases with optional features in gitweb.
>
> Yes, but as can be seen from how you enable various other caching
> engines the setup of those is non-trivial, this is and either way
> caching *HAS* to be explicitly turned on by the admin/user since they
> are going to have to do *some* configuration, or at least be aware that
> their webapp is going to chew up some sort of resource.
I wonder if there is any data that describes when one should enable
caching, and when one can do without it, e.g. depending on the number
and total size of repositories presented via gitweb.
IMHO cache storage is orthogonal to expire policy, which in turn is
orthogonal on cache use in gitweb. And those parts should be kept separate
(and tested independently), even if we decide on homegrown caching
solution.
>> Second, if you can't use CGI::Cache directly, you can always steal the
>> idea from it, then the change to gitweb itself would be minimal:
>>
>> "Internally, the CGI::Cache module ties the output file descriptor
>> (usually STDOUT) to an internal variable to which all output is saved."
>
> I thought about that 3 years ago, and decided it wasn't a good option
> for gitweb. Why? There's too many assumptions throughout the code that
> when you do a print it will go immediately out. Things like error
> messages and such. Breaking out the prints into prints (which will do
> what is expected) and passing around the output in the $output variables
> makes it a lot simpler easier to differentiate about how / what your
> looking at and a *LOT* easier to debug.
Note that in quite a few places we print directly to output, streaming
the response, for performance (to reduce latency). If all data must be
first gathered in $output variable (increasing memory pressure in the
case of large files for 'blob_plain', large snapshots, large patches in
'patch' and 'patches' views) then we must wait for it to finish, and not
get data as soon as it is available.
Besides instead of just capturing STDOUT in tied variable (STDERR goes
to web server log courtesy of CGI.pm) we can tee it, i.e. capture it
to $output variable as it is streamed to web browser. See Capture::Tiny
(although I am not sure how it would interact with CGI.pm logging) and
e.g. PerlIO::tee mechanism from PerlIO::Util.
Going the route of CGI::Cache would mean minimal changes to gitweb...
and no diference in performance if caching is turned off (see streaming).
>> P.S. I'll postpone critique of the patch itself for now. The above issues
>> are much more important.
>
> That's fine. The issues your raising aren't new though, and stem back
> to before I created gitweb-caching, got rehashed with Lea's patches and
> not surprisingly are back on the table now. Like I said above, there is
> no one caching strategy that's perfect in all cases here and that's
> again why I eventually plan to merge Lea's changes (which uses
> memcached) in as well, I'm just trying to get code that I'm getting
> considerable demand for, that's proven, upstream.
Well, there are two solutions. One is first to decide on proper solution
for gitweb caching. Another is to have _some_ caching and then improve it.
So below there are a few initial comments about gitweb/cache.pm code:
* gitweb/cache.pm should be, I think, a proper module (require'd or use'd)
* you do not follow coding style used elsewhere in gitweb, e.g. spaces
around {} and (), for example it is
}elsif( $cache_enable == 1 ){
and should be
} elsif ($cache_enable == 1) {
* flags that are boolean are compared to 0 and 1
* cache key should be generated from both PATH_INFO and QUERY_STRING
in generic case (unless you turn off $path_info as default, and turn off
support for path_info URLs); see %input_params hash or href(-replay=>1)
* gitweb till now does not include any variable data in error info
* duplicated code (e.g. fork / cacheUpdate + cacheDisplay / cacheUpdate...)
* inconsistent naming style: cache_fetch but cacheDisplay.
* old style open using globs instead of local filehandles:
open(cacheFile, '<', "$fullhashpath");
and should be
open(my $cache_fh, '<', $fullhashpath);
* busy wait 'do { sleep 2; open ... } while (...)' instead of non-blocking
wait like select / IO::Select.
That's all from skimming gitweb-ml-v2:gitweb/cache.pm
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: FEATURE REQUEST: Announce branch name with merge comamnd
From: Alex Riesen @ 2009-12-12 1:35 UTC (permalink / raw)
To: Jari Aalto; +Cc: git
In-Reply-To: <87zl5p1gsp.fsf@jondo.cante.net>
On Fri, Dec 11, 2009 at 19:55, Jari Aalto <jari.aalto@cante.net> wrote:
> Please announce the branch name being merged so that the listing is
> easier to follow (possibly only with --verbose, -v option). Add
> "Branch: <name>" just before the merge is attempted. somethiglike this
>
> Branch: bug--manpage-fix-hyphen
> Trying simple merge with c87c49b1e413e5dc378d7e6b16951761a1e82f6d
It is not exactly "easier" to follow in your case. It is more
text and there is no immediately visible cue that the two
lines are related. You have to give the observer this information.
Put reference name and SHA-1 on the same line?
^ permalink raw reply
* Re: [PATCH 3/3] octopus: remove dead code
From: Junio C Hamano @ 2009-12-12 1:01 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git, Junio C Hamano
In-Reply-To: <1260578339-30750-3-git-send-email-bebarino@gmail.com>
Stephen Boyd <bebarino@gmail.com> writes:
> MSG, PARENT, and CNT are never used, just assigned to.
>
> Signed-off-by: Stephen Boyd <bebarino@gmail.com>
> ---
>
> I don't know if this is wanted. Looks like maybe they're used
> as simple debug aides?
No, thanks for spotting. It is the right thing to do to just get rid of
them.
IIRC, they are remnants of ancient logic of the script, even before
91063bb (Multi-backend merge driver., 2005-09-08) that added it to the
official git history, and were used to update MRC and to come up with the
end result (experimental versions of the script used to run "commit-tree"
itself).
^ permalink raw reply
* [PATCH 3/3] octopus: remove dead code
From: Stephen Boyd @ 2009-12-12 0:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <87zl5p1gsp.fsf@jondo.cante.net>
MSG, PARENT, and CNT are never used, just assigned to.
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
I don't know if this is wanted. Looks like maybe they're used
as simple debug aides?
git-merge-octopus.sh | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/git-merge-octopus.sh b/git-merge-octopus.sh
index 99b6f8a..3cb0b31 100755
--- a/git-merge-octopus.sh
+++ b/git-merge-octopus.sh
@@ -44,9 +44,8 @@ esac
# MRC is the current "merge reference commit"
# MRT is the current "merge result tree"
-MRC=$(git rev-parse --verify -q $head) MSG= PARENT="-p $head"
+MRC=$(git rev-parse --verify -q $head)
MRT=$(git write-tree)
-CNT=1 ;# counting our head
NON_FF_MERGE=0
OCTOPUS_FAILURE=0
for SHA1 in $remotes
@@ -72,9 +71,6 @@ do
;;
esac
- CNT=`expr $CNT + 1`
- PARENT="$PARENT -p $SHA1"
-
if test "$common,$NON_FF_MERGE" = "$MRC,0"
then
# The first head being merged was a fast-forward.
--
1.6.6.rc1.45.g9aadbb
^ permalink raw reply related
* [PATCH 1/3] octopus: make merge process simpler to follow
From: Stephen Boyd @ 2009-12-12 0:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jari Aalto
In-Reply-To: <87zl5p1gsp.fsf@jondo.cante.net>
Its not very easy to understand what heads are being merged given
the current output of an octopus merge. Fix this by replacing the
sha1 with the (usually) better description in GITHEAD_<SHA1>.
Suggested-by: Jari Aalto <jari.aalto@cante.net>
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
Maybe this will work? At least it will replace the sha1 with
whatever is given on the command line.
git-merge-octopus.sh | 9 +++++----
t/t7602-merge-octopus-many.sh | 33 +++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/git-merge-octopus.sh b/git-merge-octopus.sh
index 825c52c..1c8ee0a 100755
--- a/git-merge-octopus.sh
+++ b/git-merge-octopus.sh
@@ -61,12 +61,13 @@ do
exit 2
esac
+ pretty_name="$(eval echo \$GITHEAD_$SHA1)"
common=$(git merge-base --all $SHA1 $MRC) ||
- die "Unable to find common commit with $SHA1"
+ die "Unable to find common commit with $pretty_name"
case "$LF$common$LF" in
*"$LF$SHA1$LF"*)
- echo "Already up-to-date with $SHA1"
+ echo "Already up-to-date with $pretty_name"
continue
;;
esac
@@ -81,7 +82,7 @@ do
# tree as the intermediate result of the merge.
# We still need to count this as part of the parent set.
- echo "Fast-forwarding to: $SHA1"
+ echo "Fast-forwarding to: $pretty_name"
git read-tree -u -m $head $SHA1 || exit
MRC=$SHA1 MRT=$(git write-tree)
continue
@@ -89,7 +90,7 @@ do
NON_FF_MERGE=1
- echo "Trying simple merge with $SHA1"
+ echo "Trying simple merge with $pretty_name"
git read-tree -u -m --aggressive $common $MRT $SHA1 || exit 2
next=$(git write-tree 2>/dev/null)
if test $? -ne 0
diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh
index 01e5415..7377033 100755
--- a/t/t7602-merge-octopus-many.sh
+++ b/t/t7602-merge-octopus-many.sh
@@ -49,4 +49,37 @@ test_expect_success 'merge c1 with c2, c3, c4, ... c29' '
done
'
+cat >expected <<\EOF
+Trying simple merge with c2
+Trying simple merge with c3
+Trying simple merge with c4
+Merge made by octopus.
+ c2.c | 1 +
+ c3.c | 1 +
+ c4.c | 1 +
+ 3 files changed, 3 insertions(+), 0 deletions(-)
+ create mode 100644 c2.c
+ create mode 100644 c3.c
+ create mode 100644 c4.c
+EOF
+
+test_expect_success 'merge output uses pretty names' '
+ git reset --hard c1 &&
+ git merge c2 c3 c4 >actual &&
+ test_cmp actual expected
+'
+
+cat >expected <<\EOF
+Already up-to-date with c4
+Trying simple merge with c5
+Merge made by octopus.
+ c5.c | 1 +
+ 1 files changed, 1 insertions(+), 0 deletions(-)
+ create mode 100644 c5.c
+EOF
+
+test_expect_success 'merge up-to-date output uses pretty names' '
+ git merge c4 c5 >actual &&
+ test_cmp actual expected
+'
test_done
--
1.6.6.rc1.45.g9aadbb
^ permalink raw reply related
* [PATCH 2/3] octopus: reenable fast-forward merges
From: Stephen Boyd @ 2009-12-12 0:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <87zl5p1gsp.fsf@jondo.cante.net>
The fast-forward logic is never being triggered because $common and
$MRC are never equivalent. $common is initialized to a commit id by
merge-base and MRC is initialized to HEAD. Fix this by initializing
$MRC to the commit id for HEAD so that its possible for $MRC and
$common to be equal.
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
Found this while making tests up for part 1 of this series.
git-merge-octopus.sh | 2 +-
t/t7602-merge-octopus-many.sh | 18 ++++++++++++++++++
2 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/git-merge-octopus.sh b/git-merge-octopus.sh
index 1c8ee0a..99b6f8a 100755
--- a/git-merge-octopus.sh
+++ b/git-merge-octopus.sh
@@ -44,7 +44,7 @@ esac
# MRC is the current "merge reference commit"
# MRT is the current "merge result tree"
-MRC=$head MSG= PARENT="-p $head"
+MRC=$(git rev-parse --verify -q $head) MSG= PARENT="-p $head"
MRT=$(git write-tree)
CNT=1 ;# counting our head
NON_FF_MERGE=0
diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh
index 7377033..2746169 100755
--- a/t/t7602-merge-octopus-many.sh
+++ b/t/t7602-merge-octopus-many.sh
@@ -82,4 +82,22 @@ test_expect_success 'merge up-to-date output uses pretty names' '
git merge c4 c5 >actual &&
test_cmp actual expected
'
+
+cat >expected <<\EOF
+Fast-forwarding to: c1
+Trying simple merge with c2
+Merge made by octopus.
+ c1.c | 1 +
+ c2.c | 1 +
+ 2 files changed, 2 insertions(+), 0 deletions(-)
+ create mode 100644 c1.c
+ create mode 100644 c2.c
+EOF
+
+test_expect_success 'merge fast-forward output uses pretty names' '
+ git reset --hard c0 &&
+ git merge c1 c2 >actual &&
+ test_cmp actual expected
+'
+
test_done
--
1.6.6.rc1.45.g9aadbb
^ permalink raw reply related
* Re: How to selectively recreate merge state?
From: Junio C Hamano @ 2009-12-11 23:46 UTC (permalink / raw)
To: Jay Soffian; +Cc: Jakub Narebski, Michael J Gruber, Johannes Sixt, git
In-Reply-To: <76718490912111418i6b59056eq69671979613749f7@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> Also, I think we could improve the output of "git status" during merge
> resolution, both before and after conflicts have been resolved in a
> file.
I think you are talking about something that is largely unrelated, even
though they would be a pair of good issues to discuss. The solution to
them does not have much to do with what we have been discussing so far in
this thread, and actually should be much simpler, which is a good news
;-).
> $ git status
> foo: needs merge
> # On branch master
> # Changed but not updated:
> # (use "git add <file>..." to update what will be committed)
> # (use "git checkout -- <file>..." to discard changes in working directory)
> #
> # unmerged: foo
> #
> no changes added to commit (use "git add" and/or "git commit -a")
>
> "unmerged" is good. But the instruction to use "git checkout --
> <file>" to discard changes is wrong in this context:
You should be able to change this without any "unresolve" index extension
added to the index. Just notice an unmerged entry in the index and reword
the message accordingly.
More importantly, note that "git status" lists "unmerged" entries in a
separate section in its output in 1.6.6 (and has been so on 'master' for
some time) and your problem report needs to be adjusted for a more recent
reality. Here is what you would get:
$ git status
# On branch pu
# Changes to be committed:
# (use "git reset HEAD <file>..." to unstage)
#
# modified: builtin-send-pack.c
# modified: remote.c
# modified: remote.h
# modified: transport.c
#
# Unmerged paths:
# (use "git reset HEAD <file>..." to unstage)
# (use "git add <file>..." to mark resolution)
#
# both modified: transport-helper.c
#
One problem we can see is that 'use "git reset HEAD <file>..." to unstage'
is an invalid advice if we are in the middle of a merge, but is perfectly
valid if this were during "rebase", "am -3", "cherry-pick" and "revert".
The solution to this issue is exactly the same as the next one.
> $ git status
> # On branch master
> # Changes to be committed:
> # (use "git reset HEAD <file>..." to unstage)
> #
> # modified: foo
> #
>
> Well, yes, I can use git reset, but that just keeps my side of the merge.
If the conflict was coming from "rebase", "cherry-pick", etc., there is
nothing but one side, as there is no merge going on, and what "git reset"
does is exactly what the message tells you---to unstage.
I think "git status" should notice that the next commit you would make
from this state will be a merge commit, and remove these "reset HEAD"
lines. Once you "git add" to resolve, it makes _no_ sense to reset to
HEAD, if you are concluding a merge. Until "update-index --unresolve" is
revived as a modern version (and I suspect that a more logical Porcelain
interface would be a new option "reset --unmerge <paths>..."), we should
simply drop "reset HEAD" advice when we are in a merge.
Note that the "unresolve" index extension will not help you at all in
order for you to decide if you are going to make a merge commit. You
should instead ask "does .git/MERGE_HEAD exist?", and it is something you
should be able to implement directly on top of upcoming 1.6.6 release.
^ permalink raw reply
* Re: [PATCH 6/6] GITWEB - Separate defaults from main file
From: Jakub Narebski @ 2009-12-11 22:53 UTC (permalink / raw)
To: J.H.; +Cc: git, John 'Warthog9' Hawley
In-Reply-To: <4B226C0F.2070407@kernel.org>
On Fri, 11 Dec 2009, J.H. wrote:
>>> This is also a not-so-subtle start of trying to break up gitweb into
>>> separate files for easier maintainability, having everything in a
>>> single file is just a mess and makes the whole thing more complicated
>>> than it needs to be. This is a bit of a baby step towards breaking it
>>> up for easier maintenance.
>>
>> The question is if easier maintenance and development by spliting
>> gitweb for developers offsets ease of install for users.
>
> This would just get dropped into the same location that gitweb.cgi
> exists in, there is no real difference in installation, and thus I can't
> see this as an issue for users.
To be more exact you have to know that you have to drop _generated files_,
which means (for this version of patch) gitweb.cgi and gitweb_defaults.pl
(or whatever the generated file with config variables would be named).
ATTENTION!
Your changes would make stop working all gitweb tests. Currently they
do some magic with generated gitweb config file "$(pwd)/gitweb_config.perl"
set via GITWEB_CONFIG configuration variable to be able to run
_unprocessed_ gitweb/gitweb.perl (without any substitutions). This
allow to run tests on "live" version of gitweb.
After your changes it would be no longer possible, at least not if we
want to be sure that we test the same version of gitweb as gitweb_defaults.
It would probably mean that we need to move to testing built version,
i.e. gitweb.cgi, not gitweb.perl
>>> ---
>>> .gitignore | 1 +
>>> Makefile | 15 +-
>>> gitweb/Makefile | 2 +-
>>> gitweb/gitweb.perl | 515 +++++--------------------------------------
>>> gitweb/gitweb_defaults.perl | 468 +++++++++++++++++++++++++++++++++++++++
>>> 5 files changed, 537 insertions(+), 464 deletions(-)
>>> create mode 100644 gitweb/gitweb_defaults.perl
>>>
>>>
>>> diff --git a/.gitignore b/.gitignore
>>> index ac02a58..5e48102 100644
>>> --- a/.gitignore
>>> +++ b/.gitignore
>>> @@ -151,6 +151,7 @@
>>> /git-core-*/?*
>>> /gitk-git/gitk-wish
>>> /gitweb/gitweb.cgi
>>> +/gitweb/gitweb_defaults.pl
>>
>> Hmmm... gitweb/gitweb_defaults.perl as source file, and
>> gitweb/gitweb_defaults.pl as generated file? Wouldn't it be better to
>> go with the convention used elsewhere in gitweb and use
>> gitweb/gitweb_defaults.perl.in or gitweb/gitweb_defaults.pl.in as
>> source file?
>
> I think you got confused, the committed file is .perl the generated file
> is .pl.
Maybe I wasn't entirely clean. I meant that the committed source file
should perhaps have *.in extension to denote that it is to be processed
via variable substitution, so it would be
committed file: gitweb/gitweb_defaults.pl.in
generated file: gitweb/gitweb_defaults.pl
or whatever name (*.pm?) we agree on.
>>> $(QUIET_GEN)$(RM) $@ $@+ && \
>>> sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
>>> -e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
>>> @@ -1539,7 +1541,7 @@ endif
>>> -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \
>>> -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
>>> -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
>>> - $(patsubst %.cgi,%.perl,$@) >$@+ && \
>>> + $(patsubst %.cgi,%.perl,$(patsubst %.pl, %.perl, $@)) >$@+ && \
>>
>> Why the slightly inconsistent style ("%.cgi,%perl" vs "%.pl, %perl")?
>
> Considering that the defaults is more of an include vs. a cgi it
> probably shouldn't share the standard expected executable suffix, thus I
> used .pl. Could just as easily change it to .pm, or something else but
> I think it would make the most sense to leave things we are expecting
> the webserver to directly execute as .cgi, and includes as a different
> suffix.
I was not asking about that, but about
+ $(patsubst %.cgi,%.perl,$(patsubst %.pl, %.perl, $@)) >$@+ && \
vs
+ $(patsubst %.cgi,%.perl,$(patsubst %.pl,%.perl,$@)) >$@+ && \
But after thinking about it a bit, and consulting make documentation
(in particular definition of $@ variable) this rule shouldn't work at all.
`$@'
The file name of the target of the rule. If the target is an
archive member, then `$@' is the name of the archive file. In a
pattern rule that has multiple targets (*note Introduction to
Pattern Rules: Pattern Intro.), `$@' is the name of whichever
target caused the rule's commands to be run.
What we need is to run pattern substitution for _two_ files, perhaps
using the for loop.
Also I think the order of substitutions would be better to be reversed:
$(patsubst %.pl,%.perl,$(patsubst %.cgi,%.perl,$FILE)) >$FILE+
This way the gitweb_defaults file can even have *.perl extension
>> Also wouldn't all replacements be in the new gitweb_defaults file, so
>> there would be no need then to do replacements for gitweb.cgi?
>
> Not all replacements are done in one or the other, and since it's
> basically a NOP to perform the full set of replacements on both files
> that seemed the easiest way to ensure they were done in both places.
>
>> Oh, I see there is at least one that stayed in gitweb.perl: $version
>>
>
> <snip>
O.K.
But Makefile would be (slightly) simpler if replacements were needed only
for single file of two.
>>> +# Define and than setup our configuration
>>> +#
>>> +our(
>>> + $VERSION,
>>> + $path_info,
>>> + $GIT,
[...]
>>> + $prevent_xss,
>>> + @diff_opts,
>>> + %feature
>>> );
>>
>> Why this block is required? Why not have variables defined (using
>> "our") in gitweb_defaults file?
>
> Wanted to make sure things were properly defined, if in an unexpected
> state, should a user have gitweb.cgi in place but not the defaults.
In my opinion it actually *makes worse*. I am not sure if gitweb would
work if the variables are undefined, and you would lose 'undeclared
variable' warning.
>>> diff --git a/gitweb/gitweb_defaults.perl b/gitweb/gitweb_defaults.perl
>>> new file mode 100644
>>> index 0000000..ede0daf
>>> --- /dev/null
>>> +++ b/gitweb/gitweb_defaults.perl
>>> @@ -0,0 +1,468 @@
>>> +# gitweb - simple web interface to track changes in git repositories
>>> +#
>>> +# (C) 2005-2006, Kay Sievers <kay.sievers@vrfy.org>
>>> +# (C) 2005, Christian Gierke
>>> +#
>>> +# This program is licensed under the GPLv2
This header should probably be modified, at least stating what the file
is for.
>>> +
>>> +# Base URL for relative URLs in gitweb ($logo, $favicon, ...),
>>> +# needed and used only for URLs with nonempty PATH_INFO
>>> +$base_url = $my_url;
>>
>> Why not "our $base_url = $my_url;"?
>
> same reason as the other 'our' includes above,
See comment above about pre-declaring variables actually making it
worse wrt checking.
> though why this ended up
> as a separate patch vs. the rest of the file I don't know.
Errr... what you are talking about here?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: How to selectively recreate merge state?
From: Jay Soffian @ 2009-12-11 22:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narebski, Michael J Gruber, Johannes Sixt, git
In-Reply-To: <7v3a3h48lz.fsf@alter.siamese.dyndns.org>
On Fri, Dec 11, 2009 at 2:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> - This was done about only one year after git was born. You should not
> take it granted that the workflow it wanted to support makes sense.
>
> Considering that using "git add" to mark the resolution is to declare
> that you are _finished_ with that path, using it for other purposes
> (e.g. leaving a note that says "I've looked at and have one possible
> resolution in the file in the work tree, but I haven't verified the
> result yet", which is what the commit talks about) is simply an
> (ab|mis)use of the index. Lossage of higher stage information by this
> misuse is user's problem, and there is this thing called pen & pencil
> the user can use for taking notes if s/he does not want to lose the
> original conflict information from the index.
Just a little more data. What happened in my case was that I was using
a visual merge tool and accidentally saved instead of canceled, so git
mergetool automagically added my results. I had resolved about 15
files, and made a mistake with only one, so I was sad when I couldn't
determine how to unresolve that one file (at which point I saved off
the other 14 resolutions, reset, re-did the merge).
My intuition led me to try "git reset <path>" since that's how one
normally unstages additions to the index. But of course that didn't
work, where "of course" only makes sense if you know how the index is
used during a merge.
> In fact, considering that there are many ways conflicts can be left in the
> index and there are only two ways that they are resolved in the index by
> the user (and both eventually uses a single function to do so), it would
> make perfect sense to do the following:
>
> [...excellent list of suggestions elided...]
Also, I think we could improve the output of "git status" during merge
resolution, both before and after conflicts have been resolved in a
file. Immediately after a conflict, the conflicted files are shown as
"unmerged":
$ git status
foo: needs merge
# On branch master
# Changed but not updated:
# (use "git add <file>..." to update what will be committed)
# (use "git checkout -- <file>..." to discard changes in working directory)
#
# unmerged: foo
#
no changes added to commit (use "git add" and/or "git commit -a")
"unmerged" is good. But the instruction to use "git checkout --
<file>" to discard changes is wrong in this context:
$ git checkout -- foo
error: path 'foo' is unmerged
Then, after resolving foo and adding it:
$ git status
# On branch master
# Changes to be committed:
# (use "git reset HEAD <file>..." to unstage)
#
# modified: foo
#
Well, yes, I can use git reset, but that just keeps my side of the merge.
So I think with your suggested changes to the index, we can do better
with the status output during a merge.
j.
^ 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