* Re: [PATCH] Autoconf: Disable inline for compilers that don't support it.
From: Junio C Hamano @ 2009-03-14 20:46 UTC (permalink / raw)
To: git; +Cc: Allan Caffee
In-Reply-To: <20090314010421.GA6642@linux.vnet>
Allan Caffee <allan.caffee@gmail.com> writes:
> The Autoconf macro AC_C_INLINE will redefine the inline keyword to whatever the
> current compiler supports (including possibly nothing).
>
> Signed-off-by: Allan Caffee <allan.caffee@gmail.com>
As far as I can tell, this makes scriptlet to set ac_cv_c_inline and then
the result is written to confdefs.h:
case $ac_cv_c_inline in
inline | yes) ;;
*)
case $ac_cv_c_inline in
no) ac_val=;;
*) ac_val=$ac_cv_c_inline;;
esac
cat >>confdefs.h <<_ACEOF
#ifndef __cplusplus
#define inline $ac_val
#endif
_ACEOF
;;
esac
which is used only during the ./configure run but not during the actual
build.
What am I missing?
^ permalink raw reply
* Re: [PATCH] git-push.txt: describe how to default to pushing only current branch
From: Jeff King @ 2009-03-14 20:34 UTC (permalink / raw)
To: Chris Johnsen, Junio C Hamano; +Cc: git
In-Reply-To: <1236994051-27346-1-git-send-email-chris_johnsen@pobox.com>
On Fri, Mar 13, 2009 at 08:27:31PM -0500, Chris Johnsen wrote:
> In the resulting manpage the inline commands are not very
> obvious (the HTML looks OK though). There is some sort of
> formatting in there, but it does not seem to display any
> differently from the surrounding text when I use man to view it
> on my system. Would it be better to do something like wrap
> double quotes around the inline commands to help readers viewing
> the manpage?
The problem is that they are supposed to set in a monospaced font, but
most terminals are already monospaced. This is actually a problem
throughout the documentation, although it is usually only for
single-word phrases (like `git-foo`), which don't look nearly as bad as
multi-word ones.
Actually, looking closer, the information seems to be lost entirely.
Asciidoc renders this to <literal> in the XML, but docbook seems to
throw it away when converting to a manpage. In theory it's possible to
apply our own xsl style to turn this into something else, and I think
that is a better solution than just trying to fix this one spot.
The question is how it _should_ be rendered. Monospace isn't really
useful for terminals. Maybe simply putting quotation marks around it
would cover all situations (I'm worried it will look funny for
single-word instances).
On Sat, Mar 14, 2009 at 12:26:41PM -0700, Junio C Hamano wrote:
> The new text looks reasonable. Sign-off?
>
> Any improvement suggestions from others?
It looks fine to me; my only concern is the typesetting, but I think
that should be fixed elsewhere, as outlined above.
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] test-lib.sh: Allow running the test suite against installed git
From: Junio C Hamano @ 2009-03-14 20:25 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <1236959116-3334-3-git-send-email-git@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Introduce variables externalpath and externalexecpath such that the test
> suite can be run against a git which is installed at $externalpath with
> subcommands at $externalexecpath. externalpath defaults to the git.git
> checkout, externalexecpath defaults to $externalpath. Run the suite as
>
> externalpath=somepath externalexecpath=someotherpath make test
>
> but note that this requires and uses parts of a compiled git in the
> git.git checkout: test helpers, templates and perl libraries are taken
> from there.
While I like the end result this series tries to achieve, may I suggest a
few things?
- This is like GIT_SKIP_TESTS and GIT_TEST_HTTPD in that extra
environments affect how the tests are run. It would be much easier to
use if the new environment variables are named similarly, prefixed with
GIT_, in all caps, and with underscores between words.
- When externalpath is given but not externalexecpath, you can deduce the
latter from the former by running "$externalpath/git --exec-path",
which makes running the tests against an installed git even easier.
For example, I keep many git installations under $HOME/git-snap-vX.Y.Z,
and it would be great if your patch allowed me to say something like:
$ GIT_TEST_INSTALLED=$HOME/git-snap-v1.4.4.4/bin/git make test
^ permalink raw reply
* Re: [PATCH] Remove unused assignments
From: Junio C Hamano @ 2009-03-14 20:18 UTC (permalink / raw)
To: Benjamin Kramer; +Cc: git
In-Reply-To: <49BA56D5.5050807@googlemail.com>
Benjamin Kramer <benny.kra@googlemail.com> writes:
> These variables were always overwritten or the assigned
> value was unused:
>
> builtin-diff-tree.c::cmd_diff_tree(): nr_sha1
> builtin-for-each-ref.c::opt_parse_sort(): sort_tail
> builtin-mailinfo.c::decode_header_bq(): in
> builtin-shortlog.c::insert_one_record(): len
> connect.c::git_connect(): path
> imap-send.c::v_issue_imap_cmd(): n
> pretty.c::pp_user_info(): filler
> remote::parse_refspec_internal(): llen
>
> Signed-off-by: Benjamin Kramer <benny.kra@googlemail.com>
Thanks. I eyeballed all of them and they look safe, but this patch made
me wonder...
Did you use some dataflow analysis tool to spot these?
It will never scale if a human has to sanity check output from a
mechanical process like this patch, especially when the human is already a
chokepoint of the whole process (i.e. the maintainer).
We'd need some process improvements in the longer term to handle patches
like this, but I do not think of a good one offhand.
I do not think we want to trust tool output blindly; it will not solve the
issue to give me the recipe for the mechanical process you used, to have
me run it and to make me trust the result without checking. We would want
to keep some human whose judgement we can trust in the loop to always
check the changes mechanical analysis tools may produce.
I think the only viable long term solution would be to keep doing the
manual verification I did like this round until somebody like you
accumulate enough "trust points" in the community for consistently sending
good patches like this one to allow me to apply patches from these people
blindly, trusting the sender's judgement.
But please disregard all of the above if you spotted these by manual
inspection.
I think the patch to pp_user_info() can go one step further. The filler
is used at only one place after this patch, as a strong given to one of
the placeholders in strbuf_addf(). We can simply feed a constant to the
function instead, like this:
diff --git a/pretty.c b/pretty.c
index 3a24cd5..efa7024 100644
--- a/pretty.c
+++ b/pretty.c
@@ -135,7 +135,6 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
int namelen;
unsigned long time;
int tz;
- const char *filler = " ";
if (fmt == CMIT_FMT_ONELINE)
return;
@@ -161,7 +160,7 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
} else {
strbuf_addf(sb, "%s: %.*s%.*s\n", what,
(fmt == CMIT_FMT_FULLER) ? 4 : 0,
- filler, namelen, line);
+ " ", namelen, line);
}
switch (fmt) {
case CMIT_FMT_MEDIUM:
^ permalink raw reply related
* Re: [PATCH] git-push.txt: describe how to default to pushing only current branch
From: Junio C Hamano @ 2009-03-14 19:26 UTC (permalink / raw)
To: Chris Johnsen; +Cc: git, Jeff King
In-Reply-To: <1236994051-27346-1-git-send-email-chris_johnsen@pobox.com>
The new text looks reasonable. Sign-off?
Any improvement suggestions from others?
^ permalink raw reply
* Re: Transparently encrypt repository contents with GPG
From: Junio C Hamano @ 2009-03-14 18:45 UTC (permalink / raw)
To: Michael J Gruber
Cc: Sverre Rabbelier, Thomas Rast, Matthias Nothhaft, git, Jeff King
In-Reply-To: <49BB920A.20301@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Since both the cleaned and the smudged version are supposed to be
> "authoritative" (as opposed to the textconv'ed one) one may argue either
> way what's the right approach.
Smudged one can never be authoritative. That is the whole point of smudge
filter and in general the whole convert_to_working_tree() infrastructure.
It changes depending on who you are (e.g. on what platform you are on).
So running comparison between two clean versions is the only sane thing to
do.
You could argue textconv should work on smudged contents or on clean
contents before smudging. As long as it is done consistently, I do not
care either way too deeply, as its output is not supposed to be used for
anything but human consumption. Two equally sane arrangement would be:
(1) Start from two clean contents (run convert_to_git() if contents were
obtained from the work tree), run textconv, run diff, and output the
result literally; or
(2) Start from two smudged contents (run convert_to_working_tree() for
contents taken from the repository), run textconv, run diff, and
run clean before sending the result to the output.
The former assumes a textconv filter that wants to work on clean
contents, the latter for a one that expects smudged input. I probably
would suggest going the former approach, as it is consistent with the
general principle in other parts of the system (the internal processing
happens on clean contents).
Both of the above two assumes that the output should come in clean form;
it is consistent with the way normal diff is generated for consumption by
git-apply. You can certainly argue that the final output should be in
smudged form when textconv is used, as it is purely for human consumption,
and is not even supposed to be fed to apply.
^ permalink raw reply
* Re: [PATCH] Introduce a filter-path argument to git-daemon, for doing custom path transformations
From: Junio C Hamano @ 2009-03-14 18:23 UTC (permalink / raw)
To: Johan Sørensen; +Cc: Johannes Schindelin, git
In-Reply-To: <9e0f31700903140739g26be7981lb0fa411cdd8029e6@mail.gmail.com>
Johan Sørensen <johan@johansorensen.com> writes:
>> Do you run git-daemon from inetd, or standalone, by the way?
>
> Standalone.
>
>> I am wondering how well it would scale if you spawn an external "filter path"
>> script every time you get a request.
>
> A quick test of 250 consecutive requests with ls-remote to localhost
> (all without the --verbose flag), slowest run:
> - Baseline (no --filter-path agument): 3.39s
>
> $ cat filter.c
> #import "stdio.h"
> int main (int argc, char const *argv[]) {
> printf("%s", "/existing.git\0");
> return 0;
> }
> - 3.84s
>
> $ cat filter.rb
> #!/usr/bin/ruby
> print "/existing.git\0"
> - 4.76s
>
> So, obviously highly dependent on how long it takes the script to
> launch and how much work it does. And yes, neither of the above really
> does anything :) nor takes any increased cpu load into account
>
> Another approach is to keep the external script running and feed it on
> stdin, but that would involve a bit more micro-management of the
> external process. I will revisit that idea if I find out that's
> needed.
I actually was hoping (especially we have Dscho on Cc: list) that somebody
like you would start suggesting a "plug in" approach to load .so files,
which would lead to a easy-to-port dso support with the help from msysgit
folks we can use later in other parts of the system (e.g. customizable
filters used for diff textconv, clean/smudge, etc.)
>> (by the way, "filter path" sounds as if it checks and conditionally
>> denies access to, or something like that, which is not what you are using
>> it for. It is more about rewriting paths, a la mod_rewrite, and I think
>> the option is misnamed)
>
> Maybe --rewrite-script or --rewrite-command instead?
Perhaps.
^ permalink raw reply
* Re: [PATCH] Introduce a filter-path argument to git-daemon, for doing custom path transformations
From: Johan Sørensen @ 2009-03-14 14:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vvdqcd1zh.fsf@gitster.siamese.dyndns.org>
On Sat, Mar 14, 2009 at 7:58 AM, Junio C Hamano <gitster@pobox.com> wrote:
> However, being paranoid is a good thing when we talk about instructions we
> give to the end users. The site owner who uses this facility needs to be
> aware that the script is run as the same user that runs git-daemon, and
> that more than one instances of the script can be run at the same time.
> The script writer needs to be careful about using the same scratchpad
> location for the temporary files the script uses and not letting multiple
> instances of scripts stomping on each other's toes. These things need to
> be documented.
Will expand the docs further.
> Do you run git-daemon from inetd, or standalone, by the way?
Standalone.
> I am wondering how well it would scale if you spawn an external "filter path"
> script every time you get a request.
A quick test of 250 consecutive requests with ls-remote to localhost
(all without the --verbose flag), slowest run:
- Baseline (no --filter-path agument): 3.39s
$ cat filter.c
#import "stdio.h"
int main (int argc, char const *argv[]) {
printf("%s", "/existing.git\0");
return 0;
}
- 3.84s
$ cat filter.rb
#!/usr/bin/ruby
print "/existing.git\0"
- 4.76s
So, obviously highly dependent on how long it takes the script to
launch and how much work it does. And yes, neither of the above really
does anything :) nor takes any increased cpu load into account
Another approach is to keep the external script running and feed it on
stdin, but that would involve a bit more micro-management of the
external process. I will revisit that idea if I find out that's
needed.
> (by the way, "filter path" sounds as if it checks and conditionally
> denies access to, or something like that, which is not what you are using
> it for. It is more about rewriting paths, a la mod_rewrite, and I think
> the option is misnamed)
Maybe --rewrite-script or --rewrite-command instead?
Cheers,
JS
^ permalink raw reply
* Re: [PATCH] test-lib: write test results to test-results/<basename>-<pid>
From: SZEDER Gábor @ 2009-03-14 13:59 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Johannes Schindelin, git, gitster
In-Reply-To: <fabb9a1e0903140616q3770f89axff84755abb1f47c7@mail.gmail.com>
Hi,
On Sat, Mar 14, 2009 at 02:16:57PM +0100, Sverre Rabbelier wrote:
> On Sat, Mar 14, 2009 at 13:28, SZEDER Gábor <szeder@ira.uka.de> wrote:
> > With my proposed change there would be no need to clean 'test-results'
> > before running the tests, because test-lib.sh would take care of that
> > (not by removing and recreating 'test-results/', but by overwriting
> > (IOW: removing and recreating, but in one step) individual test result
> > files).
>
> Wouldn't that result in possible stale files being counted in the
> result (e.g., if those tests were not run this time, but they were run
> previously)?
It depends.
If you run only a few tests, then you do it with a command like 'make
t1234-foo.sh t5678-bar.sh'.
Currently this doesn't run the 'pre-clean' target, therefore if you
run different tests (e.g. 'make t1234-foo.sh ; make t5678-bar.sh'),
then a 'make aggregate-results' will include the result of both of
those tests. The same happens with my proposal, too, because the test
of the last run will not overwrite the results of the test in the
first run.
Now suppose that you want to run the same set of tests twice (or more;
e.g. 'make t1234-foo.sh ; make t1234-foo.sh ; make
aggregate-results'). Since currently the pid of the test is included
in the test result file name, there will be two files (t1234-<pid1>
and t1234-<pid2>) created under 'test-results/', and _both_ will be
counted by 'aggregate-results'. In this case my proposal is better,
because the last round will overwrite the result of the previous runs,
therefore no stale files will be counted.
Best,
Gábor
^ permalink raw reply
* Re: [PATCH] test-lib: write test results to test-results/<basename>-<pid>
From: Sverre Rabbelier @ 2009-03-14 13:16 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Johannes Schindelin, git, gitster
In-Reply-To: <20090314122833.GK6808@neumann>
Heya,
On Sat, Mar 14, 2009 at 13:28, SZEDER Gábor <szeder@ira.uka.de> wrote:
> With my proposed change there would be no need to clean 'test-results'
> before running the tests, because test-lib.sh would take care of that
> (not by removing and recreating 'test-results/', but by overwriting
> (IOW: removing and recreating, but in one step) individual test result
> files).
Wouldn't that result in possible stale files being counted in the
result (e.g., if those tests were not run this time, but they were run
previously)?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions
From: Uwe Kleine-König @ 2009-03-14 12:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: martin f krafft, Brian Campbell, git, Petr Baudis
In-Reply-To: <7vvdqcejj2.fsf@gitster.siamese.dyndns.org>
Hello,
On Fri, Mar 13, 2009 at 10:54:41PM -0700, Junio C Hamano wrote:
> martin f krafft <madduck@madduck.net> writes:
>
> > also sprach Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2009.03.12.1620 +0100]:
> >> IMHO we should reuse as much as possible from git.git. For me even
> >> requiring a git.git checkout to use its files would be OK. I consider
> >> that even better then duplicating the relevant files.
> >
> > Maybe we could even start to think about integrating TopGit back
> > into git.git?
>
> Heh, it would need massive style fixes before that happens. I am fairly
> picky on shell script styles.
me, too. That's one of my todo list items, independently from martin's
suggestion. (More to the bottom of that list, though.)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH] test-lib: write test results to test-results/<basename>-<pid>
From: SZEDER Gábor @ 2009-03-14 12:28 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster, Sverre Rabbelier
In-Reply-To: <alpine.DEB.1.00.0903141321550.10279@pacific.mpi-cbg.de>
On Sat, Mar 14, 2009 at 01:22:59PM +0100, Johannes Schindelin wrote:
> On Sat, 14 Mar 2009, SZEDER Gábor wrote:
>
> > If you have failing tests, or if you interrupt the tests, then you
> > will have stale files lying around _anyway_: not only test results
> > are left there, but also trash directories.
>
> The 'pre-clean' target actually cleans test-results/, and test-lib.sh make
> sure that the trash directory is removed and recreated.
With my proposed change there would be no need to clean 'test-results'
before running the tests, because test-lib.sh would take care of that
(not by removing and recreating 'test-results/', but by overwriting
(IOW: removing and recreating, but in one step) individual test result
files).
Best,
Gábor
^ permalink raw reply
* Re: [PATCH] test-lib: write test results to test-results/<basename>-<pid>
From: Johannes Schindelin @ 2009-03-14 12:22 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, gitster, Sverre Rabbelier
In-Reply-To: <20090314121617.GJ6808@neumann>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 378 bytes --]
Hi,
On Sat, 14 Mar 2009, SZEDER Gábor wrote:
> If you have failing tests, or if you interrupt the tests, then you
> will have stale files lying around _anyway_: not only test results
> are left there, but also trash directories.
The 'pre-clean' target actually cleans test-results/, and test-lib.sh make
sure that the trash directory is removed and recreated.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] test-lib: write test results to test-results/<basename>-<pid>
From: SZEDER Gábor @ 2009-03-14 12:16 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: SZEDER Gábor, git, gitster, Sverre Rabbelier
In-Reply-To: <alpine.DEB.1.00.0903141250450.10279@pacific.mpi-cbg.de>
Hi,
On Sat, Mar 14, 2009 at 12:53:06PM +0100, Johannes Schindelin wrote:
> Hi,
>
> On Fri, 13 Mar 2009, SZEDER Gábor wrote:
>
> > diff --git a/t/Makefile b/t/Makefile
> > index 0d65ced..2e6e205 100644
> > --- a/t/Makefile
> > +++ b/t/Makefile
> > @@ -14,14 +14,11 @@ SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
> > T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
> > TSVN = $(wildcard t91[0-9][0-9]-*.sh)
> >
> > -all: pre-clean $(T) aggregate-results clean
> > +all: $(T) aggregate-results clean
Well, this part is wrong, or at least not up-to-date. I just digged
up an ancient branch in my tree and sent out the diff, without
realizing that there were some conflicting changes since then.
> >
> > $(T):
> > @echo "*** $@ ***"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
> >
> > -pre-clean:
> > - $(RM) -r test-results
> > -
> > clean:
> > $(RM) -r 'trash directory' test-results
> >
> > @@ -33,5 +30,5 @@ full-svn-test:
> > $(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=1 LC_ALL=C
> > $(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=0 LC_ALL=en_US.UTF-8
> >
> > -.PHONY: pre-clean $(T) aggregate-results clean
> > +.PHONY: $(T) aggregate-results clean
> > .NOTPARALLEL:
>
> This is wrong. If you have failing tests, or if you interrupt the tests,
> it will never clean the test results, and after Hannes' patch you _will_
> have stale files lying around all the time.
If you have failing tests, or if you interrupt the tests, then you
will have stale files lying around _anyway_: not only test results
are left there, but also trash directories. To remove the trash
directories, you'll need to run 'make clean' (in t/), but that will
remove the test results, too, so there is no difference. But even if
you don't run 'make clean' before running the test suite again, test
results cruft from the previous run doesn't matter, because they will
be overwritten.
Best,
Gábor
^ permalink raw reply
* fetch--tool, was Re: Migrate bisect to C
From: Johannes Schindelin @ 2009-03-14 12:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Christian Couder, Nanako Shiraishi, git, Ingo Molnar,
John Tapsell, Pierre Habouzit
In-Reply-To: <7v63iccydg.fsf@gitster.siamese.dyndns.org>
Hi,
On Sat, 14 Mar 2009, Junio C Hamano wrote:
> Side note. That was how git-fetch--tool was started; it was a
> helper to partially migrate slower parts of git-fetch.sh to C. I
> suspect we can almost remove it but not quite yet...
Funny. I was looking at that last week, and yes, there is still a user.
Through several functions in parse-remote.sh, git-pull.sh uses
fetch--tool.
Now, git-pull.sh is not all that large/complicated...
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] test-lib: write test results to test-results/<basename>-<pid>
From: Johannes Schindelin @ 2009-03-14 11:53 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, gitster, Sverre Rabbelier
In-Reply-To: <20090313172002.GA16232@neumann>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1119 bytes --]
Hi,
On Fri, 13 Mar 2009, SZEDER Gábor wrote:
> diff --git a/t/Makefile b/t/Makefile
> index 0d65ced..2e6e205 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -14,14 +14,11 @@ SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
> T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
> TSVN = $(wildcard t91[0-9][0-9]-*.sh)
>
> -all: pre-clean $(T) aggregate-results clean
> +all: $(T) aggregate-results clean
>
> $(T):
> @echo "*** $@ ***"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
>
> -pre-clean:
> - $(RM) -r test-results
> -
> clean:
> $(RM) -r 'trash directory' test-results
>
> @@ -33,5 +30,5 @@ full-svn-test:
> $(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=1 LC_ALL=C
> $(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=0 LC_ALL=en_US.UTF-8
>
> -.PHONY: pre-clean $(T) aggregate-results clean
> +.PHONY: $(T) aggregate-results clean
> .NOTPARALLEL:
This is wrong. If you have failing tests, or if you interrupt the tests,
it will never clean the test results, and after Hannes' patch you _will_
have stale files lying around all the time.
I'd rather not have this change.
Ciao,
Dscho
^ permalink raw reply
* Re: Transparently encrypt repository contents with GPG
From: Michael J Gruber @ 2009-03-14 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Sverre Rabbelier, Thomas Rast, Michael J Gruber,
Matthias Nothhaft, git, Jeff King
In-Reply-To: <7vy6v9f9zn.fsf@gitster.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 13.03.2009 21:23:
> Michael J Gruber <michaeljgruber+gmane@fastmail.fm> writes:
>
>> In .gitattributes (or.git/info/a..) use
>>
>> * filter=gpg diff=gpg
>>
>> In your config:
>>
>> [filter "gpg"]
>> smudge = gpg -d -q --batch --no-tty
>> clean = gpg -ea -q --batch --no-tty -r C920A124
>> [diff "gpg"]
>> textconv = decrypt
>>
>> This gives you textual diffs even in log! You want use gpg-agent here.
>
> Don't do this.
>
> Think why the smudge/clean pair exists.
>
> The version controlled data, the contents, may not be suitable for
> consumption in the work tree in its verbatim form. For example, a cross
> platform project would want to consistently use LF line termination inside
> a repository, but on a platform whose tools expect CRLF line endings, the
> contents cannot be used verbatim. We "smudge" the contents running
> unix2dos when checking things out on such platforms, and "clean" the
> platform specific CRLF line endings by running dos2unix when checking
> things in. By doing so, you can see what really got changed between
> versions without getting distracted, and more importantly, "you" in this
> sentence is not limited to the human end users alone.
>
> git internally runs diff and xdelta to see what was changed, so that:
>
> * it can reduce storage requirement when it runs pack-objects;
>
> * it can check what path in the preimage was similar to what other path
> in the postimage, to deduce a rename;
>
> * it can check what blocks of lines in the postimage came from what other
> blocks of lines in the preimage, to pass blames across file boundaries.
>
> If your "clean" encrypts and "smudge" decrypts, it means you are refusing
> all the benifit git offers. You are making a pair of similar "smudged"
> contents totally dissimilar in their "clean" counterparts. That is simply
> backwards.
>
> As the sole raison d'etre of diff.textconv is to allow potentially lossy
> conversion (e.g. msword-to-text) applied to the preimage and postimage
> pair of contents (that are supposed to be "clean") before giving a textual
> diff to human consumption, the above config may appear to work, but if you
> really want an encrypted repository, you should be using an encrypting
> filesystem. That would give an added benefit that the work tree
> associated with your repository would also be encrypted.
Exactly. This is why I suggested using cryptfs/luks in my first response
already.
But I don't know the OP's requirements, which is why I also told him how
to do what he wanted, even though it has the drawbacks you and Jeff (and
maybe I) mentioned. Maybe it's an attempt at hosting a semi-private repo
on a public (free) server?
Besides the non-text nature of encrypted content, the problem here is
that d(e(x))=x for all x but e(d(x)) differs from x most probably, and
hopefully randomly, unless you use the right version of debian's openssl
of course ;)
That being said:
git diff calls textconv filters with smudged as well as cleaned files
(when diffing work tree files to blobs), and this does not seem right. I
hope this is not happening with the internal diff, nor with crlf!
Since both the cleaned and the smudged version are supposed to be
"authoritative" (as opposed to the textconv'ed one) one may argue either
way what's the right approach. For internal use comparing the cleaned
versions may make more sense, for displaying diff's the checked-out
form, i.e. smudged versions make more sense.
But that is another topic which would need to be substantiated with
tests. It's not completely unlikely I may come up with some, but don't
count on it...
Cheers,
Michael
^ permalink raw reply
* Re: Migrate bisect to C
From: Junio C Hamano @ 2009-03-14 8:16 UTC (permalink / raw)
To: Christian Couder
Cc: Nanako Shiraishi, git, Ingo Molnar, John Tapsell,
Johannes Schindelin, Pierre Habouzit
In-Reply-To: <200903140846.17599.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> Do you mean that you want this series to migrate both "filter_skipped" and
> "check_good_are_ancestors_of_bad" to C? Or is it ok
> if "check_good_are_ancestors_of_bad" migrates later?
One small step at a time. That's the only sane way we can get there.
> If it is ok to migrate "check_good_are_ancestors_of_bad" later, then I think
> something like the 8/7 patch I posted yesterday might be a good way,
In the final shape, you will be reading from refs/bisect namespace using
for_each_ref(), and at that point you won't have anybody feeding the
skipped from the standard input. The code you would add in [8/7] would
have to be removed if you go that route. If you make the filter_skipped
codepath to read from for_each_ref() during this round, you can still keep
that codepath even after you fully migrate everybody to C, no?
> ...
> That means that at least one <commit-id> should always be passed to "git
> rev-list".
But you do not have to even be tied to rev-list. After all, the partial
migration of filter_skipped is not "git bisect in C", but more like the
first subcommand to "git bisect--helper" command that is written in C and
can be called from shell. The next subcommand might be check_good_are...
and eventually you will have all the necessary and complex pieces the
shell version of "git bisect" currently implements as shell function as
the subcommands of "git bisect--helper". Finally, "git bisect in C" will
then make direct calls to the functions that would implement that "git
bisect--helper" command, and gets rid of the "helper" command altogether.
Side note. That was how git-fetch--tool was started; it was a
helper to partially migrate slower parts of git-fetch.sh to C. I
suspect we can almost remove it but not quite yet...
^ permalink raw reply
* Re: [PATCH] Removed static variables in blame for catering reuse of blame code
From: Junio C Hamano @ 2009-03-14 8:03 UTC (permalink / raw)
To: pi song; +Cc: git, rene.scharfe
In-Reply-To: <49B8E42B.30901@gmail.com>
pi song <pi.songs@gmail.com> writes:
> diff --git a/builtin-blame.c b/builtin-blame.c
> index 2aedd17..2e19ddf 100644
> --- a/builtin-blame.c
> +++ b/builtin-blame.c
> @@ -30,42 +30,29 @@ static const char *blame_opt_usage[] = {
> NULL
> };
>
> -static int longest_file;
> -static int longest_author;
> ...
> -static int num_read_blob;
> -static int num_get_patch;
> -static int num_commits;
I eyeballed the above removals and the additions to the new structure and
they match, which is good.
I'd suggest removing the blame_stat and make its members part of the
super-scoreboard, though.
> /*
> - * Given an origin, prepare mmfile_t structure to be used by the
> - * diff machinery
> - */
> -static void fill_origin_blob(struct origin *o, mmfile_t *file)
> ...
> -static void drop_origin_blob(struct origin *o)
> -{
> - if (o->file.ptr) {
> - free(o->file.ptr);
> - o->file.ptr = NULL;
> - }
> -}
I do not see why you had to have this huge block of removed lines, and
another identcal huge block of added lines later.
> -/*
> * Each group of lines is described by a blame_entry; it can be split
> * as we pass blame to the parents. They form a linked list in the
> * scoreboard structure, sorted by the target line number.
> @@ -176,6 +113,47 @@ struct blame_entry {
> };
>
> /*
> + * stores things that survive across multiple blame operations
> + * 1) for blame specific global parameters
> + * 2) for reusable structures (possibly for optimization purpose)
> + */
> +struct super_scoreboard {
If your goal is to make a blame library out of this, I think "scoreboard"
should be renamed to "blame_scoreboard". And this should be called
something like "blame_info".
> +/*
> + * Given an origin, prepare mmfile_t structure to be used by the
> + * diff machinery
> + */
> +static void fill_origin_blob(struct scoreboard *sb, struct origin *o, mmfile_t *file)
> +{
When the original function did not take "scoreboard", please do *not* add
scoreboard as a new parameter. Instead, make it directly take the overall
blame state "super_scoreboard" (or even narrower subpart of it when you
can). That way, when somebody later study the code, they can immediately
see that such a function does not affect what is in the scoreboard
(i.e. how the blame is passed around).
> +/*
> + * Origin is refcounted and usually we keep the blob contents to be
> + * reused.
> + */
> +static inline struct origin *origin_incref(struct origin *o)
> +...
> +static void drop_origin_blob(struct origin *o)
> +{
> + if (o->file.ptr) {
> + free(o->file.ptr);
> + o->file.ptr = NULL;
> + }
> +}
These look like duplicated/moved for no good reason.
> @@ -1122,17 +1153,21 @@ static void pass_whole_blame(struct scoreboard *sb,
> * "parent" (and "porigin"), but what we mean is to find scapegoat to
> * exonerate ourselves.
> */
> -static struct commit_list *first_scapegoat(struct rev_info *revs, struct commit *commit)
> +static struct commit_list *first_scapegoat(struct scoreboard *sb,
Same comment as fill_origin_blob() applies here.
> -static int num_scapegoats(struct rev_info *revs, struct commit *commit)
> +static int num_scapegoats(struct scoreboard *sb,
Likewise.
> @@ -1274,8 +1309,8 @@ struct commit_info
> /*
> * Parse author/committer line in the commit object buffer
> */
> -static void get_ac_line(const char *inbuf, const char *what,
> - int person_len, char *person,
> +static void get_ac_line(struct scoreboard *sb, const char *inbuf,
Likewise. You only need the mailmap here.
> @@ -1349,7 +1384,8 @@ static void get_ac_line(const char *inbuf, const char *what,
> }
> }
>
> -static void get_commit_info(struct commit *commit,
> +static void get_commit_info(struct scoreboard *sb,
Likewise.
> @@ -1430,7 +1466,8 @@ static void write_filename_info(const char *path)
> * commit. Instead of repeating this every line, emit it only once,
> * the first time each commit appears in the output.
> */
> -static int emit_one_suspect_detail(struct origin *suspect)
> +static int emit_one_suspect_detail(struct scoreboard *sb,
> + struct origin *suspect)
Likewise.
> @@ -1462,18 +1499,18 @@ static int emit_one_suspect_detail(struct origin *suspect)
> * The blame_entry is found to be guilty for the range. Mark it
> * as such, and show it in incremental output.
> */
> -static void found_guilty_entry(struct blame_entry *ent)
> +static void found_guilty_entry(struct scoreboard *sb, struct blame_entry *ent)
Likewise.
> @@ -1532,8 +1569,8 @@ static void assign_blame(struct scoreboard *sb, int opt)
> }
> }
>
> -static const char *format_time(unsigned long time, const char *tz_str,
> - int show_raw_time)
> +static const char *format_time(struct scoreboard *sb, unsigned long time,
Likewise.
Here is a fixed-up replacement along the lines I suggested above, except
that I did not bother getting rid of "struct blame_stat".
I've also fixed up a few style violations I noticed and fixed up the
commit log message a bit.
-- >8 --
From: pi song <pi.songs@gmail.com>
Date: Thu, 12 Mar 2009 21:30:03 +1100
Subject: [PATCH] blame.c: start libifying the blame infrastructure
The patch refactors builtin-blame.c to not rely on static variables by
introducing a "blame_info" structure and moving them into it. While at
it, "scoreboard" is renamed to "blame_scoreboard".
We can later use a same blame_info that holds the settings with different
scoreboards to run blame multiple times on different data.
Signed-off-by: Pi Song <pi.songs@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-blame.c | 365 +++++++++++++++++++++++++++++++------------------------
1 files changed, 205 insertions(+), 160 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index 2aedd17..efac5d4 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -30,42 +30,29 @@ static const char *blame_opt_usage[] = {
NULL
};
-static int longest_file;
-static int longest_author;
-static int max_orig_digits;
-static int max_digits;
-static int max_score_digits;
-static int show_root;
-static int reverse;
-static int blank_boundary;
-static int incremental;
-static int xdl_opts = XDF_NEED_MINIMAL;
-
-static enum date_mode blame_date_mode = DATE_ISO8601;
-static size_t blame_date_width;
-
-static struct string_list mailmap;
+/* only use for command line parameter parsing */
+static unsigned opt_blame_move_score;
1+static unsigned opt_blame_copy_score;
#ifndef DEBUG
#define DEBUG 0
#endif
-/* stats */
-static int num_read_blob;
-static int num_get_patch;
-static int num_commits;
+/*
+ * for storing stats. it can be used
+ * across multiple blame operations
+ */
+struct blame_stat {
+ int num_read_blob;
+ int num_get_patch;
+ int num_commits;
+};
#define PICKAXE_BLAME_MOVE 01
#define PICKAXE_BLAME_COPY 02
#define PICKAXE_BLAME_COPY_HARDER 04
#define PICKAXE_BLAME_COPY_HARDEST 010
-/*
- * blame for a blame_entry with score lower than these thresholds
- * is not passed to the parent using move/copy logic.
- */
-static unsigned blame_move_score;
-static unsigned blame_copy_score;
#define BLAME_DEFAULT_MOVE_SCORE 20
#define BLAME_DEFAULT_COPY_SCORE 40
@@ -86,14 +73,55 @@ struct origin {
};
/*
+ * stores things that survive across multiple blame operations
+ * 1) for blame specific global parameters
+ * 2) for reusable structures (possibly for optimization purpose)
+ */
+struct blame_info {
+ /*
+ * miscellaneous parameters collected during processing
+ * for pretty formatting purpose
+ */
+ int longest_file;
+ int longest_author;
+ int max_orig_digits;
+ int max_digits;
+ int max_score_digits;
+
+ /* formatting parameters */
+ int show_root;
+ int reverse;
+ int blank_boundary;
+ int incremental;
+ int xdl_opts;
+
+ /*
+ * blame for a blame_entry with score lower than these thresholds
+ * is not passed to the parent using move/copy logic.
+ */
+ unsigned blame_move_score;
+ unsigned blame_copy_score;
+
+ /* date formatting related */
+ enum date_mode blame_date_mode;
+ size_t blame_date_width;
+
+ /* for fast mailmap lookup */
+ struct string_list mailmap;
+
+ /* for stat collecting purpose */
+ struct blame_stat *stat;
+};
+
+/*
* Given an origin, prepare mmfile_t structure to be used by the
* diff machinery
*/
-static void fill_origin_blob(struct origin *o, mmfile_t *file)
+static void fill_origin_blob(struct blame_info *ssb, struct origin *o, mmfile_t *file)
{
if (!o->file.ptr) {
enum object_type type;
- num_read_blob++;
+ ssb->stat->num_read_blob++;
file->ptr = read_sha1_file(o->blob_sha1, &type,
(unsigned long *)(&(file->size)));
if (!file->ptr)
@@ -178,7 +206,7 @@ struct blame_entry {
/*
* The current state of the blame assignment.
*/
-struct scoreboard {
+struct blame_scoreboard {
/* the final commit (i.e. where we started digging from) */
struct commit *final;
struct rev_info *revs;
@@ -198,6 +226,8 @@ struct scoreboard {
/* look-up a line in the final buffer */
int num_lines;
int *lineno;
+
+ struct blame_info *ssb;
};
static inline int same_suspect(struct origin *a, struct origin *b)
@@ -209,14 +239,14 @@ static inline int same_suspect(struct origin *a, struct origin *b)
return !strcmp(a->path, b->path);
}
-static void sanity_check_refcnt(struct scoreboard *);
+static void sanity_check_refcnt(struct blame_scoreboard *);
/*
* If two blame entries that are next to each other came from
* contiguous lines in the same origin (i.e. <commit, path> pair),
* merge them together.
*/
-static void coalesce(struct scoreboard *sb)
+static void coalesce(struct blame_scoreboard *sb)
{
struct blame_entry *ent, *next;
@@ -258,7 +288,7 @@ static struct origin *make_origin(struct commit *commit, const char *path)
/*
* Locate an existing origin or create a new one.
*/
-static struct origin *get_origin(struct scoreboard *sb,
+static struct origin *get_origin(struct blame_scoreboard *sb,
struct commit *commit,
const char *path)
{
@@ -301,7 +331,7 @@ static int fill_blob_sha1(struct origin *origin)
* We have an origin -- check if the same path exists in the
* parent and return an origin structure to represent it.
*/
-static struct origin *find_origin(struct scoreboard *sb,
+static struct origin *find_origin(struct blame_scoreboard *sb,
struct commit *parent,
struct origin *origin)
{
@@ -409,7 +439,7 @@ static struct origin *find_origin(struct scoreboard *sb,
* We have an origin -- find the path that corresponds to it in its
* parent and return an origin structure to represent it.
*/
-static struct origin *find_rename(struct scoreboard *sb,
+static struct origin *find_rename(struct blame_scoreboard *sb,
struct commit *parent,
struct origin *origin)
{
@@ -454,7 +484,7 @@ static struct origin *find_rename(struct scoreboard *sb,
* Link in a new blame entry to the scoreboard. Entries that cover the
* same line range have been removed from the scoreboard previously.
*/
-static void add_blame_entry(struct scoreboard *sb, struct blame_entry *e)
+static void add_blame_entry(struct blame_scoreboard *sb, struct blame_entry *e)
{
struct blame_entry *ent, *prev = NULL;
@@ -497,7 +527,7 @@ static void dup_entry(struct blame_entry *dst, struct blame_entry *src)
dst->score = 0;
}
-static const char *nth_line(struct scoreboard *sb, int lno)
+static const char *nth_line(struct blame_scoreboard *sb, int lno)
{
return sb->final_buf + sb->lineno[lno];
}
@@ -564,7 +594,7 @@ static void split_overlap(struct blame_entry *split,
* in split. Adjust the linked list of blames in the scoreboard to
* reflect the split.
*/
-static void split_blame(struct scoreboard *sb,
+static void split_blame(struct blame_scoreboard *sb,
struct blame_entry *split,
struct blame_entry *e)
{
@@ -646,7 +676,7 @@ static void decref_split(struct blame_entry *split)
* Helper for blame_chunk(). blame_entry e is known to overlap with
* the patch hunk; split it and pass blame to the parent.
*/
-static void blame_overlap(struct scoreboard *sb, struct blame_entry *e,
+static void blame_overlap(struct blame_scoreboard *sb, struct blame_entry *e,
int tlno, int plno, int same,
struct origin *parent)
{
@@ -661,7 +691,7 @@ static void blame_overlap(struct scoreboard *sb, struct blame_entry *e,
/*
* Find the line number of the last line the target is suspected for.
*/
-static int find_last_in_target(struct scoreboard *sb, struct origin *target)
+static int find_last_in_target(struct blame_scoreboard *sb, struct origin *target)
{
struct blame_entry *e;
int last_in_target = -1;
@@ -680,7 +710,7 @@ static int find_last_in_target(struct scoreboard *sb, struct origin *target)
* blame_entry e and its parent. Find and split the overlap, and
* pass blame to the overlapping part to the parent.
*/
-static void blame_chunk(struct scoreboard *sb,
+static void blame_chunk(struct blame_scoreboard *sb,
int tlno, int plno, int same,
struct origin *target, struct origin *parent)
{
@@ -697,7 +727,7 @@ static void blame_chunk(struct scoreboard *sb,
}
struct blame_chunk_cb_data {
- struct scoreboard *sb;
+ struct blame_scoreboard *sb;
struct origin *target;
struct origin *parent;
long plno;
@@ -717,7 +747,7 @@ static void blame_chunk_cb(void *data, long same, long p_next, long t_next)
* for the lines it is suspected to its parent. Run diff to find
* which lines came from parent and pass blame for them.
*/
-static int pass_blame_to_parent(struct scoreboard *sb,
+static int pass_blame_to_parent(struct blame_scoreboard *sb,
struct origin *target,
struct origin *parent)
{
@@ -731,12 +761,12 @@ static int pass_blame_to_parent(struct scoreboard *sb,
if (last_in_target < 0)
return 1; /* nothing remains for this target */
- fill_origin_blob(parent, &file_p);
- fill_origin_blob(target, &file_o);
- num_get_patch++;
+ fill_origin_blob(sb->ssb, parent, &file_p);
+ fill_origin_blob(sb->ssb, target, &file_o);
+ sb->ssb->stat->num_get_patch++;
memset(&xpp, 0, sizeof(xpp));
- xpp.flags = xdl_opts;
+ xpp.flags = sb->ssb->xdl_opts;
memset(&xecfg, 0, sizeof(xecfg));
xecfg.ctxlen = 0;
xdi_diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, &xpp, &xecfg);
@@ -755,7 +785,7 @@ static int pass_blame_to_parent(struct scoreboard *sb,
*
* Compute how trivial the lines in the blame_entry are.
*/
-static unsigned ent_score(struct scoreboard *sb, struct blame_entry *e)
+static unsigned ent_score(struct blame_scoreboard *sb, struct blame_entry *e)
{
unsigned score;
const char *cp, *ep;
@@ -782,7 +812,7 @@ static unsigned ent_score(struct scoreboard *sb, struct blame_entry *e)
* so far, by comparing this and best_so_far and copying this into
* bst_so_far as needed.
*/
-static void copy_split_if_better(struct scoreboard *sb,
+static void copy_split_if_better(struct blame_scoreboard *sb,
struct blame_entry *best_so_far,
struct blame_entry *this)
{
@@ -816,7 +846,7 @@ static void copy_split_if_better(struct scoreboard *sb,
*
* All line numbers are 0-based.
*/
-static void handle_split(struct scoreboard *sb,
+static void handle_split(struct blame_scoreboard *sb,
struct blame_entry *ent,
int tlno, int plno, int same,
struct origin *parent,
@@ -835,7 +865,7 @@ static void handle_split(struct scoreboard *sb,
}
struct handle_split_cb_data {
- struct scoreboard *sb;
+ struct blame_scoreboard *sb;
struct blame_entry *ent;
struct origin *parent;
struct blame_entry *split;
@@ -856,7 +886,7 @@ static void handle_split_cb(void *data, long same, long p_next, long t_next)
* we can pass blames to it. file_p has the blob contents for
* the parent.
*/
-static void find_copy_in_blob(struct scoreboard *sb,
+static void find_copy_in_blob(struct blame_scoreboard *sb,
struct blame_entry *ent,
struct origin *parent,
struct blame_entry *split,
@@ -887,7 +917,7 @@ static void find_copy_in_blob(struct scoreboard *sb,
* file_p partially may match that image.
*/
memset(&xpp, 0, sizeof(xpp));
- xpp.flags = xdl_opts;
+ xpp.flags = sb->ssb->xdl_opts;
memset(&xecfg, 0, sizeof(xecfg));
xecfg.ctxlen = 1;
memset(split, 0, sizeof(struct blame_entry [3]));
@@ -900,7 +930,7 @@ static void find_copy_in_blob(struct scoreboard *sb,
* See if lines currently target is suspected for can be attributed to
* parent.
*/
-static int find_move_in_parent(struct scoreboard *sb,
+static int find_move_in_parent(struct blame_scoreboard *sb,
struct origin *target,
struct origin *parent)
{
@@ -912,7 +942,7 @@ static int find_move_in_parent(struct scoreboard *sb,
if (last_in_target < 0)
return 1; /* nothing remains for this target */
- fill_origin_blob(parent, &file_p);
+ fill_origin_blob(sb->ssb, parent, &file_p);
if (!file_p.ptr)
return 0;
@@ -921,11 +951,11 @@ static int find_move_in_parent(struct scoreboard *sb,
made_progress = 0;
for (e = sb->ent; e; e = e->next) {
if (e->guilty || !same_suspect(e->suspect, target) ||
- ent_score(sb, e) < blame_move_score)
+ ent_score(sb, e) < sb->ssb->blame_move_score)
continue;
find_copy_in_blob(sb, e, parent, split, &file_p);
if (split[1].suspect &&
- blame_move_score < ent_score(sb, &split[1])) {
+ sb->ssb->blame_move_score < ent_score(sb, &split[1])) {
split_blame(sb, split, e);
made_progress = 1;
}
@@ -944,7 +974,7 @@ struct blame_list {
* Count the number of entries the target is suspected for,
* and prepare a list of entry and the best split.
*/
-static struct blame_list *setup_blame_list(struct scoreboard *sb,
+static struct blame_list *setup_blame_list(struct blame_scoreboard *sb,
struct origin *target,
int min_score,
int *num_ents_p)
@@ -973,7 +1003,7 @@ static struct blame_list *setup_blame_list(struct scoreboard *sb,
/*
* Reset the scanned status on all entries.
*/
-static void reset_scanned_flag(struct scoreboard *sb)
+static void reset_scanned_flag(struct blame_scoreboard *sb)
{
struct blame_entry *e;
for (e = sb->ent; e; e = e->next)
@@ -985,7 +1015,7 @@ static void reset_scanned_flag(struct scoreboard *sb)
* across file boundary from the parent commit. porigin is the path
* in the parent we already tried.
*/
-static int find_copy_in_parent(struct scoreboard *sb,
+static int find_copy_in_parent(struct blame_scoreboard *sb,
struct origin *target,
struct commit *parent,
struct origin *porigin,
@@ -998,7 +1028,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
struct blame_list *blame_list;
int num_ents;
- blame_list = setup_blame_list(sb, target, blame_copy_score, &num_ents);
+ blame_list = setup_blame_list(sb, target, sb->ssb->blame_copy_score, &num_ents);
if (!blame_list)
return 1; /* nothing remains for this target */
@@ -1053,7 +1083,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
norigin = get_origin(sb, parent, p->one->path);
hashcpy(norigin->blob_sha1, p->one->sha1);
- fill_origin_blob(norigin, &file_p);
+ fill_origin_blob(sb->ssb, norigin, &file_p);
if (!file_p.ptr)
continue;
@@ -1070,7 +1100,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
for (j = 0; j < num_ents; j++) {
struct blame_entry *split = blame_list[j].split;
if (split[1].suspect &&
- blame_copy_score < ent_score(sb, &split[1])) {
+ sb->ssb->blame_copy_score < ent_score(sb, &split[1])) {
split_blame(sb, split, blame_list[j].ent);
made_progress = 1;
}
@@ -1082,7 +1112,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
if (!made_progress)
break;
- blame_list = setup_blame_list(sb, target, blame_copy_score, &num_ents);
+ blame_list = setup_blame_list(sb, target, sb->ssb->blame_copy_score, &num_ents);
if (!blame_list) {
retval = 1;
break;
@@ -1098,7 +1128,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
* The blobs of origin and porigin exactly match, so everything
* origin is suspected for can be blamed on the parent.
*/
-static void pass_whole_blame(struct scoreboard *sb,
+static void pass_whole_blame(struct blame_scoreboard *sb,
struct origin *origin, struct origin *porigin)
{
struct blame_entry *e;
@@ -1122,17 +1152,21 @@ static void pass_whole_blame(struct scoreboard *sb,
* "parent" (and "porigin"), but what we mean is to find scapegoat to
* exonerate ourselves.
*/
-static struct commit_list *first_scapegoat(struct rev_info *revs, struct commit *commit)
+static struct commit_list *first_scapegoat(struct blame_info *ssb,
+ struct rev_info *revs,
+ struct commit *commit)
{
- if (!reverse)
+ if (!ssb->reverse)
return commit->parents;
return lookup_decoration(&revs->children, &commit->object);
}
-static int num_scapegoats(struct rev_info *revs, struct commit *commit)
+static int num_scapegoats(struct blame_info *ssb,
+ struct rev_info *revs,
+ struct commit *commit)
{
int cnt;
- struct commit_list *l = first_scapegoat(revs, commit);
+ struct commit_list *l = first_scapegoat(ssb, revs, commit);
for (cnt = 0; l; l = l->next)
cnt++;
return cnt;
@@ -1140,7 +1174,7 @@ static int num_scapegoats(struct rev_info *revs, struct commit *commit)
#define MAXSG 16
-static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
+static void pass_blame(struct blame_scoreboard *sb, struct origin *origin, int opt)
{
struct rev_info *revs = sb->revs;
int i, pass, num_sg;
@@ -1149,7 +1183,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
struct origin *sg_buf[MAXSG];
struct origin *porigin, **sg_origin = sg_buf;
- num_sg = num_scapegoats(revs, commit);
+ num_sg = num_scapegoats(sb->ssb, revs, commit);
if (!num_sg)
goto finish;
else if (num_sg < ARRAY_SIZE(sg_buf))
@@ -1162,11 +1196,11 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
* common cases, then we look for renames in the second pass.
*/
for (pass = 0; pass < 2; pass++) {
- struct origin *(*find)(struct scoreboard *,
+ struct origin *(*find)(struct blame_scoreboard *,
struct commit *, struct origin *);
find = pass ? find_rename : find_origin;
- for (i = 0, sg = first_scapegoat(revs, commit);
+ for (i = 0, sg = first_scapegoat(sb->ssb, revs, commit);
i < num_sg && sg;
sg = sg->next, i++) {
struct commit *p = sg->item;
@@ -1198,8 +1232,8 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
}
}
- num_commits++;
- for (i = 0, sg = first_scapegoat(revs, commit);
+ sb->ssb->stat->num_commits++;
+ for (i = 0, sg = first_scapegoat(sb->ssb, revs, commit);
i < num_sg && sg;
sg = sg->next, i++) {
struct origin *porigin = sg_origin[i];
@@ -1217,7 +1251,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
* Optionally find moves in parents' files.
*/
if (opt & PICKAXE_BLAME_MOVE)
- for (i = 0, sg = first_scapegoat(revs, commit);
+ for (i = 0, sg = first_scapegoat(sb->ssb, revs, commit);
i < num_sg && sg;
sg = sg->next, i++) {
struct origin *porigin = sg_origin[i];
@@ -1231,7 +1265,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
* Optionally find copies from parents' files.
*/
if (opt & PICKAXE_BLAME_COPY)
- for (i = 0, sg = first_scapegoat(revs, commit);
+ for (i = 0, sg = first_scapegoat(sb->ssb, revs, commit);
i < num_sg && sg;
sg = sg->next, i++) {
struct origin *porigin = sg_origin[i];
@@ -1274,8 +1308,8 @@ struct commit_info
/*
* Parse author/committer line in the commit object buffer
*/
-static void get_ac_line(const char *inbuf, const char *what,
- int person_len, char *person,
+static void get_ac_line(struct string_list *mailmap, const char *inbuf,
+ const char *what, int person_len, char *person,
int mail_len, char *mail,
unsigned long *time, const char **tz)
{
@@ -1323,7 +1357,7 @@ static void get_ac_line(const char *inbuf, const char *what,
maillen = timepos - tmp;
memcpy(mail, mailpos, maillen);
- if (!mailmap.nr)
+ if (!mailmap->nr)
return;
/*
@@ -1338,7 +1372,7 @@ static void get_ac_line(const char *inbuf, const char *what,
/*
* Now, convert both name and e-mail using mailmap
*/
- if(map_user(&mailmap, mail+1, mail_len-1, person, tmp-person-1)) {
+ if (map_user(mailmap, mail+1, mail_len-1, person, tmp-person-1)) {
/* Add a trailing '>' to email, since map_user returns plain emails
Note: It already has '<', since we replace from mail+1 */
mailpos = memchr(mail, '\0', mail_len);
@@ -1349,7 +1383,8 @@ static void get_ac_line(const char *inbuf, const char *what,
}
}
-static void get_commit_info(struct commit *commit,
+static void get_commit_info(struct string_list *mailmap,
+ struct commit *commit,
struct commit_info *ret,
int detailed)
{
@@ -1378,7 +1413,7 @@ static void get_commit_info(struct commit *commit,
message = reencoded ? reencoded : commit->buffer;
ret->author = author_name;
ret->author_mail = author_mail;
- get_ac_line(message, "\nauthor ",
+ get_ac_line(mailmap, message, "\nauthor ",
sizeof(author_name), author_name,
sizeof(author_mail), author_mail,
&ret->author_time, &ret->author_tz);
@@ -1390,7 +1425,7 @@ static void get_commit_info(struct commit *commit,
ret->committer = committer_name;
ret->committer_mail = committer_mail;
- get_ac_line(message, "\ncommitter ",
+ get_ac_line(mailmap, message, "\ncommitter ",
sizeof(committer_name), committer_name,
sizeof(committer_mail), committer_mail,
&ret->committer_time, &ret->committer_tz);
@@ -1430,7 +1465,8 @@ static void write_filename_info(const char *path)
* commit. Instead of repeating this every line, emit it only once,
* the first time each commit appears in the output.
*/
-static int emit_one_suspect_detail(struct origin *suspect)
+static int emit_one_suspect_detail(struct string_list *mailmap,
+ struct origin *suspect)
{
struct commit_info ci;
@@ -1438,7 +1474,7 @@ static int emit_one_suspect_detail(struct origin *suspect)
return 0;
suspect->commit->object.flags |= METAINFO_SHOWN;
- get_commit_info(suspect->commit, &ci, 1);
+ get_commit_info(mailmap, suspect->commit, &ci, 1);
printf("author %s\n", ci.author);
printf("author-mail %s\n", ci.author_mail);
printf("author-time %lu\n", ci.author_time);
@@ -1462,18 +1498,18 @@ static int emit_one_suspect_detail(struct origin *suspect)
* The blame_entry is found to be guilty for the range. Mark it
* as such, and show it in incremental output.
*/
-static void found_guilty_entry(struct blame_entry *ent)
+static void found_guilty_entry(struct blame_info *ssb, struct blame_entry *ent)
{
if (ent->guilty)
return;
ent->guilty = 1;
- if (incremental) {
+ if (ssb->incremental) {
struct origin *suspect = ent->suspect;
printf("%s %d %d %d\n",
sha1_to_hex(suspect->commit->object.sha1),
ent->s_lno + 1, ent->lno + 1, ent->num_lines);
- emit_one_suspect_detail(suspect);
+ emit_one_suspect_detail(&ssb->mailmap, suspect);
write_filename_info(suspect->path);
maybe_flush_or_die(stdout, "stdout");
}
@@ -1484,7 +1520,7 @@ static void found_guilty_entry(struct blame_entry *ent)
* is still unknown, pick one blame_entry, and allow its current
* suspect to pass blames to its parents.
*/
-static void assign_blame(struct scoreboard *sb, int opt)
+static void assign_blame(struct blame_scoreboard *sb, int opt)
{
struct rev_info *revs = sb->revs;
@@ -1508,7 +1544,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
commit = suspect->commit;
if (!commit->object.parsed)
parse_commit(commit);
- if (reverse ||
+ if (sb->ssb->reverse ||
(!(commit->object.flags & UNINTERESTING) &&
!(revs->max_age != -1 && commit->date < revs->max_age)))
pass_blame(sb, suspect, opt);
@@ -1518,13 +1554,13 @@ static void assign_blame(struct scoreboard *sb, int opt)
mark_parents_uninteresting(commit);
}
/* treat root commit as boundary */
- if (!commit->parents && !show_root)
+ if (!commit->parents && !sb->ssb->show_root)
commit->object.flags |= UNINTERESTING;
/* Take responsibility for the remaining entries */
for (ent = sb->ent; ent; ent = ent->next)
if (same_suspect(ent->suspect, suspect))
- found_guilty_entry(ent);
+ found_guilty_entry(sb->ssb, ent);
origin_decref(suspect);
if (DEBUG) /* sanity */
@@ -1532,8 +1568,8 @@ static void assign_blame(struct scoreboard *sb, int opt)
}
}
-static const char *format_time(unsigned long time, const char *tz_str,
- int show_raw_time)
+static const char *format_time(struct blame_info *ssb, unsigned long time,
+ const char *tz_str, int show_raw_time)
{
static char time_buf[128];
const char *time_str;
@@ -1545,10 +1581,10 @@ static const char *format_time(unsigned long time, const char *tz_str,
}
else {
tz = atoi(tz_str);
- time_str = show_date(time, tz, blame_date_mode);
+ time_str = show_date(time, tz, ssb->blame_date_mode);
time_len = strlen(time_str);
memcpy(time_buf, time_str, time_len);
- memset(time_buf + time_len, ' ', blame_date_width - time_len);
+ memset(time_buf + time_len, ' ', ssb->blame_date_width - time_len);
}
return time_buf;
}
@@ -1562,7 +1598,7 @@ static const char *format_time(unsigned long time, const char *tz_str,
#define OUTPUT_SHOW_SCORE 0100
#define OUTPUT_NO_AUTHOR 0200
-static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent)
+static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent)
{
int cnt;
const char *cp;
@@ -1576,7 +1612,7 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent)
ent->s_lno + 1,
ent->lno + 1,
ent->num_lines);
- if (emit_one_suspect_detail(suspect) ||
+ if (emit_one_suspect_detail(&sb->ssb->mailmap, suspect) ||
(suspect->commit->object.flags & MORE_THAN_ONE_PATH))
write_filename_info(suspect->path);
@@ -1596,7 +1632,7 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent)
}
}
-static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
+static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int opt)
{
int cnt;
const char *cp;
@@ -1605,7 +1641,7 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
char hex[41];
int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
- get_commit_info(suspect->commit, &ci, 1);
+ get_commit_info(&sb->ssb->mailmap, suspect->commit, &ci, 1);
strcpy(hex, sha1_to_hex(suspect->commit->object.sha1));
cp = nth_line(sb, ent->lno);
@@ -1614,7 +1650,7 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? 40 : 8;
if (suspect->commit->object.flags & UNINTERESTING) {
- if (blank_boundary)
+ if (sb->ssb->blank_boundary)
memset(hex, ' ', length);
else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {
length--;
@@ -1625,31 +1661,31 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
printf("%.*s", length, hex);
if (opt & OUTPUT_ANNOTATE_COMPAT)
printf("\t(%10s\t%10s\t%d)", ci.author,
- format_time(ci.author_time, ci.author_tz,
- show_raw_time),
+ format_time(sb->ssb, ci.author_time,
+ ci.author_tz, show_raw_time),
ent->lno + 1 + cnt);
else {
if (opt & OUTPUT_SHOW_SCORE)
printf(" %*d %02d",
- max_score_digits, ent->score,
+ sb->ssb->max_score_digits, ent->score,
ent->suspect->refcnt);
if (opt & OUTPUT_SHOW_NAME)
- printf(" %-*.*s", longest_file, longest_file,
- suspect->path);
+ printf(" %-*.*s", sb->ssb->longest_file,
+ sb->ssb->longest_file, suspect->path);
if (opt & OUTPUT_SHOW_NUMBER)
- printf(" %*d", max_orig_digits,
+ printf(" %*d", sb->ssb->max_orig_digits,
ent->s_lno + 1 + cnt);
if (!(opt & OUTPUT_NO_AUTHOR)) {
- int pad = longest_author - utf8_strwidth(ci.author);
+ int pad = sb->ssb->longest_author - utf8_strwidth(ci.author);
printf(" (%s%*s %10s",
ci.author, pad, "",
- format_time(ci.author_time,
+ format_time(sb->ssb, ci.author_time,
ci.author_tz,
show_raw_time));
}
printf(" %*d) ",
- max_digits, ent->lno + 1 + cnt);
+ sb->ssb->max_digits, ent->lno + 1 + cnt);
}
do {
ch = *cp++;
@@ -1659,7 +1695,7 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
}
}
-static void output(struct scoreboard *sb, int option)
+static void output(struct blame_scoreboard *sb, int option)
{
struct blame_entry *ent;
@@ -1693,7 +1729,7 @@ static void output(struct scoreboard *sb, int option)
* To allow quick access to the contents of nth line in the
* final image, prepare an index in the scoreboard.
*/
-static int prepare_lines(struct scoreboard *sb)
+static int prepare_lines(struct blame_scoreboard *sb)
{
const char *buf = sb->final_buf;
unsigned long len = sb->final_buf_size;
@@ -1758,7 +1794,7 @@ static int lineno_width(int lines)
* How many columns do we need to show line numbers, authors,
* and filenames?
*/
-static void find_alignment(struct scoreboard *sb, int *option)
+static void find_alignment(struct blame_scoreboard *sb, int *option)
{
int longest_src_lines = 0;
int longest_dst_lines = 0;
@@ -1773,14 +1809,14 @@ static void find_alignment(struct scoreboard *sb, int *option)
if (strcmp(suspect->path, sb->path))
*option |= OUTPUT_SHOW_NAME;
num = strlen(suspect->path);
- if (longest_file < num)
- longest_file = num;
+ if (sb->ssb->longest_file < num)
+ sb->ssb->longest_file = num;
if (!(suspect->commit->object.flags & METAINFO_SHOWN)) {
suspect->commit->object.flags |= METAINFO_SHOWN;
- get_commit_info(suspect->commit, &ci, 1);
+ get_commit_info(&sb->ssb->mailmap, suspect->commit, &ci, 1);
num = utf8_strwidth(ci.author);
- if (longest_author < num)
- longest_author = num;
+ if (sb->ssb->longest_author < num)
+ sb->ssb->longest_author = num;
}
num = e->s_lno + e->num_lines;
if (longest_src_lines < num)
@@ -1791,16 +1827,16 @@ static void find_alignment(struct scoreboard *sb, int *option)
if (largest_score < ent_score(sb, e))
largest_score = ent_score(sb, e);
}
- max_orig_digits = lineno_width(longest_src_lines);
- max_digits = lineno_width(longest_dst_lines);
- max_score_digits = lineno_width(largest_score);
+ sb->ssb->max_orig_digits = lineno_width(longest_src_lines);
+ sb->ssb->max_digits = lineno_width(longest_dst_lines);
+ sb->ssb->max_score_digits = lineno_width(largest_score);
}
/*
* For debugging -- origin is refcounted, and this asserts that
* we do not underflow.
*/
-static void sanity_check_refcnt(struct scoreboard *sb)
+static void sanity_check_refcnt(struct blame_scoreboard *sb)
{
int baa = 0;
struct blame_entry *ent;
@@ -1851,7 +1887,7 @@ static const char *add_prefix(const char *prefix, const char *path)
* Parsing of (comma separated) one item in the -L option
*/
static const char *parse_loc(const char *spec,
- struct scoreboard *sb, long lno,
+ struct blame_scoreboard *sb, long lno,
long begin, long *ret)
{
char *term;
@@ -1927,7 +1963,7 @@ static const char *parse_loc(const char *spec,
/*
* Parsing of -L option
*/
-static void prepare_blame_range(struct scoreboard *sb,
+static void prepare_blame_range(struct blame_scoreboard *sb,
const char *bottomtop,
long lno,
long *bottom, long *top)
@@ -1947,17 +1983,17 @@ static void prepare_blame_range(struct scoreboard *sb,
static int git_blame_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "blame.showroot")) {
- show_root = git_config_bool(var, value);
+ ((struct blame_info *)cb)->show_root = git_config_bool(var, value);
return 0;
}
if (!strcmp(var, "blame.blankboundary")) {
- blank_boundary = git_config_bool(var, value);
+ ((struct blame_info *)cb)->blank_boundary = git_config_bool(var, value);
return 0;
}
if (!strcmp(var, "blame.date")) {
if (!value)
return config_error_nonbool(var);
- blame_date_mode = parse_date_format(value);
+ ((struct blame_info *)cb)->blame_date_mode = parse_date_format(value);
return 0;
}
return git_default_config(var, value, cb);
@@ -2079,7 +2115,7 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
return commit;
}
-static const char *prepare_final(struct scoreboard *sb)
+static const char *prepare_final(struct blame_scoreboard *sb)
{
int i;
const char *final_commit_name = NULL;
@@ -2107,7 +2143,7 @@ static const char *prepare_final(struct scoreboard *sb)
return final_commit_name;
}
-static const char *prepare_initial(struct scoreboard *sb)
+static const char *prepare_initial(struct blame_scoreboard *sb)
{
int i;
const char *final_commit_name = NULL;
@@ -2155,7 +2191,7 @@ static int blame_copy_callback(const struct option *option, const char *arg, int
*opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
if (arg)
- blame_copy_score = parse_score(arg);
+ opt_blame_copy_score = parse_score(arg);
return 0;
}
@@ -2166,7 +2202,7 @@ static int blame_move_callback(const struct option *option, const char *arg, int
*opt |= PICKAXE_BLAME_MOVE;
if (arg)
- blame_move_score = parse_score(arg);
+ opt_blame_move_score = parse_score(arg);
return 0;
}
@@ -2185,9 +2221,15 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
{
struct rev_info revs;
const char *path;
- struct scoreboard sb;
struct origin *o;
struct blame_entry *ent;
+ static struct blame_stat stat;
+ static struct blame_info ssb = {
+ .xdl_opts = XDF_NEED_MINIMAL,
+ .blame_date_mode = DATE_ISO8601,
+ .stat = &stat
+ };
+ static struct blame_scoreboard sb;
long dashdash_pos, bottom, top, lno;
const char *final_commit_name = NULL;
enum object_type type;
@@ -2198,9 +2240,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
static const char *revs_file = NULL;
static const char *contents_from = NULL;
static const struct option options[] = {
- OPT_BOOLEAN(0, "incremental", &incremental, "Show blame entries as we find them, incrementally"),
- OPT_BOOLEAN('b', NULL, &blank_boundary, "Show blank SHA-1 for boundary commits (Default: off)"),
- OPT_BOOLEAN(0, "root", &show_root, "Do not treat root commits as boundaries (Default: off)"),
+ OPT_BOOLEAN(0, "incremental", &ssb.incremental, "Show blame entries as we find them, incrementally"),
+ OPT_BOOLEAN('b', NULL, &ssb.blank_boundary, "Show blank SHA-1 for boundary commits (Default: off)"),
+ OPT_BOOLEAN(0, "root", &ssb.show_root, "Do not treat root commits as boundaries (Default: off)"),
OPT_BOOLEAN(0, "show-stats", &show_stats, "Show work cost statistics"),
OPT_BIT(0, "score-debug", &output_option, "Show output score for blame entries", OUTPUT_SHOW_SCORE),
OPT_BIT('f', "show-name", &output_option, "Show original filename (Default: auto)", OUTPUT_SHOW_NAME),
@@ -2210,7 +2252,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
OPT_BIT('t', NULL, &output_option, "Show raw timestamp (Default: off)", OUTPUT_RAW_TIMESTAMP),
OPT_BIT('l', NULL, &output_option, "Show long commit SHA1 (Default: off)", OUTPUT_LONG_OBJECT_NAME),
OPT_BIT('s', NULL, &output_option, "Suppress author name and timestamp (Default: off)", OUTPUT_NO_AUTHOR),
- OPT_BIT('w', NULL, &xdl_opts, "Ignore whitespace differences", XDF_IGNORE_WHITESPACE),
+ OPT_BIT('w', NULL, &ssb.xdl_opts, "Ignore whitespace differences", XDF_IGNORE_WHITESPACE),
OPT_STRING('S', NULL, &revs_file, "file", "Use revisions from <file> instead of calling git-rev-list"),
OPT_STRING(0, "contents", &contents_from, "file", "Use <file>'s contents as the final image"),
{ OPTION_CALLBACK, 'C', NULL, &opt, "score", "Find line copies within and across files", PARSE_OPT_OPTARG, blame_copy_callback },
@@ -2222,9 +2264,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
struct parse_opt_ctx_t ctx;
int cmd_is_annotate = !strcmp(argv[0], "annotate");
- git_config(git_blame_config, NULL);
+ git_config(git_blame_config, &ssb);
init_revisions(&revs, NULL);
- revs.date_mode = blame_date_mode;
+ revs.date_mode = ssb.blame_date_mode;
save_commit_buffer = 0;
dashdash_pos = 0;
@@ -2243,7 +2285,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
if (!strcmp(ctx.argv[0], "--reverse")) {
ctx.argv[0] = "--children";
- reverse = 1;
+ ssb.reverse = 1;
}
parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
}
@@ -2252,42 +2294,45 @@ parse_done:
if (cmd_is_annotate) {
output_option |= OUTPUT_ANNOTATE_COMPAT;
- blame_date_mode = DATE_ISO8601;
+ ssb.blame_date_mode = DATE_ISO8601;
} else {
- blame_date_mode = revs.date_mode;
+ ssb.blame_date_mode = revs.date_mode;
}
+ ssb.blame_move_score = opt_blame_move_score;
+ ssb.blame_copy_score = opt_blame_copy_score;
+
/* The maximum width used to show the dates */
- switch (blame_date_mode) {
+ switch (ssb.blame_date_mode) {
case DATE_RFC2822:
- blame_date_width = sizeof("Thu, 19 Oct 2006 16:00:04 -0700");
+ ssb.blame_date_width = sizeof("Thu, 19 Oct 2006 16:00:04 -0700");
break;
case DATE_ISO8601:
- blame_date_width = sizeof("2006-10-19 16:00:04 -0700");
+ ssb.blame_date_width = sizeof("2006-10-19 16:00:04 -0700");
break;
case DATE_RAW:
- blame_date_width = sizeof("1161298804 -0700");
+ ssb.blame_date_width = sizeof("1161298804 -0700");
break;
case DATE_SHORT:
- blame_date_width = sizeof("2006-10-19");
+ ssb.blame_date_width = sizeof("2006-10-19");
break;
case DATE_RELATIVE:
/* "normal" is used as the fallback for "relative" */
case DATE_LOCAL:
case DATE_NORMAL:
- blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
+ ssb.blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
break;
}
- blame_date_width -= 1; /* strip the null */
+ ssb.blame_date_width -= 1; /* strip the null */
if (DIFF_OPT_TST(&revs.diffopt, FIND_COPIES_HARDER))
opt |= (PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE |
PICKAXE_BLAME_COPY_HARDER);
- if (!blame_move_score)
- blame_move_score = BLAME_DEFAULT_MOVE_SCORE;
- if (!blame_copy_score)
- blame_copy_score = BLAME_DEFAULT_COPY_SCORE;
+ if (!ssb.blame_move_score)
+ ssb.blame_move_score = BLAME_DEFAULT_MOVE_SCORE;
+ if (!ssb.blame_copy_score)
+ ssb.blame_copy_score = BLAME_DEFAULT_COPY_SCORE;
/*
* We have collected options unknown to us in argv[1..unk]
@@ -2341,9 +2386,9 @@ parse_done:
setup_revisions(argc, argv, &revs, NULL);
memset(&sb, 0, sizeof(sb));
-
+ sb.ssb = &ssb;
sb.revs = &revs;
- if (!reverse)
+ if (!sb.ssb->reverse)
final_commit_name = prepare_final(&sb);
else if (contents_from)
die("--contents and --children do not blend well.");
@@ -2391,7 +2436,7 @@ parse_done:
sha1_to_hex(o->blob_sha1),
path);
}
- num_read_blob++;
+ stat.num_read_blob++;
lno = prepare_lines(&sb);
bottom = top = 0;
@@ -2422,14 +2467,14 @@ parse_done:
die("reading graft file %s failed: %s",
revs_file, strerror(errno));
- read_mailmap(&mailmap, NULL);
+ read_mailmap(&sb.ssb->mailmap, NULL);
- if (!incremental)
+ if (!sb.ssb->incremental)
setup_pager();
assign_blame(&sb, opt);
- if (incremental)
+ if (sb.ssb->incremental)
return 0;
coalesce(&sb);
@@ -2446,9 +2491,9 @@ parse_done:
}
if (show_stats) {
- printf("num read blob: %d\n", num_read_blob);
- printf("num get patch: %d\n", num_get_patch);
- printf("num commits: %d\n", num_commits);
+ printf("num read blob: %d\n", stat.num_read_blob);
+ printf("num get patch: %d\n", stat.num_get_patch);
+ printf("num commits: %d\n", stat.num_commits);
}
return 0;
}
--
1.6.2
^ permalink raw reply related
* Migrate bisect to C (was: [PATCH 1/7] strbuf: add "include_delim" parameter to "strbuf_split")
From: Christian Couder @ 2009-03-14 7:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nanako Shiraishi, git, Ingo Molnar, John Tapsell,
Johannes Schindelin, Pierre Habouzit
In-Reply-To: <7vfxhhj4mh.fsf@gitster.siamese.dyndns.org>
Le vendredi 13 mars 2009, Junio C Hamano a écrit :
>
> I also do not agree that you have to keep list of skip both in shell and
> rev-list when you go the route I suggested. I think a separate bisect.c
> you did is a good first step to make not just the bisect machinery but
> the whole bisect command into a built-in, and even if we do not do the
> full rewrite in C in one go, moving these "shell script reads from
> refs/bisect only to feed the result to rev-list --bisect" pattern to
> "shell script updates refs/bisect and let rev-list --bisect read from
> there" pattern would be a good initial step. Oh, and I did not mean it
> only for "skip", but also doing this for "good" and "bad" as well.
>
> For example, you read "refs/bisect/skip-*" and keep that in $skip to:
>
> - feed it to filter_skipped() which you are making built-in with this
> series;
>
> - feed it to check_good_are_ancestors_of_bad that in turn calls
> check_merge_bases;
>
> and its use is contained in bisect_next() alone. After this series is
> done, we can move the logic in check_good_are... to bisect.c and you do
> not have to read refs/bisect/skip-* in the shell anymore. IOW, we can
> migrate away from the "shell reads from refs/bisect/ and feeds that to
> rev-list --bisect" pattern incrementally.
Do you mean that you want this series to migrate both "filter_skipped" and
"check_good_are_ancestors_of_bad" to C? Or is it ok
if "check_good_are_ancestors_of_bad" migrates later?
If it is ok to migrate "check_good_are_ancestors_of_bad" later, then I think
something like the 8/7 patch I posted yesterday might be a good way,
because I think a "--bisect-read-refs" option that read refs
from "refs/bisect/*" would not fit well in "git rev-list".
Because, the "git rev-list" usage is:
git rev-list [OPTION] <commit-id>... [ -- paths... ]
That means that at least one <commit-id> should always be passed to "git
rev-list".
So it would be strange to have to pass a commit on the command line when
using the "--bisect-read-refs" option. And I think it would not be very
consistent to change the usage like this:
git rev-list [OPTION] [ --bisect-read-refs | <commit-id>... ] [ --
paths... ]
Also when we migrate "check_good_are_ancestors_of_bad" to C, we will
probably have to move the code that checks out the source code
("bisect_checkout" shell function),
because "check_good_are_ancestors_of_bad" can call "bisect_checkout".
And I don't think that the checkout behavior would fit well in "git
rev-list".
That's why I suggested to add a new "git rev-bisect" plumbing command that
would read refs from "refs/bisect/*" and that could later be fitted with
the "bisect_checkout" and "check_good_are_ancestors_of_bad" behavior.
Best regards,
Christian.
^ permalink raw reply
* [PATCH] builtin-tag.c: remove global variable to use the callback data of git-config.
From: Carlos Rica @ 2009-03-14 7:17 UTC (permalink / raw)
To: git, johannes.schindelin, gitster
Signed-off-by: Carlos Rica <jasampler@gmail.com>
---
Here I declare a struct to wrap the new local array along with its size.
QUESTION: An alternative to this is strbuf, would it be preferable?
builtin-tag.c | 43 ++++++++++++++++++++++++++-----------------
1 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/builtin-tag.c b/builtin-tag.c
index 01e7374..2b2d728 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -21,8 +21,6 @@ static const char * const git_tag_usage[] = {
NULL
};
-static char signingkey[1000];
-
struct tag_filter {
const char *pattern;
int lines;
@@ -156,7 +154,12 @@ static int verify_tag(const char *name, const char *ref,
return 0;
}
-static int do_sign(struct strbuf *buffer)
+struct char_array {
+ char *buf;
+ size_t size;
+};
+
+static int do_sign(struct char_array *signingkey, struct strbuf *buffer)
{
struct child_process gpg;
const char *args[4];
@@ -164,11 +167,12 @@ static int do_sign(struct strbuf *buffer)
int len;
int i, j;
- if (!*signingkey) {
- if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
- sizeof(signingkey)) > sizeof(signingkey) - 1)
+ if (!signingkey->buf[0]) {
+ if (strlcpy(signingkey->buf,
+ git_committer_info(IDENT_ERROR_ON_NO_NAME),
+ signingkey->size) > signingkey->size - 1)
return error("committer info too long.");
- bracket = strchr(signingkey, '>');
+ bracket = strchr(signingkey->buf, '>');
if (bracket)
bracket[1] = '\0';
}
@@ -183,7 +187,7 @@ static int do_sign(struct strbuf *buffer)
gpg.out = -1;
args[0] = "gpg";
args[1] = "-bsau";
- args[2] = signingkey;
+ args[2] = signingkey->buf;
args[3] = NULL;
if (start_command(&gpg))
@@ -220,9 +224,10 @@ static const char tag_template[] =
"# Write a tag message\n"
"#\n";
-static void set_signingkey(const char *value)
+static void set_signingkey(struct char_array *signingkey, const char *value)
{
- if (strlcpy(signingkey, value, sizeof(signingkey)) >= sizeof(signingkey))
+ if (strlcpy(signingkey->buf, value, signingkey->size)
+ >= signingkey->size)
die("signing key value too long (%.10s...)", value);
}
@@ -231,7 +236,7 @@ static int git_tag_config(const char *var, const char *value, void *cb)
if (!strcmp(var, "user.signingkey")) {
if (!value)
return config_error_nonbool(var);
- set_signingkey(value);
+ set_signingkey((struct char_array *) cb, value);
return 0;
}
@@ -266,9 +271,10 @@ static void write_tag_body(int fd, const unsigned char *sha1)
free(buf);
}
-static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)
+static int build_tag_object(struct strbuf *buf, int sign,
+ struct char_array *signingkey, unsigned char *result)
{
- if (sign && do_sign(buf) < 0)
+ if (sign && do_sign(signingkey, buf) < 0)
return error("unable to sign the tag");
if (write_sha1_file(buf->buf, buf->len, tag_type, result) < 0)
return error("unable to write tag file");
@@ -277,6 +283,7 @@ static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)
static void create_tag(const unsigned char *object, const char *tag,
struct strbuf *buf, int message, int sign,
+ struct char_array *signingkey,
unsigned char *prev, unsigned char *result)
{
enum object_type type;
@@ -331,7 +338,7 @@ static void create_tag(const unsigned char *object, const char *tag,
strbuf_insert(buf, 0, header_buf, header_len);
- if (build_tag_object(buf, sign, result) < 0) {
+ if (build_tag_object(buf, sign, signingkey, result) < 0) {
if (path)
fprintf(stderr, "The tag message has been left in %s\n",
path);
@@ -374,6 +381,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
+ char keyarr[1000] = {'\0'};
+ struct char_array signingkey = { keyarr, sizeof(keyarr) };
struct option options[] = {
OPT_BOOLEAN('l', NULL, &list, "list tag names"),
{ OPTION_INTEGER, 'n', NULL, &lines, NULL,
@@ -403,14 +412,14 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_END()
};
- git_config(git_tag_config, NULL);
+ git_config(git_tag_config, &signingkey);
argc = parse_options(argc, argv, options, git_tag_usage, 0);
msgfile = parse_options_fix_filename(prefix, msgfile);
if (keyid) {
sign = 1;
- set_signingkey(keyid);
+ set_signingkey(&signingkey, keyid);
}
if (sign)
annotate = 1;
@@ -474,7 +483,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (annotate)
create_tag(object, tag, &buf, msg.given || msgfile,
- sign, prev, object);
+ sign, &signingkey, prev, object);
lock = lock_any_ref_for_update(ref, prev, 0);
if (!lock)
--
1.5.4.3
^ permalink raw reply related
* Re: [PATCH] Introduce a filter-path argument to git-daemon, for doing custom path transformations
From: Junio C Hamano @ 2009-03-14 6:58 UTC (permalink / raw)
To: Johan Sørensen; +Cc: Johannes Schindelin, git
In-Reply-To: <9e0f31700903121206m3adbabacra655c5d340365f43@mail.gmail.com>
Johan Sørensen <johan@johansorensen.com> writes:
>> More importantly, you might want to point out the security concerns of
>> running a script with the full permissions of git-daemon. (AFAICT from
>> your patch you are not dropping any privileges at any point.)
>
> Do you really think this is needed? It doesn't seem like running the
> hook scripts does anything more than trusting the script author and
> permissions of the hook scripts (?). I see the path-filter script
> exactly the same way, with the exception of having to double-check the
> user supplied path the script receives.
If I am not misreading the patch (I only skimmed it), the script is what
is given to the git-daemon process from its command line, so it is under
total control of the site owner. It is much much much less problematic
than the security worry of allowing random hook scripts to be installed in
the repositories hosted at a hosting site. I think Dscho is being a bit
too paranoid in this particular case.
However, being paranoid is a good thing when we talk about instructions we
give to the end users. The site owner who uses this facility needs to be
aware that the script is run as the same user that runs git-daemon, and
that more than one instances of the script can be run at the same time.
The script writer needs to be careful about using the same scratchpad
location for the temporary files the script uses and not letting multiple
instances of scripts stomping on each other's toes. These things need to
be documented.
Do you run git-daemon from inetd, or standalone, by the way? I am
wondering how well it would scale if you spawn an external "filter path"
script (by the way, "filter path" sounds as if it checks and conditionally
denies access to, or something like that, which is not what you are using
it for. It is more about rewriting paths, a la mod_rewrite, and I think
the option is misnamed) every time you get a request.
^ permalink raw reply
* Re: [PATCH][v2] http authentication via prompts (with correct line lengths)
From: Junio C Hamano @ 2009-03-14 6:43 UTC (permalink / raw)
To: Mike Gaffney; +Cc: Mike Ralphson, git
In-Reply-To: <49BA55E2.1060604@gmail.com>
Mike Gaffney <mr.gaffo@gmail.com> writes:
> I was going to try and clean this up this weekend or early next week. I'm also
> trying to encourage open source submissions at work and was using this
> as an example patch to get people going (we need the fix to use git). So
> I do plan finishing this, just have to do it when I have time.
Thanks.
^ permalink raw reply
* Re: [PATCH] read-tree A B C: do not create a bogus index and do not segfault
From: Junio C Hamano @ 2009-03-14 6:41 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Daniel Barkalow, git, Jiri Olsa, Johannes Schindelin
In-Reply-To: <alpine.LFD.2.00.0903120929250.32478@localhost.localdomain>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Thu, 12 Mar 2009, Daniel Barkalow wrote:
>
>> I think it might be a good idea to take this as evidence that nobody is
>> using read-tree with multiple trees without merge, and just disallow it.
>
> Hmm. It _has_ been used. It's been useful for really odd things
> historically, namely trying to merge different trees by hand. So while I
> agree that we could probably remove it, it _is_ a very interesting
> feature in theory, and since we have the code..
>
> So I'd say "add a few tests for the known horrible cases" should be the
> first approach. If it ever actually breaks again and becomes a big
> maintenance headache, maybe _then_ remove the feature as not being worth
> the pain?
I've added trivial tests and will start cooking it by letting it go
through the usual pu/next/master/maint cycle.
I think you are thinking about something like the "gitk" merge (aka "The
coolest merge EVER!" [*1*]), but you can do the same thing more safely
with -m option, giving an empty tree as the common ancestor. Especially
if you run it in the aggressive mode, "addition" from our tree and their
tree, when it is an overlay, will not overlap, and will all cleanly
resolve to stage #0.
I suspect you can also use a single tree "read-tree", immediately followed
by another "read-tree --prefix=''" to read in two trees into the index and
write the results out.
[Footnote]
*1* http://article.gmane.org/gmane.comp.version-control.git/5126
^ permalink raw reply
* Re: [PATCH][v2] http authentication via prompts (with correct line lengths)
From: Junio C Hamano @ 2009-03-14 5:55 UTC (permalink / raw)
To: Mike Ralphson; +Cc: Daniel Stenberg, Mike Gaffney, git
In-Reply-To: <e2b179460903130353p1d3c1cb2n8286c2a284724156@mail.gmail.com>
Mike Ralphson <mike.ralphson@gmail.com> writes:
> 2009/3/13 Daniel Stenberg <daniel@haxx.se>:
>>Driven by use cases such as this, I also recently produced the
>>"symbols-in-versions" document in the libcurl tree which should
>> help apps to know what should works when:
>
>> http://cool.haxx.se/cvs.cgi/curl/docs/libcurl/symbols-in-versions?rev=HEAD&content-type=text/vnd.viewcvs-markup
>
> Very helpful, thanks.
Yeah, I wish we new about it much earlier. Thanks, Daniel.
> Junio, if I check all the unprotected CURL* options against this list,
> would that give us our absolute minimum supported version? If so,
> would it then be ok to remove any unnecessary ifdefs for lower
> versions if they exist?
Sounds like a good plan. Please get the ball rolling.
^ 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