* Re: What's cooking in git.git (Oct 2011, #03; Fri, 7)
From: Heiko Voigt @ 2011-10-10 20:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jens Lehmann, Brad King, Fredrik Gustafsson
In-Reply-To: <7vsjn4tukt.fsf@alter.siamese.dyndns.org>
Hi Junio,
sorry for the late reply I was taking some time off from email. Here now
some information on the two topics I am involved with that got stalled:
On Fri, Oct 07, 2011 at 01:28:34PM -0700, Junio C Hamano wrote:
> --------------------------------------------------
> [Stalled]
>
> * hv/submodule-merge-search (2011-08-26) 5 commits
> - submodule: Search for merges only at end of recursive merge
> - allow multiple calls to submodule merge search for the same path
> - submodule: Demonstrate known breakage during recursive merge
The three patches above belong to the merge-search fix topic. I think
they should be good to go.
> - push: Don't push a repository with unpushed submodules
> - push: teach --recurse-submodules the on-demand option
> (this branch is tangled with fg/submodule-auto-push.)
These two belong into the fg/submodule-auto-push topic. It seems they
got mixed into this while dicussing the two topics.
> The second from the bottom one needs to be replaced with a properly
> written commit log message.
I will look into that.
> * fg/submodule-auto-push (2011-09-11) 2 commits
> - submodule.c: make two functions static
> - push: teach --recurse-submodules the on-demand option
> (this branch is tangled with hv/submodule-merge-search.)
>
> What the topic aims to achieve may make sense, but the implementation
> looked somewhat suboptimal.
We will also have a look at the final cleanups we need here. (Fredrik?)
Cheers Heiko
^ permalink raw reply
* Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt
From: Junio C Hamano @ 2011-10-10 20:56 UTC (permalink / raw)
To: Sitaram Chamarty; +Cc: Phil Hord, Sitaram Chamarty, git
In-Reply-To: <20111008131015.GA28213@sita-lt.atc.tcs.com>
Sitaram Chamarty <sitaramc@gmail.com> writes:
> However, I'm not sure the file names that 'git difftool'
> comes up with are in a predictable order. That would mess
> up the test, but I can neither make it fail not find
> definitive information on the order in which the changed
> files are processed.
Hmm, that may be an issue, I would think.
> git-difftool--helper.sh | 9 +++++----
> t/t7800-difftool.sh | 44 +++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 48 insertions(+), 5 deletions(-)
>
> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
> index 8452890..0468446 100755
> --- a/git-difftool--helper.sh
> +++ b/git-difftool--helper.sh
> @@ -38,15 +38,16 @@ launch_merge_tool () {
>
> # $LOCAL and $REMOTE are temporary files so prompt
> # the user with the real $MERGED name before launching $merge_tool.
> + ans=y
> if should_prompt
> then
> printf "\nViewing: '$MERGED'\n"
> if use_ext_cmd
> then
> - printf "Hit return to launch '%s': " \
> + printf "Launch '%s' [Y/n]: " \
> "$GIT_DIFFTOOL_EXTCMD"
> else
> - printf "Hit return to launch '%s': " "$merge_tool"
> + printf "Launch '%s' [Y/n]: " "$merge_tool"
> fi
> read ans
> fi
> @@ -54,9 +55,9 @@ launch_merge_tool () {
> if use_ext_cmd
> then
> export BASE
> - eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"'
> + test "$ans" != "n" && eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"'
> else
> - run_merge_tool "$merge_tool"
> + test "$ans" != "n" && run_merge_tool "$merge_tool"
> fi
> }
I also found suggestion by Charles Bailey to return from the launch
function when the user says "no" easier to follow.
> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index 395adfc..f547e0b 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -38,7 +38,18 @@ restore_test_defaults()
> prompt_given()
> {
> prompt="$1"
> - test "$prompt" = "Hit return to launch 'test-tool': branch"
> + test "$prompt" = "Launch 'test-tool' [Y/n]: branch"
> +}
> +
> +stdin_contains()
> +{
> + grep >/dev/null "$1"
> +}
> +
> +stdin_doesnot_contain()
> +{
> + grep >/dev/null "$1" && return 1
> + return 0
> }
Doesn't
! grep >/dev/null "$1"
work in this case?
I also wondered if this is easier to read:
pipe | stdin_contains m2 &&
! pipe | stdin_contains master
but I do not think it is (we cannot say "pipe | ! stdin_contains master").
In any case, here is what I ended up queuing. Thanks.
-- >8 --
From: Sitaram Chamarty <sitaramc@gmail.com>
Date: Sat, 8 Oct 2011 18:40:15 +0530
Subject: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt
This is useful if you forgot to restrict the diff to the paths you want
to see, or selecting precisely the ones you want is too much typing.
[jc: with a change to return from the function upon 'n' by Charles Bailey
and a small tweak in stdin_doesnot_contain() in the test]
Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-difftool--helper.sh | 9 ++++++---
t/t7800-difftool.sh | 43 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index 8452890..e6558d1 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -43,12 +43,15 @@ launch_merge_tool () {
printf "\nViewing: '$MERGED'\n"
if use_ext_cmd
then
- printf "Hit return to launch '%s': " \
+ printf "Launch '%s' [Y/n]: " \
"$GIT_DIFFTOOL_EXTCMD"
else
- printf "Hit return to launch '%s': " "$merge_tool"
+ printf "Launch '%s' [Y/n]: " "$merge_tool"
+ fi
+ if read ans && test "$ans" = n
+ then
+ return
fi
- read ans
fi
if use_ext_cmd
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 395adfc..7fc2b3a 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -38,7 +38,17 @@ restore_test_defaults()
prompt_given()
{
prompt="$1"
- test "$prompt" = "Hit return to launch 'test-tool': branch"
+ test "$prompt" = "Launch 'test-tool' [Y/n]: branch"
+}
+
+stdin_contains()
+{
+ grep >/dev/null "$1"
+}
+
+stdin_doesnot_contain()
+{
+ ! stdin_contains "$1"
}
# Create a file on master and change it on branch
@@ -265,4 +275,35 @@ test_expect_success PERL 'difftool --extcmd cat arg2' '
test "$diff" = branch
'
+# Create a second file on master and a different version on branch
+test_expect_success PERL 'setup with 2 files different' '
+ echo m2 >file2 &&
+ git add file2 &&
+ git commit -m "added file2" &&
+
+ git checkout branch &&
+ echo br2 >file2 &&
+ git add file2 &&
+ git commit -a -m "branch changed file2" &&
+ git checkout master
+'
+
+test_expect_success PERL 'say no to the first file' '
+ diff=$((echo n; echo) | git difftool -x cat branch) &&
+
+ echo "$diff" | stdin_contains m2 &&
+ echo "$diff" | stdin_contains br2 &&
+ echo "$diff" | stdin_doesnot_contain master &&
+ echo "$diff" | stdin_doesnot_contain branch
+'
+
+test_expect_success PERL 'say no to the second file' '
+ diff=$((echo; echo n) | git difftool -x cat branch) &&
+
+ echo "$diff" | stdin_contains master &&
+ echo "$diff" | stdin_contains branch &&
+ echo "$diff" | stdin_doesnot_contain m2 &&
+ echo "$diff" | stdin_doesnot_contain br2
+'
+
test_done
--
1.7.7.138.g7f41b6
^ permalink raw reply related
* Re: [PATCHv5/RFC 0/6] Moving gitweb documentation to manpages
From: Junio C Hamano @ 2011-10-10 18:47 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Drew Northup, Jonathan Nieder
In-Reply-To: <1318098723-12813-1-git-send-email-jnareb@gmail.com>
Thanks for working on this.
^ permalink raw reply
* Re: [PATCHv5/RFC 3/6] gitweb: Add manpage for gitweb
From: Junio C Hamano @ 2011-10-10 18:56 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Drew Northup, Jonathan Nieder
In-Reply-To: <1318098723-12813-4-git-send-email-jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> diff --git a/Documentation/gitweb.txt b/Documentation/gitweb.txt
> new file mode 100644
> index 0000000..2acdb3b
> --- /dev/null
> +++ b/Documentation/gitweb.txt
> @@ -0,0 +1,703 @@
> +gitweb(1)
> +=========
> +
> +NAME
> +----
> +gitweb - Git web interface (web frontend to Git repositories)
> +
> +SYNOPSIS
> +--------
> +To get started with gitweb, run linkgit:git-instaweb[1] from a git repository.
> +This would configure and start your web server, and run web browser pointing to
> +gitweb page.
> +
> +See http://git.kernel.org/?p=git/git.git;a=tree;f=gitweb[] or
> +http://repo.or.cz/w/git.git/tree/HEAD:/gitweb/[] for gitweb source code,
> +browsed using gitweb itself.
This doesn't quite look like a "SYNOPSIS" section. Shouldn't everything
after the first line be at the beginning of "DESCRIPTION"?
^ permalink raw reply
* Re: [PATCHv5/RFC 1/6] Documentation: Preparation for gitweb manpages
From: Junio C Hamano @ 2011-10-10 18:53 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Drew Northup, Jonathan Nieder
In-Reply-To: <1318098723-12813-2-git-send-email-jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> This patch adds infrastructure for easy generation of only
> gitweb-related manpages. It adds a currently empty 'gitweb-doc'
> target to Documentation/Makefile, and a 'doc' proxy target to
> gitweb/Makefile.
I tend to agree with your after-three-dash comment that this separation is
not necessary, it may be expedient while working on the series, but wants
to be removed once the series is complete.
> This way to build only gitweb documentation in both 'man' and 'html'
> formats one can use
>
> make -C gitweb doc
>
> or
>
> cd gitweb; make doc
>
> This somewhat follows the idea of 'full-svn-test' and 'gitweb-test' in
> t/Makefile.
It seems that this follows the idea backward in that the existing practice
fo full-svn-test (and valgrind test) is to allow _excluding_ stuff that
the user may not care about or the user cannot afford to run; in that
sense 'gitweb-test' target is also backwards.
> gitweb manpages would reside in the gitweb/ directory, "make doc"
> would invoke "make -C gitweb doc" to generate formatted docs.
>
> The gitweb.conf(5) and gitweb(1) manpages will be added in subsequent
> commits.
>
> [Commit message improved with help of Jonathan Nieder]
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This commit is not strictly necessary: it only adds "doc" target to
> gitweb/Makefile, and "gitweb-doc" target to Documentation/Makefile;
> neither is run when e.g. generating RPM.
>
> They are here because they would be here if documentation source was
> kept along with gitweb script in the 'gitweb/' subdirectory, and to
> make it easier and faster to test the changes.
>
> Documentation/Makefile | 3 +++
> gitweb/Makefile | 7 ++++++-
> 2 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 6346a75..44be67b 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -170,6 +170,9 @@ info: git.info gitman.info
>
> pdf: user-manual.pdf
>
> +GITWEB_DOC = $(filter gitweb.%,$(DOC_HTML) $(DOC_MAN1) $(DOC_MAN5) $(DOC_MAN7))
> +gitweb-doc: $(GITWEB_DOC)
> +
> install: install-man
>
> install-man: man
> diff --git a/gitweb/Makefile b/gitweb/Makefile
> index 1c85b5f..3014d80 100644
> --- a/gitweb/Makefile
> +++ b/gitweb/Makefile
> @@ -174,6 +174,11 @@ test-installed:
> GITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \
> $(MAKE) -C ../t gitweb-test
>
> +### Documentation
> +
> +doc:
> + $(MAKE) -C ../Documentation gitweb-doc
> +
> ### Installation rules
>
> install: all
> @@ -187,5 +192,5 @@ install: all
> clean:
> $(RM) gitweb.cgi static/gitweb.min.js static/gitweb.min.css GITWEB-BUILD-OPTIONS
>
> -.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE
> +.PHONY: all clean install test test-installed doc .FORCE-GIT-VERSION-FILE FORCE
^ permalink raw reply
* Re: [PATCH] config: display key_delim for config --bool --get-regexp
From: Junio C Hamano @ 2011-10-10 20:59 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <1318251291-30297-1-git-send-email-Matthieu.Moy@imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> The previous logic in show_config was to print the delimiter when the
> value was set, but Boolean variables have an implicit value "true" when
> they appear with no value in the config file. As a result, we got:
>
> git_Config --get-regexp '.*\.Boolean' #1. Ok: example.boolean
> git_Config --bool --get-regexp '.*\.Boolean' #2. NO: example.booleantrue
>
> Fix this by defering the display of the separator until after the value
> to display has been computed.
>
> Reported-by: Brian Foster <brian.foster@maxim-ic.com>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
Thanks. Will queue for maintenance track.
> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
> index 3e140c1..dffccf8 100755
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
> @@ -333,6 +333,12 @@ test_expect_success 'get-regexp variable with no value' \
> 'git config --get-regexp novalue > output &&
> cmp output expect'
>
> +echo 'novalue.variable true' > expect
> +
> +test_expect_success 'get-regexp --bool variable with no value' \
> + 'git config --bool --get-regexp novalue > output &&
> + cmp output expect'
> +
> echo 'emptyvalue.variable ' > expect
>
> test_expect_success 'get-regexp variable with empty value' \
This matches the style of the surrounding code, but we may want to update
this to a more modern style.
^ permalink raw reply
* Re: [PATCH v3] Teach merge the '[-e|--edit]' option
From: Junio C Hamano @ 2011-10-10 21:05 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Todd A. Jacobs
In-Reply-To: <1318099192-60860-1-git-send-email-jaysoffian@gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> Implemented internally instead of as "git merge --no-commit && git commit"
> so that "merge --edit" is otherwise consistent (hooks, etc) with "merge".
>
> Note: the edit message does not include the status information that one
> gets with "commit --status" and it is cleaned up after editing like one
> gets with "commit --cleanup=default". A later patch could add the status
> information if desired.
>
> Note: previously we were not calling stripspace() after running the
> prepare-commit-msg hook. Now we are, stripping comments and
> leading/trailing whitespace lines if --edit is given, otherwise only
> stripping leading/trailing whitespace lines if not given --edit.
>
> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
> ---
Thanks.
> +static void prepare_to_commit(void)
> +{
> + struct strbuf msg = STRBUF_INIT;
> + strbuf_addbuf(&msg, &merge_msg);
> + strbuf_addch(&msg, '\n');
> + write_merge_msg(&msg);
> run_hook(get_index_file(), "prepare-commit-msg",
> git_path("MERGE_MSG"), "merge", NULL, NULL);
> - read_merge_msg();
> + if (option_edit) {
> + if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
> + abort_commit(NULL);
> + }
> + read_merge_msg(&msg);
> + stripspace(&msg, option_edit);
> + if (!msg.len)
> + abort_commit(_("Empty commit message."));
> + strbuf_release(&merge_msg);
> + strbuf_addbuf(&merge_msg, &msg);
> + strbuf_release(&msg);
> }
An abstraction very nicely done.
I am not sure about the '\n' you unconditionally added at the end of the
existing message.
I think running stripspace(&msg, option_edit) is a good change, even
though some people might feel it is a regression. "git commit" also cleans
up the whitespace cruft left by prepare-commit-message hook when the
editor is not in use, and this change makes it consistent.
> @@ -1015,6 +1041,36 @@ static int setup_with_upstream(const char ***argv)
> ...
> +static void write_merge_state(void)
> +{
> + int fd;
> + struct commit_list *j;
> + struct strbuf buf = STRBUF_INIT;
> +
> + for (j = remoteheads; j; j = j->next)
> + strbuf_addf(&buf, "%s\n",
> + sha1_to_hex(j->item->object.sha1));
> + fd = open(git_path("MERGE_HEAD"), O_WRONLY | O_CREAT, 0666);
> + if (fd < 0)
> + die_errno(_("Could not open '%s' for writing"),
> + git_path("MERGE_HEAD"));
> + if (write_in_full(fd, buf.buf, buf.len) != buf.len)
> + die_errno(_("Could not write to '%s'"), git_path("MERGE_HEAD"));
> + close(fd);
> + strbuf_addch(&merge_msg, '\n');
> + write_merge_msg(&merge_msg);
> + fd = open(git_path("MERGE_MODE"), O_WRONLY | O_CREAT | O_TRUNC, 0666);
> + if (fd < 0)
> + die_errno(_("Could not open '%s' for writing"),
> + git_path("MERGE_MODE"));
> + strbuf_reset(&buf);
> + if (!allow_fast_forward)
> + strbuf_addf(&buf, "no-ff");
> + if (write_in_full(fd, buf.buf, buf.len) != buf.len)
> + die_errno(_("Could not write to '%s'"), git_path("MERGE_MODE"));
> + close(fd);
> +}
Again very nicely done.
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 87aac835a1..8c6b811718 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -643,4 +643,19 @@ test_expect_success 'amending no-ff merge commit' '
>
> test_debug 'git log --graph --decorate --oneline --all'
>
> +cat >editor <<\EOF
> +#!/bin/sh
> +# strip comments and blank lines from end of message
> +sed -e '/^#/d' < "$1" | sed -e :a -e '/^\n*$/{$d;N;ba' -e '}' > expected
> +EOF
> +chmod 755 editor
I am not sure about this one. Wouldn't this want to be editing the given
file to make sure that the edited content appear in the result, not just
testing the additional stripspace() call you added in the codepath?
> +test_expect_success 'merge --no-ff --edit' '
> + git reset --hard c0 &&
> + EDITOR=./editor git merge --no-ff --edit c1 &&
> + verify_parents $c0 $c1 &&
> + git cat-file commit HEAD | sed "1,/^$/d" > actual &&
> + test_cmp actual expected
> +'
> +
> test_done
So perhaps this one on top? I am just suspecting that your additional '\n'
is to make sure we do not write out a file with an incomplete line with
this patch, but that change is not explained in your commit log message,
so I am not sure.
builtin/merge.c | 3 ++-
t/t7600-merge.sh | 10 +++++++++-
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index a8dbf4a..09ffc07 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -867,7 +867,8 @@ static void prepare_to_commit(void)
{
struct strbuf msg = STRBUF_INIT;
strbuf_addbuf(&msg, &merge_msg);
- strbuf_addch(&msg, '\n');
+ if (msg.len && msg.buf[msg.len-1] != '\n')
+ strbuf_addch(&msg, '\n');
write_merge_msg(&msg);
run_hook(get_index_file(), "prepare-commit-msg",
git_path("MERGE_MSG"), "merge", NULL, NULL);
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 8c6b811..3008e4e 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -645,6 +645,12 @@ test_debug 'git log --graph --decorate --oneline --all'
cat >editor <<\EOF
#!/bin/sh
+# Add a new message string that was not in the template
+(
+ echo "Merge work done on the side branch c1"
+ echo
+ cat <"$1"
+) >"$1.tmp" && mv "$1.tmp" "$1"
# strip comments and blank lines from end of message
sed -e '/^#/d' < "$1" | sed -e :a -e '/^\n*$/{$d;N;ba' -e '}' > expected
EOF
@@ -654,7 +660,9 @@ test_expect_success 'merge --no-ff --edit' '
git reset --hard c0 &&
EDITOR=./editor git merge --no-ff --edit c1 &&
verify_parents $c0 $c1 &&
- git cat-file commit HEAD | sed "1,/^$/d" > actual &&
+ git cat-file commit HEAD >raw &&
+ grep "work done on the side branch" raw &&
+ sed "1,/^$/d" >actual raw &&
test_cmp actual expected
'
^ permalink raw reply related
* Re: [PATCHv5/RFC 1/6] Documentation: Preparation for gitweb manpages
From: Jakub Narebski @ 2011-10-10 21:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Drew Northup, Jonathan Nieder
In-Reply-To: <7vwrcck1jm.fsf@alter.siamese.dyndns.org>
On Mon, 10 Oct 2011, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > This patch adds infrastructure for easy generation of only
> > gitweb-related manpages. It adds a currently empty 'gitweb-doc'
> > target to Documentation/Makefile, and a 'doc' proxy target to
> > gitweb/Makefile.
>
> I tend to agree with your after-three-dash comment that this separation is
> not necessary, it may be expedient while working on the series, but wants
> to be removed once the series is complete.
Also it is a bit of historical remains, as in original patch by Drew
the manpage (the AsciiDoc source) was in 'gitweb/' directory.
Anyway, I'll remove this patch from the future versions of this patch
series (if there would be need for next version).
[...]
> > ---
> > This commit is not strictly necessary: it only adds "doc" target to
> > gitweb/Makefile, and "gitweb-doc" target to Documentation/Makefile;
> > neither is run when e.g. generating RPM.
> >
> > They are here because they would be here if documentation source was
> > kept along with gitweb script in the 'gitweb/' subdirectory, and to
> > make it easier and faster to test the changes.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCHv5/RFC 3/6] gitweb: Add manpage for gitweb
From: Jakub Narebski @ 2011-10-10 22:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Drew Northup, Jonathan Nieder
In-Reply-To: <7vr52kk1jj.fsf@alter.siamese.dyndns.org>
On Mon, 10 Oct 2011, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > diff --git a/Documentation/gitweb.txt b/Documentation/gitweb.txt
> > new file mode 100644
> > index 0000000..2acdb3b
> > --- /dev/null
> > +++ b/Documentation/gitweb.txt
> > @@ -0,0 +1,703 @@
> > +gitweb(1)
> > +=========
> > +
> > +NAME
> > +----
> > +gitweb - Git web interface (web frontend to Git repositories)
> > +
> > +SYNOPSIS
> > +--------
> > +To get started with gitweb, run linkgit:git-instaweb[1] from a git repository.
> > +This would configure and start your web server, and run web browser pointing to
> > +gitweb page.
> > +
> > +See http://git.kernel.org/?p=git/git.git;a=tree;f=gitweb[] or
> > +http://repo.or.cz/w/git.git/tree/HEAD:/gitweb/[] for gitweb source code,
> > +browsed using gitweb itself.
>
> This doesn't quite look like a "SYNOPSIS" section. Shouldn't everything
> after the first line be at the beginning of "DESCRIPTION"?
Did you mean something like (it would have to be slightly adjusted,
of course):
+SYNOPSIS
+--------
+To get started with gitweb, run linkgit:git-instaweb[1] from a git repository.
+
+DESCRIPTION
+-----------
+This would configure and start your web server, and run web browser pointing to
+gitweb page.
The problem is that catering to old AsciiDoc (but still used by some of
long-term-support Linux distributions) requires to have "SYNOPSIS"
section... but there is no natural synopsis for non self-hostable web
application, is there?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCHv5/RFC 3/6] gitweb: Add manpage for gitweb
From: Jonathan Nieder @ 2011-10-10 22:18 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, git, Drew Northup
In-Reply-To: <201110110002.24665.jnareb@gmail.com>
Jakub Narebski wrote:
> The problem is that catering to old AsciiDoc (but still used by some of
> long-term-support Linux distributions) requires to have "SYNOPSIS"
> section... but there is no natural synopsis for non self-hostable web
> application, is there?
I personally think something like
SYNOPSIS
--------
/usr/share/gitweb/gitweb.cgi
git instaweb
or perhaps something like
SYNOPSIS
--------
http://<site>/?p=<project>.git;a=<action>;h=<object>;<parameters>
http://<site>/<project>/<action>/<object>?<parameters>
would be best.
^ permalink raw reply
* Re: [PATCH v3] Teach merge the '[-e|--edit]' option
From: Jay Soffian @ 2011-10-10 22:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Todd A. Jacobs
In-Reply-To: <7vd3e4k162.fsf@alter.siamese.dyndns.org>
On Mon, Oct 10, 2011 at 2:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jay Soffian <jaysoffian@gmail.com> writes:
>> +static void prepare_to_commit(void)
>> +{
>> + struct strbuf msg = STRBUF_INIT;
>> + strbuf_addbuf(&msg, &merge_msg);
>> + strbuf_addch(&msg, '\n');
>> + write_merge_msg(&msg);
>> run_hook(get_index_file(), "prepare-commit-msg",
>> git_path("MERGE_MSG"), "merge", NULL, NULL);
>> - read_merge_msg();
>> + if (option_edit) {
>> + if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
>> + abort_commit(NULL);
>> + }
>> + read_merge_msg(&msg);
>> + stripspace(&msg, option_edit);
>> + if (!msg.len)
>> + abort_commit(_("Empty commit message."));
>> + strbuf_release(&merge_msg);
>> + strbuf_addbuf(&merge_msg, &msg);
>> + strbuf_release(&msg);
>> }
>
> An abstraction very nicely done.
<blush> :-)
> I am not sure about the '\n' you unconditionally added at the end of the
> existing message.
Right, the old code does that when the merge fails, counting on (I
think) git-commit to then take care of any extra newlines. My
reasoning was tack it on before running prepare-commit-msg, then run
stripspace() after the hook and and editor, which will take care of
any excess newlines. I guess this would be a regression if someone's
prepare-commit-msg hook blindly appends to the commit message.
> I think running stripspace(&msg, option_edit) is a good change, even
> though some people might feel it is a regression. "git commit" also cleans
> up the whitespace cruft left by prepare-commit-message hook when the
> editor is not in use, and this change makes it consistent.
Correct.
>> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
>> index 87aac835a1..8c6b811718 100755
>> --- a/t/t7600-merge.sh
>> +++ b/t/t7600-merge.sh
>> @@ -643,4 +643,19 @@ test_expect_success 'amending no-ff merge commit' '
>>
>> test_debug 'git log --graph --decorate --oneline --all'
>>
>> +cat >editor <<\EOF
>> +#!/bin/sh
>> +# strip comments and blank lines from end of message
>> +sed -e '/^#/d' < "$1" | sed -e :a -e '/^\n*$/{$d;N;ba' -e '}' > expected
>> +EOF
>> +chmod 755 editor
>
> I am not sure about this one. Wouldn't this want to be editing the given
> file to make sure that the edited content appear in the result, not just
> testing the additional stripspace() call you added in the codepath?
Yep.
I added this test under the previous iteration of the patch when I was
concerned that commit message make it through the external commit code
path correctly. It doesn't really make sense with this iteration now
that I think about it. The part about stripping comments and newlines
is no longer needed.
>> +test_expect_success 'merge --no-ff --edit' '
>> + git reset --hard c0 &&
>> + EDITOR=./editor git merge --no-ff --edit c1 &&
>> + verify_parents $c0 $c1 &&
>> + git cat-file commit HEAD | sed "1,/^$/d" > actual &&
>> + test_cmp actual expected
>> +'
>> +
>> test_done
>
> So perhaps this one on top? I am just suspecting that your additional '\n'
> is to make sure we do not write out a file with an incomplete line with
> this patch, but that change is not explained in your commit log message,
> so I am not sure.
I assumed the '\n' was needed as it's added (unconditionally) before
writing MERGE_MSG when the merge fails. I didn't notice that when I
added the prepare-commit-msg hook support to merge.c (65969d43d1).
> builtin/merge.c | 3 ++-
> t/t7600-merge.sh | 10 +++++++++-
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index a8dbf4a..09ffc07 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -867,7 +867,8 @@ static void prepare_to_commit(void)
> {
> struct strbuf msg = STRBUF_INIT;
> strbuf_addbuf(&msg, &merge_msg);
> - strbuf_addch(&msg, '\n');
> + if (msg.len && msg.buf[msg.len-1] != '\n')
> + strbuf_addch(&msg, '\n');
> write_merge_msg(&msg);
> run_hook(get_index_file(), "prepare-commit-msg",
> git_path("MERGE_MSG"), "merge", NULL, NULL);
I'm guessing the '\n' is always needed (per above), but I'm not sure.
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 8c6b811..3008e4e 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -645,6 +645,12 @@ test_debug 'git log --graph --decorate --oneline --all'
>
> cat >editor <<\EOF
> #!/bin/sh
> +# Add a new message string that was not in the template
> +(
> + echo "Merge work done on the side branch c1"
> + echo
> + cat <"$1"
> +) >"$1.tmp" && mv "$1.tmp" "$1"
> # strip comments and blank lines from end of message
> sed -e '/^#/d' < "$1" | sed -e :a -e '/^\n*$/{$d;N;ba' -e '}' > expected
> EOF
> @@ -654,7 +660,9 @@ test_expect_success 'merge --no-ff --edit' '
> git reset --hard c0 &&
> EDITOR=./editor git merge --no-ff --edit c1 &&
> verify_parents $c0 $c1 &&
> - git cat-file commit HEAD | sed "1,/^$/d" > actual &&
> + git cat-file commit HEAD >raw &&
> + grep "work done on the side branch" raw &&
> + sed "1,/^$/d" >actual raw &&
> test_cmp actual expected
> '
Okay. A test that the merge is aborted if the message is empty would
also be good.
I'll try to find time to send another iteration with better tests. May
not be till next week though.
j.
^ permalink raw reply
* Re: [PATCH] git-svn: Allow certain refs to be ignored
From: Eric Wong @ 2011-10-10 22:58 UTC (permalink / raw)
To: Michael Olson; +Cc: git, Junio C Hamano
In-Reply-To: <7vvcs0s7xa.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> Asking Eric to comment when he has time to do so.
>
> I find these pattern matches that are not anchored on either side
> somewhat disturbing (e.g. --ignore-refs=master would ignore master2)
> but ignore-paths codepath seems to follow the same pattern, so perhaps it
> is in line with what git-svn users want. I dunno.
As stated last year, I remember wanting globs instead of regexps, but
we already made the regexp mistake with ignore-paths, too :(
I don't think it's horrible with regexps, and if git-svn users find it
useful, it's fine by me.
> Michael Olson <mwolson@gnu.org> writes:
> > Re-sent by request of Piotr Krukowiecki. This is against v1.7.4.1,
> > and I've been using it stably for a while.
Michael: can you please rebase against latest and resend? Thanks.
^ permalink raw reply
* Re: Git User's Survey 2011 very short summary
From: Andrew Ardill @ 2011-10-10 23:09 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <201110101721.51830.jnareb@gmail.com>
Thanks for your work organising this, as always it is very interesting to read.
Regards,
Andrew Ardill
^ permalink raw reply
* Re: [PATCHv5/RFC 1/6] Documentation: Preparation for gitweb manpages
From: Junio C Hamano @ 2011-10-10 23:10 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Drew Northup, Jonathan Nieder
In-Reply-To: <201110102352.25456.jnareb@gmail.com>
I probably do not have time to look into this, but just FYI my trial merge
to 'pu' of this topic is failing like this:
asciidoc: ERROR: gitweb.conf.txt: line 484: illegal style name: Default: ()
asciidoc: ERROR: gitweb.conf.txt: line 494: illegal style name: Default: 300
^ permalink raw reply
* [PATCH v2] git-svn: Allow certain refs to be ignored
From: Michael Olson @ 2011-10-10 23:27 UTC (permalink / raw)
To: Git Mailing List
Implement a new --ignore-refs option which specifies a regex of refs
to ignore while importing svn history.
This is a useful supplement to the --ignore-paths option, as that
option only operates on the contents of branches and tags, not the
branches and tags themselves.
Signed-off-by: Michael Olson <mwolson@gnu.org>
---
Rebased against git master.
git-svn.perl | 38 +++++++++++++++++++++++++++++++++-----
1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 351e743..fed1734 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -94,7 +94,8 @@ $_q ||= 0;
my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
'config-dir=s' => \$Git::SVN::Ra::config_dir,
'no-auth-cache' => \$Git::SVN::Prompt::_no_auth_cache,
- 'ignore-paths=s' => \$SVN::Git::Fetcher::_ignore_regex );
+ 'ignore-paths=s' => \$SVN::Git::Fetcher::_ignore_regex,
+ 'ignore-refs=s' => \$Git::SVN::Ra::_ignore_refs_regex );
my %fc_opts = ( 'follow-parent|follow!' => \$Git::SVN::_follow_parent,
'authors-file|A=s' => \$_authors,
'authors-prog=s' => \$_authors_prog,
@@ -388,9 +389,12 @@ sub do_git_init_db {
command_noisy('config', "$pfx.$i", $icv{$i});
$set = $i;
}
- my $ignore_regex = \$SVN::Git::Fetcher::_ignore_regex;
- command_noisy('config', "$pfx.ignore-paths", $$ignore_regex)
- if defined $$ignore_regex;
+ my $ignore_paths_regex = \$SVN::Git::Fetcher::_ignore_regex;
+ command_noisy('config', "$pfx.ignore-paths", $$ignore_paths_regex)
+ if defined $$ignore_paths_regex;
+ my $ignore_refs_regex = \$Git::SVN::Ra::_ignore_refs_regex;
+ command_noisy('config', "$pfx.ignore-refs", $$ignore_refs_regex)
+ if defined $$ignore_refs_regex;
if (defined $SVN::Git::Fetcher::_preserve_empty_dirs) {
my $fname = \$SVN::Git::Fetcher::_placeholder_filename;
@@ -2119,6 +2123,8 @@ sub read_all_remotes {
$r->{$1}->{url} = $2;
} elsif (m!^(.+)\.pushurl=\s*(.*)\s*$!) {
$r->{$1}->{pushurl} = $2;
+ } elsif (m!^(.+)\.ignore-refs=\s*(.*)\s*$!) {
+ $r->{$1}->{ignore_refs_regex} = $2;
} elsif (m!^(.+)\.(branches|tags)=$svn_refspec$!) {
my ($remote, $t, $local_ref, $remote_ref) =
($1, $2, $3, $4);
@@ -2155,6 +2161,16 @@ sub read_all_remotes {
}
} keys %$r;
+ foreach my $remote (keys %$r) {
+ foreach ( grep { defined $_ }
+ map { $r->{$remote}->{$_} } qw(branches tags) ) {
+ foreach my $rs ( @$_ ) {
+ $rs->{ignore_refs_regex} =
+ $r->{$remote}->{ignore_refs_regex};
+ }
+ }
+ }
+
$r;
}
@@ -5310,7 +5326,7 @@ sub apply_diff {
}
package Git::SVN::Ra;
-use vars qw/@ISA $config_dir $_log_window_size/;
+use vars qw/@ISA $config_dir $_ignore_refs_regex $_log_window_size/;
use strict;
use warnings;
my ($ra_invalid, $can_do_switch, %ignored_err, $RA);
@@ -5768,6 +5784,17 @@ sub get_dir_globbed {
@finalents;
}
+# return value: 0 -- don't ignore, 1 -- ignore
+sub is_ref_ignored {
+ my ($g, $p) = @_;
+ my $refname = $g->{ref}->full_path($p);
+ return 1 if defined($g->{ignore_refs_regex}) &&
+ $refname =~ m!$g->{ignore_refs_regex}!;
+ return 0 unless defined($_ignore_refs_regex);
+ return 1 if $refname =~ m!$_ignore_refs_regex!o;
+ return 0;
+}
+
sub match_globs {
my ($self, $exists, $paths, $globs, $r) = @_;
@@ -5804,6 +5831,7 @@ sub match_globs {
next unless /$g->{path}->{regex}/;
my $p = $1;
my $pathname = $g->{path}->full_path($p);
+ next if is_ref_ignored($g, $p);
next if $exists->{$pathname};
next if ($self->check_path($pathname, $r) !=
$SVN::Node::dir);
--
1.7.4.1
^ permalink raw reply related
* Garbage collection creates many unpacked objects.
From: Martin Fick @ 2011-10-10 23:30 UTC (permalink / raw)
To: git
If I clone linus' kernel, delete all the tags, and then run
git gc, it ends up expanding into about 5K of unpacked
objects. The .git size goes from 473M to 511M. This seems
a bit strange no? Shouldn't gcing yield a smaller repo an
fewer unpacked refs?
If I do this on our internal kernel repo (which has 2Ktags),
it gets much more pathological, it expands to about 1M
objects and grows to about 7G!!!
This seems to happen with all versions which I tested,
1.6.0, 1.7.6 and 1.7.7
Any thoughts?
-Martin
--
Employee of Qualcomm Innovation Center, Inc. which is a
member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt
From: Sitaram Chamarty @ 2011-10-10 23:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Phil Hord, Sitaram Chamarty, git
In-Reply-To: <7v8voslg4l.fsf@alter.siamese.dyndns.org>
On Tue, Oct 11, 2011 at 2:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I also wondered if this is easier to read:
>
> pipe | stdin_contains m2 &&
> ! pipe | stdin_contains master
> but I do not think it is (we cannot say "pipe | ! stdin_contains master").
Agreed on both counts.
"pipe | ( ! grep master )" does work, but I suspect that is an
inconsistency in the shell so I didn't want to use it. IIRC the "(
list )" constrict is not supposed to make *that* much difference.
Have to check when I have time.
> In any case, here is what I ended up queuing. Thanks.
> +stdin_doesnot_contain()
> +{
> + ! stdin_contains "$1"
> }
(facepalm) Why didn't I think of that!
Thanks :-)
^ permalink raw reply
* Re: Garbage collection creates many unpacked objects.
From: Shawn Pearce @ 2011-10-10 23:49 UTC (permalink / raw)
To: Martin Fick; +Cc: git
In-Reply-To: <201110101730.43302.mfick@codeaurora.org>
On Mon, Oct 10, 2011 at 16:30, Martin Fick <mfick@codeaurora.org> wrote:
> If I clone linus' kernel, delete all the tags, and then run
> git gc, it ends up expanding into about 5K of unpacked
> objects. The .git size goes from 473M to 511M. This seems
> a bit strange no? Shouldn't gcing yield a smaller repo an
> fewer unpacked refs?
>
> If I do this on our internal kernel repo (which has 2Ktags),
> it gets much more pathological, it expands to about 1M
> objects and grows to about 7G!!!
This is caused by unreachable objects being evicted from packs and
stored as loose objects for up to 2 weeks, until the next `git prune`
run. I think you can avoid this by using `git repack -a -d` instead of
`git gc`, but at the risk that a concurrent modification of the
repository may result in corruption due to a race condition between
the object being needed, and the object being deleted.
^ permalink raw reply
* Re: [PATCH 2/3] Fix some "variable might be used uninitialized" warnings
From: Junio C Hamano @ 2011-10-10 23:59 UTC (permalink / raw)
To: Ramsay Jones; +Cc: GIT Mailing-list
In-Reply-To: <4E90751C.4030409@ramsay1.demon.co.uk>
Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>> [I don't think traverse_trees() would ever be called with n == 0 anyway; the call
>> site in builtin/merge-tree.c is called with the constant 3, and the call-chains(s)
>> which start from unpack_trees() are protected by "if (len)", where 'len' is unsigned.]
>
> When patches don't even make it to pu I just assume you hate them so much that
> there is not much chance of them being applied and simply forget about them.
> In this case, since compiler warnings are a bugbear of mine, I'm hoping that
> you just forgot about this one ... :-D [if not, sorry for the noise].
Thanks for a reminder.
The reason a patch may not hit 'pu', unless I or other people whose
judgement I trust explicity say "the approach taken by the patch is
utterly wrong" is either because (1) the discussion for or against the
topic is still going strong and there is little chance of it getting
forgotten by everybody, (2) I do not see much discussion for or against
the topic, and I am indifferent, or (3) the patch was just lost in the
noise.
So a good default strategy for a series that do not hit 'pu' is to
re-post. Such a perseverance was what took format-patch to hit Linus's
tree in June-July 2005 timeframe---we wouldn't have the command today, had
I given up back then ;-).
^ permalink raw reply
* Re: [PATCH v3 5/5] attr.c: respect core.ignorecase when matching attribute patterns
From: Junio C Hamano @ 2011-10-11 0:00 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Brandon Casey, git, peff, j.sixt, Brandon Casey
In-Reply-To: <4E91BAC8.9060606@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 10/06/2011 08:22 PM, Brandon Casey wrote:
>> The last set of tests is performed only on a case-insensitive filesystem.
>> Those tests make sure that git handles the case where the .gitignore file
>> resides in a subdirectory and the user supplies a path that does not match
>> the case in the filesystem. In that case^H^H^H^Hsituation, part of the
>> path supplied by the user is effectively interpreted case-insensitively,
>> and part of it is dependent on the setting of core.ignorecase. git should
>> only match the portion of the path below the directory holding the
>> .gitignore file according to the setting of core.ignorecase.
>
> ... It is hard to imagine that
> anybody *wants* part of a single filename to be matched
> case-insensitively and another part to be matched case-sensitively.
That is not necessarily coming from the user's wish. When a command that
takes a pathspec from the user is run from a subdirectory, almost always
the output from $(git rev-parse --show-prefix) is prefixed to them. This
value is read from the filesystem, and unless on a system whose readdir()
may lie to us, we do not _have_ to ignore the case of this part of the
substring of a resulting pathspec element to get a successful match, even
though we _do_ want to match the user-supplied part case insensitively
if/when the user says he wants us to with core.ignorecase.
Having said that, it may not hurt in practice if we matched the prefix
part case insensitvely, because prefix=foobar/ obtained on a filesystem
where core.ignorecase is true would mean there won't be FooBar/ and other
case-permuted names on the filesystem at the same time.
But of course I may be missing something...
^ permalink raw reply
* Re: [PATCH v2 1/7] invalidate_ref_cache(): rename function from invalidate_cached_refs()
From: Junio C Hamano @ 2011-10-11 0:00 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland
In-Reply-To: <1318235064-25915-2-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> It is the cache that is being invalidated, not the references.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
Although I think one can say "ref cache is the container for cached refs"
and invalidating the "ref cache" as the container and invalidating the
"cached refs" as a collection mean essentially the same thing, probably
the new name makes more sense.
> refs.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 2cb93e2..56e4254 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -223,7 +223,7 @@ static struct cached_refs *get_cached_refs(const char *submodule)
> return refs;
> }
>
> -static void invalidate_cached_refs(void)
> +static void invalidate_ref_cache(void)
> {
> struct cached_refs *refs = cached_refs;
> while (refs) {
> @@ -1212,7 +1212,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
> ret |= repack_without_ref(refname);
>
> unlink_or_warn(git_path("logs/%s", lock->ref_name));
> - invalidate_cached_refs();
> + invalidate_ref_cache();
> unlock_ref(lock);
> return ret;
> }
> @@ -1511,7 +1511,7 @@ int write_ref_sha1(struct ref_lock *lock,
> unlock_ref(lock);
> return -1;
> }
> - invalidate_cached_refs();
> + invalidate_ref_cache();
> if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
> (strcmp(lock->ref_name, lock->orig_ref_name) &&
> log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
^ permalink raw reply
* Re: [PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask
From: Junio C Hamano @ 2011-10-11 0:00 UTC (permalink / raw)
To: David Aguilar
Cc: Jonathan Nieder, git@vger.kernel.org, Tanguy Ortolo,
Charles Bailey, SebastianSchuberth
In-Reply-To: <1C9F1683-4C6E-4D49-86D3-3A47B2843F23@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> thanks. I agree that the tar is overkill. I think I copied that snippet
> from templates/makefile. does that have the same bug?
Likely, as I recall that I wrote the installation of templates in an
expecially sloppy way, thinking somebody would fix it in short order
anyway. Instead the sloppiness seems to have been copied and pasted to
make things worse...
Thanks for fixing the mess---all's well that ends well.
^ permalink raw reply
* Re: [PATCH v3] Teach merge the '[-e|--edit]' option
From: Junio C Hamano @ 2011-10-11 0:00 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Todd A. Jacobs
In-Reply-To: <CAG+J_Dz37etot0nNkq+1gTUy8R0vVJpsRQuvwrTSczXRWy7mkA@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
>> I am not sure about the '\n' you unconditionally added at the end of the
>> existing message.
>
> Right, the old code does that when the merge fails, counting on (I
> think) git-commit to then take care of any extra newlines.
Ahh, that explains it. I was originally about to suggest running the
stripspace() only when we run the editor, and saw a failure from an
unrelated test and realized that running stripspace() on the result of
prepare-commit-msg is the right thing to do after all, because that is
what is done by "git commit". Yes, the current code does rely on the
stripspace to remove it, so there is no need to make it conditional.
So if we drop the "conditionally add '\n'" part in builtin/merge.c from my
"how about this on top" patch and we should be ready to go, right?
Thanks.
^ permalink raw reply
* Re: [PATCH v2 0/7] Provide API to invalidate refs cache
From: Junio C Hamano @ 2011-10-11 0:02 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> These patches apply on top of mh/iterate-refs, which is in next but
> not in master.
Building your series on mh/iterate-refs would unfortunately make the
conflict resolution worse. It would have been better if this were based on
a merge between mh/iterate-refs and jp/get-ref-dir-unsorted (which already
has happened on 'master' as of fifteen minutes ago).
I could rebase your series, but it always is more error prone to have
somebody who is not the original author rebase a series than the original
author build for the intended base tree from the beginning.
Thanks.
^ permalink raw reply
* Re: [PATCH v3] Teach merge the '[-e|--edit]' option
From: Jay Soffian @ 2011-10-11 0:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Todd A. Jacobs
In-Reply-To: <7v1uukieh2.fsf@alter.siamese.dyndns.org>
On Mon, Oct 10, 2011 at 5:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> So if we drop the "conditionally add '\n'" part in builtin/merge.c from my
> "how about this on top" patch and we should be ready to go, right?
Yes. I can send a followup patch adding an additional test case next
week (for the case where the editor zeros out the message).
Thanks Junio!
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