aboutsummaryrefslogtreecommitdiffstats
path: root/builtin/commit.c
AgeCommit message (Collapse)AuthorFilesLines
2017-11-06Merge branch 'ma/lockfile-fixes'Junio C Hamano1-5/+5
An earlier update made it possible to use an on-stack in-core lockfile structure (as opposed to having to deliberately leak an on-heap one). Many codepaths have been updated to take advantage of this new facility. * ma/lockfile-fixes: read_cache: roll back lock in `update_index_if_able()` read-cache: leave lock in right state in `write_locked_index()` read-cache: drop explicit `CLOSE_LOCK`-flag cache.h: document `write_locked_index()` apply: remove `newfd` from `struct apply_state` apply: move lockfile into `apply_state` cache-tree: simplify locking logic checkout-index: simplify locking logic tempfile: fix documentation on `delete_tempfile()` lockfile: fix documentation on `close_lock_file_gently()` treewide: prefer lockfiles on the stack sha1_file: do not leak `lock_file`
2017-10-07read-cache: drop explicit `CLOSE_LOCK`-flagMartin Ågren1-5/+5
`write_locked_index()` takes two flags: `COMMIT_LOCK` and `CLOSE_LOCK`. At most one is allowed. But it is also possible to use no flag, i.e., `0`. But when `write_locked_index()` calls `do_write_index()`, the temporary file, a.k.a. the lockfile, will be closed. So passing `0` is effectively the same as `CLOSE_LOCK`, which seems like a bug. We might feel tempted to restructure the code in order to close the file later, or conditionally. It also feels a bit unfortunate that we simply "happen" to close the lock by way of an implementation detail of lockfiles. But note that we need to close the temporary file before `stat`-ing it, at least on Windows. See 9f41c7a6b (read-cache: close index.lock in do_write_index, 2017-04-26). Drop `CLOSE_LOCK` and make it explicit that `write_locked_index()` always closes the lock. Whether it is also committed is governed by the remaining flag, `COMMIT_LOCK`. This means we neither have nor suggest that we have a mode to write the index and leave the file open. Whatever extra contents we might eventually want to write, we should probably write it from within `write_locked_index()` itself anyway. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-03Merge branch 'jk/no-optional-locks'Junio C Hamano1-1/+4
Some commands (most notably "git status") makes an opportunistic update when performing a read-only operation to help optimize later operations in the same repository. The new "--no-optional-locks" option can be passed to Git to disable them. * jk/no-optional-locks: git: add --no-optional-locks option
2017-09-29Merge branch 'ma/leakplugs'Junio C Hamano1-5/+10
Memory leaks in various codepaths have been plugged. * ma/leakplugs: pack-bitmap[-write]: use `object_array_clear()`, don't leak object_array: add and use `object_array_pop()` object_array: use `object_array_clear()`, not `free()` leak_pending: use `object_array_clear()`, not `free()` commit: fix memory leak in `reduce_heads()` builtin/commit: fix memory leak in `prepare_index()`
2017-09-27git: add --no-optional-locks optionJeff King1-1/+4
Some tools like IDEs or fancy editors may periodically run commands like "git status" in the background to keep track of the state of the repository. Some of these commands may refresh the index and write out the result in an opportunistic way: if they can get the index lock, then they update the on-disk index with any updates they find. And if not, then their in-core refresh is lost and just has to be recomputed by the next caller. But taking the index lock may conflict with other operations in the repository. Especially ones that the user is doing themselves, which _aren't_ opportunistic. In other words, "git status" knows how to back off when somebody else is holding the lock, but other commands don't know that status would be happy to drop the lock if somebody else wanted it. There are a couple possible solutions: 1. Have some kind of "pseudo-lock" that allows other commands to tell status that they want the lock. This is likely to be complicated and error-prone to implement (and maybe even impossible with just dotlocks to work from, as it requires some inter-process communication). 2. Avoid background runs of commands like "git status" that want to do opportunistic updates, preferring instead plumbing like diff-files, etc. This is awkward for a couple of reasons. One is that "status --porcelain" reports a lot more about the repository state than is available from individual plumbing commands. And two is that we actually _do_ want to see the refreshed index. We just don't want to take a lock or write out the result. Whereas commands like diff-files expect us to refresh the index separately and write it to disk so that they can depend on the result. But that write is exactly what we're trying to avoid. 3. Ask "status" not to lock or write the index. This is easy to implement. The big downside is that any work done in refreshing the index for such a call is lost when the process exits. So a background process may end up re-hashing a changed file multiple times until the user runs a command that does an index refresh themselves. This patch implements the option 3. The idea (and the test) is largely stolen from a Git for Windows patch by Johannes Schindelin, 67e5ce7f63 (status: offer *not* to lock the index and update it, 2016-08-12). The twist here is that instead of making this an option to "git status", it becomes a "git" option and matching environment variable. The reason there is two-fold: 1. An environment variable is carried through to sub-processes. And whether an invocation is a background process or not should apply to the whole process tree. So you could do "git --no-optional-locks foo", and if "foo" is a script or alias that calls "status", you'll still get the effect. 2. There may be other programs that want the same treatment. I've punted here on finding more callers to convert, since "status" is the obvious one to call as a repeated background job. But "git diff"'s opportunistic refresh of the index may be a good candidate. The test is taken from 67e5ce7f63, and it's worth repeating Johannes's explanation: Note that the regression test added in this commit does not *really* verify that no index.lock file was written; that test is not possible in a portable way. Instead, we verify that .git/index is rewritten *only* when `git status` is run without `--no-optional-locks`. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-24refs: pass NULL to resolve_ref_unsafe() if hash is not neededRené Scharfe1-2/+1
This allows us to get rid of some write-only variables, among them seven SHA1 buffers. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-24builtin/commit: fix memory leak in `prepare_index()`Martin Ågren1-5/+10
Release `pathspec` and the string list `partial`. When we clear the string list, make sure we do not free the `util` pointers. That would result in double-freeing, since we set them up as `item->util = item` in `list_paths()`. Initialize the string list early, so that we can always release it. That introduces some unnecessary overhead in various code paths, but means there is one and only one way out of the function. If we ever accumulate more things we need to free, it should be straightforward to do so. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-08add UNLEAK annotation for reducing leak false positivesJeff King1-1/+2
It's a common pattern in git commands to allocate some memory that should last for the lifetime of the program and then not bother to free it, relying on the OS to throw it away. This keeps the code simple, and it's fast (we don't waste time traversing structures or calling free at the end of the program). But it also triggers warnings from memory-leak checkers like valgrind or LSAN. They know that the memory was still allocated at program exit, but they don't know _when_ the leaked memory stopped being useful. If it was early in the program, then it's probably a real and important leak. But if it was used right up until program exit, it's not an interesting leak and we'd like to suppress it so that we can see the real leaks. This patch introduces an UNLEAK() macro that lets us do so. To understand its design, let's first look at some of the alternatives. Unfortunately the suppression systems offered by leak-checking tools don't quite do what we want. A leak-checker basically knows two things: 1. Which blocks were allocated via malloc, and the callstack during the allocation. 2. Which blocks were left un-freed at the end of the program (and which are unreachable, but more on that later). Their suppressions work by mentioning the function or callstack of a particular allocation, and marking it as OK to leak. So imagine you have code like this: int cmd_foo(...) { /* this allocates some memory */ char *p = some_function(); printf("%s", p); return 0; } You can say "ignore allocations from some_function(), they're not leaks". But that's not right. That function may be called elsewhere, too, and we would potentially want to know about those leaks. So you can say "ignore the callstack when main calls some_function". That works, but your annotations are brittle. In this case it's only two functions, but you can imagine that the actual allocation is much deeper. If any of the intermediate code changes, you have to update the suppression. What we _really_ want to say is that "the value assigned to p at the end of the function is not a real leak". But leak-checkers can't understand that; they don't know about "p" in the first place. However, we can do something a little bit tricky if we make some assumptions about how leak-checkers work. They generally don't just report all un-freed blocks. That would report even globals which are still accessible when the leak-check is run. Instead they take some set of memory (like BSS) as a root and mark it as "reachable". Then they scan the reachable blocks for anything that looks like a pointer to a malloc'd block, and consider that block reachable. And then they scan those blocks, and so on, transitively marking anything reachable from a global as "not leaked" (or at least leaked in a different category). So we can mark the value of "p" as reachable by putting it into a variable with program lifetime. One way to do that is to just mark "p" as static. But that actually affects the run-time behavior if the function is called twice (you aren't likely to call main() twice, but some of our cmd_*() functions are called from other commands). Instead, we can trick the leak-checker by putting the value into _any_ reachable bytes. This patch keeps a global linked-list of bytes copied from "unleaked" variables. That list is reachable even at program exit, which confers recursive reachability on whatever values we unleak. In other words, you can do: int cmd_foo(...) { char *p = some_function(); printf("%s", p); UNLEAK(p); return 0; } to annotate "p" and suppress the leak report. But wait, couldn't we just say "free(p)"? In this toy example, yes. But UNLEAK()'s byte-copying strategy has several advantages over actually freeing the memory: 1. It's recursive across structures. In many cases our "p" is not just a pointer, but a complex struct whose fields may have been allocated by a sub-function. And in some cases (e.g., dir_struct) we don't even have a function which knows how to free all of the struct members. By marking the struct itself as reachable, that confers reachability on any pointers it contains (including those found in embedded structs, or reachable by walking heap blocks recursively. 2. It works on cases where we're not sure if the value is allocated or not. For example: char *p = argc > 1 ? argv[1] : some_function(); It's safe to use UNLEAK(p) here, because it's not freeing any memory. In the case that we're pointing to argv here, the reachability checker will just ignore our bytes. 3. Likewise, it works even if the variable has _already_ been freed. We're just copying the pointer bytes. If the block has been freed, the leak-checker will skip over those bytes as uninteresting. 4. Because it's not actually freeing memory, you can UNLEAK() before we are finished accessing the variable. This is helpful in cases like this: char *p = some_function(); return another_function(p); Writing this with free() requires: int ret; char *p = some_function(); ret = another_function(p); free(p); return ret; But with unleak we can just write: char *p = some_function(); UNLEAK(p); return another_function(p); This patch adds the UNLEAK() macro and enables it automatically when Git is compiled with SANITIZE=leak. In normal builds it's a noop, so we pay no runtime cost. It also adds some UNLEAK() annotations to show off how the feature works. On top of other recent leak fixes, these are enough to get t0000 and t0001 to pass when compiled with LSAN. Note the case in commit.c which actually converts a strbuf_release() into an UNLEAK. This code was already non-leaky, but the free didn't do anything useful, since we're exiting. Converting it to an annotation means that non-leak-checking builds pay no runtime cost. The cost is minimal enough that it's probably not worth going on a crusade to convert these kinds of frees to UNLEAKS. I did it here for consistency with the "sb" leak (though it would have been equally correct to go the other way, and turn them both into strbuf_release() calls). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-26Merge branch 'bw/submodule-config-cleanup'Junio C Hamano1-1/+0
Code clean-up to avoid mixing values read from the .gitmodules file and values read from the .git/config file. * bw/submodule-config-cleanup: submodule: remove gitmodules_config unpack-trees: improve loading of .gitmodules submodule-config: lazy-load a repository's .gitmodules file submodule-config: move submodule-config functions to submodule-config.c submodule-config: remove support for overlaying repository config diff: stop allowing diff to have submodules configured in .git/config submodule: remove submodule_config callback routine unpack-trees: don't respect submodule.update submodule: don't rely on overlayed config when setting diffopts fetch: don't overlay config with submodule-config submodule--helper: don't overlay config in update-clone submodule--helper: don't overlay config in remote_submodule_branch add, reset: ensure submodules can be added or reset submodule: don't use submodule_from_name t7411: check configuration parsing errors
2017-08-23Merge branch 'ks/commit-abort-on-empty-message-fix' into maintJunio C Hamano1-5/+5
"git commit" when seeing an totally empty message said "you did not edit the message", which is clearly wrong. The message has been corrected. * ks/commit-abort-on-empty-message-fix: commit: check for empty message before the check for untouched template
2017-08-23Merge branch 'kw/commit-keep-index-when-pre-commit-is-not-run'Junio C Hamano1-6/+9
"git commit" used to discard the index and re-read from the filesystem just in case the pre-commit hook has updated it in the middle; this has been optimized out when we know we do not run the pre-commit hook. * kw/commit-keep-index-when-pre-commit-is-not-run: commit: skip discarding the index if there is no pre-commit hook
2017-08-16commit: skip discarding the index if there is no pre-commit hookKevin Willford1-6/+9
If there is not a pre-commit hook, there is no reason to discard the index and reread it. This change checks to presence of a pre-commit hook and then only discards the index if there was one. Signed-off-by: Kevin Willford <kewillf@microsoft.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-11Merge branch 'ks/commit-abort-on-empty-message-fix'Junio C Hamano1-5/+5
"git commit" when seeing an totally empty message said "you did not edit the message", which is clearly wrong. The message has been corrected. * ks/commit-abort-on-empty-message-fix: commit: check for empty message before the check for untouched template
2017-08-03submodule: remove gitmodules_configBrandon Williams1-1/+0
Now that the submodule-config subsystem can lazily read the gitmodules file we no longer need to explicitly pre-read the gitmodules by calling 'gitmodules_config()' so let's remove it. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-17commit: check for empty message before the check for untouched templateKaartic Sivaraam1-5/+5
The check for whether the template given to 'git commit' is untouched is done before the empty message check. This results in a wrong error message being displayed in the following case. When the user removes everything in template completely to abort the commit he is shown the "template untouched" error which is wrong. He should be shown the "empty message" error. Do the empty message check before checking for an untouched template thus fixing this issue. Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-17sha1_name: convert get_sha1* to get_oid*brian m. carlson1-4/+4
Now that all the callers of get_sha1 directly or indirectly use struct object_id, rename the functions starting with get_sha1 to start with get_oid. Convert the internals in sha1_name.c to use struct object_id as well, and eliminate explicit length checks where possible. Convert a use of 40 in get_oid_basic to GIT_SHA1_HEXSZ. Outside of sha1_name.c and cache.h, this transition was made with the following semantic patch: @@ expression E1, E2; @@ - get_sha1(E1, E2.hash) + get_oid(E1, &E2) @@ expression E1, E2; @@ - get_sha1(E1, E2->hash) + get_oid(E1, E2) @@ expression E1, E2; @@ - get_sha1_committish(E1, E2.hash) + get_oid_committish(E1, &E2) @@ expression E1, E2; @@ - get_sha1_committish(E1, E2->hash) + get_oid_committish(E1, E2) @@ expression E1, E2; @@ - get_sha1_treeish(E1, E2.hash) + get_oid_treeish(E1, &E2) @@ expression E1, E2; @@ - get_sha1_treeish(E1, E2->hash) + get_oid_treeish(E1, E2) @@ expression E1, E2; @@ - get_sha1_commit(E1, E2.hash) + get_oid_commit(E1, &E2) @@ expression E1, E2; @@ - get_sha1_commit(E1, E2->hash) + get_oid_commit(E1, E2) @@ expression E1, E2; @@ - get_sha1_tree(E1, E2.hash) + get_oid_tree(E1, &E2) @@ expression E1, E2; @@ - get_sha1_tree(E1, E2->hash) + get_oid_tree(E1, E2) @@ expression E1, E2; @@ - get_sha1_blob(E1, E2.hash) + get_oid_blob(E1, &E2) @@ expression E1, E2; @@ - get_sha1_blob(E1, E2->hash) + get_oid_blob(E1, E2) @@ expression E1, E2, E3, E4; @@ - get_sha1_with_context(E1, E2, E3.hash, E4) + get_oid_with_context(E1, E2, &E3, E4) @@ expression E1, E2, E3, E4; @@ - get_sha1_with_context(E1, E2, E3->hash, E4) + get_oid_with_context(E1, E2, E3, E4) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-10Merge branch 'ks/typofix-commit-c-comment'Junio C Hamano1-1/+1
Typofix. * ks/typofix-commit-c-comment: builtin/commit.c: fix a typo in the comment
2017-07-10Merge branch 'ks/commit-assuming-only-warning-removal'Junio C Hamano1-8/+1
An old message shown in the commit log template was removed, as it has outlived its usefulness. * ks/commit-assuming-only-warning-removal: commit-template: distinguish status information unconditionally commit-template: remove outdated notice about explicit paths
2017-07-06builtin/commit.c: fix a typo in the commentKaartic Sivaraam1-1/+1
Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-30Merge branch 'ks/status-initial-commit'Junio C Hamano1-0/+1
"git status" has long shown essentially the same message as "git commit"; the message it gives while preparing for the root commit, i.e. "Initial commit", was hard to understand for some new users. Now it says "No commits yet" to stress more on the current status (rather than the commit the user is preparing for, which is more in line with the focus of "git commit"). * ks/status-initial-commit: status: contextually notify user about an initial commit
2017-06-30commit-template: distinguish status information unconditionallyKaartic Sivaraam1-2/+1
The commit template adds the status information without adding a new line to distinguish them in the absence of optional parts. This results in difficulty in interpreting it's content, specifically for inexperienced users. Unconditionally, add new lines to separate the status message from the other parts of the commit-template to make it more readable. Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-30commit-template: remove outdated notice about explicit pathsKaartic Sivaraam1-6/+0
The notice that "git commit <paths>" default to "git commit --only <paths>" was there since 756e3ee0 ("Merge branch 'jc/commit'", 2006-02-14). Back then, existing users of Git expected the command doing "git commit --include <paths>", and after the behaviour of the command was changed to align with other people's "$scm commit <paths>", the text was added to help them transition their expectations. Remove the message that now has outlived its usefulness. Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-26Merge branch 'lb/status-stash-count'Junio C Hamano1-0/+6
"git status" learned to optionally give how many stash entries the user has in its output. * lb/status-stash-count: glossary: define 'stash entry' status: add optional stash count information stash: update documentation to use 'stash entry'
2017-06-24Merge branch 'bw/config-h'Junio C Hamano1-0/+1
Fix configuration codepath to pay proper attention to commondir that is used in multi-worktree situation, and isolate config API into its own header file. * bw/config-h: config: don't implicitly use gitdir or commondir config: respect commondir setup: teach discover_git_directory to respect the commondir config: don't include config.h by default config: remove git_config_iter config: create config.h
2017-06-24Merge branch 'bw/ls-files-sans-the-index'Junio C Hamano1-1/+2
Code clean-up. * bw/ls-files-sans-the-index: ls-files: factor out tag calculation ls-files: factor out debug info into a function ls-files: convert show_files to take an index ls-files: convert show_ce_entry to take an index ls-files: convert prune_cache to take an index ls-files: convert ce_excluded to take an index ls-files: convert show_ru_info to take an index ls-files: convert show_other_files to take an index ls-files: convert show_killed_files to take an index ls-files: convert write_eolinfo to take an index ls-files: convert overlay_tree_on_cache to take an index tree: convert read_tree to take an index parameter convert: convert renormalize_buffer to take an index convert: convert convert_to_git to take an index convert: convert convert_to_git_filter_fd to take an index convert: convert crlf_to_git to take an index convert: convert get_cached_convert_stats_ascii to take an index
2017-06-21status: contextually notify user about an initial commitKaartic Sivaraam1-0/+1
The existing message, "Initial commit", makes sense for the commit template notifying users that it's their initial commit, but is confusing when merely checking the status of a fresh repository (or orphan branch) without having any commits yet. Change the output of "status" to say "No commits yet" when "git status" is run on a fresh repo (or orphan branch), while retaining the current "Initial commit" message displayed in the template that's displayed in the editor when the initial commit is being authored. Correspondingly change the output of "short status" to "No commits yet on " when "git status -sb" is run on a fresh repo (or orphan branch). A few alternatives considered were, * Waiting for initial commit * Your current branch does not have any commits * Current branch waiting for initial commit The most succint one among the alternatives was chosen. [with help on tests from Ævar] Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-19Merge branch 'bw/object-id'Junio C Hamano1-1/+1
Conversion from uchar[20] to struct object_id continues. * bw/object-id: (33 commits) diff: rename diff_fill_sha1_info to diff_fill_oid_info diffcore-rename: use is_empty_blob_oid tree-diff: convert path_appendnew to object_id tree-diff: convert diff_tree_paths to struct object_id tree-diff: convert try_to_follow_renames to struct object_id builtin/diff-tree: cleanup references to sha1 diff-tree: convert diff_tree_sha1 to struct object_id notes-merge: convert write_note_to_worktree to struct object_id notes-merge: convert verify_notes_filepair to struct object_id notes-merge: convert find_notes_merge_pair_ps to struct object_id notes-merge: convert merge_from_diffs to struct object_id notes-merge: convert notes_merge* to struct object_id tree-diff: convert diff_root_tree_sha1 to struct object_id combine-diff: convert find_paths_* to struct object_id combine-diff: convert diff_tree_combined to struct object_id diff: convert diff_flush_patch_id to struct object_id patch-ids: convert to struct object_id diff: finish conversion for prepare_temp_file to struct object_id diff: convert reuse_worktree_file to struct object_id diff: convert fill_filespec to struct object_id ...
2017-06-18status: add optional stash count informationLiam Beguin1-0/+6
Introduce '--show-stash' and its configuration option 'status.showStash' to allow git-status to show information about currently stashed entries. Signed-off-by: Liam Beguin <liambeguin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15config: don't include config.h by defaultBrandon Williams1-0/+1
Stop including config.h by default in cache.h. Instead only include config.h in those files which require use of the config system. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-13Merge branch 'nd/fopen-errors'Junio C Hamano1-4/+1
We often try to open a file for reading whose existence is optional, and silently ignore errors from open/fopen; report such errors if they are not due to missing files. * nd/fopen-errors: mingw_fopen: report ENOENT for invalid file names mingw: verify that paths are not mistaken for remote nicknames log: fix memory leak in open_next_file() rerere.c: move error_errno() closer to the source system call print errno when reporting a system call error wrapper.c: make warn_on_inaccessible() static wrapper.c: add and use fopen_or_warn() wrapper.c: add and use warn_on_fopen_errors() config.mak.uname: set FREAD_READS_DIRECTORIES for Darwin, too config.mak.uname: set FREAD_READS_DIRECTORIES for Linux and FreeBSD clone: use xfopen() instead of fopen() use xfopen() in more places git_fopen: fix a sparse 'not declared' warning
2017-06-13ls-files: convert overlay_tree_on_cache to take an indexBrandon Williams1-1/+2
Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-02builtin/notes: convert to struct object_idbrian m. carlson1-1/+1
Convert most of the static functions to use struct object_id. In addition, convert copy_notes_for_rewrite and its callers. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-29Merge branch 'bm/interpret-trailers-cut-line-is-eom'Junio C Hamano1-1/+1
"git interpret-trailers", when used as GIT_EDITOR for "git commit -v", looked for and appended to a trailer block at the very end, i.e. at the end of the "diff" output. The command has been corrected to pay attention to the cut-mark line "commit -v" adds to the buffer---the real trailer block should appear just before it. * bm/interpret-trailers-cut-line-is-eom: interpret-trailers: honor the cut line
2017-05-29Merge branch 'bc/object-id'Junio C Hamano1-4/+4
Conversion from uchar[20] to struct object_id continues. * bc/object-id: (53 commits) object: convert parse_object* to take struct object_id tree: convert parse_tree_indirect to struct object_id sequencer: convert do_recursive_merge to struct object_id diff-lib: convert do_diff_cache to struct object_id builtin/ls-tree: convert to struct object_id merge: convert checkout_fast_forward to struct object_id sequencer: convert fast_forward_to to struct object_id builtin/ls-files: convert overlay_tree_on_cache to object_id builtin/read-tree: convert to struct object_id sha1_name: convert internals of peel_onion to object_id upload-pack: convert remaining parse_object callers to object_id revision: convert remaining parse_object callers to object_id revision: rename add_pending_sha1 to add_pending_oid http-push: convert process_ls_object and descendants to object_id refs/files-backend: convert many internals to struct object_id refs: convert struct ref_update to use struct object_id ref-filter: convert some static functions to struct object_id Convert struct ref_array_item to struct object_id Convert the verify_pack callback to struct object_id Convert lookup_tag to struct object_id ...
2017-05-26use xfopen() in more placesNguyễn Thái Ngọc Duy1-4/+1
xfopen() - provides error details - explains error on reading, or writing, or whatever operation - has l10n support - prints file name in the error Some of these are missing in the places that are replaced with xfopen(), which is a clear win. In some other places, it's just less code (not as clearly a win as the previous case but still is). The only slight regresssion is in remote-testsvn, where we don't report the file class (marks files) in the error messages anymore. But since this is a _test_ svn remote transport, I'm not too concerned. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-18interpret-trailers: honor the cut lineBrian Malehorn1-1/+1
If a commit message is edited with the "verbose" option, the buffer will have a cut line and diff after the log message, like so: my subject # ------------------------ >8 ------------------------ # Do not touch the line above. # Everything below will be removed. diff --git a/foo.txt b/foo.txt index 5716ca5..7601807 100644 --- a/foo.txt +++ b/foo.txt @@ -1 +1 @@ -bar +baz "git interpret-trailers" is unaware of the cut line, and assumes the trailer block would be at the end of the whole thing. This can easily be seen with: $ GIT_EDITOR='git interpret-trailers --in-place --trailer Acked-by:me' \ git commit --amend -v Teach "git interpret-trailers" to notice the cut-line and ignore the remainder of the input when looking for a place to add new trailer block. This makes it consistent with how "git commit -v -s" inserts a new Signed-off-by: line. This can be done by the same logic as the existing helper function, wt_status_truncate_message_at_cut_line(), uses, but it wants the caller to pass a strbuf to it. Because the function ignore_non_trailer() used by the command takes a <pointer, length> pair, not a strbuf, steal the logic from wt_status_truncate_message_at_cut_line() to create a new wt_status_locate_end() helper function that takes <pointer, length> pair, and make ignore_non_trailer() call it to help "interpret-trailers". Since there is only one caller of wt_status_truncate_message_at_cut_line() in cmd_commit(), rewrite it to call wt_status_locate_end() helper instead and remove the old helper that no longer has any caller. Signed-off-by: Brian Malehorn <bmalehorn@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08tree: convert parse_tree_indirect to struct object_idbrian m. carlson1-1/+1
Convert parse_tree_indirect to take a pointer to struct object_id. Update all the callers. This transformation was achieved using the following semantic patch and manual updates to the declaration and definition. Update builtin/checkout.c manually as well, since it uses a ternary expression not handled by the semantic patch. @@ expression E1; @@ - parse_tree_indirect(E1.hash) + parse_tree_indirect(&E1) @@ expression E1; @@ - parse_tree_indirect(E1->hash) + parse_tree_indirect(E1) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08Convert lookup_commit* to struct object_idbrian m. carlson1-2/+2
Convert lookup_commit, lookup_commit_or_die, lookup_commit_reference, and lookup_commit_reference_gently to take struct object_id arguments. Introduce a temporary in parse_object buffer in order to convert this function. This is required since in order to convert parse_object and parse_object_buffer, lookup_commit_reference_gently and lookup_commit_or_die would need to be converted. Not introducing a temporary would therefore require that lookup_commit_or_die take a struct object_id *, but lookup_commit would take unsigned char *, leaving a confusing and hard-to-use interface. parse_object_buffer will lose this temporary in a later patch. This commit was created with manual changes to commit.c, commit.h, and object.c, plus the following semantic patch: @@ expression E1, E2; @@ - lookup_commit_reference_gently(E1.hash, E2) + lookup_commit_reference_gently(&E1, E2) @@ expression E1, E2; @@ - lookup_commit_reference_gently(E1->hash, E2) + lookup_commit_reference_gently(E1, E2) @@ expression E1; @@ - lookup_commit_reference(E1.hash) + lookup_commit_reference(&E1) @@ expression E1; @@ - lookup_commit_reference(E1->hash) + lookup_commit_reference(E1) @@ expression E1; @@ - lookup_commit(E1.hash) + lookup_commit(&E1) @@ expression E1; @@ - lookup_commit(E1->hash) + lookup_commit(E1) @@ expression E1, E2; @@ - lookup_commit_or_die(E1.hash, E2) + lookup_commit_or_die(&E1, E2) @@ expression E1, E2; @@ - lookup_commit_or_die(E1->hash, E2) + lookup_commit_or_die(E1, E2) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-02Convert struct cache_tree to use struct object_idbrian m. carlson1-1/+1
Convert the sha1 member of struct cache_tree to struct object_id by changing the definition and applying the following semantic patch, plus the standard object_id transforms: @@ struct cache_tree E1; @@ - E1.sha1 + E1.oid.hash @@ struct cache_tree *E1; @@ - E1->sha1 + E1->oid.hash Fix up one reference to active_cache_tree which was not automatically caught by Coccinelle. These changes are prerequisites for converting parse_object. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-28status: add color config slots for branch info in "--short --branch"Stephen Kent1-0/+4
Add color config slots to be used in the status short-format when displaying local and remote tracking branch information. [jc: rebased on top of Peff's fix to 'git status' and tweaked the test to check both local and remote-tracking branch output] Signed-off-by: Stephen Kent <smkent@smkent.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-26Merge branch 'jk/war-on-git-path'Junio C Hamano1-3/+3
While handy, "git_path()" is a dangerous function to use as a callsite that uses it safely one day can be broken by changes to other code that calls it. Reduction of its use continues. * jk/war-on-git-path: am: drop "dir" parameter from am_state_init replace strbuf_addstr(git_path()) with git_path_buf() replace xstrdup(git_path(...)) with git_pathdup(...) use git_path_* helper functions branch: add edit_description() helper bisect: add git_path_bisect_terms helper
2017-04-23Merge branch 'nd/conditional-config-include'Junio C Hamano1-1/+1
$GIT_DIR may in some cases be normalized with all symlinks resolved while "gitdir" path expansion in the pattern does not receive the same treatment, leading to incorrect mismatch. This has been fixed. * nd/conditional-config-include: config: resolve symlinks in conditional include's patterns path.c: and an option to call real_path() in expand_user_path()
2017-04-20use git_path_* helper functionsJeff King1-3/+3
Long ago we added functions like git_path_merge_msg() to replace the more dangerous git_path("MERGE_MSG"). Over time some new calls to the latter have crept it. Let's convert them to use the safer form. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-14path.c: and an option to call real_path() in expand_user_path()Nguyễn Thái Ngọc Duy1-1/+1
In the next patch we need the ability to expand '~' to real_path($HOME). But we can't do that from outside because '~' is part of a pattern, not a true path. Add an option to expand_user_path() to do so. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-20builtin/commit: convert to struct object_idbrian m. carlson1-23/+23
Convert most leaf functions to use struct object_id. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-31builtin/commit.c: switch to strbuf, instead of snprintf()Elia Pinto1-6/+4
Switch to dynamic allocation with strbuf, so we can avoid dealing with magic numbers in the code and reduce the cognitive burden from the programmers. The original code is correct, but programmers no longer have to count bytes needed for static allocation to know that. As a side effect of this change, we also reduce the snprintf() calls, that may silently truncate results if the programmer is not careful. Helped-by: René Scharfe <l.s.r@web.de> Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-13builtin/commit.c: remove the PATH_MAX limitation via dynamic allocationElia Pinto1-11/+10
Remove the PATH_MAX limitation from the environment setting that points to a filename by switching to dynamic allocation. As a side effect of this change, we also reduce the snprintf() calls, that may silently truncate results if the programmer is not careful. Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-19Merge branch 'jc/lock-report-on-error'Junio C Hamano1-4/+4
Git 2.11 had a minor regression in "merge --ff-only" that competed with another process that simultanously attempted to update the index. We used to explain what went wrong with an error message, but the new code silently failed. The error message has been resurrected. * jc/lock-report-on-error: lockfile: LOCK_REPORT_ON_ERROR hold_locked_index(): align error handling with hold_lockfile_for_update() wt-status: implement opportunisitc index update correctly
2016-12-19Merge branch 'ak/commit-only-allow-empty'Junio C Hamano1-3/+1
"git commit --allow-empty --only" (no pathspec) with dirty index ought to be an acceptable way to create a new commit that does not change any paths, but it was forbidden, perhaps because nobody needed it so far. * ak/commit-only-allow-empty: commit: remove 'Clever' message for --only --amend commit: make --only --allow-empty work without paths
2016-12-09commit: remove 'Clever' message for --only --amendAndreas Krey1-2/+0
The behavior is now documented; more importantly, rewarding the user with a "Wow, you are clever" praise afterwards is not an effective way to advertise the feature--at that point the user already knows. Signed-off-by: Andreas Krey <a.krey@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-07hold_locked_index(): align error handling with hold_lockfile_for_update()Junio C Hamano1-4/+4
Callers of the hold_locked_index() function pass 0 when they want to prepare to write a new version of the index file without wishing to die or emit an error message when the request fails (e.g. somebody else already held the lock), and pass 1 when they want the call to die upon failure. This option is called LOCK_DIE_ON_ERROR by the underlying lockfile API, and the hold_locked_index() function translates the paramter to LOCK_DIE_ON_ERROR when calling the hold_lock_file_for_update(). Replace these hardcoded '1' with LOCK_DIE_ON_ERROR and stop translating. Callers other than the ones that are replaced with this change pass '0' to the function; no behaviour change is intended with this patch. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Among the callers of hold_locked_index() that passes 0: - diff.c::refresh_index_quietly() at the end of "git diff" is an opportunistic update; it leaks the lockfile structure but it is just before the program exits and nobody should care. - builtin/describe.c::cmd_describe(), builtin/commit.c::cmd_status(), sequencer.c::read_and_refresh_cache() are all opportunistic updates and they are OK. - builtin/update-index.c::cmd_update_index() takes a lock upfront but we may end up not needing to update the index (i.e. the entries may be fully up-to-date), in which case we do not need to issue an error upon failure to acquire the lock. We do diagnose and die if we indeed need to update, so it is OK. - wt-status.c::require_clean_work_tree() IS BUGGY. It asks silence, does not check the returned value. Compare with callsites like cmd_describe() and cmd_status() to notice that it is wrong to call update_index_if_able() unconditionally.
2016-12-05commit: make --only --allow-empty work without pathsAndreas Krey1-1/+1
--only is implied when paths are present, and required them unless --amend. But with --allow-empty it should be allowed as well - it is the only way to create an empty commit in the presence of staged changes. Signed-off-by: Andreas Krey <a.krey@gmx.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-29commit: make ignore_non_trailer take buf/lenJonathan Tan1-1/+1
Make ignore_non_trailer take a buf/len pair instead of struct strbuf. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-31Merge branch 'rs/commit-pptr-simplify'Junio C Hamano1-8/+6
Code simplification. * rs/commit-pptr-simplify: commit: simplify building parents list
2016-10-30commit: simplify building parents listRené Scharfe1-8/+6
Push pptr down into the FROM_MERGE branch of the if/else statement, where it's actually used, and call commit_list_append() for appending elements instead of playing tricks with commit_list_insert(). Call copy_commit_list() in the amend branch instead of open-coding it. Don't bother setting pptr in the final branch as it's not used thereafter. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-27Merge branch 'nd/ita-empty-commit'Junio C Hamano1-4/+9
When new paths were added by "git add -N" to the index, it was enough to circumvent the check by "git commit" to refrain from making an empty commit without "--allow-empty". The same logic prevented "git status" to show such a path as "new file" in the "Changes not staged for commit" section. * nd/ita-empty-commit: commit: don't be fooled by ita entries when creating initial commit commit: fix empty commit creation when there's no changes but ita entries diff: add --ita-[in]visible-in-index diff-lib: allow ita entries treated as "not yet exist in index"
2016-10-24commit: don't be fooled by ita entries when creating initial commitNguyễn Thái Ngọc Duy1-3/+8
ita entries are dropped at tree generation phase. If the entire index consists of just ita entries, the result would be a a commit with no entries, which should be caught unless --allow-empty is specified. The test "!!active_nr" is not sufficient to catch this. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-24commit: fix empty commit creation when there's no changes but ita entriesNguyễn Thái Ngọc Duy1-1/+1
If i-t-a entries are present and there is no change between the index and HEAD i-t-a entries, index_differs_from() still returns "dirty, new entries" (aka, the resulting commit is not empty), but cache-tree will skip i-t-a entries and produce the exact same tree of current commit. index_differs_from() is supposed to catch this so we can abort git-commit (unless --no-empty is specified). Update it to optionally ignore i-t-a entries when doing a diff between the index and HEAD so that it would return "no change" in this case and abort commit. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-17sequencer: use memoized sequencer directory pathJohannes Schindelin1-1/+1
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-26Merge branch 'tg/add-chmod+x-fix'Junio C Hamano1-1/+1
"git add --chmod=+x <pathspec>" added recently only toggled the executable bit for paths that are either new or modified. This has been corrected to flip the executable bit for all paths that match the given pathspec. * tg/add-chmod+x-fix: t3700-add: do not check working tree file mode without POSIXPERM t3700-add: create subdirectory gently add: modify already added files when --chmod is given read-cache: introduce chmod_index_entry update-index: add test for chmod flags
2016-09-15add: modify already added files when --chmod is givenThomas Gummerer1-1/+1
When the chmod option was added to git add, it was hooked up to the diff machinery, meaning that it only works when the version in the index differs from the version on disk. As the option was supposed to mirror the chmod option in update-index, which always changes the mode in the index, regardless of the status of the file, make sure the option behaves the same way in git add. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-08Merge branch 'jh/status-v2-porcelain'Junio C Hamano1-44/+34
Enhance "git status --porcelain" output by collecting more data on the state of the index and the working tree files, which may further be used to teach git-prompt (in contrib/) to make fewer calls to git. * jh/status-v2-porcelain: status: unit tests for --porcelain=v2 test-lib-functions.sh: add lf_to_nul helper git-status.txt: describe --porcelain=v2 format status: print branch info with --porcelain=v2 --branch status: print per-file porcelain v2 status data status: collect per-file data for --porcelain=v2 status: support --porcelain[=<version>] status: cleanup API to wt_status_print status: rename long-format print routines
2016-08-11status: print branch info with --porcelain=v2 --branchJeff Hostetler1-0/+5
Expand porcelain v2 output to include branch and tracking branch information. This includes the commit id, the branch, the upstream branch, and the ahead and behind counts. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-11status: collect per-file data for --porcelain=v2Jeff Hostetler1-0/+3
Collect extra per-file data for porcelain V2 format. The output of `git status --porcelain` leaves out many details about the current status that clients might like to have. This can force them to be less efficient as they may need to launch secondary commands (and try to match the logic within git) to accumulate this extra information. For example, a GUI IDE might want the file mode to display the correct icon for a changed item (without having to stat it afterwards). Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-10Merge branch 'os/no-verify-skips-commit-msg-too' into maintJunio C Hamano1-1/+1
"git commit --help" said "--no-verify" is only about skipping the pre-commit hook, and failed to say that it also skipped the commit-msg hook. * os/no-verify-skips-commit-msg-too: commit: describe that --no-verify skips the commit-msg hook in the help text
2016-08-08Merge branch 'pb/commit-editmsg-path' into maintJunio C Hamano1-7/+8
Code clean-up. * pb/commit-editmsg-path: builtin/commit.c: memoize git-path for COMMIT_EDITMSG
2016-08-05status: support --porcelain[=<version>]Jeff Hostetler1-3/+18
Update --porcelain argument to take optional version parameter to allow multiple porcelain formats to be supported in the future. The token "v1" is the default value and indicates the traditional porcelain format. (The token "1" is an alias for that.) Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-05status: cleanup API to wt_status_printJeff Hostetler1-42/+9
Refactor the API between builtin/commit.c and wt-status.[ch]. Hide the details of the various wt_*status_print() routines inside wt-status.c behind a single (new) wt_status_print() routine. Eliminate the switch statements from builtin/commit.c. Allow details of new status formats to be isolated within wt-status.c Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-05status: rename long-format print routinesJeff Hostetler1-2/+2
Rename the various wt_status_print*() routines to be wt_longstatus_print*() to make it clear that these routines are only concerned with the normal/long status output and reduce developer confusion as other status formats are added in the future. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-03Merge branch 'os/no-verify-skips-commit-msg-too'Junio C Hamano1-1/+1
"git commit --help" said "--no-verify" is only about skipping the pre-commit hook, and failed to say that it also skipped the commit-msg hook. * os/no-verify-skips-commit-msg-too: commit: describe that --no-verify skips the commit-msg hook in the help text
2016-07-28Merge branch 'js/find-commit-subject-ignore-leading-blanks' into maintJunio C Hamano1-1/+1
A helper function that takes the contents of a commit object and finds its subject line did not ignore leading blank lines, as is commonly done by other codepaths. Make it ignore leading blank lines to match. * js/find-commit-subject-ignore-leading-blanks: reset --hard: skip blank lines when reporting the commit subject sequencer: use skip_blank_lines() to find the commit subject commit -C: skip blank lines at the beginning of the message commit.c: make find_commit_subject() more robust pretty: make the skip_blank_lines() function public
2016-07-26commit: describe that --no-verify skips the commit-msg hook in the help textOrgad Shaneh1-1/+1
This brings the short help in line with the documentation. Signed-off-by: Orgad Shaneh <orgads@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-11Merge branch 'js/find-commit-subject-ignore-leading-blanks'Junio C Hamano1-1/+1
A helper function that takes the contents of a commit object and finds its subject line did not ignore leading blank lines, as is commonly done by other codepaths. Make it ignore leading blank lines to match. * js/find-commit-subject-ignore-leading-blanks: reset --hard: skip blank lines when reporting the commit subject sequencer: use skip_blank_lines() to find the commit subject commit -C: skip blank lines at the beginning of the message commit.c: make find_commit_subject() more robust pretty: make the skip_blank_lines() function public
2016-07-06Merge branch 'pb/commit-editmsg-path'Junio C Hamano1-7/+8
Code clean-up. * pb/commit-editmsg-path: builtin/commit.c: memoize git-path for COMMIT_EDITMSG
2016-06-29commit -C: skip blank lines at the beginning of the messageJohannes Schindelin1-1/+1
Consistent with the pretty-printing machinery, we skip leading blank lines (if any) of existing commit messages. While Git itself only produces commit objects with a single empty line between commit header and commit message, it is legal to have more than one blank line (i.e. lines containing only white space, or no characters) at the beginning of the commit message, and the pretty-printing code already handles that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-09builtin/commit.c: memoize git-path for COMMIT_EDITMSGPranit Bauva1-7/+8
This is a follow up commit for f932729c (memoize common git-path "constant" files, 10-Aug-2015). The many function calls to git_path() are replaced by git_path_commit_editmsg() and which thus eliminates the need to repeatedly compute the location of "COMMIT_EDITMSG". Mentored-by: Lars Schneider <larsxschneider@gmail.com> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-07add: add --chmod=+x / --chmod=-x optionsEdward Thomson1-1/+1
The executable bit will not be detected (and therefore will not be set) for paths in a repository with `core.filemode` set to false, though the users may still wish to add files as executable for compatibility with other users who _do_ have `core.filemode` functionality. For example, Windows users adding shell scripts may wish to add them as executable for compatibility with users on non-Windows. Although this can be done with a plumbing command (`git update-index --add --chmod=+x foo`), teaching the `git-add` command allows users to set a file executable with a command that they're already familiar with. Signed-off-by: Edward Thomson <ethomson@edwardthomson.com> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-23Merge branch 'pb/commit-verbose-config'Junio C Hamano1-0/+10
"git commit" learned to pay attention to "commit.verbose" configuration variable and act as if "--verbose" option was given from the command line. * pb/commit-verbose-config: commit: add a commit.verbose config variable t7507-commit-verbose: improve test coverage by testing number of diffs parse-options.c: make OPTION_COUNTUP respect "unspecified" values t/t7507: improve test coverage t0040-parse-options: improve test coverage test-parse-options: print quiet as integer t0040-test-parse-options.sh: fix style issues
2016-05-10commit: add a commit.verbose config variablePranit Bauva1-0/+10
Add commit.verbose configuration variable as a convenience for those who always prefer --verbose. Add tests to check the behavior introduced by this commit and also to verify that behavior of status doesn't break because of this commit. Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-02Merge branch 'ad/commit-have-m-option' into maintJunio C Hamano1-3/+3
"git commit" misbehaved in a few minor ways when an empty message is given via -m '', all of which has been corrected. * ad/commit-have-m-option: commit: do not ignore an empty message given by -m '' commit: --amend -m '' silently fails to wipe message
2016-04-22Merge branch 'ad/commit-have-m-option'Junio C Hamano1-3/+3
"git commit" misbehaved in a few minor ways when an empty message is given via -m '', all of which has been corrected. * ad/commit-have-m-option: commit: do not ignore an empty message given by -m '' commit: --amend -m '' silently fails to wipe message
2016-04-14Merge branch 'ss/commit-squash-msg' into maintJunio C Hamano1-1/+10
When "git merge --squash" stopped due to conflict, the concluding "git commit" failed to read in the SQUASH_MSG that shows the log messages from all the squashed commits. * ss/commit-squash-msg: commit: do not lose SQUASH_MSG contents
2016-04-07commit: do not ignore an empty message given by -m ''Jeff King1-3/+3
When f9568530 (builtin-commit: resurrect behavior for multiple -m options, 2007-11-11) converted a "char *message" to "struct strbuf message" to hold the messages given with the "-m" option, it incorrectly changed the checks "did we get a message with the -m option?" to "is message.len 0?". Later, we noticed one breakage from this change and corrected it with 25206778 (commit: don't start editor if empty message is given with -m, 2013-05-25). However, "we got a message with -m, even though an empty one, so we shouldn't be launching an editor" was not the only breakage. * "git commit --amend -m '' --allow-empty", even though it looks strange, is a valid request to amend the commit to have no message at all. Due to the misdetection of the presence of -m on the command line, we ended up keeping the log messsage from the original commit. * "git commit -m "$msg" -F file" should be rejected whether $msg is an empty string or not, but due to the same bug, was not rejected when $msg is empty. * "git -c template=file -m "$msg"" should ignore the template even when $msg is empty, but it didn't and instead used the contents from the template file. Correct these by checking have_option_m, which the earlier 25206778 introduced to fix the same bug. Reported-by: Adam Dinwoodie <adam@dinwoodie.org> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-04-06Merge branch 'ss/commit-squash-msg'Junio C Hamano1-1/+10
When "git merge --squash" stopped due to conflict, the concluding "git commit" failed to read in the SQUASH_MSG that shows the log messages from all the squashed commits. * ss/commit-squash-msg: commit: do not lose SQUASH_MSG contents
2016-03-21commit: do not lose SQUASH_MSG contentsSven Strickroth1-1/+10
When concluding a conflicted "git merge --squash", the command failed to read SQUASH_MSG that was prepared by "git merge", and showed only the "# Conflicts:" list of conflicted paths. Place the contents from SQUASH_MSG at the beginning, just like we show the commit log skeleton first when concluding a normal merge, and then show the "# Conflicts:" list, to help the user write the log message for the resulting commit. Test by Junio C Hamano <gitster@pobox.com>. Signed-off-by: Sven Strickroth <sven@cs-ware.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-25diff: activate diff.renames by defaultMatthieu Moy1-0/+1
Rename detection is a very convenient feature, and new users shouldn't have to dig in the documentation to benefit from it. Potential objections to activating rename detection are that it sometimes fail, and it is sometimes slow. But rename detection is already activated by default in several cases like "git status" and "git merge", so activating diff.renames does not fundamentally change the situation. When the rename detection fails, it now fails consistently between "git diff" and "git status". This setting does not affect plumbing commands, hence well-written scripts will not be affected. Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-01-28Merge branch 'jc/strbuf-getline'Junio C Hamano1-1/+1
The preliminary clean-up for jc/peace-with-crlf topic. * jc/strbuf-getline: strbuf: give strbuf_getline() to the "most text friendly" variant checkout-index: there are only two possible line terminations update-index: there are only two possible line terminations check-ignore: there are only two possible line terminations check-attr: there are only two possible line terminations mktree: there are only two possible line terminations strbuf: introduce strbuf_getline_{lf,nul}() strbuf: make strbuf_getline_crlf() global strbuf: miniscule style fix
2016-01-20Merge branch 'js/fopen-harder'Junio C Hamano1-1/+1
Some codepaths used fopen(3) when opening a fixed path in $GIT_DIR (e.g. COMMIT_EDITMSG) that is meant to be left after the command is done. This however did not work well if the repository is set to be shared with core.sharedRepository and the umask of the previous user is tighter. They have been made to work better by calling unlink(2) and retrying after fopen(3) fails with EPERM. * js/fopen-harder: Handle more file writes correctly in shared repos commit: allow editing the commit message even in shared repos
2016-01-15strbuf: introduce strbuf_getline_{lf,nul}()Junio C Hamano1-1/+1
The strbuf_getline() interface allows a byte other than LF or NUL as the line terminator, but this is only because I wrote these codepaths anticipating that there might be a value other than NUL and LF that could be useful when I introduced line_termination long time ago. No useful caller that uses other value has emerged. By now, it is clear that the interface is overly broad without a good reason. Many codepaths have hardcoded preference to read either LF terminated or NUL terminated records from their input, and then call strbuf_getline() with LF or NUL as the third parameter. This step introduces two thin wrappers around strbuf_getline(), namely, strbuf_getline_lf() and strbuf_getline_nul(), and mechanically rewrites these call sites to call either one of them. The changes contained in this patch are: * introduction of these two functions in strbuf.[ch] * mechanical conversion of all callers to strbuf_getline() with either '\n' or '\0' as the third parameter to instead call the respective thin wrapper. After this step, output from "git grep 'strbuf_getline('" would become a lot smaller. An interim goal of this series is to make this an empty set, so that we can have strbuf_getline_crlf() take over the shorter name strbuf_getline(). Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-01-07commit: allow editing the commit message even in shared reposJohannes Schindelin1-1/+1
It was pointed out by Yaroslav Halchenko that the file containing the commit message is writable only by the owner, which means that we have to rewrite it from scratch in a shared repository. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-12-10Merge branch 'bc/object-id'Junio C Hamano1-4/+4
More transition from "unsigned char[40]" to "struct object_id". This needed a few merge fixups, but is mostly disentangled from other topics. * bc/object-id: remote: convert functions to struct object_id Remove get_object_hash. Convert struct object to object_id Add several uses of get_object_hash. object: introduce get_object_hash macro. ref_newer: convert to use struct object_id push_refs_with_export: convert to struct object_id get_remote_heads: convert to struct object_id parse_fetch: convert to use struct object_id add_sought_entry_mem: convert to struct object_id Convert struct ref to use object_id. sha1_file: introduce has_object_file helper.
2015-11-20Remove get_object_hash.brian m. carlson1-4/+4
Convert all instances of get_object_hash to use an appropriate reference to the hash member of the oid member of struct object. This provides no functional change, as it is essentially a macro substitution. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
2015-11-20Add several uses of get_object_hash.brian m. carlson1-4/+4
Convert most instances where the sha1 member of struct object is dereferenced to use get_object_hash. Most instances that are passed to functions that have versions taking struct object_id, such as get_sha1_hex/get_oid_hex, or instances that can be trivially converted to use struct object_id instead, are not converted. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
2015-11-16allow hooks to ignore their standard input streamClemens Buchacher1-0/+3
Since ec7dbd145 (receive-pack: allow hooks to ignore its standard input stream) the pre-receive and post-receive hooks ignore SIGPIPE. Do the same for the remaining hooks pre-push and post-rewrite, which read from standard input. The same arguments for ignoring SIGPIPE apply. Include test by Jeff King which checks that SIGPIPE does not cause pre-push hook failure. With the use of git update-ref --stdin it is fast enough to be enabled by default. Signed-off-by: Clemens Buchacher <clemens.buchacher@intel.com> Signed-off-by: Jeff King <peff@peff.net>
2015-10-16strbuf: make stripspace() part of strbufTobias Klauser1-3/+3
This function is also used in other builtins than stripspace, so it makes sense to have it in a more generic place. Since it operates on an strbuf and the function is declared in strbuf.h, move it to strbuf.c and add the corresponding prefix to its name, just like other API functions in the strbuf_* family. Also switch all current users of stripspace() to the new function name and keep a temporary wrapper inline function for any topic branches still using stripspace(). Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Tobias Klauser <tklauser@distanz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-01Merge branch ↵Junio C Hamano1-3/+1
'dt/commit-preserve-base-index-upon-opportunistic-cache-tree-update' When re-priming the cache-tree opportunistically while committing the in-core index as-is, we mistakenly invalidated the in-core index too aggressively, causing the experimental split-index code to unnecessarily rewrite the on-disk index file(s). * dt/commit-preserve-base-index-upon-opportunistic-cache-tree-update: commit: don't rewrite shared index unnecessarily
2015-08-31commit: don't rewrite shared index unnecessarilyDavid Turner1-3/+1
Remove a cache invalidation which would cause the shared index to be rewritten on as-is commits. When the cache-tree has changed, we need to update it. But we don't necessarily need to update the shared index. So setting active_cache_changed to SOMETHING_CHANGED is unnecessary. Instead, we let update_main_cache_tree just update the CACHE_TREE_CHANGED bit. In order to test this, make test-dump-split-index not segfault on missing replace_bitmap/delete_bitmap. This new codepath is not called now that the test passes, but is necessary to avoid a segfault when the new test is run with the old builtin/commit.c code. Signed-off-by: David Turner <dturner@twopensource.com> Acked-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-25Merge branch 'mh/tempfile'Junio C Hamano1-7/+8
The "lockfile" API has been rebuilt on top of a new "tempfile" API. * mh/tempfile: credential-cache--daemon: use tempfile module credential-cache--daemon: delete socket from main() gc: use tempfile module to handle gc.pid file lock_repo_for_gc(): compute the path to "gc.pid" only once diff: use tempfile module setup_temporary_shallow(): use tempfile module write_shared_index(): use tempfile module register_tempfile(): new function to handle an existing temporary file tempfile: add several functions for creating temporary files prepare_tempfile_object(): new function, extracted from create_tempfile() tempfile: a new module for handling temporary files commit_lock_file(): use get_locked_file_path() lockfile: add accessor get_lock_file_path() lockfile: add accessors get_lock_file_fd() and get_lock_file_fp() create_bundle(): duplicate file descriptor to avoid closing it twice lockfile: move documentation to lockfile.h and lockfile.c
2015-08-10memoize common git-path "constant" filesJeff King1-16/+16
One of the most common uses of git_path() is to pass a constant, like git_path("MERGE_MSG"). This has two drawbacks: 1. The return value is a static buffer, and the lifetime is dependent on other calls to git_path, etc. 2. There's no compile-time checking of the pathname. This is OK for a one-off (after all, we have to spell it correctly at least once), but many of these constant strings appear throughout the code. This patch introduces a series of functions to "memoize" these strings, which are essentially globals for the lifetime of the program. We compute the value once, take ownership of the buffer, and return the cached value for subsequent calls. cache.h provides a helper macro for defining these functions as one-liners, and defines a few common ones for global use. Using a macro is a little bit gross, but it does nicely document the purpose of the functions. If we need to touch them all later (e.g., because we learned how to change the git_dir variable at runtime, and need to invalidate all of the stored values), it will be much easier to have the complete list. Note that the shared-global functions have separate, manual declarations. We could do something clever with the macros (e.g., expand it to a declaration in some places, and a declaration _and_ a definition in path.c). But there aren't that many, and it's probably better to stay away from too-magical macros. Likewise, if we abandon the C preprocessor in favor of generating these with a script, we could get much fancier. E.g., normalizing "FOO/BAR-BAZ" into "git_path_foo_bar_baz". But the small amount of saved typing is probably not worth the resulting confusion to readers who want to grep for the function's definition. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10lockfile: add accessor get_lock_file_path()Michael Haggerty1-7/+8
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-29convert "enum date_mode" into a structJeff King1-2/+2
In preparation for adding date modes that may carry extra information beyond the mode itself, this patch converts the date_mode enum into a struct. Most of the conversion is fairly straightforward; we pass the struct as a pointer and dereference the type field where necessary. Locations that declare a date_mode can use a "{}" constructor. However, the tricky case is where we use the enum labels as constants, like: show_date(t, tz, DATE_NORMAL); Ideally we could say: show_date(t, tz, &{ DATE_NORMAL }); but of course C does not allow that. Likewise, we cannot cast the constant to a struct, because we need to pass an actual address. Our options are basically: 1. Manually add a "struct date_mode d = { DATE_NORMAL }" definition to each caller, and pass "&d". This makes the callers uglier, because they sometimes do not even have their own scope (e.g., they are inside a switch statement). 2. Provide a pre-made global "date_normal" struct that can be passed by address. We'd also need "date_rfc2822", "date_iso8601", and so forth. But at least the ugliness is defined in one place. 3. Provide a wrapper that generates the correct struct on the fly. The big downside is that we end up pointing to a single global, which makes our wrapper non-reentrant. But show_date is already not reentrant, so it does not matter. This patch implements 3, along with a minor macro to keep the size of the callers sane. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-05Merge branch 'pt/xdg-config-path' into maintJunio C Hamano1-5/+3
Code clean-up for xdg configuration path support. * pt/xdg-config-path: path.c: remove home_config_paths() git-config: replace use of home_config_paths() git-commit: replace use of home_config_paths() credential-store.c: replace home_config_paths() with xdg_config_home() dir.c: replace home_config_paths() with xdg_config_home() attr.c: replace home_config_paths() with xdg_config_home() path.c: implement xdg_config_home() t0302: "unreadable" test needs POSIXPERM t0302: test credential-store support for XDG_CONFIG_HOME git-credential-store: support XDG_CONFIG_HOME git-credential-store: support multiple credential files
2015-05-26Merge branch 'nd/untracked-cache'Junio C Hamano1-2/+3
Teach the index to optionally remember already seen untracked files to speed up "git status" in a working tree with tons of cruft. * nd/untracked-cache: (24 commits) git-status.txt: advertisement for untracked cache untracked cache: guard and disable on system changes mingw32: add uname() t7063: tests for untracked cache update-index: test the system before enabling untracked cache update-index: manually enable or disable untracked cache status: enable untracked cache untracked-cache: temporarily disable with $GIT_DISABLE_UNTRACKED_CACHE untracked cache: mark index dirty if untracked cache is updated untracked cache: print stats with $GIT_TRACE_UNTRACKED_STATS untracked cache: avoid racy timestamps read-cache.c: split racy stat test to a separate function untracked cache: invalidate at index addition or removal untracked cache: load from UNTR index extension untracked cache: save to an index extension ewah: add convenient wrapper ewah_serialize_strbuf() untracked cache: don't open non-existent .gitignore untracked cache: mark what dirs should be recursed/saved untracked cache: record/validate dir mtime and reuse cached output untracked cache: make a wrapper around {open,read,close}dir() ...
2015-05-11Merge branch 'pt/xdg-config-path'Junio C Hamano1-5/+3
Code clean-up for xdg configuration path support. * pt/xdg-config-path: path.c: remove home_config_paths() git-config: replace use of home_config_paths() git-commit: replace use of home_config_paths() credential-store.c: replace home_config_paths() with xdg_config_home() dir.c: replace home_config_paths() with xdg_config_home() attr.c: replace home_config_paths() with xdg_config_home() path.c: implement xdg_config_home()
2015-05-11Merge branch 'nd/multiple-work-trees'Junio C Hamano1-1/+1
A replacement for contrib/workdir/git-new-workdir that does not rely on symbolic links and make sharing of objects and refs safer by making the borrowee and borrowers aware of each other. * nd/multiple-work-trees: (41 commits) prune --worktrees: fix expire vs worktree existence condition t1501: fix test with split index t2026: fix broken &&-chain t2026 needs procondition SANITY git-checkout.txt: a note about multiple checkout support for submodules checkout: add --ignore-other-wortrees checkout: pass whole struct to parse_branchname_arg instead of individual flags git-common-dir: make "modules/" per-working-directory directory checkout: do not fail if target is an empty directory t2025: add a test to make sure grafts is working from a linked checkout checkout: don't require a work tree when checking out into a new one git_path(): keep "info/sparse-checkout" per work-tree count-objects: report unused files in $GIT_DIR/worktrees/... gc: support prune --worktrees gc: factor out gc.pruneexpire parsing code gc: style change -- no SP before closing parenthesis checkout: clean up half-prepared directories in --to mode checkout: reject if the branch is already checked out elsewhere prune: strategies for linked checkouts checkout: support checking out into a new working directory ...
2015-05-06git-commit: replace use of home_config_paths()Paul Tan1-5/+3
Since home_config_paths() combines two distinct functionality already implemented by expand_user_path() and xdg_config_home(), and hides the home config file path ~/.gitconfig. Make the code more explicit by replacing the use of home_config_paths() with expand_user_path() and xdg_config_home(). Signed-off-by: Paul Tan <pyokagan@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-23commit.c: fix a memory leakStefan Beller1-2/+4
Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-12status: enable untracked cacheNguyễn Thái Ngọc Duy1-2/+3
update_index_if_able() is moved down so that the updated untracked cache could be written out. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-05Merge branch 'mh/refs-have-new'Junio C Hamano1-2/+2
Simplify the ref transaction API around how "the ref should be pointing at this object" is specified. * mh/refs-have-new: refs.h: remove duplication in function docstrings update_ref(): improve documentation ref_transaction_verify(): new function to check a reference's value ref_transaction_delete(): check that old_sha1 is not null_sha1 ref_transaction_create(): check that new_sha1 is valid commit: avoid race when creating orphan commits commit: add tests of commit races ref_transaction_delete(): remove "have_old" parameter ref_transaction_update(): remove "have_old" parameter struct ref_update: move "have_old" into "flags" refs.c: change some "flags" to "unsigned int" refs: remove the gap in the REF_* constant values refs: move REF_DELETING to refs.c
2015-02-17commit: avoid race when creating orphan commitsMichael Haggerty1-1/+1
If HEAD doesn't point at anything during the initial check, then we should make sure that it *still* doesn't point at anything when we are ready to update the reference. Otherwise, another process might commit while we are working (e.g., while we are waiting for the user to edit the commit message) and we will silently overwrite it. This fixes a failing test in t7516. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-02-17ref_transaction_update(): remove "have_old" parameterMichael Haggerty1-1/+1
Instead, verify the reference's old value if and only if old_sha1 is non-NULL. ref_transaction_delete() will get the same treatment in a moment. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-02-17Merge branch 'mg/commit-author-no-match-malformed-message'Junio C Hamano1-1/+1
The error message from "git commit", when a non-existing author name was given as value to the "--author=" parameter, has been reworded to avoid misunderstanding. * mg/commit-author-no-match-malformed-message: commit: reword --author error message
2015-02-11Merge branch 'ah/usage-strings'Junio C Hamano1-2/+2
* ah/usage-strings: standardize usage info string format
2015-02-11Merge branch 'jk/blame-commit-label'Junio C Hamano1-9/+3
"git blame HEAD -- missing" failed to correctly say "HEAD" when it tried to say "No such path 'missing' in HEAD". * jk/blame-commit-label: blame.c: fix garbled error message use xstrdup_or_null to replace ternary conditionals builtin/commit.c: use xstrdup_or_null instead of envdup builtin/apply.c: use xstrdup_or_null instead of null_strdup git-compat-util: add xstrdup_or_null helper
2015-01-26commit: reword --author error messageMichael J Gruber1-1/+1
If an --author argument is specified but does not contain a '>' then git tries to find the argument within the existing authors; and gives the error message "No existing author found with '%s'" if there is no match. This is confusing for users who try to specify a valid complete author name. Rename the error message to make it clearer that the failure has two reasons in this case. (This codepath is touched only when we know already that the argument cannot be a completely wellformed author ident.) Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-01-14standardize usage info string formatAlex Henrie1-2/+2
This patch puts the usage info strings that were not already in docopt- like format into docopt-like format, which will be a litle easier for end users and a lot easier for translators. Changes include: - Placing angle brackets around fill-in-the-blank parameters - Putting dashes in multiword parameter names - Adding spaces to [-f|--foobar] to make [-f | --foobar] - Replacing <foobar>* with [<foobar>...] Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-01-13builtin/commit.c: use xstrdup_or_null instead of envdupJeff King1-9/+3
The only reason for envdup to be its own function is that we have to save the result in a temporary string. With xstrdup_or_null, we can feed the result of getenv() directly. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-12-22Merge branch 'jk/commit-date-approxidate'Junio C Hamano1-30/+18
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
2014-12-22Merge branch 'cc/interpret-trailers-more'Junio C Hamano1-26/+2
"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
2014-12-11commit: always populate GIT_AUTHOR_* variablesJeff King1-21/+5
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-12-11commit: loosen ident checks when generating templateJeff King1-9/+14
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-12-01commit: use SEQ_DIR instead of hardcoding "sequencer"Nguyễn Thái Ngọc Duy1-1/+1
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-11-10commit: make ignore_non_trailer() non staticChristian Couder1-46/+0
Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-11-10Merge branch 'jc/conflict-hint' into cc/interpret-trailers-moreJunio C Hamano1-26/+48
* 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`
2014-10-28merge & sequencer: turn "Conflicts:" hint into a commentJunio C Hamano1-16/+31
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 <gitster@pobox.com>
2014-10-28builtin/commit.c: extract ignore_non_trailer() helper functionJunio C Hamano1-26/+33
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 <gitster@pobox.com>
2014-10-21Merge branch 'rs/ref-transaction'Junio C Hamano1-3/+3
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 ...
2014-10-20Merge branch 'jn/parse-config-slot'Junio C Hamano1-12/+10
Code cleanup. * jn/parse-config-slot: color_parse: do not mention variable name in error message pass config slots as pointers instead of offsets
2014-10-15refs.c: change resolve_ref_unsafe reading argument to be a flags fieldRonnie Sahlberg1-1/+1
resolve_ref_unsafe takes a boolean argument for reading (a nonexistent ref resolves successfully for writing but not for reading). Change this to be a flags field instead, and pass the new constant RESOLVE_REF_READING when we want this behaviour. While at it, swap two of the arguments in the function to put output arguments at the end. As a nice side effect, this ensures that we can catch callers that were unaware of the new API so they can be audited. Give the wrapper functions resolve_refdup and read_ref_full the same treatment for consistency. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-15refs.c: pass the ref log message to _create/delete/update instead of _commitRonnie Sahlberg1-2/+2
Change the ref transaction API so that we pass the reflog message to the create/delete/update functions instead of to ref_transaction_commit. This allows different reflog messages for each ref update in a multi-ref transaction. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-14color_parse: do not mention variable name in error messageJeff King1-2/+1
Originally the color-parsing function was used only for config variables. It made sense to pass the variable name so that the die() message could be something like: $ git -c color.branch.plain=bogus branch fatal: bad color value 'bogus' for variable 'color.branch.plain' These days we call it in other contexts, and the resulting error messages are a little confusing: $ git log --pretty='%C(bogus)' fatal: bad color value 'bogus' for variable '--pretty format' $ git config --get-color foo.bar bogus fatal: bad color value 'bogus' for variable 'command line' This patch teaches color_parse to complain only about the value, and then return an error code. Config callers can then propagate that up to the config parser, which mentions the variable name. Other callers can provide a custom message. After this patch these three cases now look like: $ git -c color.branch.plain=bogus branch error: invalid color value: bogus fatal: unable to parse 'color.branch.plain' from command-line config $ git log --pretty='%C(bogus)' error: invalid color value: bogus fatal: unable to parse --pretty format $ git config --get-color foo.bar bogus error: invalid color value: bogus fatal: unable to parse default color value Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-14pass config slots as pointers instead of offsetsJonathan Nieder1-10/+9
Many config-parsing helpers, like parse_branch_color_slot, take the name of a config variable and an offset to the "slot" name (e.g., "color.branch.plain" is passed along with "13" to effectively pass "plain"). This is leftover from the time that these functions would die() on error, and would want the full variable name for error reporting. These days they do not use the full variable name at all. Passing a single pointer to the slot name is more natural, and lets us more easily adjust the callers to use skip_prefix to avoid manually writing offset numbers. This is effectively a continuation of 9e1a5eb, which did the same for parse_diff_color_slot. This patch covers all of the remaining similar constructs. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-14Merge branch 'rs/more-uses-of-skip-prefix'Junio C Hamano1-9/+9
* rs/more-uses-of-skip-prefix: use skip_prefix() to avoid more magic numbers
2014-10-07use skip_prefix() to avoid more magic numbersRené Scharfe1-9/+9
Continue where ae021d87 (use skip_prefix to avoid magic numbers) left off and use skip_prefix() in more places for determining the lengths of prefix strings to avoid using dependent constants and other indirect methods. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-01lockfile.h: extract new header file for the functions in lockfile.cMichael Haggerty1-0/+1
Move the interface declaration for the functions in lockfile.c from cache.h to a new file, lockfile.h. Add #includes where necessary (and remove some redundant includes of cache.h by files that already include builtin.h). Move the documentation of the lock_file state diagram from lockfile.c to the new header file. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-01lockfile: change lock_file::filename into a strbufMichael Haggerty1-6/+6
For now, we still make sure to allocate at least PATH_MAX characters for the strbuf because resolve_symlink() doesn't know how to expand the space for its return value. (That will be fixed in a moment.) Another alternative would be to just use a strbuf as scratch space in lock_file() but then store a pointer to the naked string in struct lock_file. But lock_file objects are often reused. By reusing the same strbuf, we can avoid having to reallocate the string most times when a lock_file object is reused. Helped-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-01prepare_index(): declare return value to be (const char *)Michael Haggerty1-2/+2
Declare the return value to be const to make it clear that we aren't giving callers permission to write over the string that it points at. (The return value is the filename field of a struct lock_file, which can be used by a signal handler at any time and therefore shouldn't be tampered with.) Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-09-19Merge branch 'ah/grammofix'Junio C Hamano1-1/+1
* ah/grammofix: grammofix in user-facing messages
2014-09-19Merge branch 'jk/commit-author-parsing'Junio C Hamano1-41/+48
Code clean-up. * jk/commit-author-parsing: determine_author_info(): copy getenv output determine_author_info(): reuse parsing functions date: use strbufs in date-formatting functions record_author_date(): use find_commit_header() record_author_date(): fix memory leak on malformed commit commit: provide a function to find a header in a buffer
2014-09-11Merge branch 'dt/cache-tree-repair'Junio C Hamano1-1/+13
Add a few more places in "commit" and "checkout" that make sure that the cache-tree is fully populated in the index. * dt/cache-tree-repair: cache-tree: do not try to use an invalidated subtree info to build a tree cache-tree: Write updated cache-tree after commit cache-tree: subdirectory tests test-dump-cache-tree: invalid trees are not errors cache-tree: create/update cache-tree on checkout
2014-09-11Merge branch 'rs/ref-transaction-1'Junio C Hamano1-13/+12
The second batch of the transactional ref update series. * rs/ref-transaction-1: (22 commits) update-ref --stdin: pass transaction around explicitly update-ref --stdin: narrow scope of err strbuf refs.c: make delete_ref use a transaction refs.c: make prune_ref use a transaction to delete the ref refs.c: remove lock_ref_sha1 refs.c: remove the update_ref_write function refs.c: remove the update_ref_lock function refs.c: make lock_ref_sha1 static walker.c: use ref transaction for ref updates fast-import.c: use a ref transaction when dumping tags receive-pack.c: use a reference transaction for updating the refs refs.c: change update_ref to use a transaction branch.c: use ref transaction for all ref updates fast-import.c: change update_branch to use ref transactions sequencer.c: use ref transactions for all ref updates commit.c: use ref transactions for updates replace.c: use the ref transaction functions for updates tag.c: use ref transactions when doing updates refs.c: add transaction.status and track OPEN/CLOSED refs.c: make ref_transaction_begin take an err argument ...
2014-09-11Merge branch 'rs/child-process-init'Junio C Hamano1-2/+1
Code clean-up. * rs/child-process-init: run-command: inline prepare_run_command_v_opt() run-command: call run_command_v_opt_cd_env() instead of duplicating it run-command: introduce child_process_init() run-command: introduce CHILD_PROCESS_INIT
2014-09-09Merge branch 'sb/prepare-revision-walk-error-check'Junio C Hamano1-1/+2
* sb/prepare-revision-walk-error-check: prepare_revision_walk(): check for return value in all places
2014-09-03commit.c: use ref transactions for updatesRonnie Sahlberg1-13/+12
Change commit.c to use ref transactions for all ref updates. Make sure we pass a NULL pointer to ref_transaction_update if have_old is false. Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-09-02Merge branch 'mm/config-edit-global'Junio C Hamano1-2/+33
Start "git config --edit --global" from a skeletal per-user configuration file contents, instead of a total blank, when the user does not already have any. This immediately reduces the need for a later "Have you forgotten setting core.user?" and we can add more to the template as we gain more experience. * mm/config-edit-global: commit: advertise config --global --edit on guessed identity home_config_paths(): let the caller ignore xdg path config --global --edit: create a template file if needed
2014-09-02grammofix in user-facing messagesAlex Henrie1-1/+1
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> Acked-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-29determine_author_info(): copy getenv outputJeff King1-13/+27
When figuring out the author name for a commit, we may end up either pointing to const storage from getenv("GIT_AUTHOR_*"), or to newly allocated storage based on an existing commit or the --author option. Using const pointers to getenv's return has two problems: 1. It is not guaranteed that the return value from getenv remains valid across multiple calls. 2. We do not know whether to free the values at the end, so we just leak them. We can solve both by duplicating the string returned by getenv(). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-29determine_author_info(): reuse parsing functionsJeff King1-28/+21
Rather than parsing the header manually to find the "author" field, and then parsing its sub-parts, let's use find_commit_header and split_ident_line. This is shorter and easier to read, and should do a more careful parsing job. For example, the current parser could find the end-of-email right-bracket across a newline (for a malformed commit), and calculate a bogus gigantic length for the date (by using "eol - rb"). As a bonus, this also plugs a memory leak when we pull the date field from an existing commit (we still leak the name and email buffers, which will be fixed in a later commit). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-27date: use strbufs in date-formatting functionsJeff King1-10/+10
Many of the date functions write into fixed-size buffers. This is a minor pain, as we have to take special precautions, and frequently end up copying the result into a strbuf or heap-allocated buffer anyway (for which we sometimes use strcpy!). Let's instead teach parse_date, datestamp, etc to write to a strbuf. The obvious downside is that we might need to perform a heap allocation where we otherwise would not need to. However, it turns out that the only two new allocations required are: 1. In test-date.c, where we don't care about efficiency. 2. In determine_author_info, which is not performance critical (and where the use of a strbuf will help later refactoring). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-20run-command: introduce CHILD_PROCESS_INITRené Scharfe1-2/+1
Most struct child_process variables are cleared using memset first after declaration. Provide a macro, CHILD_PROCESS_INIT, that can be used to initialize them statically instead. That's shorter, doesn't require a function call and is slightly more readable (especially given that we already have STRBUF_INIT, ARGV_ARRAY_INIT etc.). Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-12prepare_revision_walk(): check for return value in all placesStefan Beller1-1/+2
Even the documentation tells us: You should check if it returns any error (non-zero return code) and if it does not, you can start using get_revision() to do the iteration. In preparation for this commit, I grepped all occurrences of prepare_revision_walk and added error messages, when there were none. Signed-off-by: Stefan Beller <stefanbeller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-25commit: advertise config --global --edit on guessed identityMatthieu Moy1-2/+33
When the user has no user-wide configuration file, it's faster to use the newly introduced config file template than to run two commands to set user.name and user.email. Advise this to the user. The old advice is kept if the user already has a configuration file since the template feature would not trigger in this case. Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-23Merge branch 'ta/string-list-init'Junio C Hamano1-2/+1
* ta/string-list-init: replace memset with string-list initializers string-list: add string_list initializer helper function
2014-07-22Merge branch 'rs/code-cleaning'Junio C Hamano1-1/+1
* rs/code-cleaning: remote-testsvn: use internal argv_array of struct child_process in cmd_import() bundle: use internal argv_array of struct child_process in create_bundle() fast-import: use hashcmp() for SHA1 hash comparison transport: simplify fetch_objs_via_rsync() using argv_array run-command: use internal argv_array of struct child_process in run_hook_ve() use commit_list_count() to count the members of commit_lists strbuf: use strbuf_addstr() for adding C strings
2014-07-21replace memset with string-list initializersTanay Abhra1-2/+1
Using memset and then manually setting values of the string-list members is not future proof as the internal representation of string-list may change any time. Use `string_list_init()` or STRING_LIST_INIT_* macros instead of memset. Signed-off-by: Tanay Abhra <tanayabh@gmail.com> Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-17strbuf: use strbuf_addstr() for adding C stringsRené Scharfe1-1/+1
Avoid code duplication and let strbuf_addstr() call strlen() for us. Signed-off-by: Rene Scharfe <l.s.r@web.de> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-16Merge branch 'nd/split-index'Junio C Hamano1-19/+14
An experiment to use two files (the base file and incremental changes relative to it) to represent the index to reduce I/O cost of rewriting a large index when only small part of the working tree changes. * nd/split-index: (32 commits) t1700: new tests for split-index mode t2104: make sure split index mode is off for the version test read-cache: force split index mode with GIT_TEST_SPLIT_INDEX read-tree: note about dropping split-index mode or index version read-tree: force split-index mode off on --index-output rev-parse: add --shared-index-path to get shared index path update-index --split-index: do not split if $GIT_DIR is read only update-index: new options to enable/disable split index mode split-index: strip pathname of on-disk replaced entries split-index: do not invalidate cache-tree at read time split-index: the reading part split-index: the writing part read-cache: mark updated entries for split index read-cache: save deleted entries in split index read-cache: mark new entries for split index read-cache: split-index mode read-cache: save index SHA-1 after reading entry.c: update cache_changed if refresh_cache is set in checkout_entry() cache-tree: mark istate->cache_changed on prime_cache_tree() cache-tree: mark istate->cache_changed on cache tree update ...
2014-07-16Merge branch 'jk/commit-buffer-length' into maintJunio C Hamano1-2/+2
A handful of code paths had to read the commit object more than once when showing header fields that are usually not parsed. The internal data structure to keep track of the contents of the commit object has been updated to reduce the need for this double-reading, and to allow the caller find the length of the object. * jk/commit-buffer-length: reuse cached commit buffer when parsing signatures commit: record buffer length in cache commit: convert commit->buffer to a slab commit-slab: provide a static initializer use get_commit_buffer everywhere convert logmsg_reencode to get_commit_buffer use get_commit_buffer to avoid duplicate code use get_cached_commit_buffer where appropriate provide helpers to access the commit buffer provide a helper to set the commit buffer provide a helper to free commit buffer sequencer: use logmsg_reencode in get_message logmsg_reencode: return const buffer do not create "struct commit" with xcalloc commit: push commit_index update into alloc_commit_node alloc: include any-object allocations in alloc_report replace dangerous uses of strbuf_attach commit_tree: take a pointer/len pair rather than a const strbuf
2014-07-14cache-tree: Write updated cache-tree after commitDavid Turner1-1/+17
During the commit process, update the cache-tree. Write this updated cache-tree so that it's ready for subsequent commands. Add test code which demonstrates that git commit now writes the cache tree. Make all tests test the entire cache-tree, not just the root level. Signed-off-by: David Turner <dturner@twitter.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-09Merge branch 'jk/skip-prefix'Junio C Hamano1-3/+2
* jk/skip-prefix: http-push: refactor parsing of remote object names imap-send: use skip_prefix instead of using magic numbers use skip_prefix to avoid repeated calculations git: avoid magic number with skip_prefix fetch-pack: refactor parsing in get_ack fast-import: refactor parsing of spaces stat_opt: check extra strlen call daemon: use skip_prefix to avoid magic numbers fast-import: use skip_prefix for parsing input use skip_prefix to avoid repeating strings use skip_prefix to avoid magic numbers transport-helper: avoid reading past end-of-string fast-import: fix read of uninitialized argv memory apply: use skip_prefix instead of raw addition refactor skip_prefix to return a boolean avoid using skip_prefix as a boolean daemon: mark some strings as const parse_diff_color_slot: drop ofs parameter
2014-07-02Merge branch 'jk/commit-buffer-length'Junio C Hamano1-2/+2
Move "commit->buffer" out of the in-core commit object and keep track of their lengths. Use this to optimize the code paths to validate GPG signatures in commit objects. * jk/commit-buffer-length: reuse cached commit buffer when parsing signatures commit: record buffer length in cache commit: convert commit->buffer to a slab commit-slab: provide a static initializer use get_commit_buffer everywhere convert logmsg_reencode to get_commit_buffer use get_commit_buffer to avoid duplicate code use get_cached_commit_buffer where appropriate provide helpers to access the commit buffer provide a helper to set the commit buffer provide a helper to free commit buffer sequencer: use logmsg_reencode in get_message logmsg_reencode: return const buffer do not create "struct commit" with xcalloc commit: push commit_index update into alloc_commit_node alloc: include any-object allocations in alloc_report replace dangerous uses of strbuf_attach commit_tree: take a pointer/len pair rather than a const strbuf
2014-06-25Merge branch 'jl/status-added-submodule-is-never-ignored' into maintJunio C Hamano1-2/+16
"git status" (and "git commit") behaved as if changes in a modified submodule are not there if submodule.*.ignore configuration is set, which was misleading. The configuration is only to unclutter diff output during the course of development, and should not to hide changes in the "status" output to cause the users forget to commit them. * jl/status-added-submodule-is-never-ignored: commit -m: commit staged submodules regardless of ignore config status/commit: show staged submodules regardless of ignore config
2014-06-25Merge branch 'jk/commit-C-pick-empty' into maintJunio C Hamano1-3/+2
"git commit --allow-empty-message -C $commit" did not work when the commit did not have any log message. * jk/commit-C-pick-empty: commit: do not complain of empty messages from -C
2014-06-20refactor skip_prefix to return a booleanJeff King1-3/+2
The skip_prefix() function returns a pointer to the content past the prefix, or NULL if the prefix was not found. While this is nice and simple, in practice it makes it hard to use for two reasons: 1. When you want to conditionally skip or keep the string as-is, you have to introduce a temporary variable. For example: tmp = skip_prefix(buf, "foo"); if (tmp) buf = tmp; 2. It is verbose to check the outcome in a conditional, as you need extra parentheses to silence compiler warnings. For example: if ((cp = skip_prefix(buf, "foo")) /* do something with cp */ Both of these make it harder to use for long if-chains, and we tend to use starts_with() instead. However, the first line of "do something" is often to then skip forward in buf past the prefix, either using a magic constant or with an extra strlen(3) (which is generally computed at compile time, but means we are repeating ourselves). This patch refactors skip_prefix() to return a simple boolean, and to provide the pointer value as an out-parameter. If the prefix is not found, the out-parameter is untouched. This lets you write: if (skip_prefix(arg, "foo ", &arg)) do_foo(arg); else if (skip_prefix(arg, "bar ", &arg)) do_bar(arg); Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-16Merge branch 'jl/status-added-submodule-is-never-ignored'Junio C Hamano1-2/+16
submodule.*.ignore and diff.ignoresubmodules are used to ignore all submodule changes in "diff" output, but it can be confusing to apply these configuration values to status and commit. This is a backward-incompatible change, but should be so in a good way (aka bugfix). * jl/status-added-submodule-is-never-ignored: commit -m: commit staged submodules regardless of ignore config status/commit: show staged submodules regardless of ignore config
2014-06-13read-cache: new API write_locked_index instead of write_index/write_cacheNguyễn Thái Ngọc Duy1-19/+14
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-12commit_tree: take a pointer/len pair rather than a const strbufJeff King1-2/+2
While strbufs are pretty common throughout our code, it is more flexible for functions to take a pointer/len pair than a strbuf. It's easy to turn a strbuf into such a pair (by dereferencing its members), but less easy to go the other way (you can strbuf_attach, but that has implications about memory ownership). This patch teaches commit_tree (and its associated callers and sub-functions) to take such a pair for the commit message rather than a strbuf. This makes passing the buffer around slightly more verbose, but means we can get rid of some dangerous strbuf_attach calls in the next patch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-06Merge branch 'nd/status-auto-comment-char'Junio C Hamano1-0/+32
* nd/status-auto-comment-char: commit: allow core.commentChar=auto for character auto selection config: be strict on core.commentChar
2014-06-06Merge branch 'fc/status-printf-squelch-format-zero-length-warnings'Junio C Hamano1-1/+1
* fc/status-printf-squelch-format-zero-length-warnings: silence a bunch of format-zero-length warnings
2014-06-06Merge branch 'jk/commit-C-pick-empty'Junio C Hamano1-3/+2
"git commit --allow-empty-message -C $commit" did not work when the commit did not have any log message. * jk/commit-C-pick-empty: commit: do not complain of empty messages from -C
2014-06-03Merge branch 'jk/commit-date-approxidate'Junio C Hamano1-19/+60
* jk/commit-date-approxidate: commit: accept more date formats for "--date" commit: print "Date" line when the user has set date pretty: make show_ident_date public commit: use split_ident_line to compare author/committer
2014-05-19commit: allow core.commentChar=auto for character auto selectionNguyễn Thái Ngọc Duy1-0/+32
When core.commentChar is "auto", the comment char starts with '#' as in default but if it's already in the prepared message, find another char in a small subset. This should stop surprises because git strips some lines unexpectedly. Note that git is not smart enough to recognize '#' as the comment char in custom templates and convert it if the final comment char is different. It thinks '#' lines in custom templates as part of the commit message. So don't use this with custom templates. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-05-07silence a bunch of format-zero-length warningsFelipe Contreras1-1/+1
This can be observed in many versions of gcc and still exists with 4.9.0: wt-status.c: In function ‘wt_status_print_unmerged_header’: wt-status.c:191:2: warning: zero-length gnu_printf format string [-Wformat-zero-length] status_printf_ln(s, c, ""); ^ The user have long been told to pass -Wno-format-zero-length, but a patch that avoids warning altogether is not too noisy, so let's do so. Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-05-02commit: accept more date formats for "--date"Jeff King1-2/+25
Right now we pass off the string found by "--date" straight to the fmt_ident function, which will use our strict parse_date to normalize it. However, this means obvious things like "--date=now" or "--date=2.days.ago" will not work. Instead, let's fallback to the approxidate function to handle this for us. Note that we must try parse_date ourselves first, even though approxidate will try strict parsing itself. The reason is that approxidate throws away any timezone information it sees from the strict parsing, and we want to preserve it. So asking for: git commit --date="@1234567890 -0700" continues to set the date in -0700, regardless of what the local timezone is. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-05-02commit: print "Date" line when the user has set dateJeff King1-0/+19
When we make a commit and the author is not the same as the committer (e.g., because you used "-c $commit" or "--author=$somebody"), we print the author's name and email in both the commit-message template and as part of the commit summary. This is a safety check to give the user a chance to confirm that we are doing what they expect. This patch brings the same safety for the "date" field, which may be set by "-c" or by using "--date". Note that we explicitly do not set it for $GIT_AUTHOR_DATE, as it is probably not of interest when "git commit" is being fed its parameters by a script. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-05-02commit: use split_ident_line to compare author/committerJeff King1-17/+16
Instead of string-wise comparing the author/committer lines with their timestamps truncated, we can use split_ident_line and ident_cmp. These functions are more robust than our ad-hoc parsing, though in practice it should not matter, as we just generated these ident lines ourselves. However, this will also allow us easy access to the timestamp and tz fields in future patches. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-04-28commit: do not complain of empty messages from -CJeff King1-3/+2
When we pick another commit's message, we die() immediately if we find that it's empty and we are not going to run an editor (i.e., when running "-C" instead of "-c"). However, this check is redundant and harmful. It's redundant because we will already notice the empty message later, after we would have run the editor, and die there (just as we would for a regular, not "-C" case, where the user provided an empty message in the editor). It's harmful for a few reasons: 1. It does not respect --allow-empty-message. As a result, a "git rebase -i" cannot "pick" such a commit. So you cannot even go back in time to fix it with a "reword" or "edit" instruction. 2. It does not take into account other ways besides the editor to modify the message. For example, "git commit -C empty-commit -m foo" could take the author information from empty-commit, but add a message to it. There's more to do to make that work correctly (and right now we explicitly forbid "-C with -m"), but this removes one roadblock. 3. The existing check is not enough to prevent segfaults. We try to find the "\n\n" header/body boundary in the commit. If it is at the end of the string (i.e., no body), _or_ if we cannot find it at all (i.e., a truncated commit object), we consider the message empty. With "-C", that's OK; we die in either case. But with "-c", we continue on, and in the case of a truncated commit may end up dereferencing NULL+2. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-04-17commit.c: check for lock error and return earlyRonnie Sahlberg1-4/+4
Move the check for the lock failure to happen immediately after lock_any_ref_for_update(). Previously the lock and the check-if-lock-failed was separated by a handful of string manipulation statements. Moving the check to occur immediately after the failed lock makes the code slightly easier to read and makes it follow the pattern of try-to-take-a-lock(); if (check-if-lock-failed) { error(); } Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-04-08Merge branch 'jl/nor-or-nand-and'Junio C Hamano1-1/+1
Eradicate mistaken use of "nor" (that is, essentially "nor" used not in "neither A nor B" ;-)) from in-code comments, command output strings, and documentations. * jl/nor-or-nand-and: code and test: fix misuses of "nor" comments: fix misuses of "nor" contrib: fix misuses of "nor" Documentation: fix misuses of "nor"
2014-04-08Merge branch 'jc/rev-parse-argh-dashed-multi-words'Junio C Hamano1-1/+1
Make sure that the help text given to describe the "<param>" part of the "git cmd --option=<param>" does not contain SP or _, e.g. "--gpg-sign=<key-id>" option for "git commit" is not spelled as "--gpg-sign=<key id>". * jc/rev-parse-argh-dashed-multi-words: parse-options: make sure argh string does not have SP or _ update-index: teach --cacheinfo a new syntax "mode,sha1,path" parse-options: multi-word argh should use dash to separate words
2014-04-07commit -m: commit staged submodules regardless of ignore configJens Lehmann1-2/+16
The previous commit fixed the problem that the staged but that ignored submodules did not show up in the status output of the commit command and weren't committed afterwards either. But when commit doesn't generate the status output (e.g. when used in a script with '-m') the ignored submodule will still not be committed. This is because in that case a different code path is taken which calls index_differs_from() instead of calling the wt_status functions. Fix that by calling index_differs_from() from builtin/commit.c with a diff_options argument value that tells it not ignore any submodule changes unless the '--ignore-submodules' option is used. Even though this option isn't yet implemented for cmd_commit() but only for cmd_status() this prepares cmd_commit() to correctly handle the '--ignore-submodules' option later. As status and commit share the same ignore_submodule_arg variable this makes the code more robust against accidental breakage and documents how to correctly call index_differs_from(). Change the expected result of the test documenting this problem from failure to success. Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-03-31code and test: fix misuses of "nor"Justin Lebar1-1/+1
Signed-off-by: Justin Lebar <jlebar@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-03-28Merge branch 'bp/commit-p-editor'Junio C Hamano1-7/+28
When it is not necessary to edit a commit log message (e.g. "git commit -m" is given a message without specifying "-e"), we used to disable the spawning of the editor by overriding GIT_EDITOR, but this means all the uses of the editor, other than to edit the commit log message, are also affected. * bp/commit-p-editor: run-command: mark run_hook_with_custom_index as deprecated merge hook tests: fix and update tests merge: fix GIT_EDITOR override for commit hook commit: fix patch hunk editing with "commit -p -m" test patch hunk editing with "commit -p -m" merge hook tests: use 'test_must_fail' instead of '!' merge hook tests: fix missing '&&' in test
2014-03-25Merge branch 'nd/commit-editor-cleanup'Junio C Hamano1-3/+11
"git commit --cleanup=<mode>" learned a new mode, scissors. * nd/commit-editor-cleanup: commit: add --cleanup=scissors wt-status.c: move cut-line print code out to wt_status_add_cut_line wt-status.c: make cut_line[] const to shrink .data section a bit
2014-03-24parse-options: multi-word argh should use dash to separate wordsJunio C Hamano1-1/+1
"When you need to use space, use dash" is a strange way to say that you must not use a space. Because it is more common for the command line descriptions to use dashed-multi-words, you do not even want to use spaces in these places. Rephrase the documentation to avoid this strangeness. Fix a few existing multi-word argument help strings, i.e. - GPG key-ids given to -S/--gpg-sign are "key-id"; - Refs used for storing notes are "notes-ref"; and - Expiry timestamps given to --expire are "expiry-date". and update the corresponding documentation pages. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-03-18commit: fix patch hunk editing with "commit -p -m"Benoit Pierre1-7/+28
Don't change git environment: move the GIT_EDITOR=":" override to the hook command subprocess, like it's already done for GIT_INDEX_FILE. Signed-off-by: Benoit Pierre <benoit.pierre@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-02-27Merge branch 'ep/varscope'Junio C Hamano1-5/+6
Shrink lifetime of variables by moving their definitions to an inner scope where appropriate. * ep/varscope: builtin/gc.c: reduce scope of variables builtin/fetch.c: reduce scope of variable builtin/commit.c: reduce scope of variables builtin/clean.c: reduce scope of variable builtin/blame.c: reduce scope of variables builtin/apply.c: reduce scope of variables bisect.c: reduce scope of variable
2014-02-27Merge branch 'nd/submodule-pathspec-ending-with-slash'Junio C Hamano1-1/+1
Allow "git cmd path/", when the 'path' is where a submodule is bound to the top-level working tree, to match 'path', despite the extra and unnecessary trailing slash. * nd/submodule-pathspec-ending-with-slash: clean: use cache_name_is_other() clean: replace match_pathspec() with dir_path_match() pathspec: pass directory indicator to match_pathspec_item() match_pathspec: match pathspec "foo/" against directory "foo" dir.c: prepare match_pathspec_item for taking more flags pathspec: rename match_pathspec_depth() to match_pathspec() pathspec: convert some match_pathspec_depth() to dir_path_match() pathspec: convert some match_pathspec_depth() to ce_path_match()
2014-02-27Merge branch 'nv/commit-gpgsign-config'Junio C Hamano1-0/+4
Introduce commit.gpgsign configuration variable to force every commit to be GPG signed. The variable cannot be overriden from the command line of some of the commands that create commits except for "git commit" and "git commit-tree", but I am not convinced that it is a good idea to sprinkle support for --no-gpg-sign everywhere, which in turn means that this configuration variable may not be such a good idea. * nv/commit-gpgsign-config: test the commit.gpgsign config option commit-tree: add and document --no-gpg-sign commit-tree: add the commit.gpgsign option to sign all commits
2014-02-25commit: add --cleanup=scissorsNguyễn Thái Ngọc Duy1-3/+11
Since 1a72cfd (commit -v: strip diffs and submodule shortlogs from the commit message - 2013-12-05) we have a less fragile way to cut out "git status" at the end of a commit message but it's only enabled for stripping submodule shortlogs. Add new cleanup option that reuses the same mechanism for the entire "git status" without accidentally removing lines starting with '#'. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-02-24commit-tree: add the commit.gpgsign option to sign all commitsNicolas Vigier1-0/+4
If you want to GPG sign all your commits, you have to add the -S option all the time. The commit.gpgsign config option allows to sign all commits automatically. Signed-off-by: Nicolas Vigier <boklm@mars-attacks.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-02-24pathspec: convert some match_pathspec_depth() to ce_path_match()Nguyễn Thái Ngọc Duy1-1/+1
This helps reduce the number of match_pathspec_depth() call sites and show how match_pathspec_depth() is used. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-01-31builtin/commit.c: reduce scope of variablesElia Pinto1-5/+6
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-12-17Merge branch 'cc/starts-n-ends-with'Junio C Hamano1-5/+5
Remove a few duplicate implementations of prefix/suffix comparison functions, and rename them to starts_with and ends_with. * cc/starts-n-ends-with: replace {pre,suf}fixcmp() with {starts,ends}_with() strbuf: introduce starts_with() and ends_with() builtin/remote: remove postfixcmp() and use suffixcmp() instead environment: normalize use of prefixcmp() by removing " != 0"
2013-12-17Merge branch 'jl/commit-v-strip-marker'Junio C Hamano1-6/+3
"git commit -v" appends the patch to the log message before editing, and then removes the patch when the editor returned control. However, the patch was not stripped correctly when the first modified path was a submodule. * jl/commit-v-strip-marker: commit -v: strip diffs and submodule shortlogs from the commit message
2013-12-05commit -v: strip diffs and submodule shortlogs from the commit messageJens Lehmann1-6/+3
When using the '-v' option of "git commit" the diff added to the commit message temporarily for editing is stripped off after the user exited the editor by searching for "\ndiff --git " and truncating the commmit message there if it is found. But this approach has two problems: - when the commit message itself contains a line starting with "diff --git" it will be truncated there prematurely; and - when the "diff.submodule" setting is set to "log", the diff may start with "Submodule <hash1>..<hash2>", which will be left in the commit message while it shouldn't. Fix that by introducing a special scissor separator line starting with the comment character ('#' or the core.commentChar config if set) followed by two lines describing what it is for. The scissor line - which will not be translated - is used to reliably detect the start of the diff so it can be chopped off from the commit message, no matter what the user enters there. Turn a known test failure fixed by this change into a successful test; also add one for a diff starting with a submodule log and another one for proper handling of the comment char. Reported-by: Ari Pollak <ari@debian.org> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-12-05replace {pre,suf}fixcmp() with {starts,ends}_with()Christian Couder1-5/+5
Leaving only the function definitions and declarations so that any new topic in flight can still make use of the old functions, replace existing uses of the prefixcmp() and suffixcmp() with new API functions. The change can be recreated by mechanically applying this: $ git grep -l -e prefixcmp -e suffixcmp -- \*.c | grep -v strbuf\\.c | xargs perl -pi -e ' s|!prefixcmp\(|starts_with\(|g; s|prefixcmp\(|!starts_with\(|g; s|!suffixcmp\(|ends_with\(|g; s|suffixcmp\(|!ends_with\(|g; ' on the result of preparatory changes in this series. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-12-05Merge branch 'jk/robustify-parse-commit'Junio C Hamano1-2/+2
* jk/robustify-parse-commit: checkout: do not die when leaving broken detached HEAD use parse_commit_or_die instead of custom message use parse_commit_or_die instead of segfaulting assume parse_commit checks for NULL commit assume parse_commit checks commit->object.parsed log_tree_diff: die when we fail to parse a commit
2013-10-24assume parse_commit checks for NULL commitJeff King1-2/+2
The parse_commit function will check whether it was passed a NULL commit pointer, and if so, return an error. There is no need for callers to check this separately. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-09-20Merge branch 'mm/commit-template-squelch-advice-messages'Junio C Hamano1-8/+17
From the commit log template, remove irrelevant "advice" messages that are shared with "git status" output. * mm/commit-template-squelch-advice-messages: commit: disable status hints when writing to COMMIT_EDITMSG wt-status: turn advice_status_hints into a field of wt_status commit: factor status configuration is a helper function