git/builtin/commit.c, branch v2.3.0

Merge branch 'jk/commit-date-approxidate'

2014-12-22T20:28:14Z

Recent update to "git commit" broke amending an existing commit with bogus author/committer lines without a valid e-mail address. * jk/commit-date-approxidate: commit: always populate GIT_AUTHOR_* variables commit: loosen ident checks when generating template

Merge branch 'cc/interpret-trailers-more'

2014-12-22T20:26:24Z

"git interpret-trailers" learned to properly handle the "Conflicts:" block at the end. * cc/interpret-trailers-more: trailer: add test with an old style conflict block trailer: reuse ignore_non_trailer() to ignore conflict lines commit: make ignore_non_trailer() non static merge & sequencer: turn "Conflicts:" hint into a comment builtin/commit.c: extract ignore_non_trailer() helper function merge & sequencer: unify codepaths that write "Conflicts:" hint builtin/merge.c: drop a parameter that is never used

commit: always populate GIT_AUTHOR_* variables

2014-12-11T23:34:37Z

To figure out the author ident for a commit, we call determine_author_info(). This function collects information from the environment, other commits (in the case of "--amend" or "-c/-C"), and the "--author" option. It then uses fmt_ident to generate the final ident string that goes into the commit object. fmt_ident is therefore responsible for any quality or validation checks on what is allowed to go into a commit. Before returning, though, we call split_ident_line on the result, and feed the individual components to hooks via the GIT_AUTHOR_* variables. Furthermore, we do extra validation by feeding the split to sane_ident_split(), which is pickier than fmt_ident (in particular, it will complain about an empty email field). If this parsing or validation fails, we skip updating the environment variables. This is bad, because it means that hooks may silently see a different ident than what we are putting into the commit. We should drop the extra sane_ident_split checks entirely, and take whatever fmt_ident has fed us (and what will go into the commit object). If parsing fails, we should actually abort here rather than continuing (and feeding the hooks bogus data). However, split_ident_line should never fail here. The ident was just generated by fmt_ident, so we know that it's sane. We can use assert_split_ident to double-check this. Note that we also teach that assertion to check that we found a date (it always should, but until now, no caller cared whether we found a date or not). Checking the return value of sane_ident_split is enough to ensure we have the name/email pointers set, and checking date_begin is enough to know that all of the date/tz variables are set. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

commit: loosen ident checks when generating template

2014-12-11T23:34:35Z

When we generate the commit-message template, we try to report an author or committer ident that will be of interest to the user: an author that does not match the committer, or a committer that was auto-configured. When doing so, if we encounter what we consider to be a bogus ident, we immediately die. This is a bad idea, because our use of the idents here is purely informational. Any ident rules should be enforced elsewhere, because commits that do not invoke the editor will not even hit this code path (e.g., "git commit -mfoo" would work, but "git commit" would not). So at best, we are redundant with other checks, and at worse, we actively prevent commits that should otherwise be allowed. We should therefore do the minimal parsing we can to get a value and not do any validation (i.e., drop the call to sane_ident_split()). In theory we could notice when even our minimal parsing fails to work, and do the sane thing for each check (e.g., if we have an author but can't parse the committer, assume they are different and print the author). But we can actually simplify this even further. We know that the author and committer strings we are parsing have been generated by us earlier in the program, and therefore they must be parseable. We could just call split_ident_line without even checking its return value, knowing that it will put _something_ in the name/mail fields. Of course, to protect ourselves against future changes to the code, it makes sense to turn this into an assert, so we are not surprised if our assumption fails. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

commit: make ignore_non_trailer() non static

2014-11-10T17:59:19Z

Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano

Merge branch 'jc/conflict-hint' into cc/interpret-trailers-more

2014-11-10T17:56:39Z

* jc/conflict-hint: merge & sequencer: turn "Conflicts:" hint into a comment builtin/commit.c: extract ignore_non_trailer() helper function merge & sequencer: unify codepaths that write "Conflicts:" hint builtin/merge.c: drop a parameter that is never used git-tag.txt: Add a missing hyphen to `-s`

merge & sequencer: turn "Conflicts:" hint into a comment

2014-10-28T21:04:28Z

Just like other hints such as "Changes to be committed" we show in the editor to remind the committer what paths were involved in the resulting commit to help improving their log message, this section is merely a reminder. Traditionally, it was not made into comments primarily because it has to be generated outside the wt-status infrastructure, and also because it was meant as a bit stronger reminder than the others (i.e. explaining how you resolved conflicts is much more important than mentioning what you did to every paths involved in the commit). But that still does not make this hint a part of the log message proper, and not showing it as a comment is inviting mistakes. Note that we still notice "Conflicts:" followed by list of indented pathnames as an old-style cruft and insert a new Signed-off-by: before it. This is so that "commit --amend -s" adds the new S-o-b at the right place when used on an older commit. Signed-off-by: Junio C Hamano

builtin/commit.c: extract ignore_non_trailer() helper function

2014-10-28T19:44:09Z

Extract a helper function from prepare_to_commit() to determine where to place a new Signed-off-by: line, which is essentially the true "end" of the log message, ignoring the trailing "Conflicts:" line and everything below it. The detection _should_ make sure the "Conflicts:" line it finds is truly the conflict hint block by checking everything that follows is a HT indented pathname to avoid false positive, but this logic will be revamped in a later patch to ignore comments and blanks anyway, so it is left as-is in this step. Signed-off-by: Junio C Hamano

Merge branch 'rs/ref-transaction'

2014-10-21T20:28:10Z

The API to update refs have been restructured to allow introducing a true transactional updates later. We would even allow storing refs in backends other than the traditional filesystem-based one. * rs/ref-transaction: (25 commits) ref_transaction_commit: bail out on failure to remove a ref lockfile: remove unable_to_lock_error refs.c: do not permit err == NULL remote rm/prune: print a message when writing packed-refs fails for-each-ref: skip and warn about broken ref names refs.c: allow listing and deleting badly named refs test: put tests for handling of bad ref names in one place packed-ref cache: forbid dot-components in refnames branch -d: simplify by using RESOLVE_REF_READING branch -d: avoid repeated symref resolution reflog test: test interaction with detached HEAD refs.c: change resolve_ref_unsafe reading argument to be a flags field refs.c: make write_ref_sha1 static fetch.c: change s_update_ref to use a ref transaction refs.c: ref_transaction_commit: distinguish name conflicts from other errors refs.c: pass a list of names to skip to is_refname_available refs.c: call lock_ref_sha1_basic directly from commit refs.c: refuse to lock badly named refs in lock_ref_sha1_basic rename_ref: don't ask read_ref_full where the ref came from refs.c: pass the ref log message to _create/delete/update instead of _commit ...

Merge branch 'jn/parse-config-slot'

2014-10-20T19:23:48Z

Code cleanup. * jn/parse-config-slot: color_parse: do not mention variable name in error message pass config slots as pointers instead of offsets