* [PATCH 4/4] Add sample pre-push hook script
From: Aaron Schrab @ 2012-12-28 22:57 UTC (permalink / raw)
To: git
In-Reply-To: <1356735452-21667-1-git-send-email-aaron@schrab.com>
Create a sample of a script for a pre-push hook. The main purpose is to
illustrate how a script may parse the parameters which are supplied to
such a hook. The script may also be useful to some people as-is for
avoiding to push commits which are marked as a work in progress.
Signed-off-by: Aaron Schrab <aaron@schrab.com>
---
templates/hooks--pre-push.sample | 63 ++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
create mode 100644 templates/hooks--pre-push.sample
diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample
new file mode 100644
index 0000000..1d3b4a3
--- /dev/null
+++ b/templates/hooks--pre-push.sample
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+# An example hook script to verify what is about to be pushed.
+# Called by "git push" after it has checked the remote status, but before
+# anything has been pushed. If this script exits with a non-zero status
+# nothing will be pushed.
+#
+# This hook is called with the following parameters:
+#
+# $1 -- Name of the remote to which the push is being done
+# $2 -- URL to which the push is being done
+#
+# If pushing without using a named remote those arguments will be equal.
+#
+# Further arguments provide information about the commits which are being
+# pushed in the form:
+#
+# <local ref>:<local sha1>:<remote ref>:<remote sha1>
+#
+# This sample shows how to prevent push of commits where the log
+# message starts with "WIP" (work in progress).
+
+remote="$1"
+url="$2"
+shift 2
+
+z40=0000000000000000000000000000000000000000
+
+old_ifs="$IFS"
+for to_push in "$@"
+do
+ # Split the value into its parts
+ IFS=:
+ set -- $to_push
+ IFS="$old_ifs"
+
+ local_ref="$1"
+ local_sha="$2"
+ remote_ref="$3"
+ remote_sha="$4"
+
+ if [ "$local_sha" = $z40 ]
+ then
+ range=''
+ # Handle deletes
+ else
+ if [ "$remote_sha" = $z40 ]
+ then
+ range="$local_sha"
+ else
+ range="$remote_sha..$local_sha"
+ fi
+
+ commit=`git rev-list -n 1 --grep '^WIP' "$range"`
+ if [ -n "$commit" ]
+ then
+ echo "Found WIP commit in $local_ref, not pushing"
+ exit 1
+ fi
+ fi
+done
+
+exit 0
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/4] hooks: support variable number of parameters
From: Aaron Schrab @ 2012-12-28 22:57 UTC (permalink / raw)
To: git
In-Reply-To: <1356735452-21667-1-git-send-email-aaron@schrab.com>
Define the run_hook_argv() function to allow hooks to be created where
the number of parameters to be passed is variable. The existing
run_hook() function uses stdarg to allow it to receive a variable number
of arguments, but the number of arguments that a given caller is passing
is fixed at compile time. This function will allow the caller of a hook
to determine the number of arguments to pass when preparing to call the
hook.
The first use of this function will be for a pre-push hook which will
add an argument for every reference which is to be pushed.
Signed-off-by: Aaron Schrab <aaron@schrab.com>
---
run-command.c | 20 +++++++++++++-------
run-command.h | 2 ++
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/run-command.c b/run-command.c
index 49c8fa0..e07202b 100644
--- a/run-command.c
+++ b/run-command.c
@@ -2,7 +2,6 @@
#include "run-command.h"
#include "exec_cmd.h"
#include "sigchain.h"
-#include "argv-array.h"
#ifndef SHELL_PATH
# define SHELL_PATH "/bin/sh"
@@ -746,10 +745,8 @@ char *find_hook(const char *name)
int run_hook(const char *index_file, const char *name, ...)
{
- struct child_process hook;
struct argv_array argv = ARGV_ARRAY_INIT;
- const char *p, *env[2];
- char index[PATH_MAX];
+ const char *p;
va_list args;
int ret;
@@ -764,6 +761,17 @@ int run_hook(const char *index_file, const char *name, ...)
argv_array_push(&argv, p);
va_end(args);
+ ret = run_hook_argv(index_file, argv);
+ argv_array_clear(&argv);
+ return ret;
+}
+
+int run_hook_argv(const char *index_file, struct argv_array argv)
+{
+ struct child_process hook;
+ char index[PATH_MAX];
+ const char *env[2];
+
memset(&hook, 0, sizeof(hook));
hook.argv = argv.argv;
hook.no_stdin = 1;
@@ -775,7 +783,5 @@ int run_hook(const char *index_file, const char *name, ...)
hook.env = env;
}
- ret = run_command(&hook);
- argv_array_clear(&argv);
- return ret;
+ return run_command(&hook);
}
diff --git a/run-command.h b/run-command.h
index 221ce33..12faa5b 100644
--- a/run-command.h
+++ b/run-command.h
@@ -1,6 +1,7 @@
#ifndef RUN_COMMAND_H
#define RUN_COMMAND_H
+#include "argv-array.h"
#ifndef NO_PTHREADS
#include <pthread.h>
#endif
@@ -47,6 +48,7 @@ int run_command(struct child_process *);
extern char *find_hook(const char *name);
extern int run_hook(const char *index_file, const char *name, ...);
+extern int run_hook_argv(const char *index_file, struct argv_array);
#define RUN_COMMAND_NO_STDIN 1
#define RUN_GIT_CMD 2 /*If this is to be git sub-command */
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/4] pre-push hook support
From: Aaron Schrab @ 2012-12-28 22:57 UTC (permalink / raw)
To: git
There have been at least a couple of submissions to add support for a
pre-push hook, which were rejected at least partially because they didn't
provide enough information to a hook script for it to determine what was
to be pushed any better than a separate wrapper around the 'git push'
command would be able to do. In this series I attempt to address that
problem.
The first two patches in this series do a little bit of refactoring in
order to make it easier to call hooks with a variable number of arguments.
The third patch actually adds support for calling a pre-push hook. If it
exists, it will be called with the name and URL of the destination remote
(if a named remote isn't being used, the URL will be supplied for both)
followed by another argument for each ref being pushed; these arguments
take the form:
<local ref>:<local sha1>:<remote ref>:<remote sha1>
This should provide enough information for a script to easily determine
the set of commits that is being pushed, and thus make a decision if that
should be allowed.
The final patch adds a sample pre-push hook script which will deny
attempts to push commits that are marked as a work in progress.
Aaron Schrab (4):
hooks: Add function to check if a hook exists
hooks: support variable number of parameters
push: Add support for pre-push hooks
Add sample pre-push hook script
Documentation/githooks.txt | 28 ++++++++
builtin/push.c | 1 +
run-command.c | 35 ++++++---
run-command.h | 3 +
t/t5571-pre-push-hook.sh | 145 ++++++++++++++++++++++++++++++++++++++
templates/hooks--pre-push.sample | 63 +++++++++++++++++
transport.c | 25 +++++++
transport.h | 1 +
8 files changed, 292 insertions(+), 9 deletions(-)
create mode 100755 t/t5571-pre-push-hook.sh
create mode 100644 templates/hooks--pre-push.sample
--
1.7.10.4
^ permalink raw reply
* [PATCH 1/4] hooks: Add function to check if a hook exists
From: Aaron Schrab @ 2012-12-28 22:57 UTC (permalink / raw)
To: git
In-Reply-To: <1356735452-21667-1-git-send-email-aaron@schrab.com>
Create find_hook() function to determine if a given hook exists and is
executable. If it is the path to the script will be returned, otherwise
NULL is returned.
This is in support for an upcoming run_hook_argv() function which will
expect the full path to the hook script as the first element in the
argv_array. This also makes it simple for places that can use a hook to
check if a hook exists before doing, possibly lengthy, setup work which
would be pointless if no such hook is present.
The returned value is left as a static value from get_pathname() rather
than a duplicate because it is anticipated that the return value will
either be used as a boolean, or immediately added to an argv_array list
which would result in it being duplicated at that point.
Signed-off-by: Aaron Schrab <aaron@schrab.com>
---
run-command.c | 15 +++++++++++++--
run-command.h | 1 +
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/run-command.c b/run-command.c
index 3b982e4..49c8fa0 100644
--- a/run-command.c
+++ b/run-command.c
@@ -735,6 +735,15 @@ int finish_async(struct async *async)
#endif
}
+char *find_hook(const char *name)
+{
+ char *path = git_path("hooks/%s", name);
+ if (access(path, X_OK) < 0)
+ path = NULL;
+
+ return path;
+}
+
int run_hook(const char *index_file, const char *name, ...)
{
struct child_process hook;
@@ -744,11 +753,13 @@ int run_hook(const char *index_file, const char *name, ...)
va_list args;
int ret;
- if (access(git_path("hooks/%s", name), X_OK) < 0)
+ p = find_hook(name);
+ if (!p)
return 0;
+ argv_array_push(&argv, p);
+
va_start(args, name);
- argv_array_push(&argv, git_path("hooks/%s", name));
while ((p = va_arg(args, const char *)))
argv_array_push(&argv, p);
va_end(args);
diff --git a/run-command.h b/run-command.h
index 850c638..221ce33 100644
--- a/run-command.h
+++ b/run-command.h
@@ -45,6 +45,7 @@ int start_command(struct child_process *);
int finish_command(struct child_process *);
int run_command(struct child_process *);
+extern char *find_hook(const char *name);
extern int run_hook(const char *index_file, const char *name, ...);
#define RUN_COMMAND_NO_STDIN 1
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v3 00/19] new git check-ignore sub-command
From: Junio C Hamano @ 2012-12-28 21:31 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: Adam Spiers, git list
In-Reply-To: <CALWbr2wEVzjJ_Y+W9BmakvXCwdFR3OjVH+15tPaDeXsrwaO86w@mail.gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> I think they will interact, but I need to have a deeper look to Adam's series.
> If it does, do you want me to base my work on the top of his branch ?
Not necessarily. If it becomes absolutely necessary to introduce
patch dependencies, I would rather see an addition of new command
rebased on a fix.
I just wanted to make sure that parties touching the same area of
the codebase (and me who will be merging their efforts) are aware of
what is going on.
^ permalink raw reply
* Re: [PATCH v3 00/19] new git check-ignore sub-command
From: Junio C Hamano @ 2012-12-28 21:23 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1356575558-2674-1-git-send-email-git@adamspiers.org>
After skimming the series twice quickly, I found that the early part
of refactorings are excellently done. Making existing private
functions into public needs a lot more careful thought on namings, I
think, though.
The end result looks promising. Thanks for a pleasant read.
^ permalink raw reply
* Re: [PATCH v3 19/19] Add git-check-ignore sub-command
From: Junio C Hamano @ 2012-12-28 21:21 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1356575558-2674-20-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
> new file mode 100644
> index 0000000..c825736
> --- /dev/null
> +++ b/builtin/check-ignore.c
> @@ -0,0 +1,170 @@
> +#include "builtin.h"
> +#include "cache.h"
> +#include "dir.h"
> +#include "quote.h"
> +#include "pathspec.h"
> +#include "parse-options.h"
> +
> +static int quiet, verbose, stdin_paths;
> +static const char * const check_ignore_usage[] = {
> +"git check-ignore [options] pathname...",
> +"git check-ignore [options] --stdin < <list-of-paths>",
> +NULL
> +};
> +
> +static int null_term_line;
> +
> +static const struct option check_ignore_options[] = {
> + OPT__QUIET(&quiet, N_("suppress progress reporting")),
> + OPT__VERBOSE(&verbose, N_("be verbose")),
> + OPT_GROUP(""),
> + OPT_BOOLEAN(0, "stdin", &stdin_paths,
> + N_("read file names from stdin")),
> + OPT_BOOLEAN('z', NULL, &null_term_line,
> + N_("input paths are terminated by a null character")),
> + OPT_END()
> +};
> +
> +static void output_exclude(const char *path, struct exclude *exclude)
> +{
> + char *bang = exclude->flags & EXC_FLAG_NEGATIVE ? "!" : "";
> + char *dir = (exclude->flags & EXC_FLAG_MUSTBEDIR) ? "/" : "";
That's inconsistent. Either s/bang/negated/ or s/dir/slash/ (but
obviously not both).
> +static int check_ignore(const char *prefix, const char **pathspec)
> +{
> + struct dir_struct dir;
> +...
> + if (pathspec) {
> +...
> + } else {
> + printf("no pathspec\n");
> + }
Is this an error message, an informative warning, or what? The
command is presumably for script consumption downstream of a pipe.
Does it make sense to emit this message to their standard input?
Does it even have to be output? Especially what should happen when
the user says "--quiet"?
Perhaps
if (!quiet)
fprintf(stderr, "no pathspec given.\n");
or something?
> +int cmd_check_ignore(int argc, const char **argv, const char *prefix)
> +{
> + int num_ignored = 0;
I do not think you need to initialize this one.
> + if (stdin_paths) {
> + num_ignored = check_ignore_stdin_paths(prefix);
> + } else {
> + num_ignored = check_ignore(prefix, argv);
> + maybe_flush_or_die(stdout, "ignore to stdout");
> + }
> +
> + return num_ignored > 0 ? 0 : 1;
Given that num_ignored will always be >=0, that is a funny way to
say
return !num_ignored;
or if you plan to report a non-fatal error as negative return from
the two check_ignore* functions, perhaps:
return num_ignored <= 0;
> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
> new file mode 100755
> index 0000000..3937ca4
> --- /dev/null
> +++ b/t/t0008-ignores.sh
> @@ -0,0 +1,595 @@
> +#!/bin/sh
> +
> +test_description=check-ignore
> +
> +. ./test-lib.sh
> +
> +init_vars () {
> + global_excludes="$(pwd)/global-excludes"
> +}
> +
> +enable_global_excludes () {
> + init_vars
> + git config core.excludesfile "$global_excludes"
> +}
> +
> +expect_in () {
> + dest="$HOME/expected-$1" text="$2"
> + if test -z "$text"
> + then
> + >"$dest" # avoid newline
> + else
> + echo "$text" >"$dest"
> + fi
> +}
> +
> +expect () {
> + expect_in stdout "$1"
> +}
> +
> +expect_from_stdin () {
> + cat >"$HOME/expected-stdout"
> +}
> +
> +test_stderr () {
> + expected="$1"
> + expect_in stderr "$1" &&
> + test_cmp "$HOME/expected-stderr" "$HOME/stderr"
> +}
> +
> +stderr_contains () {
> + regexp="$1"
> + if grep -q "$regexp" "$HOME/stderr"
Please do not use "grep -q"; the output from commands in test
harness is normally hidden already. This only makes script more
cluttered and robs debuggers of potentially useful output when the
test script is run with "-v".
> +test_check_ignore () {
> + args="$1" expect_code="${2:-0}" global_args="$3"
> +
> + init_vars &&
> + rm -f "$HOME/stdout" "$HOME/stderr" "$HOME/cmd" &&
> + echo git $global_args check-ignore $quiet_opt $verbose_opt $args \
> + >"$HOME/cmd" &&
Does a debugger needs this "cmd" file when we already have "-v" option?
> +test_expect_success_multi () {
> + prereq=
> + if test $# -eq 4
> + then
> + prereq=$1
> + shift
> + fi
> + testname="$1" expect_verbose="$2" code="$3"
> +
> + expect=$( echo "$expect_verbose" | sed -e 's/.* //' )
> +
> + test_expect_success $prereq "$testname" "
> + expect '$expect' &&
> + $code
> + "
This is brittle when $expect may have single quotes, isn't it?
Perhaps
test_expect_success $prereq "$testname" '
expect "$expect" && eval "$code"
'
may fix it, but in general I hate to see each test script trying to
invent their own mini-harness like this (and getting them wrong).
^ permalink raw reply
* Re: [PATCH v3 16/19] pathspec.c: move reusable code from builtin/add.c
From: Adam Spiers @ 2012-12-28 21:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list
In-Reply-To: <7vy5ghhpi0.fsf@alter.siamese.dyndns.org>
On Fri, Dec 28, 2012 at 8:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
>> diff --git a/pathspec.c b/pathspec.c
>> new file mode 100644
>> index 0000000..8aea0d2
>> --- /dev/null
>> +++ b/pathspec.c
>> @@ -0,0 +1,99 @@
>> +#include "cache.h"
>> +#include "dir.h"
>> +#include "pathspec.h"
>> +
>> +void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
>> +{
>
> It did not matter while it was an implementation detail of "git
> add", but as a public function, something needs to clarify that this
> "fill"s matches *against the index*, not against a tree or the files
> in the current directory. The same comment applies to all the
> internal functions this patch exposes to the outside world.
Prior to submitting the v3 series, I attempted to understand
fill_pathspec_matches() and find_used_pathspec() well enough to
document them all, but I failed. Perhaps some kind soul could explain
what is the exact purpose of these functions, and in particular the
role of char *seen in both?
^ permalink raw reply
* Re: [PATCH v3 17/19] pathspec.c: extract new validate_path() for reuse
From: Adam Spiers @ 2012-12-28 21:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list
In-Reply-To: <7v38ypj490.fsf@alter.siamese.dyndns.org>
On Fri, Dec 28, 2012 at 8:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
>> This will be reused by a new git check-ignore command.
>>
>> Signed-off-by: Adam Spiers <git@adamspiers.org>
>> ---
>> pathspec.c | 20 ++++++++++++++------
>> pathspec.h | 1 +
>> 2 files changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/pathspec.c b/pathspec.c
>> index 8aea0d2..6724121 100644
>> --- a/pathspec.c
>> +++ b/pathspec.c
>> @@ -77,9 +77,20 @@ void treat_gitlinks(const char **pathspec)
>> }
>>
>> /*
>> + * Dies if the given path refers to a file inside a symlinked
>> + * directory.
>> + */
>> +void validate_path(const char *path, const char *prefix)
>
> The name needs to be a lot more specific.
>
> There may be 47 different kinds of "validations" various callers may
> want to do on a path, but this function only caters to one kind of
> callers that want to make sure that the path refers to something
> that we would directly add to our index.
>
>> +{
>> + if (has_symlink_leading_path(path, strlen(path))) {
>> + int len = prefix ? strlen(prefix) : 0;
>> + die(_("'%s' is beyond a symbolic link"), path + len);
>> + }
>> +}
Good point. Which do you prefer of these suggested names?
- die_if_path_beyond_symlink()
- validate_path_not_beyond_symlink()
- die_if_symlink_leading_path()
- validate_no_symlink_leading_path()
- validate_path_addable_to_index()
Or something else?
^ permalink raw reply
* Re: [PATCH v3 16/19] pathspec.c: move reusable code from builtin/add.c
From: Junio C Hamano @ 2012-12-28 20:48 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1356575558-2674-17-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> diff --git a/pathspec.c b/pathspec.c
> new file mode 100644
> index 0000000..8aea0d2
> --- /dev/null
> +++ b/pathspec.c
> @@ -0,0 +1,99 @@
> +#include "cache.h"
> +#include "dir.h"
> +#include "pathspec.h"
> +
> +void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
> +{
It did not matter while it was an implementation detail of "git
add", but as a public function, something needs to clarify that this
"fill"s matches *against the index*, not against a tree or the files
in the current directory. The same comment applies to all the
internal functions this patch exposes to the outside world.
^ permalink raw reply
* Re: [PATCH v3 16/19] pathspec.c: move reusable code from builtin/add.c
From: Adam Spiers @ 2012-12-28 20:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list
In-Reply-To: <7v8v8hj4t0.fsf@alter.siamese.dyndns.org>
On Fri, Dec 28, 2012 at 8:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
>> diff --git a/pathspec.h b/pathspec.h
>> new file mode 100644
>> index 0000000..8bb670b
>> --- /dev/null
>> +++ b/pathspec.h
>> @@ -0,0 +1,5 @@
>> +extern char *find_used_pathspec(const char **pathspec);
>> +extern void fill_pathspec_matches(const char **pathspec, char *seen, int specs);
>> +extern const char *treat_gitlink(const char *path);
>> +extern void treat_gitlinks(const char **pathspec);
>> +extern const char **validate_pathspec(const char **argv, const char *prefix);
>
> Protect this against multiple inclusion with "#ifndef PATHSPEC_H".
Yep good idea, how should I submit this? It will cause a trivially
resolvable conflict with the next patch in the series (17/19):
pathspec.c: extract new validate_path() for reuse
but I'd prefer not to re-roll 16--19 when just 16 and 17 are sufficient.
^ permalink raw reply
* Re: [PATCH v3 17/19] pathspec.c: extract new validate_path() for reuse
From: Junio C Hamano @ 2012-12-28 20:44 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1356575558-2674-18-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> This will be reused by a new git check-ignore command.
>
> Signed-off-by: Adam Spiers <git@adamspiers.org>
> ---
> pathspec.c | 20 ++++++++++++++------
> pathspec.h | 1 +
> 2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/pathspec.c b/pathspec.c
> index 8aea0d2..6724121 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -77,9 +77,20 @@ void treat_gitlinks(const char **pathspec)
> }
>
> /*
> + * Dies if the given path refers to a file inside a symlinked
> + * directory.
> + */
> +void validate_path(const char *path, const char *prefix)
The name needs to be a lot more specific.
There may be 47 different kinds of "validations" various callers may
want to do on a path, but this function only caters to one kind of
callers that want to make sure that the path refers to something
that we would directly add to our index.
> +{
> + if (has_symlink_leading_path(path, strlen(path))) {
> + int len = prefix ? strlen(prefix) : 0;
> + die(_("'%s' is beyond a symbolic link"), path + len);
> + }
> +}
^ permalink raw reply
* Re: [PATCH v3 18/19] setup.c: document get_pathspec()
From: Adam Spiers @ 2012-12-28 20:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list
In-Reply-To: <7v7go1j4ml.fsf@alter.siamese.dyndns.org>
On Fri, Dec 28, 2012 at 8:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
>> Since we have just created a new pathspec-handling library, now is a
>> good time to add some comments explaining get_pathspec().
>>
>> Signed-off-by: Adam Spiers <git@adamspiers.org>
>> ---
>
> Yes, but we would rather not to see new users of this function added
> to our codebase in its current form, as explained in the nearby
> comment. We would want to migrate everybody to "struct pathspec"
> based interface to support magic pathspecs in the longer term.
I see. Please feel free to drop that patch from the series or amend
as you see fit.
^ permalink raw reply
* Re: [PATCH v2] log: grep author/committer using mailmap
From: Antoine Pelisse @ 2012-12-28 20:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7va9syj9v1.fsf@alter.siamese.dyndns.org>
> This is about your rewritten implementation that hasn't escaped to
> the general public but sitting in 'next', right?
>
> Two things that immediately come to mind are:
>
> - initialization of lowermail can use strbuf_init() instead;
> - downcasing can be done in place, i.e. "lowermail.buf[i] = ...".
Yep,
I don't think it's merged to 'next', I will squash those changes appropriately.
^ permalink raw reply
* Re: [PATCH v3 18/19] setup.c: document get_pathspec()
From: Junio C Hamano @ 2012-12-28 20:36 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1356575558-2674-19-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> Since we have just created a new pathspec-handling library, now is a
> good time to add some comments explaining get_pathspec().
>
> Signed-off-by: Adam Spiers <git@adamspiers.org>
> ---
Yes, but we would rather not to see new users of this function added
to our codebase in its current form, as explained in the nearby
comment. We would want to migrate everybody to "struct pathspec"
based interface to support magic pathspecs in the longer term.
> setup.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/setup.c b/setup.c
> index 7663a4c..03d6d5c 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -249,6 +249,21 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char
> return prefix_path(prefix, prefixlen, copyfrom);
> }
>
> +/*
> + * prefix - a path relative to the root of the working tree
> + * pathspec - a list of paths underneath the prefix path
> + *
> + * Iterates over pathspec, prepending each path with prefix,
> + * and return the resulting list.
> + *
> + * If pathspec is empty, return a singleton list containing prefix.
> + *
> + * If pathspec and prefix are both empty, return an empty list.
> + *
> + * This is typically used by built-in commands such as add.c, in order
> + * to normalize argv arguments provided to the built-in into a list of
> + * paths to process, all relative to the root of the working tree.
> + */
> const char **get_pathspec(const char *prefix, const char **pathspec)
> {
> const char *entry = *pathspec;
^ permalink raw reply
* Re: git diff --ignore-space-at-eol issue
From: Antoine Pelisse @ 2012-12-28 20:33 UTC (permalink / raw)
To: John Moon; +Cc: git
In-Reply-To: <BLU163-W51DAB90003C969CDF837A2CF3F0@phx.gbl>
Then you should give the output of diff --stat, and he will be able to
ignore files with no changes.
The change was originally made for permission changes. diff --stat
needs to show files have changed even though, indeed, there is no diff
output.
You could also use --numstat and filter out files with no changes
(starting by 0 0).
On Fri, Dec 28, 2012 at 8:59 PM, John Moon <johnmoon77@hotmail.com> wrote:
>
>
>> $ git diff --ignore-space-at-eol test.txt
>> $ git diff --ignore-space-at-eol --stat test.txt
>> test.txt | 0
>> 1 file changed, 0 insertions(+), 0 deletions(-)
>> $ git diff --ignore-space-at-eol --name-status test.txt
>> M test.txt
>>
>> The idea is that even though diff doesn't show any differences, stat,
>> shortstat, numstat and name-status reports the file as being changed.
>> This is available since v1.8.1-rc0.
>
> Thanks for the info. Unfortunately it's not what i would expect.
> If i told git diff specifically to ignore line endings, why is --name-status showing me a file as being modified when the only modification is the very thing i told it to ignore.
> The same thing for --stat, why is it showing me a file with zero changes? Just my opinion though.
>
> I'll tell you why this is a problem for me, basically what i am doing is running the "git diff --ignore-space-at-eol --name-status " on my root directory to give to someone else who is not using git to give them the files that i have modified. I don't want to give them a file where only the line ending has changed.
>
> Cheers.
^ permalink raw reply
* Re: [PATCH v3 16/19] pathspec.c: move reusable code from builtin/add.c
From: Junio C Hamano @ 2012-12-28 20:32 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1356575558-2674-17-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> diff --git a/pathspec.h b/pathspec.h
> new file mode 100644
> index 0000000..8bb670b
> --- /dev/null
> +++ b/pathspec.h
> @@ -0,0 +1,5 @@
> +extern char *find_used_pathspec(const char **pathspec);
> +extern void fill_pathspec_matches(const char **pathspec, char *seen, int specs);
> +extern const char *treat_gitlink(const char *path);
> +extern void treat_gitlinks(const char **pathspec);
> +extern const char **validate_pathspec(const char **argv, const char *prefix);
Protect this against multiple inclusion with "#ifndef PATHSPEC_H".
^ permalink raw reply
* Re: [PATCH v3 00/19] new git check-ignore sub-command
From: Antoine Pelisse @ 2012-12-28 20:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Adam Spiers, git list
In-Reply-To: <CAOkDyE8gfW9TvyP=iE7gVEXOqCpOqMRjpr=Vnyd_pnummy4Qsg@mail.gmail.com>
I think they will interact, but I need to have a deeper look to Adam's series.
If it does, do you want me to base my work on the top of his branch ?
On Fri, Dec 28, 2012 at 8:39 PM, Adam Spiers <git@adamspiers.org> wrote:
> On Fri, Dec 28, 2012 at 6:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Adam Spiers <git@adamspiers.org> writes:
>>
>>> This v3 re-roll of my check-ignore series is a reasonably substantial
>>> revamp over v2, and applies on top of Junio's current
>>> nd/attr-match-optim-more branch (82dce998c202).
>>
>> Thanks.
>>
>> Does this (and should this, if it doesn't) interact with the more
>> recent discussion around "git status --untracked/--ignored" [*1*],
>> which also wants to touch the recursive directory traversal logic in
>> "dir.c"?
>
> I cannot think of a reason why they would or should interact. If I'm
> wrong, I expect that either set of unit tests would show me up :-)
^ permalink raw reply
* RE: git diff --ignore-space-at-eol issue
From: John Moon @ 2012-12-28 19:59 UTC (permalink / raw)
To: apelisse; +Cc: git
In-Reply-To: <CALWbr2y3BdqcD-62jhPSQsK3U=8-Dc=R-jxg8H0yqpgVfdHJXw@mail.gmail.com>
> $ git diff --ignore-space-at-eol test.txt
> $ git diff --ignore-space-at-eol --stat test.txt
> test.txt | 0
> 1 file changed, 0 insertions(+), 0 deletions(-)
> $ git diff --ignore-space-at-eol --name-status test.txt
> M test.txt
>
> The idea is that even though diff doesn't show any differences, stat,
> shortstat, numstat and name-status reports the file as being changed.
> This is available since v1.8.1-rc0.
Thanks for the info. Unfortunately it's not what i would expect.
If i told git diff specifically to ignore line endings, why is --name-status showing me a file as being modified when the only modification is the very thing i told it to ignore.
The same thing for --stat, why is it showing me a file with zero changes? Just my opinion though.
I'll tell you why this is a problem for me, basically what i am doing is running the "git diff --ignore-space-at-eol --name-status " on my root directory to give to someone else who is not using git to give them the files that i have modified. I don't want to give them a file where only the line ending has changed.
Cheers.
^ permalink raw reply
* Re: [PATCH v3 00/19] new git check-ignore sub-command
From: Adam Spiers @ 2012-12-28 19:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list, Antoine Pelisse
In-Reply-To: <7v38yqj9ix.fsf@alter.siamese.dyndns.org>
On Fri, Dec 28, 2012 at 6:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
>> This v3 re-roll of my check-ignore series is a reasonably substantial
>> revamp over v2, and applies on top of Junio's current
>> nd/attr-match-optim-more branch (82dce998c202).
>
> Thanks.
>
> Does this (and should this, if it doesn't) interact with the more
> recent discussion around "git status --untracked/--ignored" [*1*],
> which also wants to touch the recursive directory traversal logic in
> "dir.c"?
I cannot think of a reason why they would or should interact. If I'm
wrong, I expect that either set of unit tests would show me up :-)
^ permalink raw reply
* Re: (unknown)
From: Junio C Hamano @ 2012-12-28 19:33 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20121228164322.B102B4413A@snark.thyrsus.com>
esr@thyrsus.com (Eric S. Raymond) writes:
> From: "Eric S. Raymond" <esr@thyrsus.com>
> Date: Fri, 28 Dec 2012 11:40:59 -0500
> Subject: [PATCH] Add checks to Python scripts for version dependencies.
>
> ---
> contrib/ciabot/ciabot.py | 8 +++++++-
> ...
> diff --git a/contrib/fast-import/import-zips.py b/contrib/fast-import/import-zips.py
> index 82f5ed3..b989941 100755
> --- a/contrib/fast-import/import-zips.py
> +++ b/contrib/fast-import/import-zips.py
> @@ -9,10 +9,15 @@
> ## git log --stat import-zips
>
> from os import popen, path
> -from sys import argv, exit
> +from sys import argv, exit, hexversion
> from time import mktime
> from zipfile import ZipFile
>
> +if hexversion < 0x01060000:
I am assuming that you are carefully limiting what you import from
"sys" by adding only hexversion to the import above, but then can we
refer to sys.stderr below?
> + # The limiter is the zipfile module
> + sys.stderr.write("import-zips.py: requires Python 1.6.0 or later.\n")
> + sys.exit(1)
> +
^ permalink raw reply
* Re: [PATCH] Remove the suggestion to use parsecvs, which is currently broken.
From: Junio C Hamano @ 2012-12-28 19:28 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20121228162025.8565E4413A@snark.thyrsus.com>
esr@thyrsus.com (Eric S. Raymond) writes:
> The parsecvs code has been neglected for a long time, and the only
> public version does not even build correctly. I have been handed
> control of the project and intend to fix this, but until I do it
> cannot be recommended.
>
> Also, the project URL given for Subversion needed to be updated
> to follow their site move.
> ---
> Documentation/git-cvsimport.txt | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
> ...
> --
> <a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
>
> A ``decay in the social contract'' is detectable; there is a growing
> feeling, particularly among middle-income taxpayers, that they are not
> getting back, from society and government, their money's worth for
> taxes paid. The tendency is for taxpayers to try to take more control
> of their finances... -- IRS Strategic Plan, (May 1984)
It is funny that you keep forgetting to sign-off your patches and
even send a message with no subject, but still manage to add a
pointer to your homepage and stuff at the end.
Perhaps you would need to form a habit to do "commit -s"?
^ permalink raw reply
* Re: [PATCH v3 00/19] new git check-ignore sub-command
From: Junio C Hamano @ 2012-12-28 18:50 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list, Antoine Pelisse
In-Reply-To: <1356575558-2674-1-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> This v3 re-roll of my check-ignore series is a reasonably substantial
> revamp over v2, and applies on top of Junio's current
> nd/attr-match-optim-more branch (82dce998c202).
Thanks.
Does this (and should this, if it doesn't) interact with the more
recent discussion around "git status --untracked/--ignored" [*1*],
which also wants to touch the recursive directory traversal logic in
"dir.c"?
[Reference]
*1* http://thread.gmane.org/gmane.comp.version-control.git/212127/focus=212136
^ permalink raw reply
* Re: [PATCH v2] log: grep author/committer using mailmap
From: Junio C Hamano @ 2012-12-28 18:43 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git
In-Reply-To: <CALWbr2y63L0-ykdUNGqUOb0LhG=vpXGRcb1KkvssEZmKFJEGeQ@mail.gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> Actually, gprof seems to be unhappy about the number of call to
> strbuf_grow() in map_user() (25% of the time spent in map_user() is
> spent in strbuf_grow()).
>
> That probably comes from the repeated call to strbuf_addch() when
> lowering the email address.
This is about your rewritten implementation that hasn't escaped to
the general public but sitting in 'next', right?
Two things that immediately come to mind are:
- initialization of lowermail can use strbuf_init() instead;
- downcasing can be done in place, i.e. "lowermail.buf[i] = ...".
^ permalink raw reply
* Re: [PATCH v2] log: grep author/committer using mailmap
From: Antoine Pelisse @ 2012-12-28 18:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwqw3l49z.fsf@alter.siamese.dyndns.org>
Actually, gprof seems to be unhappy about the number of call to
strbuf_grow() in map_user() (25% of the time spent in map_user() is
spent in strbuf_grow()).
That probably comes from the repeated call to strbuf_addch() when
lowering the email address.
At this point, we are also copying the '\0' for every char we add,
doubling the copy.
This may not be much of a difference, but it seems to be called 15
millions times when running:
$ git log --author='Junio C Hamano' --use-mailmap
Maybe we should come up with another way to lower this email address afterall.
^ 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