Re: Bug: send-pack does not respect http.signingkey
Dave Borowitz writes: > Should I formally send a patch with your configuration one-liner? Yeah, the log message, that explains the motivation of the change and the decision to read which part of the configuration, is much more important than the actual patch, so please do so ;-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: send-pack does not respect http.signingkey
On Thu, Jul 16, 2015 at 3:08 PM, Dave Borowitz wrote: > On Thu, Jul 16, 2015 at 2:10 PM, Junio C Hamano wrote: >> Dave Borowitz writes: >> >>> On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano wrote: Dave Borowitz writes: > On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano wrote: > >> Perhaps something like this? > > Seems like it should work. > > Jonathan had suggested there might be some principled reason why > send-pack does not respect config options, and suggested passing it in > as a flag. But that would be more work, certainly, as it would also > have to get passed through git-remote-http somehow. I actually was wondering about exactly the same thing as Jonathan, and that is where my "Perhaps" came from. >>> >>> I will say, though, as the maintainer of a handful of custom remote >>> helpers, I would prefer a solution that does not involve changing the >>> implementation of those just to pass this configuration through. >> >> That is not a controversial part ;-) >> >>> So my >>> vote would be for send-pack to respect the normal config options. >> >> The thing is what should be included in the "normal" config options. >> >> The "something like this?" patch was deliberately narrow, including >> only the GPG thing and nothing else. But anticipating that the ref >> backend would be per repo configuration, and send-pack would want to >> read from refs (and possibly write back tracking?), we may want to >> prepare ourselves by reading a bit wider than "GPG thing and nothing >> else", e.g. git_default_config() or something like that. > > Ah, now I understand the question. I have no opinion other than that > we shouldn't let discussion about future features prevent us from > fixing this obvious signed push bug :) Should I formally send a patch with your configuration one-liner? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: send-pack does not respect http.signingkey
On Thu, Jul 16, 2015 at 2:10 PM, Junio C Hamano wrote: > Dave Borowitz writes: > >> On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano wrote: >>> Dave Borowitz writes: >>> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano wrote: > Perhaps something like this? Seems like it should work. Jonathan had suggested there might be some principled reason why send-pack does not respect config options, and suggested passing it in as a flag. But that would be more work, certainly, as it would also have to get passed through git-remote-http somehow. >>> >>> I actually was wondering about exactly the same thing as Jonathan, >>> and that is where my "Perhaps" came from. >> >> I will say, though, as the maintainer of a handful of custom remote >> helpers, I would prefer a solution that does not involve changing the >> implementation of those just to pass this configuration through. > > That is not a controversial part ;-) > >> So my >> vote would be for send-pack to respect the normal config options. > > The thing is what should be included in the "normal" config options. > > The "something like this?" patch was deliberately narrow, including > only the GPG thing and nothing else. But anticipating that the ref > backend would be per repo configuration, and send-pack would want to > read from refs (and possibly write back tracking?), we may want to > prepare ourselves by reading a bit wider than "GPG thing and nothing > else", e.g. git_default_config() or something like that. Ah, now I understand the question. I have no opinion other than that we shouldn't let discussion about future features prevent us from fixing this obvious signed push bug :) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: send-pack does not respect http.signingkey
Dave Borowitz writes: > On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano wrote: >> Dave Borowitz writes: >> >>> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano wrote: >>> Perhaps something like this? >>> >>> Seems like it should work. >>> >>> Jonathan had suggested there might be some principled reason why >>> send-pack does not respect config options, and suggested passing it in >>> as a flag. But that would be more work, certainly, as it would also >>> have to get passed through git-remote-http somehow. >> >> I actually was wondering about exactly the same thing as Jonathan, >> and that is where my "Perhaps" came from. > > I will say, though, as the maintainer of a handful of custom remote > helpers, I would prefer a solution that does not involve changing the > implementation of those just to pass this configuration through. That is not a controversial part ;-) > So my > vote would be for send-pack to respect the normal config options. The thing is what should be included in the "normal" config options. The "something like this?" patch was deliberately narrow, including only the GPG thing and nothing else. But anticipating that the ref backend would be per repo configuration, and send-pack would want to read from refs (and possibly write back tracking?), we may want to prepare ourselves by reading a bit wider than "GPG thing and nothing else", e.g. git_default_config() or something like that. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: send-pack does not respect http.signingkey
On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano wrote: > Dave Borowitz writes: > >> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano wrote: >> >>> Perhaps something like this? >> >> Seems like it should work. >> >> Jonathan had suggested there might be some principled reason why >> send-pack does not respect config options, and suggested passing it in >> as a flag. But that would be more work, certainly, as it would also >> have to get passed through git-remote-http somehow. > > I actually was wondering about exactly the same thing as Jonathan, > and that is where my "Perhaps" came from. I will say, though, as the maintainer of a handful of custom remote helpers, I would prefer a solution that does not involve changing the implementation of those just to pass this configuration through. So my vote would be for send-pack to respect the normal config options. Not sure what that would mean for -c on the command line, though. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: send-pack does not respect http.signingkey
Dave Borowitz writes: > On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano wrote: > >> Perhaps something like this? > > Seems like it should work. > > Jonathan had suggested there might be some principled reason why > send-pack does not respect config options, and suggested passing it in > as a flag. But that would be more work, certainly, as it would also > have to get passed through git-remote-http somehow. I actually was wondering about exactly the same thing as Jonathan, and that is where my "Perhaps" came from. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: send-pack does not respect http.signingkey
On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano wrote:
> Dave Borowitz writes:
>
>> When git-send-pack is exec'ed, as is done by git-remote-http, it does
>> not reread the config, so it does not respect the configured
>> http.signingkey, either from the config file or -c on the command
>> line. Thus it is currently impossible to specify a signing key over
>> HTTP, other than the default one matching the "Name " format in
>> the keyring.
>>
>> This is not an issue for git:// as send-pack is executed directly in
>> the same process that reads the config.
>
> Interesting. I agree that it would be a problem not to be able to
> specify which signing key to use.
>
> Perhaps something like this?
Seems like it should work.
Jonathan had suggested there might be some principled reason why
send-pack does not respect config options, and suggested passing it in
as a flag. But that would be more work, certainly, as it would also
have to get passed through git-remote-http somehow.
> diff --git a/builtin/send-pack.c b/builtin/send-pack.c
> index b564a77..57c3a9c 100644
> --- a/builtin/send-pack.c
> +++ b/builtin/send-pack.c
> @@ -11,6 +11,7 @@
> #include "transport.h"
> #include "version.h"
> #include "sha1-array.h"
> +#include "gpg-interface.h"
>
> static const char send_pack_usage[] =
> "git send-pack [--all | --mirror] [--dry-run] [--force]
> [--receive-pack=] [--verbose] [--thin] [:]
> [...]\n"
> @@ -113,6 +114,8 @@ int cmd_send_pack(int argc, const char **argv, const char
> *prefix)
> int from_stdin = 0;
> struct push_cas_option cas = {0};
>
> + git_config(git_gpg_config, NULL);
> +
> argv++;
> for (i = 1; i < argc; i++, argv++) {
> const char *arg = *argv;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: send-pack does not respect http.signingkey
Dave Borowitz writes:
> When git-send-pack is exec'ed, as is done by git-remote-http, it does
> not reread the config, so it does not respect the configured
> http.signingkey, either from the config file or -c on the command
> line. Thus it is currently impossible to specify a signing key over
> HTTP, other than the default one matching the "Name " format in
> the keyring.
>
> This is not an issue for git:// as send-pack is executed directly in
> the same process that reads the config.
Interesting. I agree that it would be a problem not to be able to
specify which signing key to use.
Perhaps something like this?
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index b564a77..57c3a9c 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -11,6 +11,7 @@
#include "transport.h"
#include "version.h"
#include "sha1-array.h"
+#include "gpg-interface.h"
static const char send_pack_usage[] =
"git send-pack [--all | --mirror] [--dry-run] [--force]
[--receive-pack=] [--verbose] [--thin] [:]
[...]\n"
@@ -113,6 +114,8 @@ int cmd_send_pack(int argc, const char **argv, const char
*prefix)
int from_stdin = 0;
struct push_cas_option cas = {0};
+ git_config(git_gpg_config, NULL);
+
argv++;
for (i = 1; i < argc; i++, argv++) {
const char *arg = *argv;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bug: send-pack does not respect http.signingkey
When git-send-pack is exec'ed, as is done by git-remote-http, it does
not reread the config, so it does not respect the configured
http.signingkey, either from the config file or -c on the command
line. Thus it is currently impossible to specify a signing key over
HTTP, other than the default one matching the "Name " format in
the keyring.
This is not an issue for git:// as send-pack is executed directly in
the same process that reads the config.
---
gpg-interface.c | 1 +
run-command.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/gpg-interface.c b/gpg-interface.c
index 68b0c81..e0ffcb0 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -87,6 +87,7 @@ void set_signing_key(const char *key)
int git_gpg_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "user.signingkey")) {
+ fprintf(stderr, "setting %s\n", value);
set_signing_key(value);
}
if (!strcmp(var, "gpg.program")) {
diff --git a/run-command.c b/run-command.c
index 4d73e90..39ae8d5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "run-command.h"
+#include "gpg-interface.h"
#include "exec_cmd.h"
#include "sigchain.h"
#include "argv-array.h"
@@ -344,7 +345,7 @@ fail_pipe:
cmd->err = fderr[0];
}
- trace_argv_printf(cmd->argv, "trace: run_command:");
+ trace_argv_printf(cmd->argv, "trace: run_command (%s):", get_signing_key());
fflush(NULL);
#ifndef GIT_WINDOWS_NATIVE
--
2.4.3.573.g4eafbef
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
