* Re: git-cherry-pick and git-commit --amend in version 1.7.6.4
From: Junio C Hamano @ 2011-10-05 22:32 UTC (permalink / raw)
To: Jay Soffian; +Cc: Junio C Hamano, git, nicolas.dichtel
In-Reply-To: <CAG+J_Dysix9fOCuvm5+aU7-AC4wmsxH4-MOX+yhaHEqzeN1cPg@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> On Wed, Oct 5, 2011 at 5:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> I think the sequencer state needs to be removed when the command aborts.
>
> Or written later in do_pick_commit().
>
>> This needs to be fixed before 1.7.7.1.
>
> Something like this?
Does it also refrain from creating sequencer state directory?
> diff --git i/builtin/revert.c w/builtin/revert.c
> index 3117776c2c..f7fcc88871 100644
> --- i/builtin/revert.c
> +++ w/builtin/revert.c
> @@ -384,6 +384,7 @@ static int do_pick_commit(void)
> char *defmsg = NULL;
> struct strbuf msgbuf = STRBUF_INIT;
> int res;
> + int record_cherry_pick_head = 0;
>
> if (no_commit) {
> /*
> @@ -477,7 +478,7 @@ static int do_pick_commit(void)
> strbuf_addstr(&msgbuf, ")\n");
> }
> if (!no_commit)
> - write_cherry_pick_head();
> + record_cherry_pick_head = 1;
> }
>
> if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
> @@ -514,6 +515,9 @@ static int do_pick_commit(void)
> free_message(&msg);
> free(defmsg);
>
> + if (record_cherry_pick_head)
> + write_cherry_pick_head();
> +
> return res;
> }
^ permalink raw reply
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Nguyen Thai Ngoc Duy @ 2011-10-05 22:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jay Soffian, git
In-Reply-To: <7vpqib8jzk.fsf@alter.siamese.dyndns.org>
On Thu, Oct 6, 2011 at 3:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jay Soffian <jaysoffian@gmail.com> writes:
>
>> On Wed, Oct 5, 2011 at 12:02 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>>> Could you please consider a more generic approach? What I have in mind
>>> is a mechanism to "lock" a branch, so that only commands that have the
>>> key can update it.
>>>
>>> So instead of branch.<name>.checkout, I would have something like
>>> branch.<name>.locked = <key>, where <key> is just a string. Only
>>> commands that provide the matching <key> are allowed to update the
>>> branch. In checkout case, <key> could be "checkout: worktree".
>>
>> In this case, each workdir needs its own key, so I'd have to record
>> the key somewhere, unless you meant using a key of "checkout:
>> </path/to/workdir>".
>
> That actually is how I read his message.
That's what I meant.
> I think "switch_branches()" that updates HEAD to point at a local branch
> is one good place to lock the branch, but I do not know if it is a good
> idea to hook the check into the codepaths for deletion of the branch using
> "branch -[dD]" and check-out of the branch using "checkout $branch". I
> wonder if it makes sense to add the "checking" hook into much lower level
> in the callchain, perhaps delete_ref(), rename_ref() and update_ref() to
> catch attempts to update "your" current branch by other people.
I'd aim at low-level ref manipulation because too me it affects more
than just "git checkout".
> For that
> matter, instead of switch_branches(), would it make more sense to add this
> lock/unlock logic to symbolic_ref() that repoints HEAD to other branch?
Couldn't find symbolic_ref() in current code. If you meant
create_symref(), yes that would make sense.
--
Duy
^ permalink raw reply
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Nguyen Thai Ngoc Duy @ 2011-10-05 22:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jay Soffian, git
In-Reply-To: <7vzkhf713u.fsf@alter.siamese.dyndns.org>
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.
--
Duy
^ permalink raw reply
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Junio C Hamano @ 2011-10-05 22:56 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8BHeZZqsOP_+OSPfrPdkYgKQe3LgaGfo3bERD+hWT7U0g@mail.gmail.com>
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"?
^ 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:03 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, nicolas.dichtel
In-Reply-To: <CAG+J_Dysix9fOCuvm5+aU7-AC4wmsxH4-MOX+yhaHEqzeN1cPg@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> On Wed, Oct 5, 2011 at 5:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> I think the sequencer state needs to be removed when the command aborts.
>
> Or written later in do_pick_commit().
As a general direction, I think it makes tons of sense ot delay writing
out these state files before you really commit that the user will be in
the cherry-pick (or revert) sequence.
I am not sure if do_pick_commit() is the best place to do so. Wouldn't it
be necessary to special case the first round at least? The pick can fail
in one of two ways:
- It does not even start. This is the case I illustrated in the earlier
message, and we do not want to leave sequencer state.
- It stops with conflict. At this point, it probably is OK to say that
the user is committed to go with the sequencer flow and the next step
would be to help Git resolve conflicts and proceed, and in this case we
do want the sequencer state.
And once we picked/reverted at least one commit, if there are more, the
user knows the sequencer flow is in progress, and it is perfectly fine to
see the error message from "commit --amend".
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.
^ permalink raw reply
* 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
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