* Re: [PATCHv2 5/5] git-send-email: use git credential to obtain password
From: Junio C Hamano @ 2013-02-07 18:42 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: Jeff King, Matthieu Moy, git
In-Reply-To: <0b3c9b66ccb6c8343dafd210b82c7765891d3785.1360242782.git.mina86@mina86.com>
Michal Nazarewicz <mpn@google.com> writes:
> From: Michal Nazarewicz <mina86@mina86.com>
>
> If smtp_user is provided but smtp_pass is not, instead of
> prompting for password, make git-send-email use git
> credential command instead.
>
> Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
> ---
Nice ;-)
I'd expect reviews on 4/5 from Peff and Matthiew which may result in
either Reviewed-by:'s or another round, but everything else looks in
good order.
Thanks to all three of you for working on this.
> Documentation/git-send-email.txt | 4 +--
> git-send-email.perl | 59 +++++++++++++++++++++++-----------------
> 2 files changed, 36 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 44a1f7c..0cffef8 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -164,8 +164,8 @@ Sending
> Furthermore, passwords need not be specified in configuration files
> or on the command line. If a username has been specified (with
> '--smtp-user' or a 'sendemail.smtpuser'), but no password has been
> -specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
> -user is prompted for a password while the input is masked for privacy.
> +specified (with '--smtp-pass' or 'sendemail.smtppass'), then
> +a password is obtained using 'git-credential'.
>
> --smtp-server=<host>::
> If set, specifies the outgoing SMTP server to use (e.g.
> diff --git a/git-send-email.perl b/git-send-email.perl
> index be809e5..76bbfc3 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1045,6 +1045,39 @@ sub maildomain {
> return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
> }
>
> +# Returns 1 if authentication succeeded or was not necessary
> +# (smtp_user was not specified), and 0 otherwise.
> +
> +sub smtp_auth_maybe {
> + if (!defined $smtp_authuser || $auth) {
> + return 1;
> + }
> +
> + # Workaround AUTH PLAIN/LOGIN interaction defect
> + # with Authen::SASL::Cyrus
> + eval {
> + require Authen::SASL;
> + Authen::SASL->import(qw(Perl));
> + };
> +
> + # TODO: Authentication may fail not because credentials were
> + # invalid but due to other reasons, in which we should not
> + # reject credentials.
> + $auth = Git::credential({
> + 'protocol' => 'smtp',
> + 'host' => join(':', $smtp_server, $smtp_server_port),
> + 'username' => $smtp_authuser,
> + # if there's no password, "git credential fill" will
> + # give us one, otherwise it'll just pass this one.
> + 'password' => $smtp_authpass
> + }, sub {
> + my $cred = shift;
> + return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
> + });
> +
> + return $auth;
> +}
> +
> # Returns 1 if the message was sent, and 0 otherwise.
> # In actuality, the whole program dies when there
> # is an error sending a message.
> @@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
> defined $smtp_server_port ? " port=$smtp_server_port" : "";
> }
>
> - if (defined $smtp_authuser) {
> - # Workaround AUTH PLAIN/LOGIN interaction defect
> - # with Authen::SASL::Cyrus
> - eval {
> - require Authen::SASL;
> - Authen::SASL->import(qw(Perl));
> - };
> -
> - if (!defined $smtp_authpass) {
> -
> - system "stty -echo";
> -
> - do {
> - print "Password: ";
> - $_ = <STDIN>;
> - print "\n";
> - } while (!defined $_);
> -
> - chomp($smtp_authpass = $_);
> -
> - system "stty echo";
> - }
> -
> - $auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
> - }
> + smtp_auth_maybe or die $smtp->message;
>
> $smtp->mail( $raw_from ) or die $smtp->message;
> $smtp->to( @recipients ) or die $smtp->message;
^ permalink raw reply
* Re: [PATCHv2 4/5] Git.pm: add interface for git credential command
From: Matthieu Moy @ 2013-02-07 18:46 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: Jeff King, Junio C Hamano, git
In-Reply-To: <78516627e893e54d5aafe0694d1face9a37893de.1360242782.git.mina86@mina86.com>
Michal Nazarewicz <mpn@google.com> writes:
> From: Michal Nazarewicz <mina86@mina86.com>
>
> Add a credential() function which is an interface to the git
> credential command. The code is heavily based on credential_*
> functions in <contrib/mw-to-git/git-remote-mediawiki>.
I'm no perl expert, so I cannot comment much on style (there are many
small changes compared to the mediawiki code that look like improvement
though), but:
Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Preventing merges from one branch into others
From: Tim Chase @ 2013-02-07 19:14 UTC (permalink / raw)
To: git
[tried IRC to no avail]
I've been trying to find a way to prevent myself from merging a
client-specific branch back into my dev/master branches. Is there an
easy/straightforward way to do this (perhaps via a hook)? I didn't
see any sort of "pre-merge" hook script. Visualized:
A -> B -> C [dev]
\
-> Q -> R -> S [customer-specific]
and I want to ensure that changes Q/R/S never find their way back
into dev.
So I was hoping some sort of "hey, you're an idiot for trying to merge
$CUSTOMER_BRANCH back into dev/master" warning. :-)
-tkc
^ permalink raw reply
* Re: [PATCH 1/4] Makefile: extract perl-related rules to make them available from other dirs
From: Junio C Hamano @ 2013-02-07 19:16 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <1360174292-14793-2-git-send-email-Matthieu.Moy@imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> The final goal is to make it easy to write Git commands in perl in the
> contrib/ directory. It is currently possible to do so, but without the
> benefits of Git's Makefile: adapt first line with $(PERL_PATH),
> hardcode the path to Git.pm, ...
>
> We make the perl-related part of the Makefile available from directories
> other than the toplevel so that:
>
> * Developers can include it, to avoid code duplication
>
> * Users can get a consistent behavior of "make install"
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
The goal may be worthy, but the split does not look quite right.
What business do contrib/ scripts have knowing how gitweb and
git-instaweb are built and what they depend on, for example?
^ permalink raw reply
* Re: [PATCH 3/4] Makefile: factor common configuration in git-default-config.mak
From: Junio C Hamano @ 2013-02-07 19:28 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <1360174292-14793-4-git-send-email-Matthieu.Moy@imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> Similarly to the extraction of perl-related code in perl.mak, we extract
> general default configuration from the Makefile to make it available from
> directories other than the toplevel.
>
> This is required to make perl.mak usable because it requires $(pathsep)
> to be set.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
I really think this is going in a wrong direction. Whatever you
happen to have chosen in this patch will be available to others,
while whatever are left out will not be. When adding new things,
people need to ask if it needs to be sharable or not, and the right
answer to that question will even change over time.
^ permalink raw reply
* Re: [PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script
From: Junio C Hamano @ 2013-02-07 19:28 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <1360174292-14793-5-git-send-email-Matthieu.Moy@imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> The configuration of the install directory is not reused from the
> toplevel Makefile: we assume Git is already built, hence just call
> "git --exec-path". This avoids too much surgery in the toplevel Makefile.
>
> git-remote-mediawiki.perl can now "use Git;".
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
Continuing to the comment on 3/4, I wonder if it would be a lot
simpler and more maintainable if you replaced 1/4 to 3/4 with a
smaller patch to the top-level Makefile to teach it to munge
arbitrary path/to/foo.perl to path/to/foo the same way as we do to
other path/tool.perl that are known to the top-level Makefile
(similarly, another target to install the resulting path/to/foo at
an arbitrary place). Then do something like
all::
$(MAKE) -C ../.. \
PERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \
build-perl-script
install::
$(MAKE) -C ../.. \
PERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \
install-perl-script
in this step.
^ permalink raw reply
* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
From: Junio C Hamano @ 2013-02-07 19:35 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Torsten Bögershausen, git, j6t
In-Reply-To: <5113F1B1.3010102@ramsay1.demon.co.uk>
Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> Torsten Bögershausen wrote:
>> t0070 and t1301 fail when running the test suite under cygwin.
>> Skip the failing tests by unsetting POSIXPERM.
>
> t1301 does not fail for me. (WIN XP (SP3) on NTFS)
Others run Cygwin with vfat or some other filesystem, and some of
them do not cope will with POSIXPERM, perhaps?
Not having POSIXPERM by default for Cygwin may be a saner default
than having one, if we have to pick one.
It may be debatable to have this default as platform attribute,
though.
>> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
>> ---
>> t/test-lib.sh | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 1a6c4ab..94b097e 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -669,7 +669,6 @@ case $(uname -s) in
>> test_set_prereq SED_STRIPS_CR
>> ;;
>> *CYGWIN*)
>> - test_set_prereq POSIXPERM
>> test_set_prereq EXECKEEPSPID
>> test_set_prereq NOT_MINGW
>> test_set_prereq SED_STRIPS_CR
>>
>
> So, I'm not in favour of this, FWIW.
>
> ATB,
> Ramsay Jones
^ permalink raw reply
* Re: Proposal: branch.<name>.remotepush
From: Ramkumar Ramachandra @ 2013-02-07 19:37 UTC (permalink / raw)
To: Michael Schubert; +Cc: Git List, Junio C Hamano
In-Reply-To: <5113E849.8000602@elegosoft.com>
There's a reason why remote.<name>.pushurl feels wrong. If one remote
has a different push from pull, there should be something
corresponding to refs/remotes/* for push (and the equivalent of fetch
for updating it). Second, I can't even diff between a branch on my
push URL and a local branch: the [ahead 1, behind 1] in status output
really doesn't make sense if the repository you're pushing to is
different from the one you're pulling from. In contrast, if you take
what I proposed, refs/remotes/{upstream, mine}/* already exist, and
it's easy to diff them with the corresponding local branch.
And yes, a regular `git push origin refs/for/master` is just retarded.
I don't personally use Gerrit, but the people who do should not have
to suffer.
^ permalink raw reply
* Re: Preventing merges from one branch into others
From: Junio C Hamano @ 2013-02-07 19:38 UTC (permalink / raw)
To: Tim Chase; +Cc: git
In-Reply-To: <20130207131440.716c1022@bigbox.christie.dr>
Tim Chase <git@tim.thechases.com> writes:
> ... I didn't
> see any sort of "pre-merge" hook script.
http://thread.gmane.org/gmane.comp.version-control.git/94111/focus=71069
I think yours is a canonical example of "I do not want to run
command X when these conditions hold", when "these conditions" can
be checked locally _before_ you decide to do X.
While it is theoretically possible to give X a pre-X hook in such a
case, we do not want it to be the first choice to avoid hook bloat.
^ permalink raw reply
* Re: Proposal: branch.<name>.remotepush
From: Ramkumar Ramachandra @ 2013-02-07 19:49 UTC (permalink / raw)
To: Michael Schubert; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0=53riU3xKbKkyAVS8--9VoAU5P6h88MQ9-geW=H5+a-w@mail.gmail.com>
Ramkumar Ramachandra wrote:
> And yes, a regular `git push origin refs/for/master` is just retarded.
Actually a git config remote.origin.push refs/heads/*:refs/for/* makes
more sense here.
^ permalink raw reply
* [PATCH] Use __VA_ARGS__ for all of error's arguments
From: Matt Kraai @ 2013-02-07 20:00 UTC (permalink / raw)
To: git, Max Horn, Jeff King, Junio C Hamano; +Cc: Matt Kraai
From: Matt Kraai <matt.kraai@amo.abbott.com>
QNX 6.3.2 uses GCC 2.95.3 by default, and GCC 2.95.3 doesn't remove the
comma if the error macro's variable argument is left out.
Instead of testing for a sufficiently recent version of GCC, make
__VA_ARGS__ match all of the arguments. Since this should work on any
C99-compliant compiler, also remove the tests around the macro definition.
Signed-off-by: Matt Kraai <matt.kraai@amo.abbott.com>
---
git-compat-util.h | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index cc2abee..df1681f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -305,14 +305,9 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)))
/*
* Let callers be aware of the constant return value; this can help
- * gcc with -Wuninitialized analysis. We have to restrict this trick to
- * gcc, though, because of the variadic macro and the magic ## comma pasting
- * behavior. But since we're only trying to help gcc, anyway, it's OK; other
- * compilers will fall back to using the function as usual.
+ * gcc with -Wuninitialized analysis.
*/
-#if defined(__GNUC__) && ! defined(__clang__)
-#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
-#endif
+#define error(...) (error(__VA_ARGS__), -1)
extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
extern void set_error_routine(void (*routine)(const char *err, va_list params));
--
1.8.1.GIT
^ permalink raw reply related
* Re: Proposal: branch.<name>.remotepush
From: Ramkumar Ramachandra @ 2013-02-07 20:14 UTC (permalink / raw)
To: Michael Schubert; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0=_0hZP_SjMCjooUAr+MrRoXCCzdQF8y9RhW1g7ShsC7w@mail.gmail.com>
Ramkumar Ramachandra wrote:
> Ramkumar Ramachandra wrote:
>> And yes, a regular `git push origin refs/for/master` is just retarded.
>
> Actually a git config remote.origin.push refs/heads/*:refs/for/* makes
> more sense here.
Sorry about all that confusion. The first line should be `git push
origin master:refs/for/master`, but a rule like refs/head/*:refs/for/*
is insufficient: what if I want refs/head/*:refs/heads/* for one set
of branches (private ones that I don't send for review), and
refs/heads/*:refs/for/* for another set (which I send for review)?
That certainly won't play well will the existing remote.origin.push;
it'd have to behave as an override.
^ permalink raw reply
* [PATCH 3/6] diff.c: make constant string arguments const
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>
Signed-off-by: John Keeping <john@keeping.me.uk>
---
diff.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index bf95235..73ae02d 100644
--- a/diff.c
+++ b/diff.c
@@ -2123,7 +2123,8 @@ static unsigned char *deflate_it(char *data,
return deflated;
}
-static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two, char *prefix)
+static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two,
+ const char *prefix)
{
void *cp;
void *delta;
@@ -2184,7 +2185,8 @@ static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two, char
free(data);
}
-static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two, char *prefix)
+static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two,
+ const char *prefix)
{
fprintf(file, "%sGIT binary patch\n", prefix);
emit_binary_diff_body(file, one, two, prefix);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 2/6] diff: write prefix to the correct file
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>
Write the prefix for an output line to the same file as the actual
content.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
diff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index 348f71b..bf95235 100644
--- a/diff.c
+++ b/diff.c
@@ -4485,7 +4485,7 @@ void diff_flush(struct diff_options *options)
struct strbuf *msg = NULL;
msg = options->output_prefix(options,
options->output_prefix_data);
- fwrite(msg->buf, msg->len, 1, stdout);
+ fwrite(msg->buf, msg->len, 1, options->file);
}
putc(options->line_termination, options->file);
if (options->stat_sep) {
--
1.8.1.2
^ permalink raw reply related
* [PATCH 1/6] graph: output padding for merge subsequent parents
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>
When showing merges in git-log, the same commit is shown once for each
parent. Combined with "--graph" this results in graph_show_commit()
being called once for each parent without graph_update() being called.
Currently graph_show_commit() does not print anything on subsequent
invocations for the same commit (this was changed by commit 656197a -
"graph.c: infinite loop in git whatchanged --graph -m" from the previous
behaviour of looping infinitely).
Change this so that if the graph code believes it has already shown the
commit it prints a single padding line.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
graph.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/graph.c b/graph.c
index 391a712..2a3fc5c 100644
--- a/graph.c
+++ b/graph.c
@@ -1227,6 +1227,16 @@ void graph_show_commit(struct git_graph *graph)
if (!graph)
return;
+ /*
+ * When showing a diff of a merge against each of its parents, we
+ * are called once for each parent without graph_update having been
+ * called. In this case, simply output a single padding line.
+ */
+ if (graph_is_commit_finished(graph)) {
+ graph_show_padding(graph);
+ shown_commit_line = 1;
+ }
+
while (!shown_commit_line && !graph_is_commit_finished(graph)) {
shown_commit_line = graph_next_line(graph, &msgbuf);
fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 4/6] diff: add diff_line_prefix function
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>
This is a helper function to call the diff output_prefix function and
return its value as a C string, allowing us to greatly simplify
everywhere that needs to get the output prefix.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
diff.c | 10 ++++++++++
diff.h | 3 +++
2 files changed, 13 insertions(+)
diff --git a/diff.c b/diff.c
index 73ae02d..ed14d5d 100644
--- a/diff.c
+++ b/diff.c
@@ -1105,6 +1105,16 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix)
return "";
}
+const char *diff_line_prefix(struct diff_options *opt)
+{
+ struct strbuf *msgbuf;
+ if (!opt->output_prefix)
+ return "";
+
+ msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
+ return msgbuf->buf;
+}
+
static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, unsigned long len)
{
const char *cp;
diff --git a/diff.h b/diff.h
index a47bae4..76830e2 100644
--- a/diff.h
+++ b/diff.h
@@ -174,6 +174,9 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix);
diff_get_color((o)->use_color, ix)
+const char *diff_line_prefix(struct diff_options *);
+
+
extern const char mime_boundary_leader[];
extern void diff_tree_setup_paths(const char **paths, struct diff_options *);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 0/6] Improve "git log --graph" output of merges
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
This series changes a couple of places that do not currently indent
their output when being shown with a graph.
The first patch was already posted [1] and addresses the output of "git
log --graph -c -p". Patch 2 is an independent fix I noticed while
working on the later patches.
Patches 3-5 introduce a helper function and change existing sites using
diff_options->output_prefix to use it, resulting in a net reduction by
about 60 lines. There is no functional change here.
The final patch uses the helper introduced in patch 4 to make combined
diffs should the output prefix before each line. This affects the
output of "git log --graph --cc [-p|--raw]".
[1] http://article.gmane.org/gmane.comp.version-control.git/215630
John Keeping (6):
graph: output padding for merge subsequent parents
diff: write prefix to the correct file
diff.c: make constant string arguments const
diff: add diff_line_prefix function
diff.c: use diff_line_prefix() where applicable
combine-diff.c: teach combined diffs about line prefix
combine-diff.c | 47 +++++++++++++--------
diff.c | 131 +++++++++++++++------------------------------------------
diff.h | 3 ++
graph.c | 10 +++++
4 files changed, 77 insertions(+), 114 deletions(-)
--
1.8.1.2
^ permalink raw reply
* [PATCH 5/6] diff.c: use diff_line_prefix() where applicable
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>
Signed-off-by: John Keeping <john@keeping.me.uk>
---
diff.c | 115 ++++++++++++-----------------------------------------------------
1 file changed, 20 insertions(+), 95 deletions(-)
diff --git a/diff.c b/diff.c
index ed14d5d..f441f6c 100644
--- a/diff.c
+++ b/diff.c
@@ -402,12 +402,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
int nofirst;
FILE *file = o->file;
- if (o->output_prefix) {
- struct strbuf *msg = NULL;
- msg = o->output_prefix(o, o->output_prefix_data);
- assert(msg);
- fwrite(msg->buf, msg->len, 1, file);
- }
+ fputs(diff_line_prefix(o), file);
if (len == 0) {
has_trailing_newline = (first == '\n');
@@ -625,13 +620,7 @@ static void emit_rewrite_diff(const char *name_a,
char *data_one, *data_two;
size_t size_one, size_two;
struct emit_callback ecbdata;
- char *line_prefix = "";
- struct strbuf *msgbuf;
-
- if (o && o->output_prefix) {
- msgbuf = o->output_prefix(o, o->output_prefix_data);
- line_prefix = msgbuf->buf;
- }
+ const char *line_prefix = diff_line_prefix(o);
if (diff_mnemonic_prefix && DIFF_OPT_TST(o, REVERSE_DIFF)) {
a_prefix = o->b_prefix;
@@ -827,18 +816,14 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
int minus_first, minus_len, plus_first, plus_len;
const char *minus_begin, *minus_end, *plus_begin, *plus_end;
struct diff_options *opt = diff_words->opt;
- struct strbuf *msgbuf;
- char *line_prefix = "";
+ const char *line_prefix;
if (line[0] != '@' || parse_hunk_header(line, len,
&minus_first, &minus_len, &plus_first, &plus_len))
return;
assert(opt);
- if (opt->output_prefix) {
- msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
- line_prefix = msgbuf->buf;
- }
+ line_prefix = diff_line_prefix(opt);
/* POSIX requires that first be decremented by one if len == 0... */
if (minus_len) {
@@ -962,14 +947,10 @@ static void diff_words_show(struct diff_words_data *diff_words)
struct diff_words_style *style = diff_words->style;
struct diff_options *opt = diff_words->opt;
- struct strbuf *msgbuf;
- char *line_prefix = "";
+ const char *line_prefix;
assert(opt);
- if (opt->output_prefix) {
- msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
- line_prefix = msgbuf->buf;
- }
+ line_prefix = diff_line_prefix(opt);
/* special case: only removal */
if (!diff_words->plus.text.size) {
@@ -1155,13 +1136,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
struct diff_options *o = ecbdata->opt;
- char *line_prefix = "";
- struct strbuf *msgbuf;
-
- if (o && o->output_prefix) {
- msgbuf = o->output_prefix(o, o->output_prefix_data);
- line_prefix = msgbuf->buf;
- }
+ const char *line_prefix = diff_line_prefix(o);
if (ecbdata->header) {
fprintf(ecbdata->opt->file, "%s", ecbdata->header->buf);
@@ -1485,16 +1460,11 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
const char *reset, *add_c, *del_c;
const char *line_prefix = "";
int extra_shown = 0;
- struct strbuf *msg = NULL;
if (data->nr == 0)
return;
- if (options->output_prefix) {
- msg = options->output_prefix(options, options->output_prefix_data);
- line_prefix = msg->buf;
- }
-
+ line_prefix = diff_line_prefix(options);
count = options->stat_count ? options->stat_count : data->nr;
reset = diff_get_color_opt(options, DIFF_RESET);
@@ -1746,12 +1716,7 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option
dels += deleted;
}
}
- if (options->output_prefix) {
- struct strbuf *msg = NULL;
- msg = options->output_prefix(options,
- options->output_prefix_data);
- fprintf(options->file, "%s", msg->buf);
- }
+ fprintf(options->file, "%s", diff_line_prefix(options));
print_stat_summary(options->file, total_files, adds, dels);
}
@@ -1765,12 +1730,7 @@ static void show_numstat(struct diffstat_t *data, struct diff_options *options)
for (i = 0; i < data->nr; i++) {
struct diffstat_file *file = data->files[i];
- if (options->output_prefix) {
- struct strbuf *msg = NULL;
- msg = options->output_prefix(options,
- options->output_prefix_data);
- fprintf(options->file, "%s", msg->buf);
- }
+ fprintf(options->file, "%s", diff_line_prefix(options));
if (file->is_binary)
fprintf(options->file, "-\t-\t");
@@ -1812,13 +1772,7 @@ static long gather_dirstat(struct diff_options *opt, struct dirstat_dir *dir,
{
unsigned long this_dir = 0;
unsigned int sources = 0;
- const char *line_prefix = "";
- struct strbuf *msg = NULL;
-
- if (opt->output_prefix) {
- msg = opt->output_prefix(opt, opt->output_prefix_data);
- line_prefix = msg->buf;
- }
+ const char *line_prefix = diff_line_prefix(opt);
while (dir->nr) {
struct dirstat_file *f = dir->files;
@@ -2068,15 +2022,10 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
const char *reset = diff_get_color(data->o->use_color, DIFF_RESET);
const char *set = diff_get_color(data->o->use_color, DIFF_FILE_NEW);
char *err;
- char *line_prefix = "";
- struct strbuf *msgbuf;
+ const char *line_prefix;
assert(data->o);
- if (data->o->output_prefix) {
- msgbuf = data->o->output_prefix(data->o,
- data->o->output_prefix_data);
- line_prefix = msgbuf->buf;
- }
+ line_prefix = diff_line_prefix(data->o);
if (line[0] == '+') {
unsigned bad;
@@ -2263,13 +2212,7 @@ static void builtin_diff(const char *name_a,
struct userdiff_driver *textconv_one = NULL;
struct userdiff_driver *textconv_two = NULL;
struct strbuf header = STRBUF_INIT;
- struct strbuf *msgbuf;
- char *line_prefix = "";
-
- if (o->output_prefix) {
- msgbuf = o->output_prefix(o, o->output_prefix_data);
- line_prefix = msgbuf->buf;
- }
+ const char *line_prefix = diff_line_prefix(o);
if (DIFF_OPT_TST(o, SUBMODULE_LOG) &&
(!one->mode || S_ISGITLINK(one->mode)) &&
@@ -2968,14 +2911,9 @@ static void fill_metainfo(struct strbuf *msg,
{
const char *set = diff_get_color(use_color, DIFF_METAINFO);
const char *reset = diff_get_color(use_color, DIFF_RESET);
- struct strbuf *msgbuf;
- char *line_prefix = "";
+ const char *line_prefix = diff_line_prefix(o);
*must_show_header = 1;
- if (o->output_prefix) {
- msgbuf = o->output_prefix(o, o->output_prefix_data);
- line_prefix = msgbuf->buf;
- }
strbuf_init(msg, PATH_MAX * 2 + 300);
switch (p->status) {
case DIFF_STATUS_COPIED:
@@ -3912,12 +3850,8 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)
{
int line_termination = opt->line_termination;
int inter_name_termination = line_termination ? '\t' : '\0';
- if (opt->output_prefix) {
- struct strbuf *msg = NULL;
- msg = opt->output_prefix(opt, opt->output_prefix_data);
- fprintf(opt->file, "%s", msg->buf);
- }
+ fprintf(opt->file, "%s", diff_line_prefix(opt));
if (!(opt->output_format & DIFF_FORMAT_NAME_STATUS)) {
fprintf(opt->file, ":%06o %06o %s ", p->one->mode, p->two->mode,
diff_unique_abbrev(p->one->sha1, opt->abbrev));
@@ -4187,12 +4121,7 @@ static void show_rename_copy(FILE *file, const char *renamecopy, struct diff_fil
static void diff_summary(struct diff_options *opt, struct diff_filepair *p)
{
FILE *file = opt->file;
- char *line_prefix = "";
-
- if (opt->output_prefix) {
- struct strbuf *buf = opt->output_prefix(opt, opt->output_prefix_data);
- line_prefix = buf->buf;
- }
+ const char *line_prefix = diff_line_prefix(opt);
switch(p->status) {
case DIFF_STATUS_DELETED:
@@ -4493,13 +4422,9 @@ void diff_flush(struct diff_options *options)
if (output_format & DIFF_FORMAT_PATCH) {
if (separator) {
- if (options->output_prefix) {
- struct strbuf *msg = NULL;
- msg = options->output_prefix(options,
- options->output_prefix_data);
- fwrite(msg->buf, msg->len, 1, options->file);
- }
- putc(options->line_termination, options->file);
+ fprintf(options->file, "%s%c",
+ diff_line_prefix(options),
+ options->line_termination);
if (options->stat_sep) {
/* attach patch instead of inline */
fputs(options->stat_sep, options->file);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 6/6] combine-diff.c: teach combined diffs about line prefix
From: John Keeping @ 2013-02-07 20:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>
When running "git log --graph --cc -p" the diff output for merges is not
indented by the graph structure, unlike the diffs of non-merge commits
(added in commit 7be5761 - diff.c: Output the text graph padding before
each diff line).
Fix this by teaching the combined diff code to output diff_line_prefix()
before each line.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
combine-diff.c | 47 ++++++++++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index bb1cc96..35c817b 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -526,7 +526,8 @@ static void show_line_to_eol(const char *line, int len, const char *reset)
saw_cr_at_eol ? "\r" : "");
}
-static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
+static void dump_sline(struct sline *sline, const char *line_prefix,
+ unsigned long cnt, int num_parent,
int use_color, int result_deleted)
{
unsigned long mark = (1UL<<num_parent);
@@ -582,7 +583,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
rlines -= null_context;
}
- fputs(c_frag, stdout);
+ printf("%s%s", line_prefix, c_frag);
for (i = 0; i <= num_parent; i++) putchar(combine_marker);
for (i = 0; i < num_parent; i++)
show_parent_lno(sline, lno, hunk_end, i, null_context);
@@ -614,7 +615,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
struct sline *sl = &sline[lno++];
ll = (sl->flag & no_pre_delete) ? NULL : sl->lost_head;
while (ll) {
- fputs(c_old, stdout);
+ printf("%s%s", line_prefix, c_old);
for (j = 0; j < num_parent; j++) {
if (ll->parent_map & (1UL<<j))
putchar('-');
@@ -627,6 +628,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
if (cnt < lno)
break;
p_mask = 1;
+ fputs(line_prefix, stdout);
if (!(sl->flag & (mark-1))) {
/*
* This sline was here to hang the
@@ -680,11 +682,13 @@ static void reuse_combine_diff(struct sline *sline, unsigned long cnt,
static void dump_quoted_path(const char *head,
const char *prefix,
const char *path,
+ const char *line_prefix,
const char *c_meta, const char *c_reset)
{
static struct strbuf buf = STRBUF_INIT;
strbuf_reset(&buf);
+ strbuf_addstr(&buf, line_prefix);
strbuf_addstr(&buf, c_meta);
strbuf_addstr(&buf, head);
quote_two_c_style(&buf, prefix, path, 0);
@@ -696,6 +700,7 @@ static void show_combined_header(struct combine_diff_path *elem,
int num_parent,
int dense,
struct rev_info *rev,
+ const char *line_prefix,
int mode_differs,
int show_file_header)
{
@@ -714,8 +719,8 @@ static void show_combined_header(struct combine_diff_path *elem,
show_log(rev);
dump_quoted_path(dense ? "diff --cc " : "diff --combined ",
- "", elem->path, c_meta, c_reset);
- printf("%sindex ", c_meta);
+ "", elem->path, line_prefix, c_meta, c_reset);
+ printf("%s%sindex ", line_prefix, c_meta);
for (i = 0; i < num_parent; i++) {
abb = find_unique_abbrev(elem->parent[i].sha1,
abbrev);
@@ -734,11 +739,12 @@ static void show_combined_header(struct combine_diff_path *elem,
DIFF_STATUS_ADDED)
added = 0;
if (added)
- printf("%snew file mode %06o",
- c_meta, elem->mode);
+ printf("%s%snew file mode %06o",
+ line_prefix, c_meta, elem->mode);
else {
if (deleted)
- printf("%sdeleted file ", c_meta);
+ printf("%s%sdeleted file ",
+ line_prefix, c_meta);
printf("mode ");
for (i = 0; i < num_parent; i++) {
printf("%s%06o", i ? "," : "",
@@ -755,16 +761,16 @@ static void show_combined_header(struct combine_diff_path *elem,
if (added)
dump_quoted_path("--- ", "", "/dev/null",
- c_meta, c_reset);
+ line_prefix, c_meta, c_reset);
else
dump_quoted_path("--- ", a_prefix, elem->path,
- c_meta, c_reset);
+ line_prefix, c_meta, c_reset);
if (deleted)
dump_quoted_path("+++ ", "", "/dev/null",
- c_meta, c_reset);
+ line_prefix, c_meta, c_reset);
else
dump_quoted_path("+++ ", b_prefix, elem->path,
- c_meta, c_reset);
+ line_prefix, c_meta, c_reset);
}
static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
@@ -782,6 +788,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
struct userdiff_driver *userdiff;
struct userdiff_driver *textconv = NULL;
int is_binary;
+ const char *line_prefix = diff_line_prefix(opt);
context = opt->context;
userdiff = userdiff_find_by_path(elem->path);
@@ -901,7 +908,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
}
if (is_binary) {
show_combined_header(elem, num_parent, dense, rev,
- mode_differs, 0);
+ line_prefix, mode_differs, 0);
printf("Binary files differ\n");
free(result);
return;
@@ -962,8 +969,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
if (show_hunks || mode_differs || working_tree_file) {
show_combined_header(elem, num_parent, dense, rev,
- mode_differs, 1);
- dump_sline(sline, cnt, num_parent,
+ line_prefix, mode_differs, 1);
+ dump_sline(sline, line_prefix, cnt, num_parent,
opt->use_color, result_deleted);
}
free(result);
@@ -990,6 +997,7 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
int i, offset;
const char *prefix;
int line_termination, inter_name_termination;
+ const char *line_prefix = diff_line_prefix(opt);
line_termination = opt->line_termination;
inter_name_termination = '\t';
@@ -1000,6 +1008,7 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
show_log(rev);
if (opt->output_format & DIFF_FORMAT_RAW) {
+ printf("%s", line_prefix);
offset = strlen(COLONS) - num_parent;
if (offset < 0)
offset = 0;
@@ -1040,6 +1049,7 @@ void show_combined_diff(struct combine_diff_path *p,
struct rev_info *rev)
{
struct diff_options *opt = &rev->diffopt;
+
if (!p->len)
return;
if (opt->output_format & (DIFF_FORMAT_RAW |
@@ -1150,8 +1160,10 @@ void diff_tree_combined(const unsigned char *sha1,
if (show_log_first && i == 0) {
show_log(rev);
+
if (rev->verbose_header && opt->output_format)
- putchar(opt->line_termination);
+ printf("%s%c", diff_line_prefix(opt),
+ opt->line_termination);
}
diff_flush(&diffopts);
}
@@ -1179,7 +1191,8 @@ void diff_tree_combined(const unsigned char *sha1,
if (opt->output_format & DIFF_FORMAT_PATCH) {
if (needsep)
- putchar(opt->line_termination);
+ printf("%s%c", diff_line_prefix(opt),
+ opt->line_termination);
for (p = paths; p; p = p->next) {
if (p->len)
show_patch_diff(p, num_parent, dense,
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH 0/6] Improve "git log --graph" output of merges
From: Junio C Hamano @ 2013-02-07 20:53 UTC (permalink / raw)
To: John Keeping; +Cc: git, Jeff King
In-Reply-To: <cover.1360267849.git.john@keeping.me.uk>
John Keeping <john@keeping.me.uk> writes:
> This series changes a couple of places that do not currently indent
> their output when being shown with a graph.
>
> The first patch was already posted [1] and addresses the output of "git
> log --graph -c -p". Patch 2 is an independent fix I noticed while
> working on the later patches.
>
> Patches 3-5 introduce a helper function and change existing sites using
> diff_options->output_prefix to use it, resulting in a net reduction by
> about 60 lines. There is no functional change here.
>
> The final patch uses the helper introduced in patch 4 to make combined
> diffs should the output prefix before each line. This affects the
> output of "git log --graph --cc [-p|--raw]".
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/215630
Overall direction looks sensible.
It is a bit scary that we are doing the the fifth one now, as
opposed to catching this repetitiveness during the initial review of
patch series that added the graph code to the diff codepath.
Thanks.
> John Keeping (6):
> graph: output padding for merge subsequent parents
> diff: write prefix to the correct file
> diff.c: make constant string arguments const
> diff: add diff_line_prefix function
> diff.c: use diff_line_prefix() where applicable
> combine-diff.c: teach combined diffs about line prefix
>
> combine-diff.c | 47 +++++++++++++--------
> diff.c | 131 +++++++++++++++------------------------------------------
> diff.h | 3 ++
> graph.c | 10 +++++
> 4 files changed, 77 insertions(+), 114 deletions(-)
^ permalink raw reply
* Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
From: Junio C Hamano @ 2013-02-07 21:05 UTC (permalink / raw)
To: Matt Kraai; +Cc: git, Max Horn, Jeff King, Matt Kraai
In-Reply-To: <1360267238-21896-1-git-send-email-kraai@ftbfs.org>
Matt Kraai <kraai@ftbfs.org> writes:
> -#if defined(__GNUC__) && ! defined(__clang__)
> -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
> -#endif
> +#define error(...) (error(__VA_ARGS__), -1)
Before your change, we only define error() macro for GCC variants,
but with your patch that no longer is the case. Does every compiler
that compiles Git correctly today support this style of varargs
macros?
^ permalink raw reply
* Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
From: John Keeping @ 2013-02-07 21:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matt Kraai, git, Max Horn, Jeff King, Matt Kraai
In-Reply-To: <7vwquj6dio.fsf@alter.siamese.dyndns.org>
On Thu, Feb 07, 2013 at 01:05:19PM -0800, Junio C Hamano wrote:
> Matt Kraai <kraai@ftbfs.org> writes:
>
> > -#if defined(__GNUC__) && ! defined(__clang__)
> > -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
> > -#endif
> > +#define error(...) (error(__VA_ARGS__), -1)
>
> Before your change, we only define error() macro for GCC variants,
> but with your patch that no longer is the case. Does every compiler
> that compiles Git correctly today support this style of varargs
> macros?
At the very least the "! defined(__clang__)" was only recently added
[1], although it shouldn't be needed once Clang 3.3 is released.
[1] http://article.gmane.org/gmane.comp.version-control.git/213787
John
^ permalink raw reply
* Re: [PATCH] Use __VA_ARGS__ for all of error's arguments
From: Matt Kraai @ 2013-02-07 21:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Max Horn, Jeff King, Matt Kraai
In-Reply-To: <7vwquj6dio.fsf@alter.siamese.dyndns.org>
On Thu, Feb 07, 2013 at 01:05:19PM -0800, Junio C Hamano wrote:
> Matt Kraai <kraai@ftbfs.org> writes:
>
> > -#if defined(__GNUC__) && ! defined(__clang__)
> > -#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
> > -#endif
> > +#define error(...) (error(__VA_ARGS__), -1)
>
> Before your change, we only define error() macro for GCC variants,
> but with your patch that no longer is the case. Does every compiler
> that compiles Git correctly today support this style of varargs
> macros?
I don't know and I don't think it's likely I can confirm this. I'll
submit a new patch that just changes the definition, since I don't
know of any problems other than mine with the current situation.
--
Matt
^ permalink raw reply
* [PATCH] Use __VA_ARGS__ for all of error's arguments
From: Matt Kraai @ 2013-02-07 21:30 UTC (permalink / raw)
To: git, Max Horn, Junio C Hamano, Jeff King; +Cc: Matt Kraai
In-Reply-To: <20130207212438.GA22253@ftbfs.org>
From: Matt Kraai <matt.kraai@amo.abbott.com>
QNX 6.3.2 uses GCC 2.95.3 by default, and GCC 2.95.3 doesn't remove the
comma if the error macro's variable argument is left out.
Instead of testing for a sufficiently recent version of GCC, make
__VA_ARGS__ match all of the arguments.
Signed-off-by: Matt Kraai <matt.kraai@amo.abbott.com>
---
git-compat-util.h | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index cc2abee..2e960a9 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -305,13 +305,10 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)))
/*
* Let callers be aware of the constant return value; this can help
- * gcc with -Wuninitialized analysis. We have to restrict this trick to
- * gcc, though, because of the variadic macro and the magic ## comma pasting
- * behavior. But since we're only trying to help gcc, anyway, it's OK; other
- * compilers will fall back to using the function as usual.
+ * gcc with -Wuninitialized analysis.
*/
#if defined(__GNUC__) && ! defined(__clang__)
-#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
+#define error(...) (error(__VA_ARGS__), -1)
#endif
extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
--
1.8.1.2.546.gfc9f004
^ permalink raw reply related
* Re: Proposal: branch.<name>.remotepush
From: Jonathan Nieder @ 2013-02-07 23:30 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Michael Schubert, Git List, Junio C Hamano
In-Reply-To: <CALkWK0=53riU3xKbKkyAVS8--9VoAU5P6h88MQ9-geW=H5+a-w@mail.gmail.com>
Hi Ram,
Ramkumar Ramachandra wrote:
> And yes, a regular `git push origin refs/for/master` is just retarded.
The usual incantation is "git push gerrit HEAD:refs/for/master". Is
the code review creation push that uses a different branchname from
the branch the integrator pulls what seems backward, or is it the need
to specify a refname at all on the command line?
I agree that a "[branch "master"] pushremote" configuration would be
handy. pushremote instead of remotepush to be less surprising to
people who have already seen pushurl.
Good luck,
Jonathan
^ 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