* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Nguyen Thai Ngoc Duy @ 2011-10-05 23:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaa9f59p5.fsf@alter.siamese.dyndns.org>
On Thu, Oct 6, 2011 at 9:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> On Thu, Oct 6, 2011 at 5:19 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> I do not necessarily think that it is a good approach to forbid the same
>>> branch to be checked out in two different places, by the way. One reason
>>> people would want to keep multiple workdirs is so that while they are
>>> still working on a branch and are not yet at a good "stop point" to even
>>> make a temporary commit to get interrupted, they find it sometimes
>>> necessary to be able to build the tip of that same branch and even make a
>>> small in-working-tree fixes (which later will be carried back to the
>>> primary branch). The problem arises only when one of the repositories try
>>> to update or delete the branch while it is checked out in another working
>>> tree.
>>
>> I think of two options:
>>
>> - detach from the already locked branch (pretty much like what we do
>> with tags now)
>>
>> - refuse normally but let "checkout -f" do it anyway. However the
>> checkout lock will remain at the original worktree. If you want to
>> update branch from the second checkout, do "commit -f" and take
>> responsibility for your action.
>
> Sorry, what problem are you trying to solve? Does that "checkout -f" meant
> to nuke the local changes that are not yet at a good "stop point"?
I meant "git checkout" on the already locked branch is refused, but
"git checkout -f" in that case will act just like "git checkout"
ignoring all locks. But I forgot that "git checkout -f" also discards
worktree changes. Maybe "git checkout --ignore-locks" instead of "git
checkout -f".
--
Duy
^ permalink raw reply
* Re: git-cherry-pick and git-commit --amend in version 1.7.6.4
From: Junio C Hamano @ 2011-10-05 23:42 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, nicolas.dichtel
In-Reply-To: <7v62k359ee.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> It's just the "commit --amend" message that says I cannot amend felt
> utterly out of place, immediately after seeing "cherry-pick" that tried to
> pick only one commit did _not_ even start.
After thinking about it a bit more, I am starting to think that it may
just be the error message given by "commit --amend".
If the sequence were like this:
$ edit foo.c ;# I want to fix foo.c in the current branch "master"
$ EDITOR=: git commit --amend ;# forgot to say "foo.c"
$ git cherry-pick other~2 other
[master 48882c9] frotz: update xyzzy
Author: Jay Soffian <jaysoffian@gmail.com>
1 files changed, 2 insertions(+), 2 deletions(-)
error: Your local changes to the following files would be overwritten by merge:
foo.c
Please, commit your changes or stash them before you can merge.
Aborting
Then at this point, amending the commit at HEAD^ is not possible anyway,
as it is not at the tip anymore. It is perfectly fine that
$ git commit --amend foo.c
fails at this point.
It is just that it initially felt irritatingly wrong if I was picking only
a single commit "other" that wanted to touch foo.c, like this:
$ edit foo.c ;# I want to fix foo.c in the current branch "master"
$ EDITOR=: git commit --amend ;# forgot to say "foo.c"
$ git cherry-pick other
error: Your local changes to the following files would be overwritten by merge:
foo.c
Please, commit your changes or stash them before you can merge.
Aborting
At this point, as it says "Please commit your changes", and it is very
clear that cherry-pick _correctly_ errored out without touching any of my
work, it is natural for me to expect that I can "commit --amend" to fix
my eariler mistake.
$ EDITOR=: git commit --amend foo.c
fatal: You are in the middle of a cherry-pick -- cannot amend.
This can only worked around halfway:
$ rm .git/CHERRY_PICK_HEAD
$ EDITOR=: git commit --amend foo.c
Things look OK so far, but then restarting the cherry-pick I wanted to do
after I fixed foo.c would fail like this:
$ git cherry-pick other
error: .git/sequencer already exists.
error: A cherry-pick or revert is in progress.
hint: Use --continue to continue the operation
hint: or --reset to forget about it
fatal: cherry-pick failed
Perhaps it would be a possible solution to teach "cherry-pick --reset" to
remove CHERRY_PICK_HEAD and the sequencer state, so that the above
transcript would become:
$ edit foo.c ;# I want to fix foo.c in the current branch "master"
$ EDITOR=: git commit --amend ;# forgot to say "foo.c"
$ git cherry-pick other
error: Your local changes to the following files would be overwritten by merge:
foo.c
Please, commit your changes or stash them before you can merge.
Aborting
$ EDITOR=: git commit --amend foo.c
fatal: You are in the middle of a cherry-pick -- cannot amend.
hint: use "git cherry-pick --reset" to discard the previous cherry-pick.
$ git cherry-pick --reset
$ EDITOR=: git commit --amend foo.c
$ git cherry-pick other
[master 48882c9] frotz: update nitfol
Author: Jay Soffian <jaysoffian@gmail.com>
1 files changed, 2 insertions(+), 2 deletions(-)
At least, that looks like something we _could_ explain to the end users.
^ permalink raw reply
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Junio C Hamano @ 2011-10-05 23:49 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8D5FGr3R0tLYOND0kKNct4e_KgYfLUK8xL2Q4uNzWczgQ@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> On Thu, Oct 6, 2011 at 9:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> I think of two options:
>>> ...
>> Sorry, what problem are you trying to solve? Does that "checkout -f" meant
>> to nuke the local changes that are not yet at a good "stop point"?
>
> I meant "git checkout" on the already locked branch is refused, but
> "git checkout -f" in that case will act just like "git checkout"
> ignoring all locks. But I forgot that "git checkout -f" also discards
> worktree changes. Maybe "git checkout --ignore-locks" instead of "git
> checkout -f".
I see what you mean, but doesn't it feel as if it is working around a
problem that is introduced only because of a wrong policy (i.e. "you
cannot check out the same branch at two places", as opposed to "viewing
them in multiple places is perfectly fine, but no touching")?
This reminds me of how we ended up handling the "scary warning" around
detached HEAD. It is not wrong nor even dangerous to detach. It is not
wrong nor even dangerous to make commits on detached HEAD. It is however
dangerous to switch away from that state without saving it to a ref, and
that is where we give warnings.
^ permalink raw reply
* Re: git-cherry-pick and git-commit --amend in version 1.7.6.4
From: Jay Soffian @ 2011-10-06 0:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, nicolas.dichtel
In-Reply-To: <7vhb3n5asv.fsf@alter.siamese.dyndns.org>
On Wed, Oct 5, 2011 at 6:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Does it also refrain from creating sequencer state directory?
I'm not familiar with the sequencer code. It's not in master is it?
What's happening here is that do_pick_commit() was creating
CHERRY_PICK_HEAD, but then git aborts several call sites away
(do_recursive_merge -> merge_trees -> git_merge_trees -> unpack_trees
-> display_error_msgs).
So I think do_pick_commit() needs to defer creating CHERRY_PICK_HEAD
till after the possible abort.
I don't know if that's the right fix for next or not, but it seems
correct for master.
j.
^ permalink raw reply
* Re: Git Bug report
From: SZEDER Gábor @ 2011-10-06 0:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Fredrik Gustafsson, Federico Lucifredi, git
In-Reply-To: <7vlisz8jur.fsf@alter.siamese.dyndns.org>
On Wed, Oct 05, 2011 at 09:49:00AM -0700, Junio C Hamano wrote:
> Fredrik Gustafsson <iveqy@iveqy.com> writes:
>
> > On Tue, Oct 04, 2011 at 05:24:03PM -0400, Federico Lucifredi wrote:
> >> Found a minor bug in git today - the error message reported is not
> >> correct when trying to access a repo that is not accessible
> >> permission-wise:
> >>
> >> > federico@skyplex:/etc$ git log
> >> > fatal: Not a git repository (or any of the parent directories): .git
> >>
> >> with correct access permissions, everything works as expected.
> >
> > So if:
> > .git/ is a directory with not enought permissions.
> > ../.git/ is a directory with enought permissions.
> >
> > git would today use ../.git. You suggest that git instead would die
> > because a .git/ exists? (I'm not saying this is wrong or right).
>
> For that matter, if you have .git/ that is a directory but is not a
> repository, and ../.git/ that is, the same situation would arise. I do not
> think we should die because .git/ is not a git repository. I do not know
> if we should even warn about it.
And what about unreadable .git files?
~/tmp/git/outside$ git init
Initialized empty Git repository in /home/szeder/tmp/git/outside/.git/
~/tmp/git/outside$ mkdir inside repo
~/tmp/git/outside$ cd inside/
~/tmp/git/outside/inside$ git init --separate-git-dir=../repo
Initialized empty Git repository in /home/szeder/tmp/git/outside/repo/
~/tmp/git/outside/inside$ git rev-parse --git-dir
/home/szeder/tmp/git/outside/repo
~/tmp/git/outside/inside$ chmod a-r .git
~/tmp/git/outside/inside$ git rev-parse --git-dir
fatal: Error opening '.git': Permission denied
Or a non-gitfile .git file?
~/tmp/git/outside/inside$ chmod a+r .git
~/tmp/git/outside/inside$ echo foo >.git
~/tmp/git/outside/inside$ git rev-parse --git-dir
fatal: Invalid gitfile format: .git
Shouldn't these also be ignored?
Best,
Gábor
^ permalink raw reply
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Jay Soffian @ 2011-10-06 0:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <7vwrcj3sow.fsf@alter.siamese.dyndns.org>
On Wed, Oct 5, 2011 at 7:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> This reminds me of how we ended up handling the "scary warning" around
> detached HEAD. It is not wrong nor even dangerous to detach. It is not
> wrong nor even dangerous to make commits on detached HEAD. It is however
> dangerous to switch away from that state without saving it to a ref, and
> that is where we give warnings.
If you have the same branch in two workdirs, then if you commit to
that branch in one workdir, you have to reset --hard in the other. In
that case, wouldn't it make more sense to just use a detached head in
the second workdir?
$ git checkout topic
fatal: branch 'topic' is currently checked out in '...'
$ git checkout topic^0
... topic is updated elsewhere ...
$ git reset --hard topic
Either way you need to use reset --hard if topic is updated outside of
the current workdir, but at least if git encourages you to detach
first, you don't accidentally undo a commit.
Also, if we wait till commit time to tell the user "sorry, topic's
been updated elsewhere", now the user is in a perilous state. They
have uncommitted work which they clearly want on topic. And they have
to think about what steps are needed to get it there.
So, I really don't think this is quite analogous to detached HEAD, nor
pushing into a repo's checked out branch. In both those cases, at
least the user's work is already committed.
Better to prevent checking out the same branch in multiple workdirs
with an override for users that want risk shooting their foot off.
j.
^ permalink raw reply
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Junio C Hamano @ 2011-10-06 0:43 UTC (permalink / raw)
To: Jay Soffian; +Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git
In-Reply-To: <CAG+J_DzZrFx2v09zNxKm2xyA82MyKRTq3AEus3QthtpZYhQn0A@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> On Wed, Oct 5, 2011 at 7:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> This reminds me of how we ended up handling the "scary warning" around
>> detached HEAD. It is not wrong nor even dangerous to detach. It is not
>> wrong nor even dangerous to make commits on detached HEAD. It is however
>> dangerous to switch away from that state without saving it to a ref, and
>> that is where we give warnings.
>
> If you have the same branch in two workdirs, then if you commit to
> that branch in one workdir, you have to reset --hard in the other. In
> that case, wouldn't it make more sense to just use a detached head in
> the second workdir?
Not at all. My build infrastructure determines where to install the built
binary based on what branch is checked out. Having a head detached at a
commit that is at the tip of one branch is not necessarily the same as
having the branch actually checked out.
> Also, if we wait till commit time to tell the user "sorry, topic's
> been updated elsewhere", now the user is in a perilous state.
Wouldn't the "elsewhere" user would be warned before being able to update
the branch? I thought the whole point of your adding "this branch is
checked out over there" is exactly so that the "elsewhere" user can come
talk to you before that happens. These two people might be yourself, of
course.
^ permalink raw reply
* Re: Git Bug report
From: Junio C Hamano @ 2011-10-06 0:44 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Fredrik Gustafsson, Federico Lucifredi, git
In-Reply-To: <20111006003318.GA9015@goldbirke>
SZEDER Gábor <szeder@ira.uka.de> writes:
> And what about unreadable .git files?
Having then inside a working tree is so sick that I do not think it
deserves consideration.
Please don't troll immediately after a big release.
^ permalink raw reply
* [PATCH v2] Split GPG interface into its own helper library
From: Junio C Hamano @ 2011-10-06 0:46 UTC (permalink / raw)
To: git
This mostly 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>
---
* This is a re-roll of what was queued as part of jc/signed-push
topic. The helper is now aware of user.signingkey configuration
and can use it across all the future users.
Makefile | 2 +
builtin/tag.c | 76 +++-----------------------------
builtin/verify-tag.c | 35 +-------------
gpg-interface.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++
gpg-interface.h | 14 ++++++
5 files changed, 144 insertions(+), 102 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..3141680 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>]",
@@ -23,8 +24,6 @@ static const char * const git_tag_usage[] = {
NULL
};
-static char signingkey[1000];
-
struct tag_filter {
const char **patterns;
int lines;
@@ -208,60 +207,7 @@ 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;
-
- 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;
+ return sign_buffer(buffer, get_signing_key());
}
static const char tag_template[] =
@@ -270,21 +216,11 @@ static const char tag_template[] =
"# Write a tag message\n"
"#\n");
-static void set_signingkey(const char *value)
-{
- if (strlcpy(signingkey, value, sizeof(signingkey)) >= sizeof(signingkey))
- die(_("signing key value too long (%.10s...)"), value);
-}
-
static int git_tag_config(const char *var, const char *value, void *cb)
{
- if (!strcmp(var, "user.signingkey")) {
- if (!value)
- return config_error_nonbool(var);
- set_signingkey(value);
- return 0;
- }
-
+ int status = git_gpg_config(var, value, cb);
+ if (status)
+ return status;
return git_default_config(var, value, cb);
}
@@ -463,7 +399,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (keyid) {
sign = 1;
- set_signingkey(keyid);
+ set_signing_key(keyid);
}
if (sign)
annotate = 1;
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..98e8154
--- /dev/null
+++ b/gpg-interface.c
@@ -0,0 +1,119 @@
+/*
+ * Copyright (c) 2011, Google Inc.
+ */
+#include "cache.h"
+#include "run-command.h"
+#include "strbuf.h"
+#include "gpg-interface.h"
+#include "sigchain.h"
+
+static char *configured_signing_key;
+
+void set_signing_key(const char *key)
+{
+ free(configured_signing_key);
+ configured_signing_key = xstrdup(key);
+}
+
+int git_gpg_config(const char *var, const char *value, void *cb)
+{
+ if (!strcmp(var, "user.signingkey")) {
+ if (!value)
+ return config_error_nonbool(var);
+ set_signing_key(value);
+ }
+ return 0;
+}
+
+const char *get_signing_key(void)
+{
+ if (configured_signing_key)
+ return configured_signing_key;
+ return git_committer_info(IDENT_ERROR_ON_NO_NAME|IDENT_NO_DATE);
+}
+
+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..4e459fe
--- /dev/null
+++ b/gpg-interface.h
@@ -0,0 +1,14 @@
+#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);
+extern int git_gpg_config(const char *, const char *, void *);
+extern void set_signing_key(const char *);
+extern const char *get_signing_key(void);
+
+#endif
--
1.7.7.138.g7f41b6
^ permalink raw reply related
* [PATCH] commit: teach --gpg-sign option
From: Junio C Hamano @ 2011-10-06 0:56 UTC (permalink / raw)
To: git
And this uses the gpg-interface.[ch] to allow signing the commit, i.e.
$ git commit --gpg-sign -m foo
You need a passphrase to unlock the secret key for
user: "Junio C Hamano <gitster@pobox.com>"
4096-bit RSA key, ID 96AFE6CB, created 2011-10-03 (main key ID 713660A7)
[master 8457d13] foo
1 files changed, 1 insertions(+), 0 deletions(-)
The lines of GPG detached signature are placed in new header lines, after
the standard tree/parent/author/committer headers, instead of tucking the
signature block at the end of the commit log message text (similar to how
signed tag is done), for multiple reasons:
- The signature won't clutter output from "git log" and friends if it is
in the extra header. If we place it at the end of the log message, we
would need to teach "git log" and friends to strip the signature block
with an option.
- Teaching new versions of "git log" and "gitk" to optionally verify and
show signatures is cleaner if we structurally know where the signature
block is (instead of scanning in the commit log message).
- The signature needs to be stripped upon various commit rewriting
operations, e.g. rebase, filter-branch, etc. They all already ignore
unknown headers, but if we place signature in the log message, all of
these tools (and third-party tools) also need to learn how a signature
block would look like.
- When we added the optional encoding header, all the tools (both in tree
and third-party) that acts on the raw commit object should have been
fixed to ignore headers they do not understand, so it is not like that
new header would be more likely to break than extra text in the commit.
A commit made with the above sample sequence would look like this:
$ git cat-file commit HEAD
tree 3cd71d90e3db4136e5260ab54599791c4f883b9d
parent b87755351a47b09cb27d6913e6e0e17e6254a4d4
author Junio C Hamano <gitster@pobox.com> 1317862251 -0700
committer Junio C Hamano <gitster@pobox.com> 1317862251 -0700
sig -----BEGIN PGP SIGNATURE-----
sig Version: GnuPG v1.4.10 (GNU/Linux)
sig
sig iQIcBAABAgAGBQJOjPtrAAoJELC16IaWr+bL4TMP/RSe2Y/jYnCkds9unO5JEnfG
sig ...
sig =dt98
sig -----END PGP SIGNATURE-----
foo
but "git log" (unless you ask for it with --pretty=raw) output is not
cluttered with the signature information.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Cf.
Message-ID: <7vfwjgui8s.fsf_-_@alter.siamese.dyndns.org>
http://thread.gmane.org/gmane.comp.version-control.git/182297/focus=182384
builtin/commit-tree.c | 24 +++++++++++++++++++++---
builtin/commit.c | 12 ++++++++++--
builtin/merge.c | 16 ++++++++++++++--
commit.c | 41 ++++++++++++++++++++++++++++++++++++++++-
commit.h | 2 +-
notes-cache.c | 2 +-
notes-merge.c | 2 +-
7 files changed, 88 insertions(+), 11 deletions(-)
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index d083795..a17811f 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -8,8 +8,9 @@
#include "tree.h"
#include "builtin.h"
#include "utf8.h"
+#include "gpg-interface.h"
-static const char commit_tree_usage[] = "git commit-tree <sha1> [(-p <sha1>)...] < changelog";
+static const char commit_tree_usage[] = "git commit-tree [-S<signer>] <sha1> [(-p <sha1>)...] < changelog";
static void new_parent(struct commit *parent, struct commit_list **parents_p)
{
@@ -25,6 +26,14 @@ static void new_parent(struct commit *parent, struct commit_list **parents_p)
commit_list_insert(parent, parents_p);
}
+static int commit_tree_config(const char *var, const char *value, void *cb)
+{
+ int status = git_gpg_config(var, value, NULL);
+ if (status)
+ return status;
+ return git_default_config(var, value, cb);
+}
+
int cmd_commit_tree(int argc, const char **argv, const char *prefix)
{
int i;
@@ -32,11 +41,19 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
unsigned char tree_sha1[20];
unsigned char commit_sha1[20];
struct strbuf buffer = STRBUF_INIT;
+ const char *sign_commit = NULL;
- git_config(git_default_config, NULL);
+ git_config(commit_tree_config, NULL);
if (argc < 2 || !strcmp(argv[1], "-h"))
usage(commit_tree_usage);
+
+ if (!memcmp(argv[1], "-S", 2)) {
+ sign_commit = argv[1] + 2;
+ argv++;
+ argc--;
+ }
+
if (get_sha1(argv[1], tree_sha1))
die("Not a valid object name %s", argv[1]);
@@ -56,7 +73,8 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
if (strbuf_read(&buffer, 0, 0) < 0)
die_errno("git commit-tree: failed to read");
- if (commit_tree(buffer.buf, tree_sha1, parents, commit_sha1, NULL)) {
+ if (commit_tree(buffer.buf, tree_sha1, parents, commit_sha1,
+ NULL, sign_commit)) {
strbuf_release(&buffer);
return 1;
}
diff --git a/builtin/commit.c b/builtin/commit.c
index cbc9613..90cf7e8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -26,6 +26,7 @@
#include "unpack-trees.h"
#include "quote.h"
#include "submodule.h"
+#include "gpg-interface.h"
static const char * const builtin_commit_usage[] = {
"git commit [options] [--] <filepattern>...",
@@ -85,6 +86,8 @@ static int all, edit_flag, also, interactive, patch_interactive, only, amend, si
static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
static int no_post_rewrite, allow_empty_message;
static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
+static char *sign_commit;
+
/*
* The default commit message cleanup mode will remove the lines
* beginning with # (shell comments) and leading and trailing
@@ -144,6 +147,8 @@ static struct option builtin_commit_options[] = {
OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
OPT_STRING(0, "cleanup", &cleanup_arg, "default", "how to strip spaces and #comments from message"),
OPT_BOOLEAN(0, "status", &include_status, "include status in commit message template"),
+ { OPTION_STRING, 'S', "gpg-sign", &sign_commit, "key id",
+ "GPG sign commit", PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
/* end commit message options */
OPT_GROUP("Commit contents options"),
@@ -1323,6 +1328,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
static int git_commit_config(const char *k, const char *v, void *cb)
{
struct wt_status *s = cb;
+ int status;
if (!strcmp(k, "commit.template"))
return git_config_pathname(&template_file, k, v);
@@ -1330,7 +1336,9 @@ static int git_commit_config(const char *k, const char *v, void *cb)
include_status = git_config_bool(k, v);
return 0;
}
-
+ status = git_gpg_config(k, v, NULL);
+ if (status)
+ return status;
return git_status_config(k, v, s);
}
@@ -1481,7 +1489,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
}
if (commit_tree(sb.buf, active_cache_tree->sha1, parents, sha1,
- author_ident.buf)) {
+ author_ident.buf, sign_commit)) {
rollback_index_files();
die(_("failed to write commit object"));
}
diff --git a/builtin/merge.c b/builtin/merge.c
index ab4077f..53cff02 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -26,6 +26,7 @@
#include "merge-recursive.h"
#include "resolve-undo.h"
#include "remote.h"
+#include "gpg-interface.h"
#define DEFAULT_TWOHEAD (1<<0)
#define DEFAULT_OCTOPUS (1<<1)
@@ -63,6 +64,7 @@ static int allow_rerere_auto;
static int abort_current_merge;
static int show_progress = -1;
static int default_to_upstream;
+static const char *sign_commit;
static struct strategy all_strategy[] = {
{ "recursive", DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -206,6 +208,8 @@ static struct option builtin_merge_options[] = {
OPT_BOOLEAN(0, "abort", &abort_current_merge,
"abort the current in-progress merge"),
OPT_SET_INT(0, "progress", &show_progress, "force progress reporting", 1),
+ { OPTION_STRING, 'S', "gpg-sign", &sign_commit, "key id",
+ "GPG sign commit", PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
OPT_END()
};
@@ -525,6 +529,8 @@ static void parse_branch_merge_options(char *bmo)
static int git_merge_config(const char *k, const char *v, void *cb)
{
+ int status;
+
if (branch && !prefixcmp(k, "branch.") &&
!prefixcmp(k + 7, branch) &&
!strcmp(k + 7 + strlen(branch), ".mergeoptions")) {
@@ -562,6 +568,10 @@ static int git_merge_config(const char *k, const char *v, void *cb)
default_to_upstream = git_config_bool(k, v);
return 0;
}
+
+ status = git_gpg_config(k, v, NULL);
+ if (status)
+ return status;
return git_diff_ui_config(k, v, cb);
}
@@ -870,7 +880,8 @@ static int merge_trivial(void)
parent->next->item = remoteheads->item;
parent->next->next = NULL;
run_prepare_commit_msg();
- commit_tree(merge_msg.buf, result_tree, parent, result_commit, NULL);
+ commit_tree(merge_msg.buf, result_tree, parent, result_commit, NULL,
+ sign_commit);
finish(result_commit, "In-index merge");
drop_save();
return 0;
@@ -900,7 +911,8 @@ static int finish_automerge(struct commit_list *common,
free_commit_list(remoteheads);
strbuf_addch(&merge_msg, '\n');
run_prepare_commit_msg();
- commit_tree(merge_msg.buf, result_tree, parents, result_commit, NULL);
+ commit_tree(merge_msg.buf, result_tree, parents, result_commit,
+ NULL, sign_commit);
strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
finish(result_commit, buf.buf);
strbuf_release(&buf);
diff --git a/commit.c b/commit.c
index 97b4327..969435d 100644
--- a/commit.c
+++ b/commit.c
@@ -6,6 +6,7 @@
#include "diff.h"
#include "revision.h"
#include "notes.h"
+#include "gpg-interface.h"
int save_commit_buffer = 1;
@@ -814,6 +815,41 @@ struct commit_list *reduce_heads(struct commit_list *heads)
return result;
}
+static int do_sign_commit(struct strbuf *buf, const char *keyid)
+{
+ struct strbuf sig = STRBUF_INIT;
+ int inspos, copypos;
+ const char gpg_sig[] = "sig ";
+ const int header_len = sizeof(gpg_sig) - 1;
+
+ /* find the end of the header */
+ inspos = strstr(buf->buf, "\n\n") - buf->buf + 1;
+ copypos = buf->len;
+
+ strbuf_addbuf(&sig, buf);
+
+ if (!keyid || !*keyid)
+ keyid = get_signing_key();
+ if (sign_buffer(&sig, keyid)) {
+ strbuf_release(&sig);
+ return -1;
+ }
+
+ while (sig.buf[copypos]) {
+ const char *bol = sig.buf + copypos;
+ const char *eol = strchrnul(bol, '\n');
+ int len = (eol - bol) + !!*eol;
+ strbuf_insert(buf, inspos, gpg_sig, header_len);
+ inspos += header_len;
+ strbuf_insert(buf, inspos, bol, len);
+ inspos += len;
+ copypos += len;
+ }
+ strbuf_release(&sig);
+ return 0;
+}
+
+
static const char commit_utf8_warn[] =
"Warning: commit message does not conform to UTF-8.\n"
"You may want to amend it after fixing the message, or set the config\n"
@@ -821,7 +857,7 @@ static const char commit_utf8_warn[] =
int commit_tree(const char *msg, unsigned char *tree,
struct commit_list *parents, unsigned char *ret,
- const char *author)
+ const char *author, const char *sign_commit)
{
int result;
int encoding_is_utf8;
@@ -864,6 +900,9 @@ int commit_tree(const char *msg, unsigned char *tree,
if (encoding_is_utf8 && !is_utf8(buffer.buf))
fprintf(stderr, commit_utf8_warn);
+ if (sign_commit && do_sign_commit(&buffer, sign_commit))
+ return -1;
+
result = write_sha1_file(buffer.buf, buffer.len, commit_type, ret);
strbuf_release(&buffer);
return result;
diff --git a/commit.h b/commit.h
index 12d100b8..8c2419b 100644
--- a/commit.h
+++ b/commit.h
@@ -175,6 +175,6 @@ struct commit_list *reduce_heads(struct commit_list *heads);
extern int commit_tree(const char *msg, unsigned char *tree,
struct commit_list *parents, unsigned char *ret,
- const char *author);
+ const char *author, const char *sign_commit);
#endif /* COMMIT_H */
diff --git a/notes-cache.c b/notes-cache.c
index 4c8984e..c36a960 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -56,7 +56,7 @@ int notes_cache_write(struct notes_cache *c)
if (write_notes_tree(&c->tree, tree_sha1))
return -1;
- if (commit_tree(c->validity, tree_sha1, NULL, commit_sha1, NULL) < 0)
+ if (commit_tree(c->validity, tree_sha1, NULL, commit_sha1, NULL, NULL) < 0)
return -1;
if (update_ref("update notes cache", c->tree.ref, commit_sha1, NULL,
0, QUIET_ON_ERR) < 0)
diff --git a/notes-merge.c b/notes-merge.c
index e1aaf43..c29c434 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -546,7 +546,7 @@ void create_notes_commit(struct notes_tree *t, struct commit_list *parents,
/* else: t->ref points to nothing, assume root/orphan commit */
}
- if (commit_tree(msg, tree_sha1, parents, result_sha1, NULL))
+ if (commit_tree(msg, tree_sha1, parents, result_sha1, NULL, NULL))
die("Failed to commit notes tree to database");
}
--
1.7.7.138.g7f41b6
^ permalink raw reply related
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Jay Soffian @ 2011-10-06 0:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <7vsjn73q6j.fsf@alter.siamese.dyndns.org>
On Wed, Oct 5, 2011 at 8:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Not at all. My build infrastructure determines where to install the built
> binary based on what branch is checked out. Having a head detached at a
> commit that is at the tip of one branch is not necessarily the same as
> having the branch actually checked out.
That's fair, but I'm willing to wager that's a minority use-case. Not
that it shouldn't be possible, but perhaps it should require telling
git that's really what you want to do with checkout --force.
>> Also, if we wait till commit time to tell the user "sorry, topic's
>> been updated elsewhere", now the user is in a perilous state.
>
> Wouldn't the "elsewhere" user would be warned before being able to update
> the branch? I thought the whole point of your adding "this branch is
> checked out over there" is exactly so that the "elsewhere" user can come
> talk to you before that happens. These two people might be yourself, of
> course.
So you're envisioning this?
$ git commit foo.c
Warning, master is also checked out in workdir2
How does that help the user? Now they have to go to workdir2 and reset
--hard. Is that really something we want to encourage?
And what if they do this:
$ cd workdir1
$ edit foo.c
... time passes...
$ cd workdir2
$ edit foo.c
$ git commit foo.c
Warning, master is also checked out in workdir1
j.
^ permalink raw reply
* Re: Git Bug report
From: SZEDER Gábor @ 2011-10-06 1:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Fredrik Gustafsson, Federico Lucifredi, git
In-Reply-To: <7vobxv3q49.fsf@alter.siamese.dyndns.org>
On Wed, Oct 05, 2011 at 05:44:54PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
>
> > And what about unreadable .git files?
>
> Having then inside a working tree is so sick that I do not think it
> deserves consideration.
I'm not sure why is this any different than having a .git directory
that is not a repository inside a working tree.
> Please don't troll immediately after a big release.
I didn't mean to troll; it just happened that I came across this issue
this weekend while trying to optimize the bash completion code...
^ permalink raw reply
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Junio C Hamano @ 2011-10-06 1:15 UTC (permalink / raw)
To: Jay Soffian; +Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git
In-Reply-To: <CAG+J_DxXcvF3tBPkf7ZEtiXvEK80zYJvP1rNx-PagM8TV-1KSA@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> So you're envisioning this?
>
> $ git commit foo.c
> Warning, master is also checked out in workdir2
No. I would rather think it needs to be forced.
^ permalink raw reply
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Jay Soffian @ 2011-10-06 1:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <7v62k253ad.fsf@alter.siamese.dyndns.org>
On Wed, Oct 5, 2011 at 9:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jay Soffian <jaysoffian@gmail.com> writes:
>
>> So you're envisioning this?
>>
>> $ git commit foo.c
>> Warning, master is also checked out in workdir2
>
> No. I would rather think it needs to be forced.
Now they do what? Either commit --force or create a new branch?
Wouldn't it have been better to create the new branch before they
started editing?
Here's what I'm trying to avoid:
$ cd workdir2
$ git checkout master
$ edit foo.c
$ git commit foo.c
By default, committing to a branch that is checked out in more than
one location is denied, because it will make the index and work tree
inconsistent in the other locations and will require 'git reset --hard'
to match the work tree to HEAD in each of those other locations.
Either switch to a new branch first with 'git checkout -b <new_branch>'
or use 'git commit --force' to override this message.
User: "crap, I wanted that on master". Now they do what. Something like:
$ git checkout -b foo
$ git commit foo.c
$ cd workdir1
$ git cherry-pick foo
$ git branch -d foo
Or maybe they use stash instead. In either case, I just think that's a
terrible user experience compared to:
$ cd workdir2
$ git checkout master
error: master already checked out in workdir1
$ cd workdir1
$ edit foo.c
$ git commit foo.c
I guess it depends what you mostly use your workdirs for. For me, it's
to have different branches checked out, not to have the same branch
checked out in multiple locations. I want git to help me up front, not
when I go to commit.
j.
^ permalink raw reply
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Junio C Hamano @ 2011-10-06 1:57 UTC (permalink / raw)
To: Jay Soffian; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <CAG+J_Dz++SG28a=DhZ+Doz1np21jMavYpc0hKfe1rgq-dHZLPA@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> On Wed, Oct 5, 2011 at 9:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jay Soffian <jaysoffian@gmail.com> writes:
>>
>>> So you're envisioning this?
>>>
>>> $ git commit foo.c
>>> Warning, master is also checked out in workdir2
>>
>> No. I would rather think it needs to be forced.
>
> Now they do what? Either commit --force or create a new branch?
> Wouldn't it have been better to create the new branch before they
> started editing?
If they are going to commit, and if they knew that they are going to
commit, yes.
But why do you want to forbid people from just checking things out if they
are not interested in committing? That is where I think you are going
backwards.
> I guess it depends what you mostly use your workdirs for. For me, it's
> to have different branches checked out, not to have the same branch
> checked out in multiple locations.
Then you wouldn't have any problem if commit refused to make commit on the
branch that is checked out elsewhere, no?
I am not saying we should never have an option to _warn_ checking out the
same branch in multiple places. I am saying it is wrong to forbid doing so
by default.
^ permalink raw reply
* Re: [PATCH 2/4] cleanup: use internal memory allocation wrapper functions everywhere
From: Brandon Casey @ 2011-10-06 2:00 UTC (permalink / raw)
To: Johannes Sixt
Cc: peff@peff.net, git@vger.kernel.org, gitster@pobox.com,
sunshine@sunshineco.com, bharrosh@panasas.com,
trast@student.ethz.ch, zapped@mail.ru
In-Reply-To: <CA+sFfMf73K3yv_5K633DKOsVufMV6rTjd+SSunq4sBikt4jCsg@mail.gmail.com>
[resend without html bits added by "gmail offline"]
So, it seems that of all of Google's email clients, only full desktop
gmail allows you to send plain text email (if you're careful to make
sure "Rich formatting" has not been clicked). The new offline gmail
sends html, gmail android app sends html, gmail mobile web sends html.
Google's war on plain text continues...
Or have I overlooked the switch that makes gmail send plain text and
only plain text?
On Wed, Oct 5, 2011 at 7:53 PM, Brandon Casey <drafnel@gmail.com> wrote:
> On Thursday, September 15, 2011, Brandon Casey wrote:
>>
>> On Thu, Sep 15, 2011 at 1:52 AM, Johannes Sixt <j.sixt@viscovery.net>
>> wrote:
>> > Am 9/15/2011 3:59, schrieb Brandon Casey:
>> >> The "x"-prefixed versions of strdup, malloc, etc. will check whether
>> >> the
>> >> allocation was successful and terminate the process otherwise.
>> >>
>> >> A few uses of malloc were left alone since they already implemented a
>> >> graceful path of failure or were in a quasi external library like
>> >> xdiff.
>> >>
>> >> Signed-off-by: Brandon Casey <drafnel@gmail.com>
>> >> ---
>> >> ...
>> >> compat/mingw.c | 2 +-
>> >> compat/qsort.c | 2 +-
>> >> compat/win32/syslog.c | 2 +-
>> >
>> > There is a danger that the high-level die() routine (which is used by
>> > the
>> > x-wrappers) uses one of the low-level compat/ routines. IOW, in the case
>> > of errors, recursion might occur. Therefore, I would prefer that the
>> > compat/ routines do their own error reporting (preferably via return
>> > values and errno).
>>
>> Thanks. Will do.
>
> Hi Johannes,
> I have taken a closer look at the possibility of recursion with respect to
> die() and the functions in compat/. It appears the risk is only related to
> vsnprintf/snprintf at the moment. So as long as we avoid calling xmalloc et
> al from within snprintf.c, I think we should be safe from recursion.
> I'm inclined to keep the additions to mingw.c and win32/syslog.c since they
> both already use the x-wrappers or strbuf, even though they could easily be
> worked around. The other file that was touched is compat/qsort, which
> returns void and doesn't have a good alternative error handling path. So,
> I'm inclined to keep that one too.
> Sound reasonable?
> -Brandon
>
^ permalink raw reply
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Nguyen Thai Ngoc Duy @ 2011-10-06 2:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwrcj3sow.fsf@alter.siamese.dyndns.org>
On Thu, Oct 6, 2011 at 10:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> On Thu, Oct 6, 2011 at 9:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> I think of two options:
>>>> ...
>>> Sorry, what problem are you trying to solve? Does that "checkout -f" meant
>>> to nuke the local changes that are not yet at a good "stop point"?
>>
>> I meant "git checkout" on the already locked branch is refused, but
>> "git checkout -f" in that case will act just like "git checkout"
>> ignoring all locks. But I forgot that "git checkout -f" also discards
>> worktree changes. Maybe "git checkout --ignore-locks" instead of "git
>> checkout -f".
>
> I see what you mean, but doesn't it feel as if it is working around a
> problem that is introduced only because of a wrong policy (i.e. "you
> cannot check out the same branch at two places", as opposed to "viewing
> them in multiple places is perfectly fine, but no touching")?
Well, we could do change the default so "git checkout" == "git
checkout --ignore-locks".
"git commit --ignore-locks" would commit without checking locks. "git
commit" could either:
- reject because it does not hold the lock (to hostile?)
- detach automatically then commit
The latter has a benefit that we can now checkout tags without
detaching from the beginning. "git branch" would show tag name until
you commit.
--
Duy
^ permalink raw reply
* Re: How do I investigate apparently random git clone reports of "error: File ... has bad hash"?
From: Thorkil Naur @ 2011-10-06 2:34 UTC (permalink / raw)
To: Andreas Schwab; +Cc: cvs-ghc, git
In-Reply-To: <m2r53metpo.fsf@igel.home>
Hello Andreas,
Thank you very much for your response. I have separately received the
advice of upgrading my curl installation:
> Date: Fri, 30 Sep 2011 09:08:56 +0200
> From: Karel Gardas <karel.gardas@centrum.cz>
> To: cvs-ghc <cvs-ghc@haskell.org>
> Subject: Re: tn23 (x86 OSX HEAD), build 442, Failure
> ...
> Hello,
>
> my opensolaris builder machine also suffered from the same issue like
> tn23 and sometimes even mbolingbroke and others. Symptoms are you are
> not able to grab the ghc code or subrepos code. The solution is
> simple: (1) either remove curl from your path or (2) update curl to
> the latest version (7.21.7 works for me) and make sure it is really
> using its latest libcurl. Once I did (2) here I've never seen the
> issue again.
> ...
Additional details:
> http://www.haskell.org/pipermail/cvs-ghc/2011-October/066434.html
So it appears that upgrading curl has removed the problem.
Best regards
Thorkil
On Sun, Sep 11, 2011 at 09:59:15PM +0200, Andreas Schwab wrote:
> Thorkil Naur <naur@post11.tele.dk> writes:
>
> >> $ git clone http://darcs.haskell.org/ghc.git/ build8
> >> Cloning into build8...
> >> error: File 42988feeeb76f5cb92b541e9dac277e073bcb3ef has bad hash
> >> error: Unable to find 42988feeeb76f5cb92b541e9dac277e073bcb3ef under
> > http://darcs.haskell.org/ghc.git
> >> Cannot obtain needed blob 42988feeeb76f5cb92b541e9dac277e073bcb3ef
> >> while processing commit ffb2e81c03a01e74825b3a0223e214df59241fab.
> >> error: Fetch failed.
>
> I just tried to clone it and got this error:
>
> $ git clone http://darcs.haskell.org/ghc.git
> Cloning into ghc...
> error: Recv failure: Connection reset by peer (curl_result = 56, http_code = 0, sha1 = be6810bb027643bf0697b3d237426110f064aba1)
> error: Unable to find be6810bb027643bf0697b3d237426110f064aba1 under http://darcs.haskell.org/ghc.git
> Cannot obtain needed commit be6810bb027643bf0697b3d237426110f064aba1
> while processing commit 6942b112082fbcdff5c66f06f56fdd336861da47.
> error: Fetch failed.
>
> It looks like this is just a network problem.
>
> Btw, the repo is rather strange. It's not a bare repo, but does not
> contain a .git directory. Instead the files that are normally under
> .git are placed directly in the working tree.
>
> Andreas.
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
> "And now for something completely different."
^ permalink raw reply
* Re: [PATCHv3 3/5] Add a common is_gitfile function
From: Nguyen Thai Ngoc Duy @ 2011-10-06 2:59 UTC (permalink / raw)
To: Phil Hord; +Cc: git@vger.kernel.org, Phil Hord, Junio C Hamano, Erik Faye-Lund
In-Reply-To: <4E8C5CC2.4030505@cisco.com>
On Thu, Oct 6, 2011 at 12:33 AM, Phil Hord <hordp@cisco.com> wrote:
> diff --git a/setup.c b/setup.c
> index 61c22e6..a3d5a41 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -358,7 +358,7 @@ const char *read_gitfile(const char *path)
>
> if (stat(path, &st))
> return NULL;
> - if (!S_ISREG(st.st_mode))
> + if (!is_gitfile(path))
> return NULL;
> fd = open(path, O_RDONLY);
> if (fd < 0)
> @@ -368,9 +368,6 @@ const char *read_gitfile(const char *path)
> close(fd);
> if (len != st.st_size)
> die("Error reading %s", path);
> - buf[len] = '\0';
> - if (prefixcmp(buf, "gitdir: "))
> - die("Invalid gitfile format: %s", path);
> while (buf[len - 1] == '\n' || buf[len - 1] == '\r')
> len--;
> if (len < 9)
I think you are changing the behavior here. Currently if .git is a
file and not a valid gitfile, die() will be called and repo discovery
process stops. With your patch, it skips invalid .git files and moves
on.
I'm not saying it's good or bad, just pointing out.
--
Duy
^ permalink raw reply
* Re: [PATCHv3 2/5] Learn to handle gitfiles in enter_repo
From: Nguyen Thai Ngoc Duy @ 2011-10-06 3:11 UTC (permalink / raw)
To: Phil Hord; +Cc: git@vger.kernel.org, Phil Hord, Junio C Hamano, Erik Faye-Lund
In-Reply-To: <4E8C5C2E.50309@cisco.com>
On Thu, Oct 6, 2011 at 12:31 AM, Phil Hord <hordp@cisco.com> wrote:
> - if (!suffix[i] || chdir(used_path))
> + if (!suffix[i])
> + return NULL;
> + gitfile = read_gitfile(used_path) ;
> + if (gitfile)
> + strcpy(used_path, gitfile);
> + if (chdir(used_path))
> return NULL;
> path = validated_path;
> }
This is room for improvement, the patch is fine as it is now. We could
improve error reporting here. If .git file points to nowhere, we get
"not a repository-kind of message. Except daemon.c, enter_repo()
callers always die() if enter_repo() returns NULL. We could move the
die() part (with improved error message) into enter_repo().
We could update enter_repo(const char *, int) to enter_repo(const char
*, int, int gently). If gently is 1, we never die() nor report
anything (ie. what we're doing now). daemon.c will need this, the rest
of callers will be happy with gently = 0.
--
Duy
^ permalink raw reply
* Re: [PATCHv3 5/5] Add test showing git-fetch groks gitfiles
From: Nguyen Thai Ngoc Duy @ 2011-10-06 3:13 UTC (permalink / raw)
To: Phil Hord; +Cc: git@vger.kernel.org, Phil Hord, Junio C Hamano, Erik Faye-Lund
In-Reply-To: <4E8C5D31.2000406@cisco.com>
On Thu, Oct 6, 2011 at 12:35 AM, Phil Hord <hordp@cisco.com> wrote:
> Add a test for two subtly different cases: 'git fetch path/.git'
> and 'git fetch path' to confirm that transport recognizes both
> paths as git repositories when using the gitfile mechanism.
It'd be interesting to test failed cases too. I can think of two: .git
file is not a valid file and .git points to an invalid repo.
--
Duy
^ permalink raw reply
* [PATCH v2 0/2] reroll bc/attr-ignore-case (top two commits)
From: Brandon Casey @ 2011-10-06 4:00 UTC (permalink / raw)
To: gitster; +Cc: git, peff, Brandon Casey
This is the re-roll of this series that I said I would do.
This just swaps the order of the top two patches around and *hopefully*
improves the commit messages.
My original "attr.c: respect core.ignorecase.." patch was broken and I
intended to mark it as WIP. Your patch fixes it, but it's nicer if your
patch comes first, that way the test suite (and git) is never broken.
This is built on top of aae0befee293ebdf9aa6391a074eb0e7c10cdf75 in
bc/attr-ignore-case, which hopefully makes it easy to replace the top two
commits in that branch with these two commits.
I know this is already in next, and you seem poised to merge to master,
so feel free to drop these if you want.
Brandon Casey (1):
attr.c: respect core.ignorecase when matching attribute patterns
Junio C Hamano (1):
attr: read core.attributesfile from git_default_core_config
attr.c | 20 ++++------------
builtin/check-attr.c | 2 +
cache.h | 1 +
config.c | 3 ++
environment.c | 1 +
t/t0003-attributes.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-
6 files changed, 71 insertions(+), 16 deletions(-)
--
1.7.7.1.ge3b6f
^ permalink raw reply
* [PATCH v2 1/2] attr: read core.attributesfile from git_default_core_config
From: Brandon Casey @ 2011-10-06 4:00 UTC (permalink / raw)
To: gitster; +Cc: git, peff, Brandon Casey
In-Reply-To: <1317873614-3057-1-git-send-email-drafnel@gmail.com>
From: Junio C Hamano <gitster@pobox.com>
This code calls git_config from a helper function to parse the config entry
it is interested in. Calling git_config in this way may cause a problem if
the helper function can be called after a previous call to git_config by
another function since the second call to git_config may reset some
variable to the value in the config file which was previously overridden.
The above is not a problem in this case since the function passed to
git_config only parses one config entry and the variable it sets is not
assigned outside of the parsing function. But a programmer who desires
all of the standard config options to be parsed may be tempted to modify
git_attr_config() so that it falls back to git_default_config() and then it
_would_ be vulnerable to the above described behavior.
So, move the call to git_config up into the top-level cmd_* function and
move the responsibility for parsing core.attributesfile into the main
config file parser.
Which is only the logical thing to do ;-)
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
attr.c | 15 ++-------------
builtin/check-attr.c | 2 ++
cache.h | 1 +
config.c | 3 +++
environment.c | 1 +
5 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/attr.c b/attr.c
index 0793859..124337d 100644
--- a/attr.c
+++ b/attr.c
@@ -20,8 +20,6 @@ static const char git_attr__unknown[] = "(builtin)unknown";
#define ATTR__UNSET NULL
#define ATTR__UNKNOWN git_attr__unknown
-static const char *attributes_file;
-
/* This is a randomly chosen prime. */
#define HASHSIZE 257
@@ -494,14 +492,6 @@ static int git_attr_system(void)
return !git_env_bool("GIT_ATTR_NOSYSTEM", 0);
}
-static int git_attr_config(const char *var, const char *value, void *dummy)
-{
- if (!strcmp(var, "core.attributesfile"))
- return git_config_pathname(&attributes_file, var, value);
-
- return 0;
-}
-
static void bootstrap_attr_stack(void)
{
if (!attr_stack) {
@@ -521,9 +511,8 @@ static void bootstrap_attr_stack(void)
}
}
- git_config(git_attr_config, NULL);
- if (attributes_file) {
- elem = read_attr_from_file(attributes_file, 1);
+ if (git_attributes_file) {
+ elem = read_attr_from_file(git_attributes_file, 1);
if (elem) {
elem->origin = NULL;
elem->prev = attr_stack;
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 708988a..abb1165 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -92,6 +92,8 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
struct git_attr_check *check;
int cnt, i, doubledash, filei;
+ git_config(git_default_config, NULL);
+
argc = parse_options(argc, argv, prefix, check_attr_options,
check_attr_usage, PARSE_OPT_KEEP_DASHDASH);
diff --git a/cache.h b/cache.h
index 607c2ea..8d95fb2 100644
--- a/cache.h
+++ b/cache.h
@@ -589,6 +589,7 @@ extern int warn_ambiguous_refs;
extern int shared_repository;
extern const char *apply_default_whitespace;
extern const char *apply_default_ignorewhitespace;
+extern const char *git_attributes_file;
extern int zlib_compression_level;
extern int core_compression_level;
extern int core_compression_seen;
diff --git a/config.c b/config.c
index 4183f80..d3bcaa0 100644
--- a/config.c
+++ b/config.c
@@ -491,6 +491,9 @@ static int git_default_core_config(const char *var, const char *value)
return 0;
}
+ if (!strcmp(var, "core.attributesfile"))
+ return git_config_pathname(&git_attributes_file, var, value);
+
if (!strcmp(var, "core.bare")) {
is_bare_repository_cfg = git_config_bool(var, value);
return 0;
diff --git a/environment.c b/environment.c
index e96edcf..d60b73f 100644
--- a/environment.c
+++ b/environment.c
@@ -29,6 +29,7 @@ const char *git_log_output_encoding;
int shared_repository = PERM_UMASK;
const char *apply_default_whitespace;
const char *apply_default_ignorewhitespace;
+const char *git_attributes_file;
int zlib_compression_level = Z_BEST_SPEED;
int core_compression_level;
int core_compression_seen;
--
1.7.7.1.ge3b6f
^ permalink raw reply related
* [PATCH v2 2/2] attr.c: respect core.ignorecase when matching attribute patterns
From: Brandon Casey @ 2011-10-06 4:00 UTC (permalink / raw)
To: gitster; +Cc: git, peff, Brandon Casey
In-Reply-To: <1317873614-3057-1-git-send-email-drafnel@gmail.com>
When core.ignorecase is true, the file globs configured in the
.gitattributes file should be matched case-insensitively against the paths
in the working directory. Let's do so.
Plus, add some tests.
The last set of tests is performed only on a case-insensitive filesystem.
Those tests make sure that git handles the case where the .gitignore file
resides in a subdirectory and the user supplies a path that does not match
the case in the filesystem. In that case^H^H^H^Hsituation, part of the
path supplied by the user is effectively interpreted case-insensitively,
and part of it is dependent on the setting of core.ignorecase. git should
only match the portion of the path below the directory holding the
.gitignore file according to the setting of core.ignorecase.
This is also partly future-proofing. Currently, git builds the attr stack
based on the path supplied by the user, so we don't have to do anything
special (like use strcmp_icase) to handle the parts of that path that don't
match the filesystem with respect to case. If git instead built the attr
stack by scanning the repository, then the paths in the origin field would
not necessarily match the paths supplied by the user. If someone makes a
change like that in the future, these tests will notice.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
attr.c | 5 ++-
t/t0003-attributes.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 62 insertions(+), 3 deletions(-)
diff --git a/attr.c b/attr.c
index 124337d..76b079f 100644
--- a/attr.c
+++ b/attr.c
@@ -11,6 +11,7 @@
#include "cache.h"
#include "exec_cmd.h"
#include "attr.h"
+#include "dir.h"
const char git_attr__true[] = "(builtin)true";
const char git_attr__false[] = "\0(builtin)false";
@@ -631,7 +632,7 @@ static int path_matches(const char *pathname, int pathlen,
/* match basename */
const char *basename = strrchr(pathname, '/');
basename = basename ? basename + 1 : pathname;
- return (fnmatch(pattern, basename, 0) == 0);
+ return (fnmatch_icase(pattern, basename, 0) == 0);
}
/*
* match with FNM_PATHNAME; the pattern has base implicitly
@@ -645,7 +646,7 @@ static int path_matches(const char *pathname, int pathlen,
return 0;
if (baselen != 0)
baselen++;
- return fnmatch(pattern, pathname + baselen, FNM_PATHNAME) == 0;
+ return fnmatch_icase(pattern, pathname + baselen, FNM_PATHNAME) == 0;
}
static int macroexpand_one(int attr_nr, int rem);
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index ae2f1da..47a70c4 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -9,7 +9,7 @@ attr_check () {
path="$1"
expect="$2"
- git check-attr test -- "$path" >actual 2>err &&
+ git $3 check-attr test -- "$path" >actual 2>err &&
echo "$path: test: $2" >expect &&
test_cmp expect actual &&
test_line_count = 0 err
@@ -27,6 +27,7 @@ test_expect_success 'setup' '
echo "onoff test -test"
echo "offon -test test"
echo "no notest"
+ echo "A/e/F test=A/e/F"
) >.gitattributes &&
(
echo "g test=a/g" &&
@@ -93,6 +94,63 @@ test_expect_success 'attribute test' '
'
+test_expect_success 'attribute matching is case sensitive when core.ignorecase=0' '
+
+ test_must_fail attr_check F f "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/F f "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/c/F f "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/G a/g "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/B/g a/b/g "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/b/G a/b/g "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/b/H a/b/h "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/b/D/g "a/b/d/*" "-c core.ignorecase=0" &&
+ test_must_fail attr_check oNoFf unset "-c core.ignorecase=0" &&
+ test_must_fail attr_check oFfOn set "-c core.ignorecase=0" &&
+ attr_check NO unspecified "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/b/D/NO "a/b/d/*" "-c core.ignorecase=0" &&
+ attr_check a/b/d/YES a/b/d/* "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/E/f "A/e/F" "-c core.ignorecase=0"
+
+'
+
+test_expect_success 'attribute matching is case insensitive when core.ignorecase=1' '
+
+ attr_check F f "-c core.ignorecase=1" &&
+ attr_check a/F f "-c core.ignorecase=1" &&
+ attr_check a/c/F f "-c core.ignorecase=1" &&
+ attr_check a/G a/g "-c core.ignorecase=1" &&
+ attr_check a/B/g a/b/g "-c core.ignorecase=1" &&
+ attr_check a/b/G a/b/g "-c core.ignorecase=1" &&
+ attr_check a/b/H a/b/h "-c core.ignorecase=1" &&
+ attr_check a/b/D/g "a/b/d/*" "-c core.ignorecase=1" &&
+ attr_check oNoFf unset "-c core.ignorecase=1" &&
+ attr_check oFfOn set "-c core.ignorecase=1" &&
+ attr_check NO unspecified "-c core.ignorecase=1" &&
+ attr_check a/b/D/NO "a/b/d/*" "-c core.ignorecase=1" &&
+ attr_check a/b/d/YES unspecified "-c core.ignorecase=1" &&
+ attr_check a/E/f "A/e/F" "-c core.ignorecase=1"
+
+'
+
+test_expect_success 'check whether FS is case-insensitive' '
+ mkdir junk &&
+ echo good >junk/CamelCase &&
+ echo bad >junk/camelcase &&
+ if test "$(cat junk/CamelCase)" != good
+ then
+ test_set_prereq CASE_INSENSITIVE_FS
+ fi
+'
+
+test_expect_success CASE_INSENSITIVE_FS 'additional case insensitivity tests' '
+ test_must_fail attr_check a/B/D/g "a/b/d/*" "-c core.ignorecase=0" &&
+ test_must_fail attr_check A/B/D/NO "a/b/d/*" "-c core.ignorecase=0" &&
+ attr_check A/b/h a/b/h "-c core.ignorecase=0" &&
+ attr_check A/b/h a/b/h "-c core.ignorecase=1" &&
+ attr_check a/B/D/g "a/b/d/*" "-c core.ignorecase=1" &&
+ attr_check A/B/D/NO "a/b/d/*" "-c core.ignorecase=1"
+'
+
test_expect_success 'unnormalized paths' '
attr_check ./f f &&
--
1.7.7.1.ge3b6f
^ permalink raw reply related
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Jay Soffian @ 2011-10-06 4:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <7v1uuq51c3.fsf@alter.siamese.dyndns.org>
On Wed, Oct 5, 2011 at 9:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jay Soffian <jaysoffian@gmail.com> writes:
>> Now they do what? Either commit --force or create a new branch?
>> Wouldn't it have been better to create the new branch before they
>> started editing?
>
> If they are going to commit, and if they knew that they are going to
> commit, yes.
Committing is obviously the common case for a checked-out branch.
> But why do you want to forbid people from just checking things out if they
> are not interested in committing? That is where I think you are going
> backwards.
Because if they do decide to commit, it's now harder for them to do so.
It would be great if git could intervene after the checkout, but
before they edit any files, so that they don't have uncommitted work.
Obviously that's not possible, so git should prevent them from getting
to that point.
Let's consider the various situations:
1. master is checked out w/edits in workdir1, user wants to work on
topic in workdir2.
There's nothing to warn about in workdir2 neither at checkout nor commit time.
2. master is checked out w/edits in workdir1, user wants examine
unedited master in workdir2
At checkout time in workdir2:
My preference: checkout advices user to use --detach or --force.
Your preference: checkout is silent.
Now user decides they want to commit to master in workdir2 (which is
insane, they've got uncommitted changes to it in workdir1). What
happens?
In my scenario, the commit happens on a detached HEAD. When they
eventually switch back to a branch, git tells them how to move their
commit to a branch.
In your scenario, commit complains. User now has to --force, stash, or
create a new branch.
It's just seems insane to me putting in obstacles to the user
committing their work. That's where I think you are going backwards.
You have a use case where using a detached HEAD doesn't work because
you've scripted around the same branch multiply checked out. I think
that's probably an exceedingly rare use case, and justifies "checkout
--force".
>> I guess it depends what you mostly use your workdirs for. For me, it's
>> to have different branches checked out, not to have the same branch
>> checked out in multiple locations.
>
> Then you wouldn't have any problem if commit refused to make commit on the
> branch that is checked out elsewhere, no?
Yes, I would, because by that point, I've already made the mistake of
checking out the same branch twice. I want git to prevent me from
doing that by accident. Because I don't want to ever be in the
situation which comes next, which is that I've got uncommitted work
for the same branch in two places!
> I am not saying we should never have an option to _warn_ checking out the
> same branch in multiple places. I am saying it is wrong to forbid doing so
> by default.
I am not saying we should never have an option to allow checking out
the same branch in multiple places. I am saying it is wrong to allow
doing so by default.
j.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox