* [PATCH 3/2] Split GPG interface into its own helper library
From: Junio C Hamano @ 2011-09-08 4:37 UTC (permalink / raw)
To: git; +Cc: Shawn O. Pearce
In-Reply-To: <7vbouw2hqg.fsf@alter.siamese.dyndns.org>
This moves existing code from builtin/tag.c (for signing) and
builtin/verify-tag.c (for verifying) to a new gpg-interface.c file to
provide a more generic library interface.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Makefile | 2 +
builtin/tag.c | 60 ++++----------------------------
builtin/verify-tag.c | 35 ++-----------------
gpg-interface.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++
gpg-interface.h | 11 ++++++
5 files changed, 117 insertions(+), 85 deletions(-)
create mode 100644 gpg-interface.c
create mode 100644 gpg-interface.h
diff --git a/Makefile b/Makefile
index 8d6d451..2183223 100644
--- a/Makefile
+++ b/Makefile
@@ -530,6 +530,7 @@ LIB_H += exec_cmd.h
LIB_H += fsck.h
LIB_H += gettext.h
LIB_H += git-compat-util.h
+LIB_H += gpg-interface.h
LIB_H += graph.h
LIB_H += grep.h
LIB_H += hash.h
@@ -620,6 +621,7 @@ LIB_OBJS += entry.o
LIB_OBJS += environment.o
LIB_OBJS += exec_cmd.o
LIB_OBJS += fsck.o
+LIB_OBJS += gpg-interface.o
LIB_OBJS += graph.o
LIB_OBJS += grep.o
LIB_OBJS += hash.o
diff --git a/builtin/tag.c b/builtin/tag.c
index 667515e..e9d36fa 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -14,6 +14,7 @@
#include "parse-options.h"
#include "diff.h"
#include "revision.h"
+#include "gpg-interface.h"
static const char * const git_tag_usage[] = {
"git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
@@ -208,60 +209,13 @@ static int verify_tag(const char *name, const char *ref,
static int do_sign(struct strbuf *buffer)
{
- struct child_process gpg;
- const char *args[4];
- char *bracket;
- int len;
- int i, j;
+ const char *key;
- if (!*signingkey) {
- if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
- sizeof(signingkey)) > sizeof(signingkey) - 1)
- return error(_("committer info too long."));
- bracket = strchr(signingkey, '>');
- if (bracket)
- bracket[1] = '\0';
- }
-
- /* When the username signingkey is bad, program could be terminated
- * because gpg exits without reading and then write gets SIGPIPE. */
- signal(SIGPIPE, SIG_IGN);
-
- memset(&gpg, 0, sizeof(gpg));
- gpg.argv = args;
- gpg.in = -1;
- gpg.out = -1;
- args[0] = "gpg";
- args[1] = "-bsau";
- args[2] = signingkey;
- args[3] = NULL;
-
- if (start_command(&gpg))
- return error(_("could not run gpg."));
-
- if (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {
- close(gpg.in);
- close(gpg.out);
- finish_command(&gpg);
- return error(_("gpg did not accept the tag data"));
- }
- close(gpg.in);
- len = strbuf_read(buffer, gpg.out, 1024);
- close(gpg.out);
-
- if (finish_command(&gpg) || !len || len < 0)
- return error(_("gpg failed to sign the tag"));
-
- /* Strip CR from the line endings, in case we are on Windows. */
- for (i = j = 0; i < buffer->len; i++)
- if (buffer->buf[i] != '\r') {
- if (i != j)
- buffer->buf[j] = buffer->buf[i];
- j++;
- }
- strbuf_setlen(buffer, j);
-
- return 0;
+ if (*signingkey)
+ key = signingkey;
+ else
+ key = git_committer_info(IDENT_ERROR_ON_NO_NAME|IDENT_NO_DATE);
+ return sign_buffer(buffer, key);
}
static const char tag_template[] =
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 3134766..8b4f742 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -11,6 +11,7 @@
#include "run-command.h"
#include <signal.h>
#include "parse-options.h"
+#include "gpg-interface.h"
static const char * const verify_tag_usage[] = {
"git verify-tag [-v|--verbose] <tag>...",
@@ -19,42 +20,12 @@ static const char * const verify_tag_usage[] = {
static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
{
- struct child_process gpg;
- const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
- char path[PATH_MAX];
- size_t len;
- int fd, ret;
+ int len;
- fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
- if (fd < 0)
- return error("could not create temporary file '%s': %s",
- path, strerror(errno));
- if (write_in_full(fd, buf, size) < 0)
- return error("failed writing temporary file '%s': %s",
- path, strerror(errno));
- close(fd);
-
- /* find the length without signature */
len = parse_signature(buf, size);
if (verbose)
write_in_full(1, buf, len);
-
- memset(&gpg, 0, sizeof(gpg));
- gpg.argv = args_gpg;
- gpg.in = -1;
- args_gpg[2] = path;
- if (start_command(&gpg)) {
- unlink(path);
- return error("could not run gpg.");
- }
-
- write_in_full(gpg.in, buf, len);
- close(gpg.in);
- ret = finish_command(&gpg);
-
- unlink_or_warn(path);
-
- return ret;
+ return verify_signed_buffer(buf, size, len);
}
static int verify_tag(const char *name, int verbose)
diff --git a/gpg-interface.c b/gpg-interface.c
new file mode 100644
index 0000000..b83cca1
--- /dev/null
+++ b/gpg-interface.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 2011, Google Inc.
+ */
+#include "cache.h"
+#include "run-command.h"
+#include "strbuf.h"
+#include "gpg-interface.h"
+#include "sigchain.h"
+
+int sign_buffer(struct strbuf *buffer, const char *signing_key)
+{
+ struct child_process gpg;
+ const char *args[4];
+ ssize_t len;
+ int i, j;
+
+ memset(&gpg, 0, sizeof(gpg));
+ gpg.argv = args;
+ gpg.in = -1;
+ gpg.out = -1;
+ args[0] = "gpg";
+ args[1] = "-bsau";
+ args[2] = signing_key;
+ args[3] = NULL;
+
+ if (start_command(&gpg))
+ return error(_("could not run gpg."));
+
+ /*
+ * When the username signingkey is bad, program could be terminated
+ * because gpg exits without reading and then write gets SIGPIPE.
+ */
+ sigchain_push(SIGPIPE, SIG_IGN);
+
+ if (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {
+ close(gpg.in);
+ close(gpg.out);
+ finish_command(&gpg);
+ return error(_("gpg did not accept the data"));
+ }
+ close(gpg.in);
+ len = strbuf_read(buffer, gpg.out, 1024);
+ close(gpg.out);
+
+ sigchain_pop(SIGPIPE);
+
+ if (finish_command(&gpg) || !len || len < 0)
+ return error(_("gpg failed to sign the data"));
+
+ /* Strip CR from the line endings, in case we are on Windows. */
+ for (i = j = 0; i < buffer->len; i++)
+ if (buffer->buf[i] != '\r') {
+ if (i != j)
+ buffer->buf[j] = buffer->buf[i];
+ j++;
+ }
+ strbuf_setlen(buffer, j);
+
+ return 0;
+}
+
+int verify_signed_buffer(const char *buf, size_t total, size_t payload)
+{
+ struct child_process gpg;
+ const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
+ char path[PATH_MAX];
+ int fd, ret;
+
+ fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
+ if (fd < 0)
+ return error("could not create temporary file '%s': %s",
+ path, strerror(errno));
+ if (write_in_full(fd, buf, total) < 0)
+ return error("failed writing temporary file '%s': %s",
+ path, strerror(errno));
+ close(fd);
+
+ memset(&gpg, 0, sizeof(gpg));
+ gpg.argv = args_gpg;
+ gpg.in = -1;
+ args_gpg[2] = path;
+ if (start_command(&gpg)) {
+ unlink(path);
+ return error("could not run gpg.");
+ }
+
+ write_in_full(gpg.in, buf, payload);
+ close(gpg.in);
+ ret = finish_command(&gpg);
+
+ unlink_or_warn(path);
+
+ return ret;
+}
diff --git a/gpg-interface.h b/gpg-interface.h
new file mode 100644
index 0000000..7689357
--- /dev/null
+++ b/gpg-interface.h
@@ -0,0 +1,11 @@
+#ifndef GPG_INTERFACE_H
+#define GPG_INTERFACE_H
+
+/*
+ * Copyright (c) 2011, Google Inc.
+ */
+
+extern int sign_buffer(struct strbuf *buffer, const char *signing_key);
+extern int verify_signed_buffer(const char *buffer, size_t total, size_t payload);
+
+#endif
--
1.7.7.rc0.188.g3793ac
^ permalink raw reply related
* Re: [PATCHv2 1/2] remote: write correct fetch spec when renaming remote 'remote'
From: Junio C Hamano @ 2011-09-08 3:43 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git, Jeff King
In-Reply-To: <alpine.DEB.2.00.1109062136350.12564@debian>
Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
> same pattern both when updating refspecs and when renaming refs. Of
> course, we can never be certain that a ref "refs/remotes/origin/foo"
> is really related to the remote called "origin". The user could have
> simply created the ref manually. Is that what you are getting at?
You have two separate and independent code that are not linked together
but should logically be.
One updates fetch refspec whose RHS is "refs/remotes/$OLD/<anything>" to
"refs/remotes/$NEW/<the same thing>". If you do not find any such fetch
refspec, then you do not update these configuration variables, which is
good.
Later in the same mv() function, the other one renames refs/remotes/$OLD/
to refs/remotes/$NEW/, even when you did not find any fetch refspec that
stores under "refs/remotes/$OLD/<anything>" in the earlier logic.
Now, these actual refs may have been placed manually by the user. They may
have been placed by an old config that the user may have edited. You
simply do not know.
But you know one thing. You _do_ know is that these refs did _not_ come
from any "[remote "$OLD"] fetch = ..." configuration, and by inference, it
will not come from any "[remote "$NEW"] fetch = ...", in other words, they
do not have any relation with the "$NEW" remote. So I do not see a good
reason to move them from refs/remotes/$OLD/ to refs/remotes/$NEW/. That
was what I was pointing out.
^ permalink raw reply
* Re: Pushing with --mirror over HTTP?
From: Eli Barzilay @ 2011-09-08 2:58 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20110907213950.GI13364@sigill.intra.peff.net>
5 hours ago, Jeff King wrote:
> On Mon, Sep 05, 2011 at 12:05:37AM -0400, Eli Barzilay wrote:
>
> > Is there anything broken with pushing with mirror over HTTP? I'm
> > trying that with a github url, and I get a broken-looking error
> > message:
> >
> > remote part of refspec is not a valid name in :.have
>
> It's probably nothing to do with http, but rather with alternate
> object databases on the server (which GitHub uses heavily). The
> server hands out fake ".have" refs telling you it has some other
> branch tips to base packs off of. So I suspect the "push --mirror"
> code is simply wrong for trying to update those refs (it may be
> exacerbated by using http, though, as the remote helper code seems
> to have some extra checks).
Ah -- I thought that this was some result of parsing some text message
or something like that, maybe if the error was
remote part of refspec is not a valid name in ":.have"
or even
remote part of refspec is not a valid name in: :.have
it would have been clearer? Seems like it's a good place for this
since some `:foo' is likely to appear there, and the colon can be
confused as part of the text.
Also, maybe the man page should say something about `--mirror' not
working well with such servers? It looks to me like mirroring to
github and to google code would be pretty popular.
> > and with the google code, I get:
> >
> > error: unable to push to unqualified destination: HEAD
> >
> > Pushing to both of these work fine without `--mirror'.
>
> This one, I'm not sure. It may be related.
>
> > (BTW, as a workaround, I'm using
> > push --force --tags <url> :
> > is this achieving the same effect for a repo without weird refs?)
>
> Not quite. I think:
>
> git push --force <url> refs/*:refs/*
>
> would be closer.
Thanks -- I'll use that instead.
> But even that's not quite right. I believe that "--mirror" will
> also delete any remote refs that don't exist locally (which is why
> you are seeing the ":.have" refspec above, which attempts to delete
> it).
Is there some way of doing that? (We do use a branch during releases
that is deleted after the release, so I need to propagate these
deletions.)
--
((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:
http://barzilay.org/ Maze is Life!
^ permalink raw reply
* Re: [PATCH v2] date.c: Support iso8601 timezone formats
From: Haitao Li @ 2011-09-08 2:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vaaag4794.fsf@alter.siamese.dyndns.org>
>> + } else {
>> + /* Only hours specified */
>
> That comment belongs to inside the following if() {...}.
>
>> + if (n == 1 || n == 2) {
>
> ... i.e. here.
>
Good catch!
>> - if (min < 60 && n > 2) {
>> + if (n > 0 && min < 60 && hour < 25) {
>
> What is this "hour < 25" about? Aren't we talking about the UTC offset
> value that come after the [-+] sign?
>
> I do not mind adding a new check, but I do mind if it adds a check with
> not much value. Even at Pacific/Kiritimati, the offset is 14; the new
> check seems a bit too lenient.
>
I think it's not "too" lenient. UTC+14 was "invented" in 1995 [1].
Maybe UTC+15 would be added someday for some reason? How about
changing to "hour < 24", this is how ICU checks offset validity [2].
1. http://en.wikipedia.org/wiki/UTC%2B14#History
2. http://bugs.icu-project.org/trac/browser/icu/tags/release-4-8-1/source/i18n/timezone.cpp#L1482
^ permalink raw reply
* Re: [PATCH v3 1/1] sha1_file: normalize alt_odb path before comparing and storing
From: wanghui @ 2011-09-08 2:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, tali
In-Reply-To: <7vk49k2nsz.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Wang Hui <Hui.Wang@windriver.com> writes:
>
>
>> From: Hui Wang <Hui.Wang@windriver.com>
>>
>> When it needs to compare and add an alt object path to the
>> alt_odb_list, we normalize this path first since comparing normalized
>> path is easy to get correct result.
>>
>> Use strbuf to replace some string operations, since it is cleaner and
>> safer.
>>
>
> Thanks, will queue.
>
>
>> diff --git a/sha1_file.c b/sha1_file.c
>> index f7c3408..fa2484b 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -248,27 +248,27 @@ static int link_alt_odb_entry(const char * entry, int len, const char * relative
>> ...
>> + /* Drop the last '/' from path can make memcmp more accurate */
>> + if (pathbuf.buf[pfxlen-1] == '/')
>> + pfxlen -= 1;
>>
>
> By the way, I do not necessarily agree with the above comment. As long as
> you consistently strip the trailing slashes from all directory paths, or
> you consistently leave a single trailing slash after all directory paths,
> you can get accurate comparison either way.
>
> Side note: I tend to prefer keeping a single trailing slash when I
> know what we are talking about is a directory in general, because
> you do not have to worry about the corner case near the root.
> Compare ('/' and '/bin/') vs ('/' and '/bin').
>
> In this particular case, the real reason you want to remove the trailing
> slash is that the invariants of ent->base[] demands it (after all, it
> places another slash immediately after it), and making pathbuf.buf[] an
> empty string (i.e. pfxlen == 0) would still be OK to represent an
> alternate object store at the root level (this function assigns '/' at
> ent->base[pfxlen] immediately before returning, and that '/' names the
> root directory).
>
>
Yes, your concern is right, I didn't even think about root directory
situation. If the pathbuf.buf is really '/', stripping last slash will
make pathbuf.buf a empty string, this will make is_directory() return
false, this is a bug, how about replace those three line codes to:
/* Except root dir, all paths are stripped the last slash if they have */
if (pathbuf.buf[pfxlen-1] == '/' && pxflen != 1)
pxflen -= 1;
>> + entlen = pfxlen + 43; /* '/' + 2 hex + '/' + 38 hex + NUL */
>> + ent = xmalloc(sizeof(*ent) + entlen);
>> + memcpy(ent->base, pathbuf.buf, pfxlen);
>> + strbuf_release(&pathbuf);
>>
>> ent->name = ent->base + pfxlen + 1;
>> ent->base[pfxlen + 3] = '/';
>>
>
>
^ permalink raw reply
* Re: [PATCHv2 1/2] remote: write correct fetch spec when renaming remote 'remote'
From: Martin von Zweigbergk @ 2011-09-08 1:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Martin von Zweigbergk, git, Jeff King
In-Reply-To: <7vaaah6zx0.fsf@alter.siamese.dyndns.org>
On Tue, 6 Sep 2011, Junio C Hamano wrote:
> It is somewhat bothering that we do not say "we didn't do any magic" here
> when we did not move the tracking branch specifications, but that is not a
> new problem, so I am OK with this change.
If I understand you correctly, this is the same concern that Jeff had
and that I tried to address in patch 3/2.
> I however suspect that you would want to keep the record of what you
> changed here, so that the renaming of actual refs done in [PATCH 2/2] is
> limited to those that you updated the specifications for, no?
Sorry, I don't think I really understand. Are you worried that we
might rename too many refs, i.e. unrelated ones? We match exactly the
same pattern both when updating refspecs and when renaming refs. Of
course, we can never be certain that a ref "refs/remotes/origin/foo"
is really related to the remote called "origin". The user could have
simply created the ref manually. Is that what you are getting at?
Martin
^ permalink raw reply
* Re: [PATCH v17 1/7] bisect: move argument parsing before state modification.
From: Jon Seymour @ 2011-09-08 1:23 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Christian Couder, git, gitster, jnareb, jrnieder
In-Reply-To: <4E67B2F2.9070806@kdbg.org>
On Thu, Sep 8, 2011 at 4:07 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 07.09.2011 08:16, schrieb Christian Couder:
> IOW, I think the new behavior is *much* better than the old behavior.
>
There is perhaps no surprise that I agree with Hannes. Certainly, it
seemed saner to me to do argument validation before state update. [
Also, the earlier iterations of the --no-checkout series needed the
new behaviour. Not sure if that is still true, but I suspect it is ].
jon.
^ permalink raw reply
* Re: [PATCH 2/2] push -s: skeleton
From: Robin H. Johnson @ 2011-09-07 23:55 UTC (permalink / raw)
To: Git Mailing List; +Cc: Shawn O. Pearce
In-Reply-To: <7vbouw2hqg.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 2111 bytes --]
On Wed, Sep 07, 2011 at 01:57:27PM -0700, Junio C Hamano wrote:
> If a tag is GPG-signed, and if you trust the cryptographic robustness of
> the SHA-1 and GPG, you can guarantee that all the history leading to the
> signed commit is not tampered with. However, it would be both cumbersome
> and cluttering to sign each and every commit. Especially if you strive to
> keep your history clean by tweaking, rewriting and polishing your commits
> before pushing the resulting history out, many commits you will create
> locally end up not mattering at all, and it is a waste of time to sign
> them.
Thanks to pcloud for including me on the thread. I do find the idea of
these push-certificates very interesting and useful, but I think they
will do best to augment signed commits, not replace them.
There's a couple of related things we've been considering on the Gentoo
side:
- detached signatures of blobs (either the SHA1 of the blob or the blob
itself)
- The signature covering the message+blob details, but NOT the chain of
history: this opens up the ability to cherry-pick and rebase iff there
are no conflicts and the blobs are identical, all while preserving the
signature.
- concerns about a pre-image attack against Git. tl;dr version:
1. Attacker prepares decoy file in advance, that hashes to the same as
the malicious file.
2. Attacker sends decoy in as an innocuous real commit.
3. Months later, the attacker breaks into the system and alters the
packfile to include the new malicious file.
4. All new clones from that point forward get the malicious version.
Re your comment on always needing to resign commits above, we'd been
considering post-signing commits, not when they are initially made.
After your commit is clean and ready to ship, you can fire the commit
ids into the signature tool, which can generate a detached signature
note for each commit.
--
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail : robbat2@gentoo.org
GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 330 bytes --]
^ permalink raw reply
* Re: The imporantance of including http credential caching in 1.7.7
From: Junio C Hamano @ 2011-09-07 23:38 UTC (permalink / raw)
To: Philip Oakley; +Cc: Kyle Neath, Michael J Gruber, git
In-Reply-To: <33309DB6F935472497D9790A26046452@PhilipOakley>
"Philip Oakley" <philipoakley@iee.org> writes:
> Would it not be possible for GitHub to provide for those key users such
> a trial version that includes the patches identified to obtain the
> "real-world success reports" that are needed, as mentioned in the "Re:
> What's cooking in git.git (Aug 2011, #07; Wed, 24)"
>
> This should help satisfy the needs from both sides, even if you can
> only push it to a few clients.
That would not help very much, as (1) we know what Jeff included as sample
keystores are more or less cooked and good, but (2) nobody in your style
of trial will come up with different keystore to exercise the API to make
sure that will not paint us into a corner we cannot upgrade without major
pain in the backward compatibility area. The "real-world success reports"
you can generate would only for (1) but at this point we are not worried
about that. We are more (much more) worried about (2).
^ permalink raw reply
* Re: [PATCH] RelNotes/1.7.7: minor fixes
From: Junio C Hamano @ 2011-09-07 23:29 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <61f2b23c1717455d9b2088b1b8f3350756d757e0.1315396440.git.git@drmicha.warpmail.net>
Thanks, I obviously kant speel.
^ permalink raw reply
* Re: [PATCH 2/2] push -s: skeleton
From: Shawn Pearce @ 2011-09-07 23:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpqjc0zaf.fsf@alter.siamese.dyndns.org>
On Wed, Sep 7, 2011 at 15:21, Junio C Hamano <gitster@pobox.com> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
>
>> Yes. Above we flushed the req_buf and send that in an HTTP request.
>> You need to hoist this block above the "if (args->stateless_rpc)"
>> segment.
>
> What do you mean by "hoist"? For the req advertisement, it seems that you
> are not hoisting anything but duplicating the code, turning safe_write()
> followed by flush into packet-buf-flush and sending the result over the
> sideband. Shouldn't this new data be sent over the sideband-to-http the
> same way?
>
> Unless you do not want signed push over http, that is...
We do.
> diff --git a/builtin/send-pack.c b/builtin/send-pack.c
> index 3193f34..37e0313 100644
> --- a/builtin/send-pack.c
> +++ b/builtin/send-pack.c
> @@ -379,9 +379,13 @@ int send_pack(struct send_pack_args *args,
> packet_buf_write(&req_buf, "%.*s",
> (int)(ep - cp), cp);
> }
> - /* Do we need anything funky for stateless rpc? */
> - safe_write(out, req_buf.buf, req_buf.len);
> - packet_flush(out);
> + if (args->stateless_rpc) {
> + packet_buf_flush(&req_buf);
> + send_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);
> + } else {
> + safe_write(out, req_buf.buf, req_buf.len);
> + packet_flush(out);
> + }
This sounds too late to me. I think you just caused 2 HTTP POSTs, one
a partial one with the commands and no pack data, and another with the
push certificate and the pack. Neither is useful.
--
Shawn.
^ permalink raw reply
* Re: The imporantance of including http credential caching in 1.7.7
From: Philip Oakley @ 2011-09-07 23:01 UTC (permalink / raw)
To: Kyle Neath, Michael J Gruber; +Cc: git
In-Reply-To: <CAFcyEthuf49_kOmoLmoSSbNJN+iOBpicP4-eFAV5wL5_RffwGg@mail.gmail.com>
From: "Kyle Neath" <kneath@gmail.com>
> Junio C Hamano <gitster@pobox.com> wrote:
>> If this were a new, insignificant, and obscure feature in a piece of
>> software with mere 20k users, it may be OK to release a new version with
>> the feature in an uncooked shape.
>
> Michael J Gruber <git@drmicha.warpmail.net> wrote:
>> So, it's been a year or more that you've been aware of the importance of
>> this issue (from your/github's perspective), and we hear about it now,
>> at the end of the rc phase.
>
> I apologize if it sounds like that. I've been discussing this situation
> with
> many people (including Jeff King) for a very long time now, and it was my
> understanding that the credential caching was done and simply waiting for
> a
> new release. This is the first I've heard that it will not be included in
> 1.7.7, so I'm voicing my opinion now. Admittedly, late in the game - and I
> apologize for that.
>
> I'd be happy to help in any capacity I can. Unfortunately I'm no C hacker,
> and
> I've accepted that as a character flaw (it's something I'm working on).
> I'm
> afraid I can't be of much help with the actual code. What I can provide is
> an
> alternate viewpoint to the core team. A viewpoint of someone who's spent 3
> years trying to make git easier for newcomers.
Help in any capacity :
Would it not be possible for GitHub to provide for those key users such a
trial version
that includes the patches identified to obtain the "real-world success
reports" that
are needed, as mentioned in the "Re: What's cooking in git.git (Aug 2011,
#07; Wed, 24)"
This should help satisfy the needs from both sides, even if you can only
push it to a few clients.
>>What's cooking in git.git (Aug 2011, #07; Wed, 24)
>> Looked mostly reasonable except for the limitation that it is not clear
>> how to deal with a site at which a user needs to use different passwords
>> for different repositories. Will keep it in "next" at least for one
>> cycle,
>> until we start hearing real-world success reports on the list.
^ permalink raw reply
* Re: [RFC/PATCH] fetch: bigger forced-update warnings
From: Thomas Rast @ 2011-09-07 22:42 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn Pearce, Junio C Hamano, Michael J Gruber, git
In-Reply-To: <20110907212042.GG13364@sigill.intra.peff.net>
Jeff King wrote:
> On Mon, Sep 05, 2011 at 02:14:57PM -0700, Shawn O. Pearce wrote:
>
> > > Right. What I mean is, what should the bigger warning look like?
> >
> > Its a bikeshed. I refuse to paint bikesheds. :-)
[...]
> + if (uncommon_forced_update)
> + warning("HEY STUPID FIX YOUR TOPICS");
Whatever comes out of the bikeshedding, I'm going to keep a patch
locally that refreshes the mental picture of Shawn shouting that!
That being said, I think there should be a multiline warning pointing
the user at the "recovering from upstream rebase" section in
git-rebase(1). At least by default with an advice.* setting to
disable it.
> + if (!prefixcmp(message, "fetch: fast-forward"))
> + uc->fastforward++;
> + else if (!prefixcmp(message, "fetch: forced-update\n"))
> + uc->forced++;
That doesn't work: fetch puts the whole command line there.
E.g.
git fetch altgit
--> fetch altgit: fast-forward
git fetch altgit next:refs/remotes/next
--> fetch altgit next:remotes/altgit/next: fast-forward
There's also a minor subtlety here that I had to double-check first:
the message for a branch creation is 'storing head', so the later
check
> + return uc.fastforward && uc.forced <= 1; /* 1 for the one we just did */
never triggers at the second fetch.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH 2/2] push -s: skeleton
From: Junio C Hamano @ 2011-09-07 22:40 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Junio C Hamano, Robin H. Johnson, git, Shawn O. Pearce
In-Reply-To: <CACsJy8Cy_Nn3EExV0D=RWtft+1pc9RBdJgpmES4AeQgYsUfU3A@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>> ...
>> 4. A new phase to record the push certificate is introduced in the
>> codepath after the receiving end runs receive_hook(). It is envisioned
>> that this phase:
>>
>> a. parses the updated-to object names, and appends the push
>> certificate (still GPG signed) to a note attached to each of the
>> objects that will sit at the tip of the refs;
>
> I recall Gentoo wanted something like this (recording who pushes
> what). Pulling Robin in if he has any comments.
As the beauty of this approach is that we can update and tailor what the
receiving end does using the information given from the server, it is a
strange thing to do to chomp this list in the middle at a funny place
here.
^ permalink raw reply
* Re: [PATCH 2/2] push -s: skeleton
From: Nguyen Thai Ngoc Duy @ 2011-09-07 22:21 UTC (permalink / raw)
To: Junio C Hamano, Robin H. Johnson; +Cc: git, Shawn O. Pearce
In-Reply-To: <7vbouw2hqg.fsf@alter.siamese.dyndns.org>
On Thu, Sep 8, 2011 at 6:57 AM, Junio C Hamano <gitster@pobox.com> wrote:
> A better alternative could be to sign a "push certificate" (for the lack
> of better name) every time you push, asserting that what commits you are
> pushing to update which refs. The basic workflow goes like this:
>
> 1. You push out your work with "git push -s";
>
> 2. "git push", as usual, learns where the remote refs are and which refs
> are to be updated with this push. It prepares a text file in memory
> that looks like this using this information:
>
> Push-Certificate-Version: 1
> Pusher: Junio C Hamano <gitster@pobox.com> 1315427886 -0700
> Update: e83c51633... d4e58965f... refs/heads/master
> Update: 5a144a288... 7931f38a2... refs/heads/next
>
> An actual push certificate records full 40-char object name, but it is
> ellided for brevity here.
>
> The user then is asked to sign this push certificate using GPG. The
> result is carried to the other side (i.e. receive-pack). In the
> protocol exchange, this step comes immediately after the sender tells
> what the result of the push should be, before it sends the pack data.
>
> 3. The receiving end will keep the signed push certificate in core,
> receives the pack data and unpacks (or stores and runs index-pack)
> as usual.
>
> 4. A new phase to record the push certificate is introduced in the
> codepath after the receiving end runs receive_hook(). It is envisioned
> that this phase:
>
> a. parses the updated-to object names, and appends the push
> certificate (still GPG signed) to a note attached to each of the
> objects that will sit at the tip of the refs;
I recall Gentoo wanted something like this (recording who pushes
what). Pulling Robin in if he has any comments.
--
Duy
^ permalink raw reply
* Re: [PATCH 2/2] push -s: skeleton
From: Junio C Hamano @ 2011-09-07 22:21 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <CAJo=hJtz6fa4XfC-4ghryP_nfg3sbcrE2bKauj+F7w2Z_8Ckvw@mail.gmail.com>
Shawn Pearce <spearce@spearce.org> writes:
> Yes. Above we flushed the req_buf and send that in an HTTP request.
> You need to hoist this block above the "if (args->stateless_rpc)"
> segment.
What do you mean by "hoist"? For the req advertisement, it seems that you
are not hoisting anything but duplicating the code, turning safe_write()
followed by flush into packet-buf-flush and sending the result over the
sideband. Shouldn't this new data be sent over the sideband-to-http the
same way?
Unless you do not want signed push over http, that is...
builtin/send-pack.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 3193f34..37e0313 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -379,9 +379,13 @@ int send_pack(struct send_pack_args *args,
packet_buf_write(&req_buf, "%.*s",
(int)(ep - cp), cp);
}
- /* Do we need anything funky for stateless rpc? */
- safe_write(out, req_buf.buf, req_buf.len);
- packet_flush(out);
+ if (args->stateless_rpc) {
+ packet_buf_flush(&req_buf);
+ send_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);
+ } else {
+ safe_write(out, req_buf.buf, req_buf.len);
+ packet_flush(out);
+ }
}
strbuf_release(&req_buf);
^ permalink raw reply related
* Re: git push output goes into stderr
From: Jeff King @ 2011-09-07 21:57 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: Junio C Hamano, Lynn Lin, git
In-Reply-To: <20110906074916.GC28490@ecki>
On Tue, Sep 06, 2011 at 09:49:16AM +0200, Clemens Buchacher wrote:
> On Sun, Sep 04, 2011 at 05:57:53PM -0700, Junio C Hamano wrote:
> > Lynn Lin <lynn.xin.lin@gmail.com> writes:
> >
> > > When I create a local branch and then push it to remote. I find that
> > > the output without error goes into stderr, is this expected?
> >
> > Progress output are sent to the stderr stream.
>
> But it's not only progress output that goes to stderr in case of
> git push. Even the summary written in tranport_print_push_status
> goes to stderr, unless we specify git push --porcelain. Can't we
> let that part of the output go to stdout unconditionally?
We could, though it makes more sense on stderr to me.
Stdout has always been about "the main program output" and stderr about
diagnostic messages. With a program whose main function is to generate
output (e.g., "git tag -l", it's very easy to know that the list of tags
is the main program output (which you don't want to pollute with
anything else), and any problems or even general chattiness goes to
stderr.
But with a program whose main function is to perform an action, like
"git push", I think there are really two ways to look at it:
1. There is no main output; any progress or status update is just
diagnostic chat, and should go to stderr.
2. The main output is the status report; it goes to stdout, and
progress updates go to stderr.
I think both are equally valid mental models, and both are consistent
with the philosophy above. If we switch, I wouldn't be surprised to see
somebody say "why is this going to stdout, it should be on stderr". In
fact, I seem to recall that we've had this discussion before on the
list.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] fetch: bigger forced-update warnings
From: Jeff King @ 2011-09-07 21:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn Pearce, Michael J Gruber, git
In-Reply-To: <7vty8o10kj.fsf@alter.siamese.dyndns.org>
On Wed, Sep 07, 2011 at 02:53:32PM -0700, Junio C Hamano wrote:
> > Some branches are expected to rewind, so the prominent
> > warning would be annoying. However, git doesn't know what
> > the expectation is for a particular branch. We can have it
> > guess by peeking at the lost couple of reflog entries. If we
>
> s/lost/last/
Oops, thanks.
> This is slightly offtopic, but I have been wondering if this approach do
> the right thing for "git pull". Wouldn't the underlying "git fetch" give a
> warning, and then the calling "git pull" go ahead and make a merge,
> scrolling the warning away with the merge/update summary diffstat? That
> would be a larger change if "git pull" needs to stash away the warning
> message, do its thing and then spit out the warning later.
I think this particular warning has nothing to do with git-pull. But
rather, that we should _always_ abort a pull with a forced-update.
Because the only sane things to do there are:
1. Stop and look around, and see if you should be doing a "git reset"
first.
or
2. "git pull --rebase"
But proceeding with the pull just seems like a disaster. So it is not
about "this usually fast forwards, but isn't now, so let's make the
warning bigger". It is more about noticing that it is a forced-update at
all. Maybe that's what you meant by "off-topic". :)
> > +static int forced_update_is_uncommon(const char *ref)
> > +{
> > + struct update_counts uc;
> > + memset(&uc, 0, sizeof(&uc));
> > + if (for_each_recent_reflog_ent(ref, count_updates, 4096, &uc) < 0)
> > + for_each_reflog_ent(ref, count_updates, &uc);
> > + return uc.fastforward && uc.forced <= 1; /* 1 for the one we just did */
> > +}
>
> Looks sensible.
Now we just need to paint the shed. :)
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] fetch: bigger forced-update warnings
From: Junio C Hamano @ 2011-09-07 21:53 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn Pearce, Michael J Gruber, git
In-Reply-To: <20110907212042.GG13364@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Subject: fetch: bigger forced-update warnings
>
> The default fetch refspec allows forced-updates. We already
> print "forced update" in the status table, but it's easy to
> miss. Let's make the warning a little more prominent.
>
> Some branches are expected to rewind, so the prominent
> warning would be annoying. However, git doesn't know what
> the expectation is for a particular branch. We can have it
> guess by peeking at the lost couple of reflog entries. If we
s/lost/last/
> see all fast forwards, then a new forced-update is probably
> noteworthy. If we see something that force-updates all the
> time, it's probably boring and not worth displaying the big
> warning (we keep the status table "forced update" note, of
> course).
>
> Signed-off-by: Jeff King <peff@peff.net>
This is slightly offtopic, but I have been wondering if this approach do
the right thing for "git pull". Wouldn't the underlying "git fetch" give a
warning, and then the calling "git pull" go ahead and make a merge,
scrolling the warning away with the merge/update summary diffstat? That
would be a larger change if "git pull" needs to stash away the warning
message, do its thing and then spit out the warning later.
> +static int forced_update_is_uncommon(const char *ref)
> +{
> + struct update_counts uc;
> + memset(&uc, 0, sizeof(&uc));
> + if (for_each_recent_reflog_ent(ref, count_updates, 4096, &uc) < 0)
> + for_each_reflog_ent(ref, count_updates, &uc);
> + return uc.fastforward && uc.forced <= 1; /* 1 for the one we just did */
> +}
Looks sensible.
^ permalink raw reply
* Re: [RFC/PATCH] fetch: bigger forced-update warnings
From: Shawn Pearce @ 2011-09-07 21:39 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Michael J Gruber, git
In-Reply-To: <20110907212042.GG13364@sigill.intra.peff.net>
On Wed, Sep 7, 2011 at 14:20, Jeff King <peff@peff.net> wrote:
> + if (uncommon_forced_update)
> + warning("HEY STUPID FIX YOUR TOPICS");
<action>
<type>paint</type>
<object>bikeshed</object>
<why>because-i-can</why>
How about:
warning("!!! REMOTE BRANCH REWOUND HISTORY !!!");
warning(" Check status report for branches that rewound.");
</action>
--
Shawn.
^ permalink raw reply
* Re: Pushing with --mirror over HTTP?
From: Jeff King @ 2011-09-07 21:39 UTC (permalink / raw)
To: Eli Barzilay; +Cc: git
In-Reply-To: <20068.19089.303108.950233@winooski.ccs.neu.edu>
On Mon, Sep 05, 2011 at 12:05:37AM -0400, Eli Barzilay wrote:
> Is there anything broken with pushing with mirror over HTTP? I'm
> trying that with a github url, and I get a broken-looking error
> message:
>
> remote part of refspec is not a valid name in :.have
It's probably nothing to do with http, but rather with alternate object
databases on the server (which GitHub uses heavily). The server hands
out fake ".have" refs telling you it has some other branch tips to base
packs off of. So I suspect the "push --mirror" code is simply wrong for
trying to update those refs (it may be exacerbated by using http,
though, as the remote helper code seems to have some extra checks).
> and with the google code, I get:
>
> error: unable to push to unqualified destination: HEAD
>
> Pushing to both of these work fine without `--mirror'.
This one, I'm not sure. It may be related.
> (BTW, as a workaround, I'm using
> push --force --tags <url> :
> is this achieving the same effect for a repo without weird refs?)
Not quite. I think:
git push --force <url> refs/*:refs/*
would be closer. But even that's not quite right. I believe that
"--mirror" will also delete any remote refs that don't exist locally
(which is why you are seeing the ":.have" refspec above, which attempts
to delete it).
-Peff
^ permalink raw reply
* Re: [PATCH] Documentation: "on for all" configuration of notes.rewriteRef
From: Jeff King @ 2011-09-07 21:35 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, John S. Urban", Tor Arntsen, knittl, Junio C Hamano
In-Reply-To: <201109072329.18338.trast@student.ethz.ch>
On Wed, Sep 07, 2011 at 11:29:17PM +0200, Thomas Rast wrote:
> Admittedly I never considered the problem of supposedly-immutable
> notes here. The whole point was to help users who had no idea that
> the string put there should probably start with refs/notes/.
>
> So maybe the patch should instead say something along the lines of, to
> enable rewriting for the notes ref called foo, put refs/notes/foo --
> which to a core gitter of course sounds extremely redundant.
>
> But what about the general issue of users who *have* put refs/notes/*,
> and then some software comes along that does not expect them to be
> rewritten? Do we declare the software broken, or discourage from such
> blanket rewriting?
I think putting "refs/notes/*" is a perfectly reasonable thing from the
user's perspective, and I'd hate to take away that convenience (and
especially, I think in the long run, we'd like to have a hierarchy of
notes that have rewriting turned on by default).
The cache code is probably what should be changed, then. It can move to
"refs/cache", I guess, though I'm not too happy with that. The notes
code assumes refs/notes in several places, and it's nice to be able to
look at the cache trees with "--notes=cache/foo".
Maybe some way of saying "every notes tree gets rewriting, except ones
in refs/notes/cache"?
Right now it's not a big problem. The only such immutable cache in a
released version of git is the textconv cache, and it only contains
blobs. Which, AFAIK, cannot be subject to rewriting. So we could put it
off until another such cache comes along.
-Peff
^ permalink raw reply
* Re: [PATCH] Documentation: "on for all" configuration of notes.rewriteRef
From: Thomas Rast @ 2011-09-07 21:29 UTC (permalink / raw)
To: Jeff King; +Cc: git, John S. Urban", Tor Arntsen, knittl, Junio C Hamano
In-Reply-To: <20110907212310.GH13364@sigill.intra.peff.net>
Jeff King wrote:
> On Sun, Sep 04, 2011 at 10:27:04PM +0200, Thomas Rast wrote:
>
> > Users had problems finding a working setting for notes.rewriteRef.
> > Document how to enable rewriting for all notes.
>
> Hmm. Is this a safe thing to recommend?
>
> I think the idea of storing something like generation numbers in
> git-notes is dead at this point, but it would be quite disastrous to
> have generation numbers copied to rebased commits. Ditto for something
> like a patch-id cache. Should these sorts of immutable cache notes, if
> and when they do come about, go into a separate hierarchy?
Admittedly I never considered the problem of supposedly-immutable
notes here. The whole point was to help users who had no idea that
the string put there should probably start with refs/notes/.
So maybe the patch should instead say something along the lines of, to
enable rewriting for the notes ref called foo, put refs/notes/foo --
which to a core gitter of course sounds extremely redundant.
But what about the general issue of users who *have* put refs/notes/*,
and then some software comes along that does not expect them to be
rewritten? Do we declare the software broken, or discourage from such
blanket rewriting?
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH] Documentation: "on for all" configuration of notes.rewriteRef
From: Jeff King @ 2011-09-07 21:23 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, John S. Urban", Tor Arntsen, knittl, Junio C Hamano
In-Reply-To: <f415402994735a60664e1f9f85be490a68b25ed3.1315167848.git.trast@student.ethz.ch>
On Sun, Sep 04, 2011 at 10:27:04PM +0200, Thomas Rast wrote:
> Users had problems finding a working setting for notes.rewriteRef.
> Document how to enable rewriting for all notes.
Hmm. Is this a safe thing to recommend?
I think the idea of storing something like generation numbers in
git-notes is dead at this point, but it would be quite disastrous to
have generation numbers copied to rebased commits. Ditto for something
like a patch-id cache. Should these sorts of immutable cache notes, if
and when they do come about, go into a separate hierarchy?
-Peff
^ permalink raw reply
* [RFC/PATCH] fetch: bigger forced-update warnings
From: Jeff King @ 2011-09-07 21:20 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Junio C Hamano, Michael J Gruber, git
In-Reply-To: <CAJo=hJvFSegSzTOMj824PoG=soj75JMChfRnjyz4rNgUcVM=Jw@mail.gmail.com>
On Mon, Sep 05, 2011 at 02:14:57PM -0700, Shawn O. Pearce wrote:
> > Right. What I mean is, what should the bigger warning look like?
>
> Its a bikeshed. I refuse to paint bikesheds. :-)
Hmph. Somebody has to write the patch. :P
> > Also, you suggested caching to avoid looking through the whole reflog
> > each time. I think you could probably just sample the last 10 or so
> > reflog entries to get an idea.
>
> Good point. 10 or so last records might be representative of the
> branch's recent behavior, which is all that matters to the user who
> wants this warning.
Actually, because recent ones are near the end, it's much easier to say
"look at the last 4096 bytes of reflogs" rather than "look at exactly
10". For our purposes, it's about the same (actually 4096 is probably
more like 18-20, depending on the exact size of each entry. But it's a
page, so it's probably reasonable).
-- >8 --
Subject: fetch: bigger forced-update warnings
The default fetch refspec allows forced-updates. We already
print "forced update" in the status table, but it's easy to
miss. Let's make the warning a little more prominent.
Some branches are expected to rewind, so the prominent
warning would be annoying. However, git doesn't know what
the expectation is for a particular branch. We can have it
guess by peeking at the lost couple of reflog entries. If we
see all fast forwards, then a new forced-update is probably
noteworthy. If we see something that force-updates all the
time, it's probably boring and not worth displaying the big
warning (we keep the status table "forced update" note, of
course).
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/fetch.c | 39 +++++++++++++++++++++++++++++++++++++--
1 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 93c9938..93bfefa 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -208,6 +208,34 @@ static struct ref *get_ref_map(struct transport *transport,
return ref_map;
}
+struct update_counts {
+ unsigned fastforward;
+ unsigned forced;
+};
+
+static int count_updates(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp, int tz,
+ const char *message, void *data)
+{
+ struct update_counts *uc = data;
+ /* We could check the ancestry of osha1 and nsha1, but this is way
+ * cheaper */
+ if (!prefixcmp(message, "fetch: fast-forward"))
+ uc->fastforward++;
+ else if (!prefixcmp(message, "fetch: forced-update\n"))
+ uc->forced++;
+ return 0;
+}
+
+static int forced_update_is_uncommon(const char *ref)
+{
+ struct update_counts uc;
+ memset(&uc, 0, sizeof(&uc));
+ if (for_each_recent_reflog_ent(ref, count_updates, 4096, &uc) < 0)
+ for_each_reflog_ent(ref, count_updates, &uc);
+ return uc.fastforward && uc.forced <= 1; /* 1 for the one we just did */
+}
+
#define STORE_REF_ERROR_OTHER 1
#define STORE_REF_ERROR_DF_CONFLICT 2
@@ -239,7 +267,8 @@ static int s_update_ref(const char *action,
static int update_local_ref(struct ref *ref,
const char *remote,
- char *display)
+ char *display,
+ int *uncommon_forced_update)
{
struct commit *current = NULL, *updated;
enum object_type type;
@@ -336,6 +365,8 @@ static int update_local_ref(struct ref *ref,
TRANSPORT_SUMMARY_WIDTH, quickref, REFCOL_WIDTH, remote,
pretty_ref,
r ? _("unable to update local ref") : _("forced update"));
+ if (!r && forced_update_is_uncommon(ref->name))
+ *uncommon_forced_update = 1;
return r;
} else {
sprintf(display, "! %-*s %-*s -> %s %s",
@@ -355,6 +386,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
const char *what, *kind;
struct ref *rm;
char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
+ int uncommon_forced_update = 0;
fp = fopen(filename, "a");
if (!fp)
@@ -428,7 +460,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
fputc('\n', fp);
if (ref) {
- rc |= update_local_ref(ref, what, note);
+ rc |= update_local_ref(ref, what, note,
+ &uncommon_forced_update);
free(ref);
} else
sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
@@ -450,6 +483,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
error(_("some local refs could not be updated; try running\n"
" 'git remote prune %s' to remove any old, conflicting "
"branches"), remote_name);
+ if (uncommon_forced_update)
+ warning("HEY STUPID FIX YOUR TOPICS");
return rc;
}
--
1.7.6.10.g62f04
^ 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