Git development
 help / color / mirror / Atom feed
* Re: [PATCH] commit: teach --gpg-sign option
From: Michael J Gruber @ 2011-10-09 16:32 UTC (permalink / raw)
  Cc: Git Mailing List, Robin H. Johnson, Junio C Hamano
In-Reply-To: <4E8EBAFE.8020805@drmicha.warpmail.net>

Michael J Gruber venit, vidit, dixit 07.10.2011 10:40:
> [readding JCH to cc whom you dropped]
> Robin H. Johnson venit, vidit, dixit 07.10.2011 00:24:
>> On Wed, Oct 05, 2011 at 05:56:55PM -0700,  Junio C Hamano wrote:
>>> 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(-)
>> I like it, but I have a couple of questions: 
>> 1. Are the sig lines used in computed SHA1/commitid of a given commit (I
>>    see examples w/ --amend and that would usually change the SHA1)?
> 
> Yes, just like with tag objects.
> 
>> 2. Can we allow more than one person sign a commit?
> 
> I don't think we support it now (tags) but we could allow concatenating
> signatures since they are detached.

Quick update:
Sticking two signatures into a signed tag works perfectly with current
git, both signatures are verified and displayed.

So, it might make sense to have "commit --amend" append to an existing
signature.

> There's a somewhat delicate issue here: The signature (tag/commit) is a
> signature on the contents of the object, and is itself not part of the
> contents (or else we would have a chicken-egg-problem).
> 
> The sha1 of the object is determined by the content+header, i.e.
> including the signature.

NB: "header" is the wrong term here, it's "data" I think.

> So, by adding a signature, you change the sha1, but any existing
> signature remains valid.
> 
> This is also how you can try to achieve a specific sha1 for a given
> object content...
> 

^ permalink raw reply

* Re: A Basic Git Question About File Tracking
From: Scott Chacon @ 2011-10-09 16:57 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Jon Forrest, git, Jonathan Nieder
In-Reply-To: <m3ipnz0xri.fsf@localhost.localdomain>

Jon,

2011/10/8 Jakub Narebski <jnareb@gmail.com>:
>> This spoils my understanding of what the index
>> is. I had been thinking that after you add files
>> to the index, and then commit, the index is then
>> empty. In other words, whatever's in the index
>> gets committed, and then the index is cleaned.
>>
>> On the other hand, if the definition of a tracked
>> file is a file that's in the index, then this definitely
>> clears up my understanding of tracked files.
>>
>> If every file that's 'git add'ed stays in the
>> index, how does git know which files to commit?

It may help to read a blog post I put on the Pro Git blog called
"Reset Demystified" that talks about a simplified model of the HEAD,
index and working directory.

http://progit.org/2011/07/11/reset.html

Let me know if that helps.  And you're right, the book should say "not
in the index" rather than "not be in the last commit", that would be
more technically correct. I think at that point in the book I have not
gone into any details about the index yet, so it would be confusing
without more detail.

thanks,
Scott

^ permalink raw reply

* Re: Check revision and if no files modified (looking for svnversion equivalent)
From: Jonathan Nieder @ 2011-10-09 18:10 UTC (permalink / raw)
  To: fkater@googlemail.com; +Cc: git
In-Reply-To: <20111009162902.GA3000@comppasch2>

fkater@googlemail.com wrote:

> - get the last revision (e.g. r1001)
> - check for modified files not yet checked in (e.g. r1001 M)

"git describe --dirty", maybe.  Thanks to Jean Privat for inventing
it.

> Both information is required for the application's build
> process (Makefiles):
>
> - to display the version in the application
> - to refuse to build a proper 'release' version if something
>   was not checked in

Ok, this seems like two different needs.

- For the former, see GIT-VERSION-GEN in git.git for inspiration.
- For the latter, I'd recommend something like

	git update-index -q --refresh &&
	git diff-index --quiet HEAD ||
	{
		echo >&2 fail
		exit 1
	}

Hope that helps,
Jonathan

^ permalink raw reply

* Re: [PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask
From: David Aguilar @ 2011-10-09 18:26 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Junio C Hamano, git@vger.kernel.org, Tanguy Ortolo,
	Charles Bailey, SebastianSchuberth
In-Reply-To: <20111009114333.GA29829@elie.hsd1.il.comcast.net>

On Oct 9, 2011, at 4:43 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:

> Jonathan Nieder wrote:
> 
>> ---
> [...]
>> Since debian/rules install is run as root, the default is for tar to
>> act as thought --preserve-permissions were passed
> 
> I should have said: "when 'make install' is run as root, ...".
> 
> Typically people building git for private use would run "make install"
> as root when installing to /usr/local, but as an unprivileged user
> when installing to $HOME.  The RPM packaging runs "make install"
> without special privileges and the Debian packaging runs it as (fake)
> root, iirc.
> 
> Sorry for the lack of clarity.

thanks. I agree that the tar is overkill. I think I copied that snippet from templates/makefile. does that have the same bug?
-- 
    David (mobile)

^ permalink raw reply

* Re: [PATCH] commit: teach --gpg-sign option
From: Michael J Gruber @ 2011-10-09 20:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaa9f3pk8.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 06.10.2011 02:56:
> 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
> 

I just noticed that this format differs from the one of signed tags.
What special reason is there for the "sig " indentation?

BTW: commit --amend --gpg-sign strips an existing signature rather than
adding one. We might want the user to have a say here.

> 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");
>  }
>  

^ permalink raw reply

* Re: [PATCH 6/6] Retain caches of submodule refs
From: Junio C Hamano @ 2011-10-09 20:10 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt
In-Reply-To: <4E918194.5060102@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> So I think I need help from a submodule guru (Heiko?) who can tell me
> what is done with submodule references and whether they might be
> modified while a git process is executing in the main module.  If so,
> then either this patch has to be withdrawn, or more work has to be put
> in to make such code invalidate the submodule reference cache.
>
> Sorry for the oversight, but I forgot that not all code necessarily uses
> the refs.c API when dealing with references (a regrettable situation, BTW).

In the longer term, I would agree with you that we very much prefer all
the ref accesses to go through the refs API, and I also foresee that the
submodule support would need to become more aware of the status of refs in
checked out submodules. For example, a recursive "submodule update" may
want to inspect the refs in a submodule directory, compare them with the
commit bound in the superproject tree for the submodule path, and decide
to spawn a "fetch && checkout $branch" in there. The same process then may
want to run "status" at the superproject level to show the result, which
in turn would inspect the relationship between the commit bound in the
superproject tree and the commit at the HEAD of the submodule directory.

So let's not rip out the commit but instead give submodule machinery an
explicit way to say "We do not know the current status of the refs in this
submodule anymore".

Thanks.

^ permalink raw reply

* Re: [PATCH] commit: teach --gpg-sign option
From: Junio C Hamano @ 2011-10-09 21:22 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git
In-Reply-To: <4E91FD57.7050808@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> I just noticed that this format differs from the one of signed tags.
> What special reason is there for the "sig " indentation?

Read the part of the message you are quoting.

^ permalink raw reply

* Re: [PATCH 1/3] traverse_trees(): allow pruning with pathspec
From: Nguyen Thai Ngoc Duy @ 2011-10-09 21:35 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Junio C Hamano, git, Linus Torvalds
In-Reply-To: <4E91C031.9030205@alum.mit.edu>

On Mon, Oct 10, 2011 at 2:39 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On 08/29/2011 11:33 PM, Junio C Hamano wrote:
>> diff --git a/tree-walk.c b/tree-walk.c
>> index 33f749e..808bb55 100644
>> --- a/tree-walk.c
>> +++ b/tree-walk.c
>> [...]
>> @@ -376,16 +396,22 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
>>                       mask |= 1ul << i;
>>                       if (S_ISDIR(entry[i].mode))
>>                               dirmask |= 1ul << i;
>> +                     e = &entry[i];
>>               }
>>               if (!mask)
>>                       break;
>> -             ret = info->fn(n, mask, dirmask, entry, info);
>> -             if (ret < 0) {
>> -                     error = ret;
>> -                     if (!info->show_all_errors)
>> -                             break;
>> +             interesting = prune_traversal(e, info, &base, interesting);
>
> According to gcc 4.2.4 (though, strangely, not gcc 4.4.3):
>
> tree-walk.c: In function ‘traverse_trees’:
> tree-walk.c:347: warning: ‘e’ may be used uninitialized in this function

False alarm. If e is not initialized in the for loop, mask would be
zero and therefore prune_traversal(e, info, &base, interesting), which
would use uninitialized "e", would never be called.
-- 
Duy

^ permalink raw reply

* Re: [PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask
From: Jonathan Nieder @ 2011-10-09 21:47 UTC (permalink / raw)
  To: David Aguilar
  Cc: Junio C Hamano, git@vger.kernel.org, Tanguy Ortolo,
	Charles Bailey, SebastianSchuberth
In-Reply-To: <1C9F1683-4C6E-4D49-86D3-3A47B2843F23@gmail.com>

David Aguilar wrote:

> thanks. I agree that the tar is overkill. I think I copied that
> snippet from templates/makefile. does that have the same bug?

It did have a similar bug before, but it was fixed (in a different
way) by v1.6.0.3~81^2 (Fix permission bits on sources checked out with
an overtight umask, 2008-08-21).  The main difference from the
mergetools/ case is that the blt/ directory is populated at build
time.

^ permalink raw reply

* Re: [PATCH] commit: teach --gpg-sign option
From: Junio C Hamano @ 2011-10-09 22:27 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git
In-Reply-To: <4E91FD57.7050808@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> BTW: commit --amend --gpg-sign strips an existing signature rather than
> adding one. We might want the user to have a say here.

I think it deserves a separate command (commit --add-gpg-sign) that is
used _only_ to add an additional signature by another person without
affecting anything else in the commit (i.e. the tree, the parents and the
author and committership information) from the viewpoint of the workflow,

Obviously that "add-signature" mode needs to be aware of the existing
signature. It is a deliberate design decision to strip existing signature
when anything in the commit changes, which is the norm for --amend.

^ permalink raw reply

* Re: [PATCH] commit: teach --gpg-sign option
From: Robin H. Johnson @ 2011-10-09 22:57 UTC (permalink / raw)
  To: Michael J Gruber, Git Mailing List; +Cc: 	Robin H. Johnson, Junio C Hamano
In-Reply-To: <4E8EBAFE.8020805@drmicha.warpmail.net>

On Fri, Oct 07, 2011 at 10:40:30AM +0200,  Michael J Gruber wrote:
> [readding JCH to cc whom you dropped]
> Robin H. Johnson venit, vidit, dixit 07.10.2011 00:24:
> > On Wed, Oct 05, 2011 at 05:56:55PM -0700,  Junio C Hamano wrote:
> >> 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(-)
> > I like it, but I have a couple of questions: 
> > 1. Are the sig lines used in computed SHA1/commitid of a given commit (I
> >    see examples w/ --amend and that would usually change the SHA1)?
> Yes, just like with tag objects.
Ok, at the core, this is going to pose a problem with multiple
signatures.

Workflow example:
1. Dev1 creates a commit, signs it, pushes to central repo.
2. Dev2 pulls, signs the tip commit, pushes it back.

Since signing model here actually alters the commit, the push by Dev2
loses the history point of a commit with only a single signature, like
if somebody pushes a rewritten history (which should usually be
prohibited).

The push certificate variant of signing does permit this case without
breaking history.

> > I think this isn't a replacement for push certificates, but has value in
> > itself. It's certainly provides better integration than the
> > signature-in-note variants.
> > 
> 
> I do think it's meant as an implementation of push certificates. I don't
> see any other value in it which could not be achieved by signed tags.
> Can you describe any?
Identify of the committer for verification.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

^ permalink raw reply

* Re: [PATCH] commit: teach --gpg-sign option
From: Junio C Hamano @ 2011-10-09 23:18 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Michael J Gruber, Git Mailing List, Junio C Hamano
In-Reply-To: <robbat2-20111009T225253-591026811Z@orbis-terrarum.net>

"Robin H. Johnson" <robbat2@gentoo.org> writes:

> Workflow example:
> 1. Dev1 creates a commit, signs it, pushes to central repo.
> 2. Dev2 pulls, signs the tip commit, pushes it back.

I personally am not sympathetic to such a "sign every and all commits by
multiple people" workflow. If you really want to do such a thing, you can
have the second and subsequent one to create a new commit on top whose
sole purpose is to hold such a signature (commit --allow-empty --gpg-sig),
or use signed tags.

^ permalink raw reply

* Re: Recovering Committed Changes in a Detached Head?
From: Junio C Hamano @ 2011-10-09 23:22 UTC (permalink / raw)
  To: Martin Fick; +Cc: SZEDER Gábor, Daly Gutierrez, git
In-Reply-To: <ab706826-75df-4410-941e-6b40ec92713c@email.android.com>

Martin Fick <mfick@codeaurora.org> writes:

> While rflog is cool,...
>
> First, maybe git could create refs for these automatically, perhaps with a name like orphans/1?  Maybe these refs would only be visible via git branch --orphans.

Instead of spelling them orphans/$n, you already have @{$n}.

^ permalink raw reply

* Re: [PATCH v2] Teach merge the '[-e|--edit]' option
From: Junio C Hamano @ 2011-10-09 23:23 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Ramkumar Ramachandra
In-Reply-To: <CAG+J_Dzrk5x0+JRC8EbrAxjZE+hD+-5mp+H=F=M8Su2WosPfmg@mail.gmail.com>

[Adding Ram to the Cc: list as this topic has much to do with resuming a
sequencer-driven workflow, and dropping Todd who does not seem to have
much to say on this topic]

Jay Soffian <jaysoffian@gmail.com> writes:

> I am complaining that git-merge implements commits internally, which
> gives it unique behavior from git-{commit/cherry-pick/revert} (the
> latter two of which just run external git-commit). I'm saying merge is
> fundamentally broken to do it this way. And maybe that's something
> that should be fixed in 2.0 -- that git-merge should just call out to
> git-commit, just like cherry-pick/revert do.

The purpose of the plumbing commands, e.g. commit-tree, is to implement a
unit of logical step at the mechanical level well without enforcing policy
decisions that may not apply to all situations.

On the other hand, the purpose of the Porcelain commands is to support a
concrete workflow element. "git commit" should support what users want to
happen when advancing the history by one commit, "git pull" should support
what users want to happen when integrating work done in another repository
to the history you are currently growing, etc. It is where we should and
do allow users to implement their own policy with hooks and configuration
variables when they want to, and it is even fine if we implemented sane
default policies with ways to override them (e.g. "commit --allow-empty",
"merge --no-ff").

Some Porcelain commands cannot complete their workflow element by
themselves in certain situations without getting help from users, and they
give control back to the user when they need such help. "git rebase", "git
am", "git merge", etc. can and do stop and ask the user to help resolving
conflicts.

The unfortunate historical accident that we may want to correct is that
some of these "we stopped in the middle and asked the user to help before
continuing" situation were presented as if "we stopped and aborted in the
middle, leaving the user to fix up the mess", which is a completely wrong
mental model. "Upon conflicts, 'git merge' stops in the middle, and you
complete it with 'git commit'" is a prime example of this. We even wrongly
label such a situation as "failed merge". It is not failed---it merely is
not auto-completed and waiting to be completed with user's help.

To understand why it is a wrong mental model, you need to imagine a world
where the logic to resolve conflicts in "git merge" is improved so that it
needs less help from the users. rerere.autoupdate is half-way there---the
user allows the merge machinery to take advantage of conflict resolutions
that the user has performed previously. Even though we currently do not
let "git merge" proceed to commit the result, it is entirely plausible to
go one step further and treat the resulting tree from applying the rerere
information as the result of the automerge. When that happens, it is very
natural for the user to expect that the rest of what "git merge" does for
a clean automerge to be carried out. After all, from the end user's point
of view, it _is_ a clean auto-merge. The only difference is how the user
helped the automerge machinery.

The root cause of the inconsistencies you are bringing up (which I agree
are annoying and I further agree that it is a worthy thing to address) is
that even though we tell the users "after helping the 'git merge', you
conclude it with 'git commit'", the concluding 'git commit' does _not_
perform what the user configured 'git merge' to do before a merge is
concluded, unlike a cleanly resolved 'git merge'.

This is merely an unfortunate historical accident. Because "git merge" did
not have any user configurable policy decisions (read: hooks) when this
"conclude with commit" was coded, "conclude with commit" was sufficient to
emulate the case where the merge did not need any help from the user.

But it no longer is true with modern Git.

With more recent changes, e.g. the sequencer work and "git cherry-pick"
that takes multiple commits, "conclude with commit" is becoming less and
less correct thing to say. The workflow elements these commands implement
do have "create a commit" as one essential part, but that is not the only
thing they do. If anything, I think the right way forward is to update the
UI with this rule for consistency:

  Some tools can stop in the middle when they cannot automatically compute
  the outcome, and give control back to the user asking for help. After
  helping these tool, the way to resume what was being done is to invoke
  the tool with the "--continue" option. All user level policy decisions
  implemented by hooks and configurations the tool normally obey when it
  does not need such help from the end user are obeyed when continuing.

I wouldn't mind if that is "invoke 'git continue' command", even though I
suspect that may make the implementation slightly more complex (I haven't
thought things through). "git commit" as a way to conclude a merge that
was stopped in the middle due to a conflict should be deprecated in the
longer term, like say in Git 2.0 someday.


[Footnote]

*1* By the way, "git merge --no-commit" is an oddball. It primarily is
used when the user does _not_ want the resulting commit but wants to
further modify the tree state (e.g. cherry picking a part of what was done
in the side branch). At the philosophical level, the user should be using
merge machinery at the "plumbing" level (e.g. merge-recursive backend),
but the interface to invoke the plumbing level merge machinery is so
arcane (they are after all designed for scripts not for humans) that
nobody does so in practice. And for that purpose, I think it is Ok for the
user to do anything after "git merge --no-commit" finishes (either leaving
conflicts or leaving a cleanly merged state), including "git commit".
Because that "git commit" is very different from the "conclude conflicted
merge with commit" which is a poor substitute for "git merge --continue"
in modern Git, I think it is perfectly fine and even preferable if it does
not obey any "git merge" semantics (i.e. user defined policy that pertains
to "merge" operations).

^ permalink raw reply

* Re: Recovering Committed Changes in a Detached Head?
From: Miles Bader @ 2011-10-10  1:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Martin Fick, SZEDER Gábor, Daly Gutierrez, git
In-Reply-To: <7vfwj1px6h.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:
>> First, maybe git could create refs for these automatically, perhaps
>> with a name like orphans/1?  Maybe these refs would only be visible
>> via git branch --orphans.
>
> Instead of spelling them orphans/$n, you already have @{$n}.

Hmm, shouldn't that be "HEAD@{$n}" (as the reflog output suggests)?

[Well, I dunno, I'm generally kind of confused by the @{...} notation,
but I just tried it out, and just @{$n} seems to refer to the current
branch, which presumably won't include the orphaned bit once one it's
been orphaned...]

Thanks,

-Miles

-- 
`The suburb is an obsolete and contradictory form of human settlement'

^ permalink raw reply

* Re: [PATCH 1/3] traverse_trees(): allow pruning with pathspec
From: Michael Haggerty @ 2011-10-10  4:42 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git, Linus Torvalds
In-Reply-To: <CACsJy8D9sJOtXj_jkVSyoAJ9TC4wmKNAD5YwsFXTYkpvM4e13w@mail.gmail.com>

On 10/09/2011 11:35 PM, Nguyen Thai Ngoc Duy wrote:
> On Mon, Oct 10, 2011 at 2:39 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> On 08/29/2011 11:33 PM, Junio C Hamano wrote:
>>> diff --git a/tree-walk.c b/tree-walk.c
>>> index 33f749e..808bb55 100644
>>> --- a/tree-walk.c
>>> +++ b/tree-walk.c
>>> [...]
>>> @@ -376,16 +396,22 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
>>>                       mask |= 1ul << i;
>>>                       if (S_ISDIR(entry[i].mode))
>>>                               dirmask |= 1ul << i;
>>> +                     e = &entry[i];
>>>               }
>>>               if (!mask)
>>>                       break;
>>> -             ret = info->fn(n, mask, dirmask, entry, info);
>>> -             if (ret < 0) {
>>> -                     error = ret;
>>> -                     if (!info->show_all_errors)
>>> -                             break;
>>> +             interesting = prune_traversal(e, info, &base, interesting);
>>
>> According to gcc 4.2.4 (though, strangely, not gcc 4.4.3):

I checked this a bit more carefully.  gcc 4.2.4 emits a warning when the
-O1 or -O2 optimization levels are used, but not with -O0.  gcc 4.4.3
does not emit a warning regardless of optimization level.

>> tree-walk.c: In function ‘traverse_trees’:
>> tree-walk.c:347: warning: ‘e’ may be used uninitialized in this function
> 
> False alarm. If e is not initialized in the for loop, mask would be
> zero and therefore prune_traversal(e, info, &base, interesting), which
> would use uninitialized "e", would never be called.

That's good to know.  Still, it might be worthwhile to initialize the
variable explicitly to avoid future confusion.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: [PATCH] commit: teach --gpg-sign option
From: Junio C Hamano @ 2011-10-10  4:58 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git
In-Reply-To: <CACBZZX6xsnAv4S8zAqi08bcqrghZ8nKdzFP=UNCqZOqrEeLFnA@mail.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> A nit: no other commit header is an abbreviation, it would be more
> consistent to use "signature" instead of "sig".

You are probably right.

^ permalink raw reply

* Re: [PATCH v2] Teach merge the '[-e|--edit]' option
From: Junio C Hamano @ 2011-10-10  5:29 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian, Ramkumar Ramachandra
In-Reply-To: <7v8votpx4n.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> To understand why it is a wrong mental model, you need to imagine a world
> where the logic to resolve conflicts in "git merge" is improved so that it
> needs less help from the users. rerere.autoupdate is half-way there---the
> user allows the merge machinery to take advantage of conflict resolutions
> that the user has performed previously. Even though we currently do not
> let "git merge" proceed to commit the result, it is entirely plausible to
> go one step further and treat the resulting tree from applying the rerere
> information as the result of the automerge. When that happens, it is very
> natural for the user to expect that the rest of what "git merge" does for
> a clean automerge to be carried out. After all, from the end user's point
> of view, it _is_ a clean auto-merge. The only difference is how the user
> helped the automerge machinery.

Addendum.

I am not suggesting that we should change rerere.autoupdate to go all the
way and record a merge commit by default automatically when rerere applies
cleanly.

I personally think that it is a sensible default to set rerere.autoupdate
to false (or not to set the variable at all) to ensure that a merge that
conflicts is always inspected by the end user, given that rerere is merely
a heuristic (even though it is a damn good one) and produces a surprising
result.

But that is a policy preference; some people want to trust rerere more
than I do and that is a valid choice for them to make. To support such a
policy preference, I am perfectly fine with introducing a third value to
rerere.autoupdate in addition to yes/no to allow commands (e.g. "merge",
"am", etc.) to continue when rerere resolved conflicts cleanly in a
situation where they would have stopped and asked user to help resolving.

By the way, on the other side of this same coin lies another use case
(different from the one in the footnote in the previous message) for
"merge --no-commit". When you know that a particular merge _will_ need
semantic adjustments, even if it were to textually merge cleanly, you
would want the command to ask you for help to come up with the final tree,
instead of trusting the clean automerge result. This often happens when
the topic branch you are about to merge has changed the semantics of an
existing function (e.g. adding a new parameter) while the branch you are
on has added new callsite to the function (or the other way around). In
such a merge, you would need to adjust the new callsite that does not know
about the additional parameter to the new function signature.  For exactly
the same reason, it is not a kosher advice to give to users of modern Git
to "interfere with the merge with 'merge --no-commit', and then conclude
with 'commit'", as 'commit' has less information than 'merge' itself what
'merge' wants to do in addition to recording the result as a 'commit'.

Either the 'commit' command needs to detect that it is conclusing the
merge and trigger the merge hooks the same way as 'merge' itself does,
(which is a bad design, as 'commit' will need to know about the clean-up
operations of all the other commands that may ask users to help and let
'commit' conclude it), or the end user instruction needs to be updated so
that 'merge --continue' is used in such a situation to give 'merge' a
chance to finish up. Again we could have "git continue" wrapper that knows
how to tell what operation was in progress and invokes "merge --continue"
when it detects that it was a 'merge' that was in progress, but that is a
mere fluff.

^ permalink raw reply

* [PATCH 0/2] Provide API to invalidate refs cache
From: Michael Haggerty @ 2011-10-10  5:46 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Michael Haggerty
In-Reply-To: <7v39f2rko5.fsf@alter.siamese.dyndns.org>

I am happy to provide the API; see the following patches.

But I won't have time to figure out who, outside of refs.c, has to
*call* invalidate_cached_refs().  The candidates that I know off the
top of my head are git-clone, git-submodule, and git-pack-refs.  It
would be great if experts in those areas would insert calls to
invalidate_cached_refs() where needed.

Even better would be if the meddlesome code were changed to use the
refs API.  I'd be happy to help expanding the refs API if needed to
accommodate your needs.

Michael Haggerty (2):
  invalidate_cached_refs(): take the submodule as parameter
  invalidate_cached_refs(): expose this function in refs API

 refs.c |   12 ++++--------
 refs.h |    8 ++++++++
 2 files changed, 12 insertions(+), 8 deletions(-)

-- 
1.7.7.rc2

^ permalink raw reply

* [PATCH 1/2] invalidate_cached_refs(): take the submodule as parameter
From: Michael Haggerty @ 2011-10-10  5:46 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Michael Haggerty
In-Reply-To: <1318225574-18785-1-git-send-email-mhagger@alum.mit.edu>

Instead of invalidating the refs cache on an all-or-nothing basis,
allow the cache for individual submodules to be invalidated.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/refs.c b/refs.c
index 2cb93e2..49b73c4 100644
--- a/refs.c
+++ b/refs.c
@@ -223,13 +223,9 @@ static struct cached_refs *get_cached_refs(const char *submodule)
 	return refs;
 }
 
-static void invalidate_cached_refs(void)
+static void invalidate_cached_refs(const char *submodule)
 {
-	struct cached_refs *refs = cached_refs;
-	while (refs) {
-		clear_cached_refs(refs);
-		refs = refs->next;
-	}
+	clear_cached_refs(get_cached_refs(submodule));
 }
 
 static struct ref_list *read_packed_refs(FILE *f)
@@ -1212,7 +1208,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 	ret |= repack_without_ref(refname);
 
 	unlink_or_warn(git_path("logs/%s", lock->ref_name));
-	invalidate_cached_refs();
+	invalidate_cached_refs(NULL);
 	unlock_ref(lock);
 	return ret;
 }
@@ -1511,7 +1507,7 @@ int write_ref_sha1(struct ref_lock *lock,
 		unlock_ref(lock);
 		return -1;
 	}
-	invalidate_cached_refs();
+	invalidate_cached_refs(NULL);
 	if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
 	    (strcmp(lock->ref_name, lock->orig_ref_name) &&
 	     log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
-- 
1.7.7.rc2

^ permalink raw reply related

* [PATCH 2/2] invalidate_cached_refs(): expose this function in refs API
From: Michael Haggerty @ 2011-10-10  5:46 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Michael Haggerty
In-Reply-To: <1318225574-18785-1-git-send-email-mhagger@alum.mit.edu>

Make invalidate_cached_refs() an official part of the refs API.  It is
currently a fact of life that code outside of refs.c mucks about with
references.  This change gives such code a way of informing the refs
module that it should no longer trust its cache.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |    2 +-
 refs.h |    8 ++++++++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index 49b73c4..0483ecc 100644
--- a/refs.c
+++ b/refs.c
@@ -223,7 +223,7 @@ static struct cached_refs *get_cached_refs(const char *submodule)
 	return refs;
 }
 
-static void invalidate_cached_refs(const char *submodule)
+void invalidate_cached_refs(const char *submodule)
 {
 	clear_cached_refs(get_cached_refs(submodule));
 }
diff --git a/refs.h b/refs.h
index 5de06e5..63dc68c 100644
--- a/refs.h
+++ b/refs.h
@@ -77,6 +77,14 @@ extern void unlock_ref(struct ref_lock *lock);
 /** Writes sha1 into the ref specified by the lock. **/
 extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg);
 
+/*
+ * Invalidate the reference cache for the specified submodule.  Use
+ * submodule=NULL to invalidate the cache for the main module.  This
+ * function must be called if references are changed via a mechanism
+ * other than the refs API.
+ */
+extern void invalidate_cached_refs(const char *submodule);
+
 /** Setup reflog before using. **/
 int log_ref_setup(const char *ref_name, char *logfile, int bufsize);
 
-- 
1.7.7.rc2

^ permalink raw reply related

* Re: [PATCH] commit: teach --gpg-sign option
From: Michael J Gruber @ 2011-10-10  6:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8votrhbr.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 09.10.2011 23:22:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> I just noticed that this format differs from the one of signed
>> tags. What special reason is there for the "sig " indentation?
> 
> Read the part of the message you are quoting.

I certainly did, and certainly did not find any mention. Do you think I
would have asked otherwise? I'm trying to be helpful by testing out a
patch in flight. That is: *I* am trying to be helpful.

This
> 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:
gave me the impression that commit signatures are done "similar to how
signed tag is done".

*After* doing several "cat-file" and after your insisting that you had
described the "sig " indent I come to the conclusion that you
implemented it this way "instead of... [doing it] similar to how signed
tag is done".

Before that, I misread those paragraphs (togetheter with the
non-existing object format doc) to mean that have a section in the
object which is ignored automatically, which is where tag signatures are
(while in fact they are not) and where commit signatures will go.

I have to say I dislike the fact that we would have different signature
formats. But I have spent too much time on this unnecessarily already.

Michael

^ permalink raw reply

* Re: [PATCH] commit: teach --gpg-sign option
From: Michael J Gruber @ 2011-10-10  6:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4nzhrebp.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 10.10.2011 00:27:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> BTW: commit --amend --gpg-sign strips an existing signature rather than
>> adding one. We might want the user to have a say here.
> 
> I think it deserves a separate command (commit --add-gpg-sign) that is
> used _only_ to add an additional signature by another person without
> affecting anything else in the commit (i.e. the tree, the parents and the
> author and committership information) from the viewpoint of the workflow,

Agreed, as I asked "the user to have a say".

> Obviously that "add-signature" mode needs to be aware of the existing
> signature. It is a deliberate design decision to strip existing signature
> when anything in the commit changes, which is the norm for --amend.

What norm? --amend keeps some header fields and discards others. In
fact, signing a commit "without changing it" (i.e. keeping tree, parents
etc., alias "--amend -C HEAD") should be the normal use case for signing
the tip of an existing branch. I mean, I have no problems adding to this:

git help fixup
`git fixup' is aliased to `commit --amend -C HEAD'

But what is the best default for the workflows that we encourage (commit
early, ...)? You answer a pull-request which happens to be a
fast-forward, sign the tip and suddenly you've taken over ownership (and
changed dates)??? Signing a commit should not do this.

Michael

^ permalink raw reply

* Git bug. gitattributes' pattern does not respect spaces in the filenames
From: Alexey Shumkin @ 2011-10-10  7:02 UTC (permalink / raw)
  To: git

Hello everyone!

There's a description for the understanding of a
situation.
I have a project on Windows. I use Git under Cygwin.
There are some *.xml in the project. But some of them are in cp1251
encoding, another ones are in UTF-8. For the first ones there is no
need of any conversion to see the git-diff, but for the *.xml's in UTF-8
I set

*.xml diff=utf8-to-cp1251

And according to this I have
$ git config diff.utf8-to-cp1251.textconv 'iconv -f utf-8 -t cp1251'

Unfortunately, *.xml's in cp1251 DOES match this pattern, too.
As far as cp1251 and UTF-8 files are in different folders,
it is logically enough to set pattern like

<folder with a UTF-8-xmls>/*.xml diff=utf8-to-cp1251

for the UTF-8 files.
BUT!
Unfortunately, <folder with a UTF-8-xmls> have spaces in its name,
so textconv filter does not work because of error of
parsing .gitattributes

I have no enough skills to patch Git to fix this error.
Is anybody interested in to do?

Thanks

^ permalink raw reply

* Re: [PATCH v2] Teach merge the '[-e|--edit]' option
From: Jakub Narebski @ 2011-10-10  7:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jay Soffian, Ramkumar Ramachandra
In-Reply-To: <7vr52lo1m3.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> By the way, on the other side of this same coin lies another use case
> (different from the one in the footnote in the previous message) for
> "merge --no-commit". When you know that a particular merge _will_ need
> semantic adjustments, even if it were to textually merge cleanly, you
> would want the command to ask you for help to come up with the final tree,
> instead of trusting the clean automerge result. This often happens when
> the topic branch you are about to merge has changed the semantics of an
> existing function (e.g. adding a new parameter) while the branch you are
> on has added new callsite to the function (or the other way around). In
> such a merge, you would need to adjust the new callsite that does not know
> about the additional parameter to the new function signature.  For exactly
> the same reason, it is not a kosher advice to give to users of modern Git
> to "interfere with the merge with 'merge --no-commit', and then conclude
> with 'commit'", as 'commit' has less information than 'merge' itself what
> 'merge' wants to do in addition to recording the result as a 'commit'.

Yet another issue is if we should blindly trust automatic merge resolution.
It is considered a good practice by some to always check (e.g. by compiling
and possibly also running tests) the result of merge, whether it required
merge conflict resolution or not.

IIRC Linus lately said that making "git merge" automatically commit
was one of bad design decisions of git, for the above reason...

-- 
Jakub Narębski

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox