Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2 1/4] strbuf: improve strbuf_get*line documentation
From: Jeff King @ 2012-02-23 10:08 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Junio C Hamano, Johannes Sixt, Jannis Pohlmann, git
In-Reply-To: <d5522f8d1154321804cb60fb4f53d538242ead88.1329988335.git.trast@student.ethz.ch>

On Thu, Feb 23, 2012 at 10:42:21AM +0100, Thomas Rast wrote:

> strbuf_getline() was not documented very clearly, though a reader
> familiar with getline() would not have had any questions about it.
> strbuf_getwholeline() was not documented at all.

Thanks for improving the existing docs. One suggestion:

>  `strbuf_getline`::
>  
> -	Read a line from a FILE* pointer. The second argument specifies the line
> -	terminator character, typically `'\n'`.
> +	Read a line from a FILE*. The second argument specifies the
> +	line terminator character, typically `'\n'`.  Reading stops
> +	after the terminator or at EOF.  The terminator is removed
> +	from the buffer before returning.  Returns 0 unless there was
> +	nothing left before EOF, in which case it returns `EOF`.

The get*line functions are unlike the rest of the strbuf API in that
they overwrite, rather than append to, the strbuf argument. Maybe:

  s/from a FILE\*/&, overwriting the existing contents of the strbuf/

?

-Peff

^ permalink raw reply

* Re: git log -z doesn't separate commits with NULs
From: Johannes Sixt @ 2012-02-23 10:17 UTC (permalink / raw)
  To: Nikolaj Shurkaev; +Cc: git
In-Reply-To: <4F46036F.3040406@gmail.com>

Am 23.02.2012 10:14, schrieb Nikolaj Shurkaev:
> 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
> 
> If I put
> echo "started"
> into the file  ~/1.sh I see that the file is called only once instead of
> multiple times.

That is because xargs calls the program with as many arguments as
possible, unless directed otherwise. Put this in the script:

	echo "started $*"

and repeat. Then try this:

 ... | xargs -0 -n 1 ~/1.sh

-- Hannes

^ permalink raw reply

* Re: git log -z doesn't separate commits with NULs
From: Jeff King @ 2012-02-23 10:24 UTC (permalink / raw)
  To: Nikolaj Shurkaev; +Cc: git
In-Reply-To: <4F46036F.3040406@gmail.com>

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: Jeff King @ 2012-02-23 10:27 UTC (permalink / raw)
  To: Luke Diamand; +Cc: Nikolaj Shurkaev, git
In-Reply-To: <4F460EB7.3030503@diamand.org>

On Thu, Feb 23, 2012 at 10:02:31AM +0000, Luke Diamand wrote:

> >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.
> >Is my understanding correct or I don't understand the documentation or
> >somehow pass wrong parameters into git log?
> 
> Just a guess, but doesn't the "--patch" option to git log ask it to
> produce a patch output? Surely that will override the -z: patch will
> not be expecting NULs.

No. You will get the patch text and the log message together, with
commits separated by NUL. Some diff output formats will also respect
"-z" to produce NULs internally (e.g., "--raw" will use it to separate
filenames), but "--patch" does not.

-Peff

^ permalink raw reply

* Re: git log -z doesn't separate commits with NULs
From: Andreas Schwab @ 2012-02-23 10:35 UTC (permalink / raw)
  To: Nikolaj Shurkaev; +Cc: git
In-Reply-To: <4F46036F.3040406@gmail.com>

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.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: What's cooking in git.git (Feb 2012, #08; Wed, 22)
From: Jakub Narebski @ 2012-02-23 11:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfwe26zwn.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> * jn/gitweb-hilite-regions (2012-02-19) 5 commits
>  - gitweb: Use esc_html_match_hl() in 'grep' search

This one is about starting unification of match highlighting in
gitweb, using esc_html_match_hl() which can show multiple matches in
line as opposed to existing hand-crafted and duplicated solutions.

>  - gitweb: Highlight matched part of shortened project description
>  - gitweb: Highlight matched part of project description when searching projects
>  - gitweb: Highlight matched part of project name when searching projects
>  - gitweb: Introduce esc_html_hl_regions

This is about adding match highlighting to project search.  Other
kinds of search in gitweb ('commit', 'grep' - both per-repository
searches) already use match highlighting.

>  (this branch uses jn/gitweb-search-optim.)

Theoretically those topics should be independent, but I think there
can be tetual conflict.  Better to build one on top of the other.

Nb. should I send those series independently?
 
> Not reviewed and do not know what this is about yet ;-).

I hope that above description helps.
 
> * jn/gitweb-search-optim (2012-02-19) 3 commits
>  - gitweb: Faster project search
>  - gitweb: Option for filling only specified info in fill_project_list_info
>  - gitweb: Refactor checking if part of project info need filling
>  (this branch is used by jn/gitweb-hilite-regions.)
> 
> The API introduced in the second step still has yucky design, but at least
> it is more clear than the previous rounds what this is trying to do.

Thanks for the comments.  I'll try to send re-roll soon.

-- 
Jakub Narebski

^ permalink raw reply

* measuring the % change between two commits
From: Sitaram Chamarty @ 2012-02-23 11:30 UTC (permalink / raw)
  To: Git Mailing List

I'm trying to come up with a % measure for the changes between two commits.

I don't mind how precisely it is calculated, since this is only for a
thresholding purpose where we can adjust the threshold if the way it
is calculated is not what we think it should be.

The logging options that put out a % figure only do so for
copied/renamed files.  Merely modified files do not get a % figure
attached.

I could do a --numstat and then do a 'wc -l' on each file I guess, but
I was hoping to avoid that.

--dirstat gives you a percentage but does not count the top level directory.

Any ideas?

-- 
Sitaram

^ permalink raw reply

* Re: [PATCH 2/4] Do attempt pretty print in ASCII-incompatible encodings
From: Peter Krefting @ 2012-02-23 11:25 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20120221182118.GA32668@sigill.intra.peff.net>

Jeff King:

> I'm not sure why we bother checking this. Using non-ASCII-superset
> encodings is broken, yes, but are people actually doing that?
[...]
> Are there actually encodings that will cause subtle breakage that we want 
> to catch?

Shift-JIS could be a problem; if implemented to the letter it would convert 
0x5C to a Yen character and 0x7E as an overline. Otherwise I expect it only 
being a problem with ISO 646 encodings, especially the ones that replace "@" 
with something else [1].

Also any ISO 2022 seven-bit encoding (ISO-2022-{CN,JP,KR}) could cause 
problems, especially if there is any preprocessing done on the string that 
does not respect its state-shifting (most 0x21--0x7E characters can be lead 
and trail bytes in their multi-byte modes).

-- 
\\// Peter - http://www.softwolves.pp.se/

  [1] Trying to send Internet e-mail from a system using the extended
      Swedish seven-bit encoding, where 0x40 mapped to "É", could
      sometimes be a challenge.

^ permalink raw reply

* Re: [ANNOUNCE] Git 1.7.9.2
From: Sitaram Chamarty @ 2012-02-23 11:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbooq6zq2.fsf@alter.siamese.dyndns.org>

On Thu, Feb 23, 2012 at 8:08 AM, Junio C Hamano <gitster@pobox.com> wrote:

>  * "git rev-list --verify" sometimes skipped verification depending on
>   the phase of the moon, which dates back to 1.7.8.x series.

phase of the moon?  should have been named --were-ify I guess :-)

^ permalink raw reply

* Re: git log -z doesn't separate commits with NULs
From: Nikolaj Shurkaev @ 2012-02-23 12:11 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <4F46122F.2040409@kdbg.org>

Hello.

You were right. I added parameter --max-args=1 to xargs command and that 
started work as I wanted initially.
Thank you very much.

--
Nikolaj

23.02.2012 13:17, Johannes Sixt пишет:
> Am 23.02.2012 10:14, schrieb Nikolaj Shurkaev:
>> 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
>>
>> If I put
>> echo "started"
>> into the file  ~/1.sh I see that the file is called only once instead of
>> multiple times.
> That is because xargs calls the program with as many arguments as
> possible, unless directed otherwise. Put this in the script:
>
> 	echo "started $*"
>
> and repeat. Then try this:
>
>   ... | xargs -0 -n 1 ~/1.sh
>
> -- Hannes
>

^ permalink raw reply

* 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox