* 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
* 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
* [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
* 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
* 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: [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: [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: [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: [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 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: [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: [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: [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: [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 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: [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: 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: Re: [PATCH 6/6] Retain caches of submodule refs
From: Heiko Voigt @ 2011-10-10 19:53 UTC (permalink / raw)
To: Michael Haggerty
Cc: Junio C Hamano, git, Jeff King, Drew Northup, Jakub Narebski
In-Reply-To: <4E918194.5060102@alum.mit.edu>
Hi Michael,
On Sun, Oct 09, 2011 at 01:12:20PM +0200, Michael Haggerty wrote:
> On 08/17/2011 12:45 AM, Junio C Hamano wrote:
> > All the changes except for this one made sense to me, but I am not sure
> > about this one. How often do we look into different submodule refs in the
> > same process over and over again?
>
> I am having pangs of uncertainty about this patch.
Currently when doing a 3 way merge of submodule hashes and we find a
conflict. We then use this api to look into the submodule to search for
commit suggestions which contain both sides.
If there are multiple submodules that conflict in this way we look into
those as well.
Since the setup_revision() api can currently not be used to safely
iterate twice over the same submodule my patch
allow multiple calls to submodule merge search for the same path
rewrites the search into using a child process. AFAIK the submodule ref
iteration api would then even be unused.
> Previous to this patch, the submodule reference cache was only used for
> the duration of one call to do_for_each_ref(). (It was not *discarded*
> until later, but the old cache was never reused.) Therefore, the
> submodule reference cache was implicitly invalidated between successive
> uses.
The implicit discarding was just done because it was the quickest way to
get a handle on submodule refs from the main process. There was no need
that they get reloaded every time.
> After this change, submodule ref caches are invalidated whenever
> invalidate_cached_refs() is called. But this function is static, and it
> is only called when main-module refs are changed.
>
> AFAIK there is no way within refs.c to add, modify, or delete a
> submodule reference. But if other code modifies submodule references
> directly, then the submodule ref cache in refs.c would become stale.
> Moreover, there is currently no API for invalidating the cache.
>
> So I think I need help from a submodule guru (Heiko?) who can tell me
> what is done with submodule references and whether they might be
> modified while a git process is executing in the main module. If so,
> then either this patch has to be withdrawn, or more work has to be put
> in to make such code invalidate the submodule reference cache.
At least in my code there is no place where a submodule ref is changed.
I only used it for merging submodule which only modifies the main
module. So I would say its currently safe to assume that submodule refs
do not get modified. If we do need that later on we can still add
invalidation for submodule refs.
Cheers Heiko
^ permalink raw reply
* Re: [PATCH 2/2] submodule::module_clone(): silence die() message from module_name()
From: Jens Lehmann @ 2011-10-10 19:34 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: git, Junio C Hamano
In-Reply-To: <1317978295-4796-2-git-send-email-rctay89@gmail.com>
BTW: this patch applies to next
Am 07.10.2011 11:04, schrieb Tay Ray Chuan:
> The die() message that may occur in module_name() is not really relevant
> to the user when called from module_clone(); the latter handles the
> "failure" (no submodule mapping) anyway.
Makes tons of sense, especially as adding a new submodule currently always
spews out the "No submodule mapping found in .gitmodules for path 'sub'"
message right before that mapping is added there. Thanks for noticing that
and ACK on that change from my side.
> Leave other callers of module_name() unchanged, as the die() message
> shown is either relevant for user consumption (such as those that exit()
> when the call fails), or will not occur at all (when called with paths
> returned by module_list()).
Hmm, while I agree on the first reasoning I'm not sure about the second.
module_list() asks the index for the submodule paths while module_name()
gets it's input from .gitmodules, so they can (and sometimes will)
disagree. When cmd_foreach() passes an empty "name" variable to the
spawned command that might still work (and even make sense), but using the
empty name in cmd_sync() to access the config is looking like an error to
me. It might make sense to add an "|| exit" at least to the callsite in
cmd_sync(). Or am I missing something here?
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
> ---
> git-submodule.sh | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index ebea35b..3adab93 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -130,7 +130,7 @@ module_clone()
>
> gitdir=
> gitdir_base=
> - name=$(module_name "$path")
> + name=$(module_name "$path" 2>/dev/null)
> base_path=$(dirname "$path")
>
> gitdir=$(git rev-parse --git-dir)
^ permalink raw reply
* Re: [PATCH 1/2] submodule: whitespace fix
From: Jens Lehmann @ 2011-10-10 18:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tay Ray Chuan, git
In-Reply-To: <1317978295-4796-1-git-send-email-rctay89@gmail.com>
Am 07.10.2011 11:04, schrieb Tay Ray Chuan:
> Replace SPs with TAB.
As these are the only lines where spaces are used for indentation in this
file this might be a worthwhile cleanup, but I really don't care that much.
Junio, what do you think?
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
> ---
> git-submodule.sh | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 928a62f..ebea35b 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -104,9 +104,9 @@ module_name()
> re=$(printf '%s\n' "$1" | sed -e 's/[].[^$\\*]/\\&/g')
> name=$( git config -f .gitmodules --get-regexp '^submodule\..*\.path$' |
> sed -n -e 's|^submodule\.\(.*\)\.path '"$re"'$|\1|p' )
> - test -z "$name" &&
> - die "$(eval_gettext "No submodule mapping found in .gitmodules for path '\$path'")"
> - echo "$name"
> + test -z "$name" &&
> + die "$(eval_gettext "No submodule mapping found in .gitmodules for path '\$path'")"
> + echo "$name"
> }
>
> #
^ permalink raw reply
* Re: [PATCH v3 5/5] attr.c: respect core.ignorecase when matching attribute patterns
From: Brandon Casey @ 2011-10-10 18:01 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Brandon Casey, gitster, git, peff, j.sixt
In-Reply-To: <4E91BAC8.9060606@alum.mit.edu>
On Sun, Oct 9, 2011 at 10:16 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> 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.
>
> Isn't this a rather perverse scenario?
No argument here. :)
> 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.
> ISTM that a person who is using a case-insensitive filesystem and has
> core-ignorecase=false is (1) a glutton for punishment and (2) probably
> very careful to make sure that the case of all pathnames is correct. So
> such a person is not likely to benefit from this schizophrenic behavior.
>
>> [...] If git instead built the attr
>> stack by scanning the repository, then the paths in the origin field would
>> not necessarily match the paths supplied by the user. If someone makes a
>> change like that in the future, these tests will notice.
>
> Your decision to treat path names as partly case-insensitive...
You are giving more credit to this patch than it deserves. It really
doesn't do much. It's not a design decision that I made to treat path
names as case-insensitive. That is a consequence of using a
case-insensitive file system. When you give git the path A/b/c and
there exists a/.gitattributes, then on a case insensitive file system
git will try to read A/.gitattributes and the filesystem will return a
handle for a/.gitattributes and git won't be able to tell the
difference. git will then apply the rules in the file to the paths
below a/ even when the path is supplied like A/. So, at the moment,
regardless of the setting of core.ignorecase, the path above a
.gitignore file is treated as case-insensitive on a case-insensitive
filesystem.
The purpose of the tests are primarily to ensure that nothing special
needs to be done concerning paths leading up to a directory containing
a .gitignore file in the core.ignorecase=1 case. For example, it is
_not_ necessary to use strncmp_icase instead of strncmp on the leading
portion of the path.
What I meant when I said "If someone makes a change like that in the
future, these tests will notice", is that they will notice that they
must now use strncmp_icase when comparing the portion of the path
above the .gitattributes file.
> will make
> this kind of improvement considerably more complicated.
> Therefore, weighing the negligible benefit of declaring this
> schizophrenic behavior against the potential big pain of remaining
> backwards-compatible with this behavior, I suggest that we either (1)
> declare that when core.ignorecase=false then the *whole* filenames
> (including the path leading up to the .gitignore file) must match
> case-sensitively, or (2) declare the behavior in this situation to be
> undefined so that nobody thinks they should depend on it.
I do not plan to make git treat leading paths case-sensitively on a
case-insensitive file system when core.ignorecase=0 any more than I
plan to make git treat leading paths case-insensitively on a
case-sensitive file system when core.ignorecase=1. For justification,
I refer back to your original comment about perversion. :)
So, #2 gets my vote.
Maybe my commit message is not clear that it is describing the current
behavior and not defining it. Instead of
git should only match the portion of the path below the directory
holding the .gitignore file according to the setting of
core.ignorecase.
maybe I should say
git will currently only match the portion of the path...
I could also remove the following test from the CASE_INSENSITIVE_FS
tests since it is really a dontcare:
attr_check A/b/h a/b/h "-c core.ignorecase=0"
We don't care what happens when the user supplies A/b/h and a/b/h
exists on disk when core.ignorecase=0, we only care that A/b/h is
interpreted correctly when core.ignorecase=1.
-Brandon
^ permalink raw reply
* Re: [PATCH 4/6] revert: Make commit descriptions in insn sheet optional
From: Junio C Hamano @ 2011-10-10 17:54 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Jonathan Nieder, Jeff King, Daniel Barkalow,
Christian Couder
In-Reply-To: <1318095407-26429-5-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> Change the instruction sheet format subtly so that a description of
> the commit after the object name is optional. As a result, an
> instruction sheet like this is now perfectly valid:
>
> pick 35b0426
> pick fbd5bbcbc2e
> pick 7362160f
>
> Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
> Acked-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> builtin/revert.c | 19 ++++++++-----------
> t/t3510-cherry-pick-sequence.sh | 14 ++++++++++++++
> 2 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 6451089..aa6c34e 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -692,26 +692,23 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
> unsigned char commit_sha1[20];
> char sha1_abbrev[40];
> enum replay_action action;
> - int insn_len = 0;
> - char *p, *q;
> + const char *p, *q;
>
> + p = start;
> if (!prefixcmp(start, "pick ")) {
> action = CHERRY_PICK;
> - insn_len = strlen("pick");
> - p = start + insn_len + 1;
> + p += strlen("pick ");
> } else if (!prefixcmp(start, "revert ")) {
> action = REVERT;
> - insn_len = strlen("revert");
> - p = start + insn_len + 1;
> + p += strlen("revert ");
> } else
> return NULL;
>
> - q = strchr(p, ' ');
> - if (!q || q - p + 1 > sizeof(sha1_abbrev))
> + q = p + strcspn(p, " \n");
> + if (q - p + 1 > sizeof(sha1_abbrev))
> return NULL;
> - q++;
> -
> - strlcpy(sha1_abbrev, p, q - p);
> + memcpy(sha1_abbrev, p, q - p);
> + sha1_abbrev[q - p] = '\0';
Since this is a part of clean-up series...
Do you even need to have a sha1_abbrev[] local array that is limited to 40
bytes here? The incoming _line_ is not "const char *start", so you should
at least be able to temporarily terminate the commit object name with NUL
(while remembering what byte there was before), give it to get_sha1(), and
then restore the byte at the end before returning from this function.
A bonus point would be to introduce a variant of get_sha1() that can take
(a pointer + len) not (a pointer to NUL terminated string). While I think
that would be a right thing to do in the longer term, it is outside of the
scope of this series.
^ permalink raw reply
* Re: [PATCH] commit: teach --gpg-sign option
From: Junio C Hamano @ 2011-10-10 16:45 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4E92919F.2030007@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> What norm? --amend keeps some header fields and discards others. In
> fact, signing a commit "without changing it" (i.e. keeping tree, parents
> etc., alias "--amend -C HEAD") should be the normal use case for signing
> the tip of an existing branch. I mean, I have no problems adding to this:
>
> git help fixup
> `git fixup' is aliased to `commit --amend -C HEAD'
You are *additionally* saying "-C HEAD" in an non-standard alias. Isn't
that enough indication that a vanila "--amend" is intended to record the
commit based on the updated context in which the new commit is made?
E.g. the authorship of the patch is still the same but committer
information is updated.
> But what is the best default for the workflows that we encourage (commit
> early, ...)? You answer a pull-request which happens to be a
> fast-forward, sign the tip and suddenly you've taken over ownership (and
> changed dates)??? Signing a commit should not do this.
I personally think a pull that is made in response to a pull-request,
i.e. the upstream merging from lieutenant, especially when the
authenticity of the puller matters, is perfectly fine with --no-ff.
Unlike the sign-less "we together made these history and nobody really
owns the result" (aka "Linus hates --no-ff merge because people do that to
leave a mark by peeing in the snow, without adding anything of value in
the history"), the whole purpose of signing a commit in the scenario you
mentioned is for the puller to leave his mark in the history.
^ permalink raw reply
* Re: [PATCH] commit: teach --gpg-sign option
From: Junio C Hamano @ 2011-10-10 16:35 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4E9291B6.2090201@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Junio C Hamano venit, vidit, dixit 09.10.2011 23:22:
>> Michael J Gruber <git@drmicha.warpmail.net> writes:
>>
>>> I just noticed that this format differs from the one of signed
>>> tags. What special reason is there for the "sig " indentation?
>>
>> Read the part of the message you are quoting.
>
> I certainly did, and certainly did not find any mention. Do you think I
> would have asked otherwise? I'm trying to be helpful by testing out a
> patch in flight. That is: *I* am trying to be helpful.
Surely, sorry and thanks. I should have pointed out where I _thought_ I
spelled out the reason but apparently in an ineffective way (as my wording
did not convey what I wanted to say at least to you) a bit more clearly.
> This
>> The lines of GPG detached signature are placed in new header lines, after
>> the standard tree/parent/author/committer headers, instead of tucking the
>> signature block at the end of the commit log message text (similar to how
>> signed tag is done), for multiple reasons:
> gave me the impression that commit signatures are done "similar to how
> signed tag is done".
Yeah, sorry if that sentence parses badly.
It was trying to say:
- the sig is in header lines
- which is different (instead of) from tucking it at the end of text
- by the way that "tucking at the end" would have been similar to
signed tag
The reasons why signatures in the header is better than tucking at the end
are enumerated in the part you did not quote in this message but in the
part you did quote in the previous message.
^ permalink raw reply
* Re: Git bug. gitattributes' pattern does not respect spaces in the filenames
From: Alexey Shumkin @ 2011-10-10 15:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vipnwooh1.fsf@alter.siamese.dyndns.org>
> Alexey Shumkin <Alex.Crezoff@gmail.com> writes:
>
> > As far as cp1251 and UTF-8 files are in different folders,
> > it is logically enough to set pattern like
> >
> > <folder with a UTF-8-xmls>/*.xml diff=utf8-to-cp1251
> >
> > for the UTF-8 files.
>
> ... IN the directory that needs conversion and not in the other one
> or at the toplevel. Problem solved, no?
Oh! yes! solved! thanks!
I did not take into account that each folder can have
its own .gitattributes-file
>
> Another idea may be to use "?" in the directory part of the
> pattern. Unless the directory structure is sick enough to have these
> directory names:
>
> dir-1/utf8-file.xml
> dir 1/cp1251-file.xml
>
> dir?1/*.xml would match the matter, so...
hmm... I like more the case above :)
but TMTOWTDI principle rulez
thanks again
^ 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