* Re: git log -z doesn't separate commits with NULs
From: Nikolaj Shurkaev @ 2012-02-23 12:17 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20120223102426.GB2912@sigill.intra.peff.net>
Hello.
Thank you very much for your tips. They really helped me. I was trying
to create patches that would affect only some given files or folders. By
this moment I have the following:
GeneratePatches.sh
---------------------
#!/bin/bash
#parameter 1 - <since>..<to>
#parameter 2 - path to file
git log -z --reverse --format=email --patch "$1" -- "$2" | xargs --null
--max-args=1 ./CreatePatchFile.sh
---------------------
and CreatePatchFile.sh
---------------------
#!/bin/bash
myPatchNumber=$(ls ./*-patch.patch 2>/dev/null | wc -l)
let "myPatchNumber += 1"
patchFile="./"$(printf "%04d" $myPatchNumber)"-patch.patch"
echo "$@" > "$patchFile"
---------------------
I call
./GeneratePatches.sh HEAD~3..HEAD SomePath
and that produces something very similar to what I want.
Perhaps there is a better way to do that.
Thank you once again.
---
Best regards,
Nikolaj
23.02.2012 13:24, Jeff King пишет:
> On Thu, Feb 23, 2012 at 12:14:23PM +0300, Nikolaj Shurkaev wrote:
>
>> I wanted to generate several files with some statistics using "git
>> log -z" command.
>> I did something like this:
>> git log -z --patch HEAD~10..HEAD -- SomePathHere | xargs -0
>> --max-chars=1000000 ~/1.sh
> I'm not sure what "1.sh" is expecting to take as input, but that will
> feed entire commits, including their commit message and entire diff, to
> the script on its command line.
>
> That seems like an awkward interface, but we don't really know what your
> script intends to do. Maybe it is worth sharing the contents of the
> script.
>
>> If I put echo "started" into the file ~/1.sh I see that the file is
>> called only once instead of multiple times.
> Yes. The point of xargs is usually to cram as many arguments into each
> invocation of "1.sh" as possible, splitting into multiple invocations
> only when we hit the argument-list memory limit that the OS imposes.
>
> If you want xargs to give each argument its own invocation of the
> script, use "xargs -n1".
>
>> I'm newbie to xargs, thus I tested with and that worked as I expected.
>> find . -type f -print0 | xargs -0 ./1.sh
>> That produced a lost of "started" lines.
> If you instrument your 1.sh more[1], you will find that is not executing
> once per file, but rather getting a large chunk of files per invocation.
>
> [1] Try adding: echo "got args: $*"
>
>> Thus I suspect there is a but in git log -z command and that doesn't
>> "Separate the commits with NULs instead of with new newlines." as
>> promised in the documents.
> You could verify that assertion by looking at the output. Try piping
> your "git log" command through "cat -A | less". When I try it, I see a
> NUL between each commit (cat -A will show it as "^@").
>
> -Peff
>
^ permalink raw reply
* Re: git log -z doesn't separate commits with NULs
From: Nikolaj Shurkaev @ 2012-02-23 12:19 UTC (permalink / raw)
To: Andreas Schwab; +Cc: git
In-Reply-To: <m262exrg65.fsf@igel.home>
Thank you.
That really helped me.
---
Nikolaj
23.02.2012 13:35, Andreas Schwab пишет:
> Nikolaj Shurkaev<snnicky@gmail.com> writes:
>
>> I did something like this:
>> git log -z --patch HEAD~10..HEAD -- SomePathHere | xargs -0
>> --max-chars=1000000 ~/1.sh
>>
>> If I put
>> echo "started"
>> into the file ~/1.sh I see that the file is called only once instead of
>> multiple times.
> If you want the command to be called once for each commit you need to
> pass --max-args=1 to xargs. Otherwise xargs will cumulate the arguments
> until --max-chars is reached, and the command is expected to loop over
> them.
>
> Andreas.
>
^ permalink raw reply
* Re: [PATCH] Improve http proxy support
From: Nelson Benítez León @ 2012-02-23 12:20 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqd397x8fc.fsf@bauges.imag.fr>
El día 22 de febrero de 2012 15:13, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> escribió:
> Nelson Benítez León <nbenitezl@gmail.com> writes:
>
>> Hi, my initial motivation for this patch was to add NTLM proxy
>> authentication [...]
>
> That sounds interesting, but please read Documentation/SubmittingPatches
> in Git's tree. The formatting of your email is wrong (giving more work
> for your maintainer) and you need to sign-off your patch to allow your
> code to be legally included.
Thank you for the advice, I read README file (couldn't find a HACKING
one) and the git website, and neither of those had a reference to
SubmittingPatches..
^ permalink raw reply
* Re: [PATCH] merge: use editor by default in interactive sessions
From: Erik Faye-Lund @ 2012-02-23 12:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vipk26p1b.fsf@alter.siamese.dyndns.org>
On Mon, Jan 23, 2012 at 11:18 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Traditionally, a cleanly resolved merge was committed by "git merge" using
> the auto-generated merge commit log message with invoking the editor.
>
> After 5 years of use in the field, it turns out that people perform too
> many unjustified merges of the upstream history into their topic branches.
> These merges are not just useless, but they are often not explained well,
> and making the end result unreadable when it gets time for merging their
> history back to their upstream.
>
> Earlier we added the "--edit" option to the command, so that people can
> edit the log message to explain and justify their merge commits. Let's
> take it one step further and spawn the editor by default when we are in an
> interactive session (i.e. the standard input and the standard output are
> pointing at the same tty device).
>
> There may be existing scripts that leave the standard input and the
> standard output of the "git merge" connected to whatever environment the
> scripts were started, and such invocation might trigger the above
> "interactive session" heuristics. GIT_MERGE_AUTOEDIT environment variable
> can be set to "no" at the beginning of such scripts to use the historical
> behaviour while the script runs.
>
> Note that this backward compatibility is meant only for scripts, and we
> deliberately do *not* support "merge.edit = yes/no/auto" configuration
> option to allow people to keep the historical behaviour.
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
> * With an update to the documentation, so that I won't forget, even
> though this topic came too late in the cycle and not meant for the
> upcoming 1.7.9.
>
> Documentation/git-merge.txt | 2 +-
> Documentation/merge-options.txt | 16 +++++++++++++---
> builtin/merge.c | 38 ++++++++++++++++++++++++++++++++++----
> t/test-lib.sh | 3 ++-
> 4 files changed, 50 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index e2e6aba..3ceefb8 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -9,7 +9,7 @@ git-merge - Join two or more development histories together
> SYNOPSIS
> --------
> [verse]
> -'git merge' [-n] [--stat] [--no-commit] [--squash]
> +'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
> [-s <strategy>] [-X <strategy-option>]
> [--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
> 'git merge' <msg> HEAD <commit>...
> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
> index 6bd0b04..f2f1d0f 100644
> --- a/Documentation/merge-options.txt
> +++ b/Documentation/merge-options.txt
> @@ -8,10 +8,20 @@ failed and do not autocommit, to give the user a chance to
> inspect and further tweak the merge result before committing.
>
> --edit::
> --e::
> +--no-edit::
> + Invoke an editor before committing successful mechanical merge to
> + further edit the auto-generated merge message, so that the user
> + can explain and justify the merge. The `--no-edit` option can be
> + used to accept the auto-generated message (this is generally
> + discouraged). The `--edit` option is still useful if you are
> + giving a draft message with the `-m` option from the command line
> + and want to edit it in the editor.
> +
> - Invoke editor before committing successful merge to further
> - edit the default merge message.
> +Older scripts may depend on the historical behaviour of not allowing the
> +user to edit the merge log message. They will see an editor opened when
> +they run `git merge`. To make it easier to adjust such scripts to the
> +updated behaviour, the environment variable `GIT_MERGE_AUTOEDIT` can be
> +set to `no` at the beginning of them.
>
> --ff::
> --no-ff::
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 99f1429..0006175 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -46,7 +46,7 @@ static const char * const builtin_merge_usage[] = {
>
> static int show_diffstat = 1, shortlog_len, squash;
> static int option_commit = 1, allow_fast_forward = 1;
> -static int fast_forward_only, option_edit;
> +static int fast_forward_only, option_edit = -1;
> static int allow_trivial = 1, have_message;
> static struct strbuf merge_msg;
> static struct commit_list *remoteheads;
> @@ -189,7 +189,7 @@ static struct option builtin_merge_options[] = {
> "create a single commit instead of doing a merge"),
> OPT_BOOLEAN(0, "commit", &option_commit,
> "perform a commit if the merge succeeds (default)"),
> - OPT_BOOLEAN('e', "edit", &option_edit,
> + OPT_BOOL('e', "edit", &option_edit,
> "edit message before committing"),
> OPT_BOOLEAN(0, "ff", &allow_fast_forward,
> "allow fast-forward (default)"),
> @@ -877,12 +877,12 @@ static void prepare_to_commit(void)
> write_merge_msg(&msg);
> run_hook(get_index_file(), "prepare-commit-msg",
> git_path("MERGE_MSG"), "merge", NULL, NULL);
> - if (option_edit) {
> + if (0 < option_edit) {
> if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
> abort_commit(NULL);
> }
> read_merge_msg(&msg);
> - stripspace(&msg, option_edit);
> + stripspace(&msg, 0 < option_edit);
> if (!msg.len)
> abort_commit(_("Empty commit message."));
> strbuf_release(&merge_msg);
> @@ -1076,6 +1076,33 @@ static void write_merge_state(void)
> close(fd);
> }
>
> +static int default_edit_option(void)
> +{
> + static const char name[] = "GIT_MERGE_AUTOEDIT";
> + const char *e = getenv(name);
> + struct stat st_stdin, st_stdout;
> +
> + if (have_message)
> + /* an explicit -m msg without --[no-]edit */
> + return 0;
> +
> + if (e) {
> + int v = git_config_maybe_bool(name, e);
> + if (v < 0)
> + die("Bad value '%s' in environment '%s'", e, name);
> + return v;
> + }
> +
> + /* Use editor if stdin and stdout are the same and is a tty */
> + return (!fstat(0, &st_stdin) &&
> + !fstat(1, &st_stdout) &&
> + isatty(0) &&
> + st_stdin.st_dev == st_stdout.st_dev &&
> + st_stdin.st_ino == st_stdout.st_ino &&
> + st_stdin.st_mode == st_stdout.st_mode);
I just stumbled over this code, and I got a bit worried; the
stat-implementation we use on Windows sets st_ino to 0, so
"st_stdin.st_ino == st_stdout.st_ino" will always evaluate to true.
Perhaps we should add a check for isatty(1) here as well? There's max
one console per process on Windows (AllocConsole fails if one has
already been create, see
http://msdn.microsoft.com/en-us/library/windows/desktop/ms681944(v=vs.85).aspx
for details), so I think if both stdin and stout are consoles, we
should be good.
Or is there something I'm missing here?
---
diff --git a/builtin/merge.c b/builtin/merge.c
index ed0f959..bef01e3 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1130,6 +1130,7 @@ static int default_edit_option(void)
return (!fstat(0, &st_stdin) &&
!fstat(1, &st_stdout) &&
isatty(0) &&
+ isatty(1) &&
st_stdin.st_dev == st_stdout.st_dev &&
st_stdin.st_ino == st_stdout.st_ino &&
st_stdin.st_mode == st_stdout.st_mode);
^ permalink raw reply related
* [PATCH] README: point to Documentation/SubmittingPatches
From: Matthieu Moy @ 2012-02-23 12:52 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <CAAUd643RHOrhm+gfW5UeXfjcG0Xr+q0nxzAVqYsh8VxhhP_m1g@mail.gmail.com>
It was indeed not obvious for new contributors to find this document in
the source tree, since there were no reference to it outside the
Documentation/ directory.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
> Thank you for the advice, I read README file (couldn't find a HACKING
> one) and the git website, and neither of those had a reference to
> SubmittingPatches..
Indeed. As a bonnus, here's a submission that should match the
guidelines (above --- is the commit message, and here is the place for
free comments).
README | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/README b/README
index 67cfeb2..d2690ec 100644
--- a/README
+++ b/README
@@ -42,10 +42,12 @@ including full documentation and Git related tools.
The user discussion and development of Git take place on the Git
mailing list -- everyone is welcome to post bug reports, feature
-requests, comments and patches to git@vger.kernel.org. To subscribe
-to the list, send an email with just "subscribe git" in the body to
-majordomo@vger.kernel.org. The mailing list archives are available at
-http://marc.theaimsgroup.com/?l=git and other archival sites.
+requests, comments and patches to git@vger.kernel.org (read
+Documentation/SubmittingPatches for instructions on patch submission).
+To subscribe to the list, send an email with just "subscribe git" in
+the body to majordomo@vger.kernel.org. The mailing list archives are
+available at http://marc.theaimsgroup.com/?l=git and other archival
+sites.
The messages titled "A note from the maintainer", "What's in
git.git (stable)" and "What's cooking in git.git (topics)" and
--
1.7.9.111.gf3fb0.dirty
^ permalink raw reply related
* [PATCH] pretty: add '*' modifier to add LF after non-empty
From: Luc Pionchon @ 2012-02-23 13:10 UTC (permalink / raw)
To: git; +Cc: gitster, Luc Pionchon
Add the '*' modifier, similar to the '+' modifier,
to add a line-feed after a non-empty placeholder.
Allow to print a head-line which content can be empty.
For example for decorations:
$ git log --graph --pretty=format:"%C(green)%*d %C(reset)%s"
^^^
* (HEAD, origin/master, origin/HEAD, master)
| Update draft release notes to 1.7.10
* Merge branch 'jc/maint-request-pull-for-tag'
|\
| * (origin/jc/maint-request-pull-for-tag)
| | request-pull: explicitly ask tags/$name to be pulled
* | Merge branch 'bl/gitweb-project-filter'
|\ \
| * | (origin/bl/gitweb-project-filter)
| | | gitweb: Make project search respect project_filter
Signed-off-by: Luc Pionchon <pionchon.luc@gmail.com>
---
Documentation/pretty-formats.txt | 4 ++++
pretty.c | 6 ++++++
t/t6006-rev-list-format.sh | 10 ++++++++++
3 files changed, 20 insertions(+), 0 deletions(-)
Hi,
I now started to use git. When formatting my 'log' output, I have been looking for a modifier to add a line feed after a (potentially empty) placeholder. Git allows to add a line feed before, but not after a placeholder. This is a small patch that adds the feature. I hope it is useful to others.
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 880b6f2..9114d49 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -159,6 +159,10 @@ If you add a `{plus}` (plus sign) after '%' of a placeholder, a line-feed
is inserted immediately before the expansion if and only if the
placeholder expands to a non-empty string.
+If you add a `*` (asterisk sign) after '%' of a placeholder, a line-feed
+is inserted immediately after the expansion if and only if the
+placeholder expands to a non-empty string.
+
If you add a `-` (minus sign) after '%' of a placeholder, line-feeds that
immediately precede the expansion are deleted if and only if the
placeholder expands to an empty string.
diff --git a/pretty.c b/pretty.c
index 8688b8f..5ebaf88 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1132,6 +1132,7 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
NO_MAGIC,
ADD_LF_BEFORE_NON_EMPTY,
DEL_LF_BEFORE_EMPTY,
+ ADD_LF_AFTER_NON_EMPTY,
ADD_SP_BEFORE_NON_EMPTY
} magic = NO_MAGIC;
@@ -1142,6 +1143,9 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
case '+':
magic = ADD_LF_BEFORE_NON_EMPTY;
break;
+ case '*':
+ magic = ADD_LF_AFTER_NON_EMPTY;
+ break;
case ' ':
magic = ADD_SP_BEFORE_NON_EMPTY;
break;
@@ -1162,6 +1166,8 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
} else if (orig_len != sb->len) {
if (magic == ADD_LF_BEFORE_NON_EMPTY)
strbuf_insert(sb, orig_len, "\n", 1);
+ else if (magic == ADD_LF_AFTER_NON_EMPTY)
+ strbuf_addstr(sb, "\n");
else if (magic == ADD_SP_BEFORE_NON_EMPTY)
strbuf_insert(sb, orig_len, " ", 1);
}
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 4442790..c692061 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -208,6 +208,16 @@ test_expect_success 'add LF before non-empty (2)' '
grep "^$" actual
'
+test_expect_success 'add LF after non-empty (1) (empty)' '
+ git show -s --pretty=format:"%*d%s%nfoo%n" HEAD^^ >actual &&
+ test $(wc -l <actual) = 2
+'
+
+test_expect_success 'add LF after non-empty (2) (non empty)' '
+ git show -s --pretty=format:"%*d%s%nfoo%n" HEAD >actual &&
+ test $(wc -l <actual) = 3
+'
+
test_expect_success 'add SP before non-empty (1)' '
git show -s --pretty=format:"%s% bThanks" HEAD^^ >actual &&
test $(wc -w <actual) = 2
--
1.7.4.1
^ permalink raw reply related
* Re: git log -z doesn't separate commits with NULs
From: Jakub Narebski @ 2012-02-23 13:15 UTC (permalink / raw)
To: Nikolaj Shurkaev; +Cc: Jeff King, git
In-Reply-To: <4F462E61.4020203@gmail.com>
Nikolaj Shurkaev <snnicky@gmail.com> writes:
> Thank you very much for your tips. They really helped me. I was trying
> to create patches that would affect only some given files or
> folders. By this moment I have the following:
>
> GeneratePatches.sh
> ---------------------
> #!/bin/bash
> #parameter 1 - <since>..<to>
> #parameter 2 - path to file
> git log -z --reverse --format=email --patch "$1" -- "$2" | xargs
> --null --max-args=1 ./CreatePatchFile.sh
> ---------------------
>
> and CreatePatchFile.sh
> ---------------------
> #!/bin/bash
>
> myPatchNumber=$(ls ./*-patch.patch 2>/dev/null | wc -l)
> let "myPatchNumber += 1"
>
> patchFile="./"$(printf "%04d" $myPatchNumber)"-patch.patch"
> echo "$@" > "$patchFile"
> ---------------------
>
> I call
> ./GeneratePatches.sh HEAD~3..HEAD SomePath
> and that produces something very similar to what I want.
>
> Perhaps there is a better way to do that.
So what git-format-patch is lacking?
--
Jakub Narebski
^ permalink raw reply
* Re: Problems with unrecognized headers in git bundles
From: Erik Faye-Lund @ 2012-02-23 13:27 UTC (permalink / raw)
To: Øyvind A. Holm; +Cc: Jannis Pohlmann, git
In-Reply-To: <CAA787rm4c1zYgQJ3kP5=ujpEK1Dda9+h_P3BBmg2yX2eZca=TA@mail.gmail.com>
On Wed, Feb 22, 2012 at 9:25 PM, Øyvind A. Holm <sunny@sunbase.org> wrote:
> On 22 February 2012 17:05, Jannis Pohlmann wrote:
>> Hi,
>>
>> creating bundles from some repositories seems to lead to bundles with
>> incorrectly formatted headers, at least with git >= 1.7.2. When
>> cloning from such bundles, git prints the following error/warning:
>>
>> $ git clone perl-clone.bundle perl-clone
>> Cloning into 'perl-clone'...
>> warning: unrecognized header: --work around mangled archname on...
>>
>> This can be reproduced easily with git from any version >= 1.7.2 or
>> from master, using the following steps:
>>
>> git clone git://perl5.git.perl.org/perl.git perl
>> GIT_DIR=perl/.git git bundle create perl-clone.bundle --all
>> git clone perl-clone.bundle perl-clone
>>
>> The content of the bundle is:
>>
>> # v2 git bundle
>> -- work around mangled archname on win32 while finding...
>> 39ec54a59ce332fc44e553f4e5eeceef88e8369e refs/heads/blead
>> 39ec54a59ce332fc44e553f4e5eeceef88e8369e refs/remotes/origin/HEAD
>
> Have researched this a bit, and I've found that all git versions back to
> when git-bundle was introduced (around v1.5.4) produces the same invalid
> line. The culprit is commit 3e8148feadabd0d0b1869fcc4d218a6475a5b0bc in
> perl.git, branch 'maint-5.005'. The log message of that commit contains
> email headers, maybe that's the reason git bundle gets confused?
For the lazy, the commit can be found here:
http://perl5.git.perl.org/perl.git/commit/3e8148feadabd0d0b1869fcc4d218a6475a5b0bc
^ permalink raw reply
* Re: git log -z doesn't separate commits with NULs
From: Nikolaj Shurkaev @ 2012-02-23 13:48 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Jeff King, git
In-Reply-To: <m34nuhelnf.fsf@localhost.localdomain>
For example there are commits that affect not only files in folder A but
files in folder B, C and so on.
If I do format-patch that will give me nice patches, but there are
modifications of folders B, C and so on there.
I do not know a way to generate patches via format-patch that affect
only files in folder A.
This is why I wrote those scripts.
23.02.2012 16:15, Jakub Narebski пишет:
> Nikolaj Shurkaev<snnicky@gmail.com> writes:
>
>> Thank you very much for your tips. They really helped me. I was trying
>> to create patches that would affect only some given files or
>> folders. By this moment I have the following:
>>
>> GeneratePatches.sh
>> ---------------------
>> #!/bin/bash
>> #parameter 1 -<since>..<to>
>> #parameter 2 - path to file
>> git log -z --reverse --format=email --patch "$1" -- "$2" | xargs
>> --null --max-args=1 ./CreatePatchFile.sh
>> ---------------------
>>
>> and CreatePatchFile.sh
>> ---------------------
>> #!/bin/bash
>>
>> myPatchNumber=$(ls ./*-patch.patch 2>/dev/null | wc -l)
>> let "myPatchNumber += 1"
>>
>> patchFile="./"$(printf "%04d" $myPatchNumber)"-patch.patch"
>> echo "$@"> "$patchFile"
>> ---------------------
>>
>> I call
>> ./GeneratePatches.sh HEAD~3..HEAD SomePath
>> and that produces something very similar to what I want.
>>
>> Perhaps there is a better way to do that.
> So what git-format-patch is lacking?
>
^ permalink raw reply
* Re: gitk: set uicolor SystemButtonFace error on X11 if .gitk created using Win32 tk
From: Erik Faye-Lund @ 2012-02-23 14:03 UTC (permalink / raw)
To: Matt Seitz (matseitz); +Cc: git
In-Reply-To: <70952A932255A2489522275A628B97C31288FA0B@xmb-sjc-233.amer.cisco.com>
On Wed, Feb 22, 2012 at 11:13 PM, Matt Seitz (matseitz)
<matseitz@cisco.com> wrote:
> Would you please change gitk to not hard-code Win32-specific color
> values when creating .gitk on a Win32 windowing system?
>
> Gitk stopped working for me on Cygwin when Cygwin changed from using a
> Win32 native version of tk to using the standard X11 version. The error
> was because gitk had previously created a .gitk file using Win32
> specific color values:
>
> https://github.com/gitster/git/commit/1924d1bc0dc99cd3460d3551671908cc76
> c09d3b
>
> I was able to work around the problem by replacing the Win32 specific
> colors in my .gitk file with the default colors gitk uses on other
> windowing systems.
>
Are you saying that the tk shipped with recent Cygwin reports "win32"
for "[tk windowingsystem]", but does not recognize the
"SystemButtonFace" etc values?
If so, perhaps adding a check for one of the constant values might
make sense. But I'd expect "[tk windowingsystem]" to report "x11" if
it's simply a Cygwin build of the X11-version as you say...
Anyway, this does sound like a regression in Cygwin; we're probably
not the only ones to use "SystemButtonFace" etc after consulting "[tk
windowingsystem]"...
Hm, no wait, the documentation seems to suggest ditching the
System-prefix to these constants:
http://www.tcl.tk/man/tcl/TkCmd/colors.htm
Does this patch help?
---
diff --git a/gitk-git/gitk b/gitk-git/gitk
index f9e936d..7e9a01f 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -11559,10 +11559,10 @@ if {[tk windowingsystem] eq "aqua"} {
set colors {green red blue magenta darkgrey brown orange}
if {[tk windowingsystem] eq "win32"} {
- set uicolor SystemButtonFace
- set bgcolor SystemWindow
- set fgcolor SystemButtonText
- set selectbgcolor SystemHighlight
+ set uicolor ButtonFace
+ set bgcolor Window
+ set fgcolor ButtonText
+ set selectbgcolor Highlight
} else {
set uicolor grey85
set bgcolor white
^ permalink raw reply related
* [PATCHv3 0/3] gitweb: Faster project search
From: Jakub Narebski @ 2012-02-23 15:42 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
[Cc-ing Junio because of his involvement in discussion]
These patches are separated from first part of previous version of
this series
"[PATCHv2 0/8] gitweb: Faster and improved project search"
http://thread.gmane.org/gmane.comp.version-control.git/190852
It is meant to replace 'jn/gitweb-search-optim' in pu
The intent of this series is to make project search faster by reducing
number of git commands (and I/O). This is done by first searching
projects, then filling all project info, rather than the reverse.
Jakub Narebski (3):
gitweb: Refactor checking if part of project info need filling
gitweb: Option for filling only specified info in
fill_project_list_info
gitweb: Faster project search
gitweb/gitweb.perl | 66 +++++++++++++++++++++++++++++++++++++++++----------
1 files changed, 53 insertions(+), 13 deletions(-)
--
1.7.9
^ permalink raw reply
* [PATCHv3 0/3] gitweb: Faster project search
From: Jakub Narebski @ 2012-02-23 15:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
[Cc-ing Junio because of his involvement in discussion]
These patches are separated from first part of previous version of
this series
"[PATCHv2 0/8] gitweb: Faster and improved project search"
http://thread.gmane.org/gmane.comp.version-control.git/190852
It is meant to replace 'jn/gitweb-search-optim' in pu
The intent of this series is to make project search faster by reducing
number of git commands (and I/O). This is done by first searching
projects, then filling all project info, rather than the reverse.
Jakub Narebski (3):
gitweb: Refactor checking if part of project info need filling
gitweb: Option for filling only specified info in
fill_project_list_info
gitweb: Faster project search
gitweb/gitweb.perl | 66 +++++++++++++++++++++++++++++++++++++++++----------
1 files changed, 53 insertions(+), 13 deletions(-)
--
1.7.9
^ permalink raw reply
* [PATCHv3 1/3] gitweb: Refactor checking if part of project info need filling
From: Jakub Narebski @ 2012-02-23 15:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <1330012488-7970-1-git-send-email-jnareb@gmail.com>
Extract the check if given keys (given parts) of project info needs to
be filled into project_info_needs_filling() subroutine. It is for now
a thin wrapper around "!exists $project_info->{$key}".
Note that !defined was replaced by more correct !exists.
While at it uniquify treating of all project info, adding checks for
'age' field before running git_get_last_activity(), and also checking
for all keys filled in code protected by conditional, and not only
one.
The code now looks like this
foreach my $project (@$project_list) {
if (given keys need to be filled) {
fill given keys
}
...
}
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch appeared for first time in v2 of this series, and split off
introduction of project_info_needs_filling() function from partial
filling via @wanted_keys... which in this version of series is not
done in project_info_needs_filling() anyway.
Almost no changes from v2:
* Remove duplication of empty line between project_info_needs_filling()
and fill_project_list_info()
gitweb/gitweb.perl | 34 ++++++++++++++++++++++++++--------
1 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 057ba5b..cdd84c7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5195,6 +5195,20 @@ sub git_project_search_form {
print "</div>\n";
}
+# entry for given @keys needs filling if at least one of keys in list
+# is not present in %$project_info
+sub project_info_needs_filling {
+ my ($project_info, @keys) = @_;
+
+ # return List::MoreUtils::any { !exists $project_info->{$_} } @keys;
+ foreach my $key (@keys) {
+ if (!exists $project_info->{$key}) {
+ return 1;
+ }
+ }
+ return;
+}
+
# fills project list info (age, description, owner, category, forks)
# for each project in the list, removing invalid projects from
# returned list
@@ -5206,24 +5220,28 @@ sub fill_project_list_info {
my $show_ctags = gitweb_check_feature('ctags');
PROJECT:
foreach my $pr (@$projlist) {
- my (@activity) = git_get_last_activity($pr->{'path'});
- unless (@activity) {
- next PROJECT;
+ if (project_info_needs_filling($pr, 'age', 'age_string')) {
+ my (@activity) = git_get_last_activity($pr->{'path'});
+ unless (@activity) {
+ next PROJECT;
+ }
+ ($pr->{'age'}, $pr->{'age_string'}) = @activity;
}
- ($pr->{'age'}, $pr->{'age_string'}) = @activity;
- if (!defined $pr->{'descr'}) {
+ if (project_info_needs_filling($pr, 'descr', 'descr_long')) {
my $descr = git_get_project_description($pr->{'path'}) || "";
$descr = to_utf8($descr);
$pr->{'descr_long'} = $descr;
$pr->{'descr'} = chop_str($descr, $projects_list_description_width, 5);
}
- if (!defined $pr->{'owner'}) {
+ if (project_info_needs_filling($pr, 'owner')) {
$pr->{'owner'} = git_get_project_owner("$pr->{'path'}") || "";
}
- if ($show_ctags) {
+ if ($show_ctags &&
+ project_info_needs_filling($pr, 'ctags')) {
$pr->{'ctags'} = git_get_project_ctags($pr->{'path'});
}
- if ($projects_list_group_categories && !defined $pr->{'category'}) {
+ if ($projects_list_group_categories &&
+ project_info_needs_filling($pr, 'category')) {
my $cat = git_get_project_category($pr->{'path'}) ||
$project_list_default_category;
$pr->{'category'} = to_utf8($cat);
--
1.7.9
^ permalink raw reply related
* [PATCHv3 2/3] gitweb: Option for filling only specified info in fill_project_list_info
From: Jakub Narebski @ 2012-02-23 15:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <1330012488-7970-1-git-send-email-jnareb@gmail.com>
Enhance fill_project_list_info() subroutine to accept optional
parameters that specify which fields in project information needs to
be filled. If none are specified then fill_project_list_info()
behaves as it used to, and ensure that all project info is filled.
This is in preparation of future lazy filling of project info in
project search and pagination of sorted list of projects.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This could have been squashed with the next commit, but this way it is
pure refactoring that shouldn't change gitweb behavior, and hopefully
makes this patch easier to review.
Changes from v2:
* Instead of overloading project_info_needs_filling() to do both
handling of which fields need to be filled, and which fields
we want to have filled, just make its caller filter its arguments.
This means that in this version of patch there are no changes to
project_info_needs_filling() subroutine.
# Instead of handling both cases (the one with set of wanted keys,
and the one where we want to fill all remaining info) in a single
filtering subroutine, use two different anonymous subroutines
to filter or not filter keys.
* Expanded comment about fill_project_list_info to include detailed
description of when it does remove projects from returned list (and
why it is), and examples for both types of usage of this subroutine.
* Rewritten major part of commit message, which in v2 was not updated
to reflect new development, and referred to behavior in v1.
[Should I include changes from v1 here, in the future?]
On Mon, 20 Feb 2012, Junio C Hamano wrote
in http://thread.gmane.org/gmane.comp.version-control.git/190852/focus=191046
JH>
JH> I must say that the approach to put the set filtering logic inside
JH> project_info_needs_filling function smells like a bad API design.
JH>
JH> In other words, wouldn't a callchain that uses a more natural set of API
JH> functions be like this?
JH>
JH> # the top-level caller that is interested only in these two fields
JH> fill_project_list_info($projlist, 'age', 'owner');
JH>
JH> # in fill_project_list_info()
JH> my $projlist = shift;
JH> my %wanted = map { $_ => 1 } @_;
JH>
JH> foreach my $pr (@$projlist) {
JH> if (project_info_needs_filling($pr,
JH> filter_set(\%wanted, 'age', 'age_string'))) {
I think this might be an even better solution, instead of handling all
keys / selected keys in filter_set(), provide different $filter_set
subroutines:
foreach my $pr (@$projlist) {
if (project_info_needs_filling($pr,
$filter_set->('age', 'age_string'))) {
gitweb/gitweb.perl | 33 +++++++++++++++++++++++++--------
1 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index cdd84c7..7fb7a55 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5209,39 +5209,56 @@ sub project_info_needs_filling {
return;
}
-# fills project list info (age, description, owner, category, forks)
+# fills project list info (age, description, owner, category, forks, etc.)
# for each project in the list, removing invalid projects from
-# returned list
+# returned list, or fill only specified info.
+#
+# Invalid projects are removed from the returned list if and only if you
+# ask 'age' or 'age_string' to be filled, because they are the only fields
+# that run unconditionally git command that requires repository, and
+# therefore do always check if project repository is invalid.
+#
+# USAGE:
+# * fill_project_list_info(\@project_list, 'descr_long', 'ctags')
+# ensures that 'descr_long' and 'ctags' fields are filled
+# * @project_list = fill_project_list_info(\@project_list)
+# ensures that all fields are filled (and invalid projects removed)
+#
# NOTE: modifies $projlist, but does not remove entries from it
sub fill_project_list_info {
- my $projlist = shift;
+ my ($projlist, @wanted_keys) = @_;
my @projects;
+ my $filter_set = sub { return @_; };
+ if (@wanted_keys) {
+ my %wanted_keys = map { $_ => 1 } @wanted_keys;
+ $filter_set = sub { return grep { $wanted_keys{$_} } @_; };
+ }
my $show_ctags = gitweb_check_feature('ctags');
PROJECT:
foreach my $pr (@$projlist) {
- if (project_info_needs_filling($pr, 'age', 'age_string')) {
+ if (project_info_needs_filling($pr, $filter_set->('age', 'age_string'))) {
my (@activity) = git_get_last_activity($pr->{'path'});
unless (@activity) {
next PROJECT;
}
($pr->{'age'}, $pr->{'age_string'}) = @activity;
}
- if (project_info_needs_filling($pr, 'descr', 'descr_long')) {
+ if (project_info_needs_filling($pr, $filter_set->('descr', 'descr_long'))) {
my $descr = git_get_project_description($pr->{'path'}) || "";
$descr = to_utf8($descr);
$pr->{'descr_long'} = $descr;
$pr->{'descr'} = chop_str($descr, $projects_list_description_width, 5);
}
- if (project_info_needs_filling($pr, 'owner')) {
+ if (project_info_needs_filling($pr, $filter_set->('owner'))) {
$pr->{'owner'} = git_get_project_owner("$pr->{'path'}") || "";
}
if ($show_ctags &&
- project_info_needs_filling($pr, 'ctags')) {
+ project_info_needs_filling($pr, $filter_set->('ctags'))) {
$pr->{'ctags'} = git_get_project_ctags($pr->{'path'});
}
if ($projects_list_group_categories &&
- project_info_needs_filling($pr, 'category')) {
+ project_info_needs_filling($pr, $filter_set->('category'))) {
my $cat = git_get_project_category($pr->{'path'}) ||
$project_list_default_category;
$pr->{'category'} = to_utf8($cat);
--
1.7.9
^ permalink raw reply related
* [PATCHv3 3/3] gitweb: Faster project search
From: Jakub Narebski @ 2012-02-23 15:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <1330012488-7970-1-git-send-email-jnareb@gmail.com>
Before searching by some field the information we search for must be
filled in, but we do not have to fill other fields that are not
involved in the search.
To be able to request filling only specified fields,
fill_project_list_info() was enhanced in previous commit to take
additional parameters which specify part of projects info to fill.
This way we can limit doing expensive calculations (like running
git-for-each-ref to get 'age' / "Last changed" info) to doing those
only for projects which we will show as search results.
This commit actually uses this interface, changing gitweb code from
the following behavior
fill all project info on all projects
search projects
to behaving like this pseudocode
fill search fields on all projects
search projects
fill all project info on search results
With this commit the number of git commands used to generate search
results is 2*<matched projects> + 1, and depends on number of matched
projects rather than number of all projects (all repositories).
Note: this is 'git for-each-ref' to find last activity, and 'git config'
for each project, and 'git --version' once.
Example performance improvements, for search that selects 2
repositories out of 12 in total:
* Before (warm cache):
"This page took 0.867151 seconds and 27 git commands to generate."
* After (warm cache):
"This page took 0.673643 seconds and 5 git commands to generate."
Now imagine that they are 5 repositories out of 5000, and cold or
trashed cache case.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Changes from v2 (v2 is the same as v1):
* Rewritten and extended commit message (though extending perhaps have
gone too far...).
Added paragraphs are "This commit actually uses this interface..."
and "Example performance improvements..."
Junio C Hamano wrote in
http://thread.gmane.org/gmane.comp.version-control.git/190852/focus=191053
[...]
JC> "must be filled in." is correct, but that happens without the previous
JC> patch. The first sentence must also say:
JC>
JC> In order to search by some field, the information we look for must
JC> be filled in, but we do not have to fill other fields that are not
JC> involved in the search.
JC>
JC> to justify the previous "fill_project_list_info can be asked to return
JC> without getting unused fields" patch.
Done. Thanks for a suggestion.
JC> The rest of the log message then makes good sense.
Ooops...
gitweb/gitweb.perl | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7fb7a55..4ceb1a6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2987,6 +2987,10 @@ sub search_projects_list {
return @$projlist
unless ($tagfilter || $searchtext);
+ # searching projects require filling to be run before it;
+ fill_project_list_info($projlist,
+ $tagfilter ? 'ctags' : (),
+ $searchtext ? ('path', 'descr') : ());
my @projects;
PROJECT:
foreach my $pr (@$projlist) {
@@ -5394,12 +5398,13 @@ sub git_project_list_body {
# filtering out forks before filling info allows to do less work
@projects = filter_forks_from_projects_list(\@projects)
if ($check_forks);
- @projects = fill_project_list_info(\@projects);
- # searching projects require filling to be run before it
+ # search_projects_list pre-fills required info
@projects = search_projects_list(\@projects,
'searchtext' => $searchtext,
'tagfilter' => $tagfilter)
if ($tagfilter || $searchtext);
+ # fill the rest
+ @projects = fill_project_list_info(\@projects);
$order ||= $default_projects_order;
$from = 0 unless defined $from;
--
1.7.9
^ permalink raw reply related
* [RFC/PATCH] Make git-{pull,rebase} no-tracking message friendlier
From: Carlos Martín Nieto @ 2012-02-23 16:05 UTC (permalink / raw)
To: git
The current message is too long and at too low a level for anybody to
understand it if they don't know about the configuration format
already.
Reformat it to show the commands a user would be expected to use,
instead of the contents of the configuration file.
---
As annoying as it is when people simply paste the output of 'git pull'
and ask "what does it mean" without even reading it, I have to admit
that as a new user, I'd also be scared off by this message. Using
git-remote and git-branch should make it less scary and more relatable
for the user. I'm not aware of a way to set branch.$branch.rebase with
porcelain, so I put in a config command there. Better
solutions/wordings welcome; I'd really like to get rid of the old
message.
git-parse-remote.sh | 23 ++++++++---------------
1 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index b24119d..96e76a9 100644
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -65,26 +65,19 @@ Please specify which branch you want to $op_type $op_prep on the command
line and try again (e.g. '$example').
See git-${cmd}(1) for details."
else
- echo "You asked me to $cmd without telling me which branch you
-want to $op_type $op_prep, and 'branch.${branch_name#refs/heads/}.merge' in
-your configuration file does not tell me, either. Please
-specify which branch you want to use on the command line and
+ echo "You asked me to $cmd without telling me which branch you want to
+$op_type $op_prep, and there is no tracking information for the current branch.
+Please specify which branch you want to use on the command line and
try again (e.g. '$example').
See git-${cmd}(1) for details.
If you often $op_type $op_prep the same branch, you may want to
-use something like the following in your configuration file:
- [branch \"${branch_name#refs/heads/}\"]
- remote = <nickname>
- merge = <remote-ref>"
- test rebase = "$op_type" &&
- echo " rebase = true"
- echo "
- [remote \"<nickname>\"]
- url = <url>
- fetch = <refspec>
+run something like:
-See git-config(1) for details."
+ git remote add <remote> <url>
+ git branch --set-upstream ${branch_name#refs/heads/} <remote>/<remote-branch>"
+ test rebase = "$op_type" &&
+ echo " git config branch.${branch_name#refs/heads/}.rebase true"
fi
exit 1
}
--
1.7.8.352.g876a6f
^ permalink raw reply related
* Re: [PATCH v2] contrib: added git-diffall
From: Tim Henigan @ 2012-02-23 16:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, davvid, stefano.lattarini
In-Reply-To: <7vipiy8m5q.fsf@alter.siamese.dyndns.org>
On Wed, Feb 22, 2012 at 6:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Tim Henigan <tim.henigan@gmail.com> writes:
>
> We encourage our log messages to describe the problem first and then
> present solution to the problem, so I would update the above perhaps like
> this:
>
> The 'git difftool' command lets the user to use an external tool
> to view diffs, but it runs the tool for one file at the time. This
> makes it tedious to review a change that spans multiple files.
>
> The "git-diffall" script instead prepares temporary directories
> with preimage and postimage files, and launches a single instance
> of an external diff tool to view the differences in them.
> diff.tool or merge.tool configuration variable is used to specify
> what external tool is used.
Understood. I will update in v3.
> I am wondering if reusing "diff.tool" or "merge.tool" is a good idea,
> though.
>
> I guess that it is OK to assume that any external tool that can compare
> two directories MUST be able to compare two individual files, and if that
> is true, it is perfectly fine to reuse the configuration. But if an
> external tool "frobdiff" that can compare two directories cannot compare
> two individual files, it will make it impossible for the user to run "git
> difftool" if diff.tool is set to "frobdiff" to use with "diffall".
>
> Another thing that comes to my mind is if a user has an external tool that
> can use "diffall", is there ever a situation where the user chooses to use
> "difftool" instead, to go files one by one. I cannot offhand imagine any.
It was my assumption that any tool that supports directory diff also
supports individual file diff. It seems like a strict subset of
directory diff case.
> Perhaps a longer term integration plan may be to lift the logic from this
>
...snip...
>
> But that is all two steps in the future.
I hope that this feature finds it way into the existing core commands.
This script is intended to be a conversation starter that is also
immediately useful as a separate command. Would it be better to begin
the long-term discussion now and skip adding this to contrib?
>> +# mktemp is not available on all platforms (missing from msysgit)
>> +# Use a hard-coded tmp dir if it is not available
>> +tmp="$(mktemp -d -t tmp.XXXXXX 2>/dev/null)" || {
>> + tmp=/tmp/git-diffall-tmp
>> +}
>
> It would not withstand malicious attacks, but doing
>
> tmp=/tmp/git-diffall-tmp.$$
>
> would at least protect you from accidental name crashes better in the
> fallback codepath.
This makes sense. I will add a unique portion to the name of the tmp dir in v3.
>> +trap 'rm -rf "$tmp" 2>/dev/null' EXIT
>
> Do you need to suppress errors, especially when you are running "rm -rf"
> with the 'f' option?
On msysgit, I found that "rm -rf $tmp" consistently fails due with a
permission error. I don't understand why the script that created the
tmp dir is not allowed to delete it. I am still looking into it, but
so far it appears to be an idiosyncrasy of msysgit.
>> +left=
>> +right=
>> +paths=
>> +path_sep=
>> +compare_staged=
>> +common_ancestor=
>> +left_dir=
>> +right_dir=
>> +diff_tool=
>> +copy_back=
>
> You can write multiple assignment on a line to save vertical space if you
> want to, and the initialization sequence like this is a good place to do
> so.
My personal preference is to keep them on separate lines. However if
the compressed style is preferred, I will change it.
>> + -x|--e|--ex|--ext|--extc|--extcm|--extcmd)
>> + diff_tool=$2
>> + shift
>> + ;;
>
> What if your command line ends with -x without $2?
Currently it results in a shift error with no useful message to the
user. I will add something for this in v3.
> Don't you want to match "-t/--tool" that "difftool" already uses?
Are you suggesting that I a) change "-x/--extcmd" to "-t/--tool" or
that b) I add support for the "difftool -t/--tool" option?
If "a", I was reusing the "difftool --extcmd" option which has the
same behavior. If "b", I will look into it.
>> + # could be commit, commit range or path limiter
>> + case "$1" in
>> + *...*)
>> + left=${1%...*}
>> + right=${1#*...}
>> + common_ancestor=1
>> + ;;
>
> Strictly speaking, that is not just a common_ancestor but is a merge_base,
> which is a common ancestor none of whose children is a common ancestor.
Understood. I will change the name to "merge_base" in v3.
>> + *..*)
>> + left=${1%..*}
>> + right=${1#*..}
>> + ;;
>> + *)
>> + if test -n "$path_sep"
>> + then
>> + paths="$paths$1 "
>> + elif test -z "$left"
>> + then
>> + left=$1
>> + elif test -z "$right"
>> + then
>> + right=$1
>> + else
>> + paths="$paths$1 "
>> + fi
>> + ;;
>> + esac
>
> Hrm, so "diffall HEAD~2 Documentation/" is not the way to compare the
> contents of the Documentation/ directory between the named commit and
> the working tree, like "diff HEAD~2 Documentation/" does.
>
> That is not a show-stopper (a double-dash is an easy workaround), but
> it is worth pointing out.
So I would need something to determine if a string represents a
commit/tag/branch or a path?. I presume it would need to handle the
corner case where a branch/tag and path have the same name. Is there
anything like this in the mergetool lib scripts today?
>> + then
>> + git diff --name-only "$left"..."$right" -- $paths > "$tmp/filelist"
>> + else
>> + git diff --name-only "$left" "$right" -- $paths > "$tmp/filelist"
>> + fi
>
> And this will not work with pathspec that have $IFS characters. If we
> really wanted to we could do that by properly quoting "$1" when you build
> $paths and then use eval when you run "git diff" here (look for 'sq' and
> 'eval' in existing scripts, e.g. "git-am.sh", if you are interested).
>
> Also you may want to write filelist using -z format to protect yourself
> from paths that contain LF, but that would require the loop "while read
> name" to be rewritten.
I just discovered that the script fails to handle files that have
spaces in their name, so some further work is needed.
>> +# Exit immediately if there are no diffs
>> +if test ! -s "$tmp/filelist"
>> +then
>> + exit 0
>> +fi
>
> Ok, you have trap set already so $tmp will disappear with this exit ;-)
I'll try not to be such a slow learner in the future...but no guarantees.
>> +if test -n "$copy_back" && test "$right_dir" != "working_tree"
>> +then
>> + echo "--copy-back is only valid when diff includes the working tree."
>> + exit 1
>> +fi
>
> I actually wondered why $right_dir needs to be populated with a copy in
> the first place (if you do not copy but give the working tree itself to
> the external tool, you do not even have to copy back).
>
> I know the answer to the question, namely, "because the external tool
> thinks files that are not in $left_dir are added files", but if you can
> find a way to tell the external tool to ignore new files (similar to how
> "diff -r" without -N works), running the tool with temporary left_dir and
> the true workdir as right_dir would be a lot cleaner solution to the
> problem.
I'll note this as "for future consideration". I spent some time
trying this is the original implementation, but could not find a
workable solution in the time I had available.
>> + while read name
>> + do
>> + ls_list=$(git ls-tree $right $name)
>> + if test -n "$ls_list"
>> + then
>> + mkdir -p "$tmp/$right_dir/$(dirname "$name")"
>> + git show "$right":"$name" > "$tmp/$right_dir/$name" || true
>> + fi
>> + done < "$tmp/filelist"
>
> "while read -r name" might make this slightly more robust; even though
> this loses leading and trailing whitespaces in filenames, we probably
> can get away without worrying about them.
>
>> +else
>> + # Mac users have gnutar rather than tar
>> + (tar --ignore-failed-read -c -T "$tmp/filelist" | (cd "$tmp/$right_dir" && tar -x)) || {
>> + gnutar --ignore-failed-read -c -T "$tmp/filelist" | (cd "$tmp/$right_dir" && gnutar -x)
>> + }
>
> What is this "--ignore-failed-read" about? Not reporting unreadable as an
> error smells really bad.
If a file was added or deleted between the two commits being compared,
tar would fail because a file was missing from "$tmp/filelist". The
"--ignore-failed-read" prevents tar from halting the script in this
case.
> If you require GNUism in your tar usage, this should be made configurable
> so that people can use alternative names (some systems come with "tar"
> that is POSIX and "gtar" that is GNU).
Is there an example showing how this could be configurable? The
problem is that the "--ignore-failed-read" was not supported in all
flavors of tar.
>> +# Copy files back to the working dir, if requested
>> +if test -n "$copy_back" && test "$right_dir" = "working_tree"
>> +then
>> + cd "$start_dir"
>> + git_top_dir=$(git rev-parse --show-toplevel)
>> + find "$tmp/$right_dir" -type f |
>> + while read file
>> + do
>> + cp "$file" "$git_top_dir/${file#$tmp/$right_dir/}"
>> + done
>> +fi
>
> This will copy new files created in $right_dir. Is that intended?
hmmm...that was not intended. If would be odd for the user to create
new files in this tmp directory, but if the diff tool automatically
generates any files then this could result in unwanted files.
^ permalink raw reply
* Submodule commits not show by git log
From: Wilfred Hughes @ 2012-02-23 16:49 UTC (permalink / raw)
To: git
Hi all,
I'm trying to track changes on a git submodule. `$ git log
path/to/submodule` is not showing any commits that changed the
submodule commit.
For example, in my repo I have a submodule at the path
./memcache_utils that has had the commit referenced changed several
times. It's definitely a submodule:
$ git submodule
13eb087304b995705693d6f0927dec2d88dfadaf datastore_utils
(heads/master-2-g13eb087)
7f5d6710b767a27f14e3e7bc009f026b3e5f0e74 memcache_utils (heads/master)
5877e2c2d82645fa44f121884291ee48cf24584d potatobase (5877e2c)
Yet the only commit shown is when there were files at that path:
$ git log memcache_utils
commit 2cbe65bf31901873b01331e95fec72724e7458eb
Author: [...]
Date: Thu Jan 26 20:44:07 2012 +0000
Experimenting with Paul G's port of cache-machine. It should
really be a git submodule but just want to see how it works so have
taken a copy from [...]
$
Have I missed something? I've tried and failed to create a minimal
test case that demonstrates this behaviour. I can't see anything in
the man pages to suggest that I'm using git log incorrectly, and
Googling doesn't help.
Is this user error, a known bug, or a new one?
Many thanks
Wilfred
^ permalink raw reply
* Re: [PATCH v2] contrib: added git-diffall
From: Junio C Hamano @ 2012-02-23 17:37 UTC (permalink / raw)
To: Stefano Lattarini; +Cc: Tim Henigan, git, davvid
In-Reply-To: <4F460D45.7000804@gmail.com>
Stefano Lattarini <stefano.lattarini@gmail.com> writes:
>>> +# mktemp is not available on all platforms (missing from msysgit)
>>> +# Use a hard-coded tmp dir if it is not available
>>> +tmp="$(mktemp -d -t tmp.XXXXXX 2>/dev/null)" || {
>>> + tmp=/tmp/git-diffall-tmp
>>> +}
>> ...
> # mktemp is not available on all platforms (missing from msysgit)
> tmp=$(mktemp -d -t tmp.XXXXXX 2>/dev/null) || {
> tmp=/tmp/git-diffall-tmp.$$
> mkdir "$tmp" || fatal "couldn't create temporary directory"
> }
>
>>> +mkdir -p "$tmp"
>>
> At which point this should be removed, of course.
Good eyes; thanks.
^ permalink raw reply
* Integration with UDDI-based registries and repositories
From: Lico Galindo @ 2012-02-23 18:48 UTC (permalink / raw)
To: git
Good afternoon,
We use the Reusable Component Services (RCS), a registry/repository
solution based on Software AG's CentraSite (a UDDI-based registry) for
registration of programming code and other IT assets. Code is registered
in RCS and may be stored either in the RCS repository or anywhere the
owner wants (storing the link in the registry). Some of the developers
have asked us to explore integrating RCS with Git or GitHub. This wiould
include loading code registered in RCS into Git, modifying the code in
whatever tool and registering the modified code back into RCS. By the
way, RCS supports developer interfaces like Eclipse and Visual Studio.
Does this make sense to you or am I misunderstanding your tool? How do
you think this would work?
Thanks,
Lico Galindo, PMP
IT Specialist
Data Standards Branch
Office of Environmental Information
Phone: 202.566.1252
Fax: 202.566.1684
^ permalink raw reply
* Re: [PATCH v2] contrib: added git-diffall
From: Junio C Hamano @ 2012-02-23 19:02 UTC (permalink / raw)
To: Tim Henigan; +Cc: git, davvid, stefano.lattarini
In-Reply-To: <CAFouetiSpsZGtLt2tG4ou-H18zigNx5xWQH4cy8GrL1eDxbjJw@mail.gmail.com>
Tim Henigan <tim.henigan@gmail.com> writes:
> On Wed, Feb 22, 2012 at 6:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> Another thing that comes to my mind is if a user has an external tool that
>> can use "diffall", is there ever a situation where the user chooses to use
>> "difftool" instead, to go files one by one. I cannot offhand imagine any.
>
> It was my assumption that any tool that supports directory diff also
> supports individual file diff. It seems like a strict subset of
> directory diff case.
> ...
>> Perhaps a longer term integration plan may be to lift the logic from this
>>
> ...snip...
>>
>> But that is all two steps in the future.
>
> I hope that this feature finds it way into the existing core commands.
> This script is intended to be a conversation starter that is also
> immediately useful as a separate command. Would it be better to begin
> the long-term discussion now and skip adding this to contrib?
I would envision we have this in contrib/ first, without even fixing the
whitespace-in-pathspec and whitespace-or-lf-in-paths issues I pointed out
in my review, and let people play with it.
My crystal ball tells an optimist in me that we will see people (you and
others) try to fix issues they hit in their real life use cases, and the
script will be improved while it is still in contrib/. And then somebody
who has worked on difftool will step up and roll it into difftool proper,
along the lines I hinted in the message you are responding to, at which
point the script will be removed from contrib/. That is the first step in
the future.
The second step in the future may or may not come. It will involve adding
an updated external diff interface on the core side and would prepare the
two temporary directories before the core side calls the difftool among
other things, and when that happens, we can lose most of the code in this
script that the first step in the future may have ported into difftool.
>> You can write multiple assignment on a line to save vertical space if you
>> want to, and the initialization sequence like this is a good place to do
>> so.
> My personal preference is to keep them on separate lines. However if
> the compressed style is preferred, I will change it.
"I wouldn't bother" was what I meant by "if you want to".
>>> + -x|--e|--ex|--ext|--extc|--extcm|--extcmd)
> ...
>> Don't you want to match "-t/--tool" that "difftool" already uses?
>
> Are you suggesting that I a) change "-x/--extcmd" to "-t/--tool" or
> that b) I add support for the "difftool -t/--tool" option?
No, I just misread the set of options "difftool" takes, without realizing
that --extcmd and --tool are two different things, and it is correct to
call your option "--extcmd" if it specifies what corresponds to what
"difftool --extcmd" specifies. Sorry for the confusion.
>> Hrm, so "diffall HEAD~2 Documentation/" is not the way to compare the
>> contents of the Documentation/ directory between the named commit and
>> the working tree, like "diff HEAD~2 Documentation/" does.
>>
>> That is not a show-stopper (a double-dash is an easy workaround), but
>> it is worth pointing out.
>
> So I would need something to determine if a string represents a
> commit/tag/branch or a path?.
"It does not have to be fixed in the first version" (aka "for future
consideration") was what I meant by "not a show-stopper".
>> And this will not work with pathspec that have $IFS characters. If we
>> really wanted to we could do that by properly quoting "$1" when you build
>> $paths and then use eval when you run "git diff" here (look for 'sq' and
>> 'eval' in existing scripts, e.g. "git-am.sh", if you are interested).
>>
>> Also you may want to write filelist using -z format to protect yourself
>> from paths that contain LF, but that would require the loop "while read
>> name" to be rewritten.
>
> I just discovered that the script fails to handle files that have
> spaces in their name, so some further work is needed.
Again, "It does not have to be fixed in the first version" (aka "for
future consideration") was what I meant by "If we really wanted to".
>> What is this "--ignore-failed-read" about? Not reporting unreadable as an
>> error smells really bad.
>
> If a file was added or deleted between the two commits being compared,
> tar would fail because a file was missing from "$tmp/filelist". The
> "--ignore-failed-read" prevents tar from halting the script in this
> case.
But it will also ignore errors coming from other causes, no? Wouldn't we
rather see an error if tar fails to read from a path that *has to* exist
in the working tree because "diff" said it does?
Again, it is just "for future consideration".
>> If you require GNUism in your tar usage, this should be made configurable
>> so that people can use alternative names (some systems come with "tar"
>> that is POSIX and "gtar" that is GNU).
>
> Is there an example showing how this could be configurable? The
> problem is that the "--ignore-failed-read" was not supported in all
> flavors of tar.
Grep for "$TAR" and also @@DIFF@@ in the Makefile, and add substitution
for @@TAR@@ in cmd_munge_script, perhaps?
By the way, I actually have an even more radical suggestion that may let
you get rid of most of the lines in your script.
If you tweak the usage so that "diffall" specific options *MUST* come
first, e.g.
USAGE=[--copy-back] [-x <cmd>] <arguments for diff>
then you can parse your argument partially, i.e.
copy_back= extcmd=
while case "$#,$1" in 0,*) break ;; *,-*) ;; *) break ;; esac
do
case "$1" in
--copy-back)
copy_back=true
;;
-x | --extcmd)
test $# != 1 || usage
extcmd=$2
shift
;;
*)
break
;;
esac
shift
done
then feed the remainder all to "diff", e.g.
diff --raw --no-abbrev "$@" |
And then you can prepare two temporary index files and stuff the output in
them, by having something like this on the downstream of the pipe:
while read -r lmode rmode lsha1 rsha1 status path
do
if test "$lmode" != $null_mode
then
GIT_INDEX_FILE=$tmp.left_index \
git update-index --add --cacheinfo $lmode $lsha1 $path
fi
if test "$rmode" != $null_mode
then
GIT_INDEX_FILE=$tmp.right_index \
git update-index --add --cacheinfo $rmode $rsha1 $path
fi
done
Side Note:
In the production version, you would probably give the "-z" option
to "diff", and write this loop in Perl so that you can cope better
with funny characters in the path. Instead of running two
instances of "git update-index" for every path, the loop would
group the entries for left and right side, and drive one instance
of "git update-index --index-info" each to populate the two index
files.
Also the above needs to be adjusted to deal with the side that
represents the working tree files; they are reported with $null_sha1
so in such a case instead of putting it in the temporary index,
you would copy the working tree file to the temporary location.
After you prepare these two temporary index files, you can then use them
to populate your left_dir and right_dir like this:
GIT_DIR=$(git rev-parse --git-dir) \
GIT_WORK_TREE=$left_dir \
GIT_INDEX_FILE=$tmp.left_index \
git checkout-index -a
With this, you do not have to worry about anything about the funny
combinations of where the two "directories" comes from when preparing
the temporary directories to be compared.
^ permalink raw reply
* Re: [PATCH v2 0/4] Making an elephant out of a getline() bug
From: Junio C Hamano @ 2012-02-23 19:04 UTC (permalink / raw)
To: Thomas Rast; +Cc: Johannes Sixt, Jeff King, Jannis Pohlmann, git
In-Reply-To: <cover.1329988335.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> So you all made very good points, and I don't want to repeat them.
I really wish you did not make an elephant out of this, or if you must,
kept the minimum bugfix part that must apply to older codebase and making
an elephant part clearly separable.
Anyway, I've reordered so that the bugfix part would apply to an older
maintenance release, and then the remainder applied on top would merge
cleanly to the bleeding edge, so there is no need to resend.
Thanks for working on this, everybody.
^ permalink raw reply
* Re: [PATCH] remote-curl: Fix push status report when all branches fail
From: Junio C Hamano @ 2012-02-23 19:11 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn Pearce, git
In-Reply-To: <20120223100434.GA3083@sigill.intra.peff.net>
Thanks.
^ permalink raw reply
* RE: gitk: set uicolor SystemButtonFace error on X11 if .gitk created using Win32 tk
From: Matt Seitz (matseitz) @ 2012-02-23 19:20 UTC (permalink / raw)
To: git; +Cc: kusmabite
In-Reply-To: <CABPQNSZqX0_YAn=mOpuRquG9OzFgwS9fQ6=YXqULxMz-hbH6Zw@mail.gmail.com>
> From: Erik Faye-Lund [mailto:kusmabite@gmail.com]
>
> Are you saying that the tk shipped with recent Cygwin reports "win32"
> for "[tk windowingsystem]", but does not recognize the
> "SystemButtonFace" etc values?
Here is what I am saying:
1. Cygwin tk 8.4.x and earlier:
-Reports "win32" for "[tk windowingsystem]"
-Recognizes the "SystemButtonFace" etc values
2. Cygwin tk 8.5.11:
-Reports "x11" for "[tk windowingsystem]"
-Does not recognize the "SystemButtonFace" etc values
> Does this patch help?
No, this has the same problem: if gitk is installed using a tk that
reports "win32" for "[tk windowingsystem]", then gitk hard codes win32
specific values into .gitk. If the user later changes to a tk that
doesn't support win32 specific values, gitk will no longer work.
Two use cases:
1. My situation: User replaces a win32 tk with an x11 tk
2. User uses a common .gitk file when running Windows and Linux.
Here's the error:
$ uname -a
CYGWIN_NT-5.1 matseitz-wxp 1.7.10(0.259/5/3) 2012-02-05 12:36 i686
Cygwin
$ wish
% tk windowingsystem
x11
% exit
$ gitk &
[1] 6312
$ Error in startup script: unknown color name "ButtonFace"
while executing
"winfo rgb . $c"
(procedure "setui" line 3)
invoked from within
"setui $uicolor"
(file "/usr/bin/gitk" line 11522)
^ permalink raw reply
* Re: [PATCH] merge: use editor by default in interactive sessions
From: Junio C Hamano @ 2012-02-23 19:23 UTC (permalink / raw)
To: kusmabite; +Cc: git
In-Reply-To: <CABPQNSZVOjOKpqv4s1ZCEQRd_yT3us3mqC9aN-KK5PHqztYQQg@mail.gmail.com>
Erik Faye-Lund <kusmabite@gmail.com> writes:
>> + /* Use editor if stdin and stdout are the same and is a tty */
>> + return (!fstat(0, &st_stdin) &&
>> + !fstat(1, &st_stdout) &&
>> + isatty(0) &&
>> + st_stdin.st_dev == st_stdout.st_dev &&
>> + st_stdin.st_ino == st_stdout.st_ino &&
>> + st_stdin.st_mode == st_stdout.st_mode);
>
> I just stumbled over this code, and I got a bit worried; the
> stat-implementation we use on Windows sets st_ino to 0, so
> "st_stdin.st_ino == st_stdout.st_ino" will always evaluate to true.
>
> Perhaps we should add a check for isatty(1) here as well? ...
> Or is there something I'm missing here?
No, the intention was not "we do this whether standard output is tty or
not", but was "we check that fd#0 and fd#1 are connected to the same
device by trusting stat() to do the right thing, so checking isatty(0)
is sufficient". As that "trusting stat()" assumption does not hold for
your platform, we would need to add isatty(1) to accomodate it.
Thanks for a set of sharp eyes.
^ 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