There are "git checkout [-p][<tree-ish>][--][<paths>...]" in the
SYNOPSIS section, and "git checkout [-p][<tree-ish>][--]<paths>..."
as the header for the section that explains the "check out paths
from index/tree-ish" mode. It is unclear if we require at least one
path, or it is entirely optional.
Actually, both are wrong. Without the "-p(atch)" option, you must
have <pathspec> (otherwise, with a commit that is a <tree-ish>, you
would be checking out that commit to build a new history on top of
it). With it, it is already clear that you are checking out paths,
it is optional. In other words, you cannot omit both.
The source of the confusion is that -p(atch) is described as if it
is just another "optional" part and its description is lumped
together with the non patch mode, even though the actual end user
experience is vastly different.
Let's split the entry into two, and describe the regular mode and
the patch mode separately. This allows us to make it clear that the
regular mode MUST be given at least one pathspec, that the patch
mode can be invoked with either '-p' or '--patch' but one of these
must be given, and that the pathspec is entirely optional in the
patch mode.
Also, revamp the explanation of "checkout paths" by removing
extraneous description at the beginning, that says "checking out
paths is not checking out a branch". Explaining what it is for and
when the user wants to use it upfront is the most direct way to help
the readers.
Noticed-by: Robert P J Day
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Bugfixes to an already graduated series.
* ls/filter-process-delayed:
write_entry: untangle symlink and regular-file cases
write_entry: avoid reading blobs in CE_RETRY case
write_entry: fix leak when retrying delayed filter
entry.c: check if file exists after checkout
entry.c: update cache entry only for existing files
An ancient bug that made Git misbehave with creation/renaming of
refs has been fixed.
* jk/refs-df-conflict:
refs_resolve_ref_unsafe: handle d/f conflicts for writes
t3308: create a real ref directory/file conflict
Improve behaviour of "git fsck" upon finding a missing object.
* rs/fsck-null-return-from-lookup:
fsck: handle NULL return of lookup_blob() and lookup_tree()
"git for-each-ref --format=..." learned a new format element,
%(trailers), to show only the commit log trailer part of the log
message.
* tb/show-trailers-in-ref-filter:
ref-filter.c: parse trailers arguments with %(contents) atom
ref-filter.c: use trailer_opts to format trailers
t6300: refactor %(trailers) tests
doc: use "`<literal>`"-style quoting for literal strings
doc: 'trailers' is the preferred way to format trailers
t4205: unfold across multiple lines
There are times when e.g. scripts want to know not only the name of the
upstream branch on the remote repository, but also the name of the
remote.
This patch offers the new suffix :remotename for the upstream and for
the push atoms, allowing to show exactly that. Example:
$ cat .git/config
...
[remote "origin"]
url = https://where.do.we.come/from
fetch = refs/heads/*:refs/remote/origin/*
[remote "hello-world"]
url = https://hello.world/git
fetch = refs/heads/*:refs/remote/origin/*
pushURL = hello.world:git
push = refs/heads/*:refs/heads/*
[branch "master"]
remote = origin
pushRemote = hello-world
...
$ git for-each-ref \
--format='%(upstream) %(upstream:remotename) %(push:remotename)' \
refs/heads/master
refs/remotes/origin/master origin hello-world
The implementation chooses *not* to DWIM the push remote if no explicit
push remote was configured; The reason is that it is possible to DWIM this
by using
%(if)%(push:remotename)%(then)
%(push:remotename)
%(else)
%(upstream:remotename)
%(end)
while it would be impossible to "un-DWIM" the information in case the
caller is really only interested in explicit push remotes.
While `:remote` would be shorter, it would also be a bit more ambiguous,
and it would also shut the door e.g. for `:remoteref` (which would
obviously refer to the corresponding ref in the remote repository).
Note: the dashless, non-CamelCased form `:remotename` follows the
example of the `:trackshort` example.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When an hook is present but the file is not set as executable then git will
ignore the hook.
For now this is silent which can be confusing.
This commit adds this warning to improve the situation:
hint: The 'pre-commit' hook was ignored because it's not set as executable.
hint: You can disable this warning with `git config advice.ignoredHook false`
To allow the old use-case of enabling/disabling hooks via the executable flag a
new setting is introduced: advice.ignoredHook.
Signed-off-by: Damien Marié <damien@dam.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The write_entry() function switches on the mode of the entry
we're going to write out. The cases for S_IFLNK and S_IFREG
are lumped together. In earlier versions of the code, this
made some sense. They have a shared preamble (which reads
the blob content), a short type-specific body, and a shared
conclusion (which writes out the file contents; always for
S_IFREG and only sometimes for S_IFLNK).
But over time this has grown to make less sense. The preamble
now has conditional bits for each type, and the S_IFREG body
has grown a lot more complicated. It's hard to follow the
logic of which code is running for which mode.
Let's give each mode its own case arm. We will still share
the conclusion code, which means we now jump to it with a
goto. Ideally we'd pull that shared code into its own
function, but it touches so much internal state in the
write_entry() function that the end result is actually
harder to follow than the goto.
While we're here, we'll touch up a few bits of whitespace to
make the beginning and endings of the cases easier to read.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When retrying a delayed filter-process request, we don't
need to send the blob to the filter a second time. However,
we read it unconditionally into a buffer, only to later
throw away that buffer. We can make this more efficient by
skipping the read in the first place when it isn't
necessary.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When write_entry() retries a delayed filter request, we
don't need to send the blob content to the filter again, and
set the pointer to NULL. But doing so means we leak the
contents we read earlier from read_blob_entry(). Let's make
sure to free it before dropping the pointer.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A common mistake when writing binary search is to allow possible
integer overflow by using the simple average:
mid = (min + max) / 2;
Instead, use the overflow-safe version:
mid = min + (max - min) / 2;
This translation is safe since the operation occurs inside a loop
conditioned on "min < max". The included changes were found using
the following git grep:
git grep '/ *2;' '*.c'
Making this cleanup will prevent future review friction when a new
binary search is contructed based on existing code.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The message spans over 2 lines but the C conconcatenation does not add
the needed space between the two lines.
Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current implementation of submodules supports on-demand fetch if
there is no .gitmodules entry for a submodule. Let's add a test to
document this behavior.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This aims to make git-submodule 'status' a built-in. Hence, the function
cmd_status() is ported from shell to C. This is done by introducing
four functions: module_status(), submodule_status_cb(),
submodule_status() and print_status().
The function module_status() acts as the front-end of the subcommand.
It parses subcommand's options and then calls the function
module_list_compute() for computing the list of submodules. Then
this functions calls for_each_listed_submodule() looping through the
list obtained.
Then for_each_listed_submodule() calls submodule_status_cb() for each of
the submodule in its list. The function submodule_status_cb() calls
submodule_status() after passing appropriate arguments to the funciton.
Function submodule_status() is responsible for generating the status
each submodule it is called for, and then calls print_status().
Finally, the function print_status() handles the printing of submodule's
status.
Function set_name_rev() is also ported from git-submodule to the
submodule--helper builtin function compute_rev_name(), which now
generates the value of the revision name as required.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Introduce function for_each_listed_submodule() and replace a loop
in module_init() with a call to it.
The new function will also be used in other parts of the
system in later patches.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the "--format=..." option of the "git for-each-ref" command (and
its friends, i.e. the listing mode of "git branch/tag"), "%(atom:)"
(e.g. "%(refname:)", "%(body:)" used to error out. Instead, treat
them as if the colon and an empty string that follows it were not
there.
* tb/ref-filter-empty-modifier:
ref-filter.c: pass empty-string as NULL to atom parsers
Fix regression of "git add -p" for users with "color.ui = always"
in their configuration, by merging the topic below and adjusting it
for the 'master' front.
* jk/ui-color-always-to-auto:
t7301: use test_terminal to check color
t4015: use --color with --color-moved
color: make "always" the same as "auto" in config
provide --color option for all ref-filter users
t3205: use --color instead of color.branch=always
t3203: drop "always" color test
t6006: drop "always" color config tests
t7502: use diff.noprefix for --verbose test
t7508: use test_terminal for color output
t3701: use test-terminal to collect color output
t4015: prefer --color to -c color.diff=always
test-terminal: set TERM=vt100
Many variables that points at a region of memory that will live
throughout the life of the program have been marked with UNLEAK
marker to help the leak checkers concentrate on real leaks..
* ma/builtin-unleak:
builtin/: add UNLEAKs
Backports a moral equivalent of 2015 fix to the poll emulation from
the upstream gnulib to fix occasional breakages on HPE NonStop.
* rb/compat-poll-fix:
poll.c: always set revents, even if to zero
Fixes for a handful memory access issues identified by valgrind.
* tg/memfixes:
sub-process: use child_process.args instead of child_process.argv
http-push: fix construction of hex value from path
path.c: fix uninitialized memory access
The feature that allows --pretty='%(trailers)' to take modifiers
like "fold" and "only" used to separate these modifiers with a
comma, i.e. "%(trailers:fold:only)", but we changed our mind and
use a comma, i.e. "%(trailers:fold,only)". Fast track this change
before this new feature becomes part of any official release.
* tb/delimit-pretty-trailers-args-with-comma:
pretty.c: delimit "%(trailers)" arguments with ","
When the flags for broken and dirty were implemented in
b0176ce6b5 (builtin/describe: introduce --broken flag, 2017-03-21)
and 9f67d2e827 (Teach "git describe" --dirty option, 2009-10-21)
the completion was not updated, although these flags are useful
completions. Add them.
Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
Helped-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix the argument order for test_cmp. When given the expected
result first the diff shows the actual output with '+' and the
expectation with '-', which is the convention for our tests.
Signed-off-by: Stefan Beller <sbeller@google.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If our call to refs_read_raw_ref() fails, we check errno to
see if the ref is simply missing, or if we encountered a
more serious error. If it's just missing, then in "write"
mode (i.e., when RESOLVE_REFS_READING is not set), this is
perfectly fine.
However, checking for ENOENT isn't sufficient to catch all
missing-ref cases. In the filesystem backend, we may also
see EISDIR when we try to resolve "a" and "a/b" exists.
Likewise, we may see ENOTDIR if we try to resolve "a/b" and
"a" exists. In both of those cases, we know that our
resolved ref doesn't exist, but we return an error (rather
than reporting the refname and returning a null sha1).
This has been broken for a long time, but nobody really
noticed because the next step after resolving without the
READING flag is usually to lock the ref and write it. But in
both of those cases, the write will fail with the same
errno due to the directory/file conflict.
There are two cases where we can notice this, though:
1. If we try to write "a" and there's a leftover directory
already at "a", even though there is no ref "a/b". The
actual write is smart enough to move the empty "a" out
of the way.
This is reasonably rare, if only because the writing
code has to do an independent resolution before trying
its write (because the actual update_ref() code handles
this case fine). The notes-merge code does this, and
before the fix in the prior commit t3308 erroneously
expected this case to fail.
2. When resolving symbolic refs, we typically do not use
the READING flag because we want to resolve even
symrefs that point to unborn refs. Even if those unborn
refs could not actually be written because of d/f
conflicts with existing refs.
You can see this by asking "git symbolic-ref" to report
the target of a symref pointing past a d/f conflict.
We can fix the problem by recognizing the other "missing"
errnos and treating them like ENOENT. This should be safe to
do even for callers who are then going to actually write the
ref, because the actual writing process will fail if the d/f
conflict is a real one (and t1404 checks these cases).
Arguably this should be the responsibility of the
files-backend to normalize all "missing ref" errors into
ENOENT (since something like EISDIR may not be meaningful at
all to a database backend). However other callers of
refs_read_raw_ref() may actually care about the distinction;
putting this into resolve_ref() is the minimal fix for now.
The new tests in t1401 use git-symbolic-ref, which is the
most direct way to check the resolution by itself.
Interestingly we actually had a test that setup this case
already, but we only used it to verify that the funny state
could be overwritten, not that it could be resolved.
We also add a new test in t3200, as "branch -m" was the
original motivation for looking into this. What happens is
this:
0. HEAD is pointing to branch "a"
1. The user asks to rename "a" to "a/b".
2. We create "a/b" and delete "a".
3. We then try to update any worktree HEADs that point to
the renamed ref (including the main repo HEAD). To do
that, we have to resolve each HEAD. But now our HEAD is
pointing at "a", and we get EISDIR due to the loose
"a/b". As a result, we think there is no HEAD, and we
do not update it. It now points to the bogus "a".
Interestingly this case used to work, but only accidentally.
Before 31824d180d (branch: fix branch renaming not updating
HEADs correctly, 2017-08-24), we'd update any HEAD which we
couldn't resolve. That was wrong, but it papered over the
fact that we were incorrectly failing to resolve HEAD.
So while the bug demonstrated by the git-symbolic-ref is
quite old, the regression to "branch -m" is recent.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A test in t3308 wants to make sure that we don't
accidentally merge into "refs/notes/dir" when it exists as a
directory, so it does:
mkdir .git/refs/notes/dir
git -c core.notesRef=refs/notes/dir merge ...
and expects the second command to fail. But that
understimates the refs code, which is smart enough to remove
useless directories in the refs hierarchy. The test
succeeded only because of a bug which prevented resolving
refs/notes/dir for writing, even though an actual ref update
would succeed.
In preparation for fixing that bug, let's switch to creating
a real ref in refs/notes/dir, which is a more realistic
situation.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`update_index_if_able()` used to always commit the lock or roll it back.
Commit 03b866477 (read-cache: new API write_locked_index instead of
write_index/write_cache, 2014-06-13) stopped rolling it back in case a
write was not even attempted. This change in behavior is not motivated
in the commit message and appears to be accidental: the `else`-path was
removed, although that changed the behavior in case the `if` shortcuts.
Reintroduce the rollback and document this behavior. While at it, move
the documentation on this function from the function definition to the
function declaration in cache.h.
If `write_locked_index(..., COMMIT_LOCK)` fails, it will roll back the
lock for us (see the previous commit).
Noticed-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If the original version of `write_locked_index()` returned with an
error, it didn't roll back the lockfile unless the error occured at the
very end, during closing/committing. See commit 03b866477 (read-cache:
new API write_locked_index instead of write_index/write_cache,
2014-06-13).
In commit 9f41c7a6b (read-cache: close index.lock in do_write_index,
2017-04-26), we learned to close the lock slightly earlier in the
callstack. That was mostly a side-effect of lockfiles being implemented
using temporary files, but didn't cause any real harm.
Recently, commit 076aa2cbd (tempfile: auto-allocate tempfiles on heap,
2017-09-05) introduced a subtle bug. If the temporary file is deleted
(i.e., the lockfile is rolled back), the tempfile-pointer in the `struct
lock_file` will be left dangling. Thus, an attempt to reuse the
lockfile, or even just to roll it back, will induce undefined behavior
-- most likely a crash.
Besides not crashing, we clearly want to make things consistent. The
guarantees which the lockfile-machinery itself provides is A) if we ask
to commit and it fails, roll back, and B) if we ask to close and it
fails, do _not_ roll back. Let's do the same for consistency.
Do not delete the temporary file in `do_write_index()`. One of its
callers, `write_locked_index()` will thereby avoid rolling back the
lock. The other caller, `write_shared_index()`, will delete its
temporary file anyway. Both of these callers will avoid undefined
behavior (crashing).
Teach `write_locked_index(..., COMMIT_LOCK)` to roll back the lock
before returning. If we have already succeeded and committed, it will be
a noop. Simplify the existing callers where we now have a superfluous
call to `rollback_lockfile()`. That should keep future readers from
wondering why the callers are inconsistent.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`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>
In 4f665f2cf3 (string-list.h: move documentation from Documentation/api/
into header, 2017-09-26) the string-list API documentation was moved to
string-list.h. The argv-array API documentation may follow a similar
course in the future. Until then, prevent the broken link from making
it to the end-user documentation.
Signed-off-by: Todd Zullinger <tmz@pobox.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
