* [PATCHv2 3/5] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360242782.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
The command_close_bidi_pipe() function will insist on closing both
input and output pipes returned by command_bidi_pipe(). With this
change it is possible to close one of the pipes in advance and
pass undef as an argument.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
> On Wed, Feb 06, 2013 at 09:47:04PM +0100, Michal Nazarewicz wrote:
>> This allows for something like:
>>
>> my ($pid, $in, $out, $ctx) = command_bidi_pipe(...);
>> print $out "write data";
>> close $out;
>> # ... do stuff with $in
>> command_close_bidi_pipe($pid, $in, undef, $ctx);
On Thu, Feb 07 2013, Jeff King <peff@peff.net> wrote:
> Should this part go into the documentation for command_close_bidi_pipe
> in Git.pm?
Done.
diff --git a/perl/Git.pm b/perl/Git.pm
index 11f310a..9dded54 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -426,13 +426,26 @@ Note that you should not rely on whatever actually is in C<CTX>;
currently it is simply the command name but in future the context might
have more complicated structure.
+C<PIPE_IN> and C<PIPE_OUT> may be C<undef> if they have been closed prior to
+calling this function. This may be useful in a query-response type of
+commands where caller first writes a query and later reads response, eg:
+
+ my ($pid, $in, $out, $ctx) = $r->command_bidi_pipe('cat-file --batch-check');
+ print $out "000000000\n";
+ close $out;
+ while (<$in>) { ... }
+ $r->command_close_bidi_pipe($pid, $in, undef, $ctx);
+
+This idiom may prevent potential dead locks caused by data sent to the output
+pipe not being flushed and thus not reaching the executed command.
+
=cut
sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
- unless (close $fh) {
+ if (defined $fh && !close $fh) {
if ($!) {
carp "error closing pipe: $!";
} elsif ($? >> 8) {
--
1.8.1.2.549.g1d13f9f
^ permalink raw reply related
* [PATCHv2 5/5] git-send-email: use git credential to obtain password
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360242782.git.mina86@mina86.com>
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>
---
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;
--
1.8.1.2.549.g1d13f9f
^ permalink raw reply related
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Ted Zlatanov @ 2013-02-07 14:30 UTC (permalink / raw)
To: Matthieu Moy
Cc: Jeff King, Junio C Hamano, Michal Nazarewicz, git,
Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <vpq4nhowqgk.fsf@grenoble-inp.fr>
On Thu, 07 Feb 2013 08:08:59 +0100 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
MM> Plus, read/write has already been used for a while in the C API, so I'd
MM> rather keep the same names for the Perl equivalent.
That makes perfect sense.
Ted
^ permalink raw reply
* Re: overriding/removing inherited credential.helper, Do not add an empty value from config credential.helper
From: Ted Zlatanov @ 2013-02-07 14:36 UTC (permalink / raw)
To: 乙酸鋰; +Cc: git
In-Reply-To: <CAHtLG6QaHOOYgVFPyOWo44-jTX__cd0dCyu+vs+Uf4_U-HxySw@mail.gmail.com>
On Sat, 10 Nov 2012 23:12:50 +0800 乙酸鋰 <ch3cooli@gmail.com> wrote:
> In credential.c, line 67:
> if (!strcmp(key, "helper"))
> string_list_append(&c->helpers, value);
> In global config, I add one credential helper.
> But I do not want to use any credential helper in a specific repository.
> Currently there is no way in local config to override and remove
> inherited credential helper.
> Of course, I do not want to change global config.
> Could you suggest a patch?
On Sat, 10 Nov 2012 23:08:04 +0800 乙酸鋰 <ch3cooli@gmail.com> wrote:
> Below is current git message when a local config credential.helper has
> an empty value. Please skip an empty value.
> $ git push --force origin master
> git: 'credential-' is not a git command. See 'git --help'.
> Did you mean this?
> credential
> Total 0 (delta 0), reused 0 (delta 0)
> To https://user@github.com/user/myrepo.git
> + d23aa6a...3405990 master -> master (forced update)
I would like that too (needed it today). Maybe the empty string (as
suggested) or "none" could be acceptable.
"none" seems friendlier and is very unlikely to collide with an actual
credential helper.
Ted
^ permalink raw reply
* Re: Credentials and the Secrets API...
From: Ted Zlatanov @ 2013-02-07 14:46 UTC (permalink / raw)
To: John Szakmeister; +Cc: git
In-Reply-To: <CAEBDL5Udooim_3Za76Q1Rt_aGXtsSv76nxRegGWRBE=WJQzfZA@mail.gmail.com>
On Thu, 27 Oct 2011 12:05:03 -0400 John Szakmeister <john@szakmeister.net> wrote:
JS> Just wanted to keep folks in the loop. It turns out that the Secrets
JS> API is still to young. I asked about the format to store credentials
JS> in (as far as attributes), and got a response from a KDE developer
JS> that says it's still to young on their front. They hope to have
JS> support in the next release of KDE. But there's still the issue of
JS> what attributes to use.
JS> With that information, I went ahead and created a
JS> gnome-credential-keyring that uses Gnome's Keyring facility. I still
JS> need to do a few more things (mainly run it against Jeff's tests), but
JS> it's generally working. Just wanted to keep folks in the loop.
JS> Hopefully, I can get patches out this weekend.
Do you think the Secrets API has matured enough? KDE has had a new
release since your post...
Thanks
Ted
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Jed Brown @ 2013-02-07 15:58 UTC (permalink / raw)
To: Michael Haggerty, Junio C Hamano; +Cc: git, Jeff King, Shawn Pearce
In-Reply-To: <51122D9D.9040100@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> A first weakness of your proposal is that even though the hidden refs
> are (optionally) fetchable, there is *no* way to discover them remotely
> or to bulk-download them; they would have to be retrieved one by one
> using out-of-band information. And if I understand correctly, there
> would be no way to push hidden references remotely (whether manually or
> from some automated process). Such processes would have to be local to
> the machine holding the repository.
I'm the author of git-fat [1], a smudge/clean filter system for managing
large files. I currently store files in the file system
(.git/fat/objects) and transfer them via rsync because I want to be able
to transfer exact subsets requested by the user. I would like to put
this data in a git repository so that I can take advantage of packfile
compression when applicable and so that I can use existing access
control, but I would need to store a separate reference to each blob (so
that I can transfer exact subsets). My refs would be named like
'fat-<SHA1_OF_SMUDGED_DATA>' and are known on the client side because
they are in the cleaned blob (which contains only this SHA1 and the
number of bytes [2]).
I believe that my use case would be well supported if git could push and
pull unadvertised refs, as long as basic operations were not slowed down
by the existence of a very large number of such refs.
[1] https://github.com/jedbrown/git-fat
[2] We could eliminate the performance problem of needing to buffer the
entire file if the smudge filter could be passed the object size as an
argument and if we could forward that size in a stream to 'git
hash-object --stdin'.
^ permalink raw reply
* Proposal: branch.<name>.remotepush
From: Ramkumar Ramachandra @ 2013-02-07 16:14 UTC (permalink / raw)
To: Git List
Hi,
This has been annoying me for a really long time, but I never really
got around to scratching this particular itch. I have a very common
scenario where I fork a project on GitHub. I have two configured
remotes: origin which points to "git://upstream" and mine which points
to "ssh://mine". By default, I always want to pull `master` from
origin and push to mine. Unfortunately, there's only a
branch.<name>.remote which specifies which remote to use for both
pulling and pushing. There's also a remote.<name>.pushurl, but I get
the feeling that this exists for an entirely different reason: when I
have a server with a highly-available read-only mirror of the
repository at git://anongit.*, and a less-available committer-only
mirror at ssh://*.
How about a branch.<name>.remotepush that specifies a special remote
for pushing, falling back to branch.<name>.remote?
Ram
^ permalink raw reply
* Re: Proposal: branch.<name>.remotepush
From: Michael Schubert @ 2013-02-07 17:45 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <CALkWK0nA4hQ0VWivk3AVVVq8Rbb-9CpQ9xFsSOsTQtvo4w08rw@mail.gmail.com>
On 02/07/2013 05:14 PM, Ramkumar Ramachandra wrote:
> This has been annoying me for a really long time, but I never really
> got around to scratching this particular itch. I have a very common
> scenario where I fork a project on GitHub. I have two configured
> remotes: origin which points to "git://upstream" and mine which points
> to "ssh://mine". By default, I always want to pull `master` from
> origin and push to mine. Unfortunately, there's only a
> branch.<name>.remote which specifies which remote to use for both
> pulling and pushing. There's also a remote.<name>.pushurl, but I get
> the feeling that this exists for an entirely different reason: when I
> have a server with a highly-available read-only mirror of the
> repository at git://anongit.*, and a less-available committer-only
> mirror at ssh://*.
>
> How about a branch.<name>.remotepush that specifies a special remote
> for pushing, falling back to branch.<name>.remote?
Additionally, it would be nice to have branch.<name>.push or similar
to configure a default destination branch for push. Gerrit users usually
want to track refs/heads/master but push to refs/for/master for example.
^ permalink raw reply
* Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
From: Junio C Hamano @ 2013-02-07 18:03 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Jeff King, Git Mailing List
In-Reply-To: <5113827A.40801@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
>>> (cd t && git grep GET_SHA1_QUIETLY HEAD:../cache.h)
>>> ../HEAD:../cache.h:#define GET_SHA1_QUIETLY 01
>>
>> Yuck.
>
> And even more yuck:
>
> (cd t && git grep --full-name GET_SHA1_QUIETLY HEAD:../cache.h)
> HEAD:../cache.h:#define GET_SHA1_QUIETLY 01
>
> Someone does not expect a "rev:" to be in there, it seems ;)
I think stepping outside of $(cwd) is an afterthought the code does
not anticipate.
^ permalink raw reply
* Re: [PATCH 2/2] count-objects: report garbage files in .git/objects/pack directory too
From: Junio C Hamano @ 2013-02-07 18:12 UTC (permalink / raw)
To: Duy Nguyen; +Cc: git
In-Reply-To: <20130207073751.GA20672@duynguyen-vnpc.dek-tpc.internal>
Duy Nguyen <pclouds@gmail.com> writes:
> I thought about that, but we may need to do extra stat() for loose
> garbage as well. As it is now, garbage is complained loudly, which
> gives me enough motivation to clean up, even without looking at how
> much disk space it uses.
I wouldn't call a single line "garbage: 4" exactly *loud*. I also
think that this is not about *motivating* you, but about giving
more information to the users to help them assess the health of
their repository themselves.
By the way, I wonder if we also want to notice .git/objects/garbage
or .git/objects/ga/rbage if we are to do this?
> -- 8< --
> Subject: count-objects: report garbage pack directory too
>
> prepare_packed_git_one() is modified to allow count-objects to hook a
> report function to so we don't need to duplicate the pack searching
> logic in count-objects.c. When report_pack_garbage is NULL, the
> overhead is insignificant.
>
> diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
> index e816823..1611d7c 100644
> --- a/Documentation/git-count-objects.txt
> +++ b/Documentation/git-count-objects.txt
> @@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB
> prune-packable: the number of loose objects that are also present in
> the packs. These objects could be pruned using `git prune-packed`.
> +
> -garbage: the number of files in loose object database that are not
> -valid loose objects
> +garbage: the number of files in object database that are not valid
> +loose objects nor valid packs
>
> GIT
> ---
> diff --git a/builtin/count-objects.c b/builtin/count-objects.c
> index 9afaa88..7fdd508 100644
> --- a/builtin/count-objects.c
> +++ b/builtin/count-objects.c
> @@ -9,6 +9,8 @@
> #include "builtin.h"
> #include "parse-options.h"
>
> +static unsigned long garbage;
> +
> static void count_objects(DIR *d, char *path, int len, int verbose,
> unsigned long *loose,
> off_t *loose_size,
> @@ -65,6 +67,16 @@ static void count_objects(DIR *d, char *path, int len, int verbose,
> }
> }
>
> +extern void (*report_pack_garbage)(const char *path, int len, const char *name);
> +static void real_report_pack_garbage(const char *path, int len, const char *name)
> +{
> + if (len)
> + error("garbage found: %.*s/%s", len, path, name);
> + else
> + error("garbage found: %s", path);
> + garbage++;
> +}
> +
> static char const * const count_objects_usage[] = {
> N_("git count-objects [-v]"),
> NULL
> @@ -76,7 +88,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
> const char *objdir = get_object_directory();
> int len = strlen(objdir);
> char *path = xmalloc(len + 50);
> - unsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;
> + unsigned long loose = 0, packed = 0, packed_loose = 0;
> off_t loose_size = 0;
> struct option opts[] = {
> OPT__VERBOSE(&verbose, N_("be verbose")),
> @@ -87,6 +99,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
> /* we do not take arguments other than flags for now */
> if (argc)
> usage_with_options(count_objects_usage, opts);
> + if (verbose)
> + report_pack_garbage = real_report_pack_garbage;
> memcpy(path, objdir, len);
> if (len && objdir[len-1] != '/')
> path[len++] = '/';
> diff --git a/sha1_file.c b/sha1_file.c
> index 40b2329..5b70e55 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -21,6 +21,7 @@
> #include "sha1-lookup.h"
> #include "bulk-checkin.h"
> #include "streaming.h"
> +#include "dir.h"
>
> #ifndef O_NOATIME
> #if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
> @@ -1000,15 +1001,19 @@ void install_packed_git(struct packed_git *pack)
> packed_git = pack;
> }
>
> +void (*report_pack_garbage)(const char *path, int len, const char *name);
> +
> static void prepare_packed_git_one(char *objdir, int local)
> {
> /* Ensure that this buffer is large enough so that we can
> append "/pack/" without clobbering the stack even if
> strlen(objdir) were PATH_MAX. */
> char path[PATH_MAX + 1 + 4 + 1 + 1];
> - int len;
> + int i, len;
> DIR *dir;
> struct dirent *de;
> + struct packed_git *p;
> + struct string_list garbage = STRING_LIST_INIT_DUP;
>
> sprintf(path, "%s/pack", objdir);
> len = strlen(path);
> @@ -1024,14 +1029,33 @@ static void prepare_packed_git_one(char *objdir, int local)
> int namelen = strlen(de->d_name);
> struct packed_git *p;
>
> - if (!has_extension(de->d_name, ".idx"))
> + if (len + namelen + 1 > sizeof(path)) {
> + if (report_pack_garbage)
> + report_pack_garbage(path, len - 1, de->d_name);
> continue;
> + }
>
> - if (len + namelen + 1 > sizeof(path))
> + strcpy(path + len, de->d_name);
> +
> + if (!has_extension(de->d_name, ".idx")) {
> + if (!report_pack_garbage)
> + continue;
> + if (is_dot_or_dotdot(de->d_name))
> + continue;
> + if (!has_extension(de->d_name, ".pack")) {
> + report_pack_garbage(path, 0, NULL);
> + continue;
> + }
Didn't I already say the logic should be inverted to whitelist the
known ones? Saying "Anything that is not '.pack' is bad" here is a
direct opposite, I think.
Add "A '.keep' file is OK" to this codeflow and see how it goes.
> + /*
> + * we can't decide right know if this .pack is
> + * garbage. Delay until we identify all good
> + * packs.
> + */
> + string_list_append(&garbage, path);
> continue;
> + }
>
> /* Don't reopen a pack we already have. */
> - strcpy(path + len, de->d_name);
> for (p = packed_git; p; p = p->next) {
> if (!memcmp(path, p->pack_name, len + namelen - 4))
> break;
> @@ -1047,6 +1071,25 @@ static void prepare_packed_git_one(char *objdir, int local)
> install_packed_git(p);
> }
> closedir(dir);
> +
> + if (!report_pack_garbage)
> + return;
> +
> + sort_string_list(&garbage);
> + for (p = packed_git; p; p = p->next) {
> + struct string_list_item *item;
> + if (!p->pack_local)
> + continue;
> + item = string_list_lookup(&garbage, p->pack_name);
> + if (item)
> + item->util = &garbage; /* anything but NULL */
> + }
> + for (i = 0; i < garbage.nr; i++) {
> + struct string_list_item *item = garbage.items + i;
> + if (!item->util)
> + report_pack_garbage(item->string, 0, NULL);
> + }
> + string_list_clear(&garbage, 0);
> }
>
> static int sort_pack(const void *a_, const void *b_)
> -- 8< --
^ permalink raw reply
* Re: overriding/removing inherited credential.helper, Do not add an empty value from config credential.helper
From: Junio C Hamano @ 2013-02-07 18:23 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: 乙酸鋰, git
In-Reply-To: <87pq0cchsz.fsf@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
>> Below is current git message when a local config credential.helper has
>> an empty value. Please skip an empty value.
>
>> $ git push --force origin master
>> git: 'credential-' is not a git command. See 'git --help'.
>> Did you mean this?
>> credential
Why isn't "do not add empty string, or any random string that ends
up referring to a helper that you do not have" a solution?
>> Total 0 (delta 0), reused 0 (delta 0)
>> To https://user@github.com/user/myrepo.git
>> + d23aa6a...3405990 master -> master (forced update)
>
> I would like that too (needed it today). Maybe the empty string (as
> suggested) or "none" could be acceptable.
Whatever you do, I do not think introducing a per-variable hack
[credential]
helper = none ;# or "helper = clear"
helper = mine ;# this is the only thing I use
like that is a sane way to go. "Clear everything you saw so far"
would be useful for variables other than "credential.helper";
shouldn't it be done by adding a general syntax to the configuration
file format and teach the configuration parser to clear the
cumulative definitions so far?
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Junio C Hamano @ 2013-02-07 18:25 UTC (permalink / raw)
To: Jeff King
Cc: Ævar Arnfjörð Bjarmason, Duy Nguyen,
Michael Haggerty, Jonathan Nieder, git, Shawn Pearce
In-Reply-To: <20130207001635.GA29318@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> If the new client can handle the old-style server's response, then the
> server can start blasting out refs (optionally after a timeout) and stop
> when the client interrupts with "hey, wait, I can speak the new
> protocol". The server just has to include "you can interrupt me" in its
> capability advertisement (obviously it would have to send out at least
> the first ref with the capabilities before the timeout).
Yeah, I would prefer people to come up with a way to share the port
and autodetect. It is *not* a requirement for the updated server to
run on a separate port at all.
^ permalink raw reply
* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
From: Ramsay Jones @ 2013-02-07 18:25 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git, j6t
In-Reply-To: <201301271557.08994.tboegi@web.de>
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)
[It's so long since I looked, but I'm pretty sure that the failure
in t0070 is caused by *git*, not by cygwin not supporting POSIXPERM]
> 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: overriding/removing inherited credential.helper, Do not add an empty value from config credential.helper
From: Ted Zlatanov @ 2013-02-07 18:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: 乙酸鋰, git
In-Reply-To: <7vvca47zl3.fsf@alter.siamese.dyndns.org>
On Thu, 07 Feb 2013 10:23:20 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> "Clear everything you saw so far" would be useful for variables
JCH> other than "credential.helper"; shouldn't it be done by adding a
JCH> general syntax to the configuration file format and teach the
JCH> configuration parser to clear the cumulative definitions so far?
That's a question for the configuration parser, right? I don't have an
informed opinion on how it should be implemented, and am OK with any
syntax to clear a variable.
Ted
^ permalink raw reply
* 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
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