Instead of having it as one of the three if/elseif/.. case arms,
test the condition and handle this special case upfront. This makes
it easier to follow the flow of logic.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
One of the first things cmd_merge() does is to see if the "--abort"
option is given and run "reset --merge" and exit. When the control
reaches this point, we know "--abort" was not given.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A broken or badly formatted commit might not record author or
committer lines or we may not find a valid name on them. The
function record_person() returned after calling get_commit_buffer()
without calling unuse_commit_buffer() on the memory it obtained in
such cases, potentially leaking it.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Once we know the number of objects in the input pack, we allocate an
array of nr_objects of struct delta_entry. On x86-64, this struct is
32 bytes long. The union delta_base, which is part of struct
delta_entry, provides enough space to store either ofs-delta (8 bytes)
or ref-delta (20 bytes).
Because ofs-delta encoding is more efficient space-wise and more
performant at runtime than ref-delta encoding, Git packers try to use
ofs-delta whenever possible, and it is expected that objects encoded
as ref-delta are minority.
In the best clone case where no ref-delta object is present, we waste
(20-8) * nr_objects bytes because of this union. That's about 38MB out
of 100MB for deltas[] with 3.4M objects, or 38%. deltas[] would be
around 62MB without the waste.
This patch attempts to eliminate that. deltas[] array is split into
two: one for ofs-delta and one for ref-delta. Many functions are also
duplicated because of this split. With this patch, ofs_deltas[] array
takes 51MB. ref_deltas[] should remain unallocated in clone case (0
bytes). This array grows as we see ref-delta. We save about half in
this case, or 25% of total bookkeeping.
The saving is more than the calculation above because some padding in
the old delta_entry struct is removed. ofs_delta_entry is 16 bytes,
including the 4 bytes padding. That's 13MB for padding, but packing
the struct could break platforms that do not support unaligned
access. If someone on 32-bit is really low on memory and only deals
with packs smaller than 2G, using 32-bit off_t would eliminate the
padding and save 27MB on top.
A note about ofs_deltas allocation. We could use ref_deltas memory
allocation strategy for ofs_deltas. But that probably just adds more
overhead on top. ofs-deltas are generally the majority (1/2 to 2/3) in
any pack. Incremental realloc may lead to too many memcpy. And if we
preallocate, say 1/2 or 2/3 of nr_objects initially, the growth rate
of ALLOC_GROW() could make this array larger than nr_objects, wasting
more memory.
Brought-up-by: Matthew Sporleder <msporleder@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The name (not user) and email setting should be in config section
"user" and not in "core" as documented in Documentation/config.txt.
Signed-off-by: Ossi Herrala <oherrala@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This reverts commit 2bf15a3330, whose
intention was good, but the verbosity levels used in merge-recursive
turns out to be rather uneven. For example, a merge of two branches
with conflicting submodule updates used to report CONFLICT: output
with --quiet but no longer (which *is* desired), while the final
"Automatic merge failed; fix conflicts and then commit" message is
still shown even with --quiet (which *is* inconsistent).
Originally reported by Bryan Turner; it is too early to declare what
the concensus is, but it seems that we would need to level the
verbosity levels used in merge strategy backends before we can go
forward. In the meantime, we'd revert to the old behaviour until
that happens.
cf. $gmane/267245
"git merge --quiet" did not squelch messages from the underlying
merge-recursive strategy.
* jk/merge-quiet:
merge: pass verbosity flag down to merge-recursive
A push into an unborn branch, with "receive.denyCurrentBranch" set
to "updateInstead", did not check out the working tree as expected.
* jc/update-instead-into-void:
push-to-deploy: allow pushing into an unborn branch and updating it
The "help-all" option is being initialized with a wrong value.
While being semantically wrong this can also cause a segmentation
fault in gcc on ARMv7 hardfloat platforms with a hardened
toolchain. Fix this by initializing with a NULL value.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When detached and checking out a branch again, git checkout warns
about commit(s) that might get lost. It says "If you want to keep
them ..." even for only one commit.
Use Q_() to allow differentiating singular vs plural.
Signed-off-by: Thomas Schneider <thosch97@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This makes "git merge --quiet" really quiet when we call
into merge-recursive.
Note that we can't just pass our flag down as-is; the two
parts of the code use different scales. We center at "0" as
normal for git-merge (with "--quiet" giving a negative
value), but merge-recursive uses "2" as its center. This
patch passes a negative value to merge-recursive rather than
"1", though, as otherwise the user would have to use "-qqq"
to squelch all messages (but the downside is that the user
cannot distinguish between levels 0-2 if without resorting
to the GIT_MERGE_VERBOSITY variable).
We may want to review and renormalize the message severities
in merge-recursive, but that does not have to happen now.
This is at least in improvement in the sense that we are
respecting "--quiet" at all.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If you create a git repository in the root directory with
"git init /", we erroneously write a core.worktree entry.
This isn't _wrong_, in the sense that it's OK to set
core.worktree when we don't need to. But it is unnecessarily
surprising if you later move the .git directory to another
path (which usually moves the relative working tree, but is
foiled if there is an explicit worktree set).
The problem is that we check whether core.worktree is
necessary by seeing if we can make the git_dir by
concatenating "/.git" onto the working tree. That would lead
to "//.git" in this instance, but we actually have "/.git"
(without the doubled slash).
We can fix this by special-casing the root directory. I also
split the logic out into its own function to make the
conditional a bit more readable (and used skip_prefix, which
I think makes it a little more obvious what is going on).
No tests, as we would need to be able to write to "/" to do
so. I did manually confirm that:
sudo git init /
cd /
git rev-parse --show-toplevel
git config core.worktree
still finds the top-level correctly (as "/"), and does not
set any core.worktree variable.
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Setting receive.denycurrentbranch to updateinstead and pushing into
the current branch, when the working tree and the index is truly
clean, is supposed to reset the working tree and the index to match
the tree of the pushed commit. This did not work when pushing into
an unborn branch.
The code that drives push-to-checkout hook needs no change, as the
interface is defined so that hook can decide what to do when the
push is coming to an unborn branch and take an appropriate action
since the beginning.
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git show-branch --topics <rev> <revs>..." displays ancestry graph, only
considering commits that are in all given revs, except the first one.
"git show-branch" displays ancestry graph for all local branches.
Unfortunately, "git show-branch --topics <rev>" only prints out the rev
info for the given rev, and nothing else, e.g.:
$ git show-branch --topics origin/master
[origin/master] Sync with 2.3.3
While there is an option to add all remote-tracking branches (-r), and
another to add all local+remote branches (-a), there is no option to add
only local branches. Adding such an option could be considered, but a
user would likely already expect that the above command line considers
the lack of rev other than for --topics as meaning all local branches,
like when there is no argument at all.
Moreover, when using -r and -a along with --topics, the first local or
remote-tracking branch, depending on alphabetic order is used instead of
the one given after --topics (any rev given on the command line is
actually simply ignored when either -r or -a is given). And if no rev is
given at all, the fact that the first alphetical branch is the base of
topics is probably not expected by users (Maybe --topics should always
require one rev on the command line?)
This change makes
"show-branch --topics $rev"
act as
"show-branch --topics $rev $(git for-each-ref refs/heads
--format='%(refname:short)')"
"show-branch -r --topics $rev ..."
act as
"show-branch --topics $rev ... $(git for-each-ref refs/remotes
--format='%(refname:short)')"
instead of
"show-branch --topics $(git for-each-ref refs/remotes
--format='%(refname:short)')"
and
"show-branch -a --topics $rev ..."
act as
"show-branch --topics $rev ... $(git for-each-ref refs/heads refs/remotes
--format='%(refname:short)')"
instead of
"show-branch --topics $(git for-each-ref refs/heads refs/remotes
--format='%(refname:short)')"
Signed-off-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`git prune --worktrees` was pruning worktrees which were non-existent OR
expired, while it rather should prune those which are orphaned AND
expired, as git-checkout documentation describes. Fix it.
Add test 'not prune proper checkouts', which uses valid but expired
worktree.
Modify test 'not prune recent checkouts' to remove the worktree before
pruning - link in worktrees still must survive. In older form it is
useless because would pass always when the other test passes.
Signed-off-by: Max Kirillov <max@max630.net>
Acked-by: Duy Nguyen <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we find an object in a packfile index, we make sure we
can still open the packfile itself (or that it is already
open), as it might have been deleted by a simultaneous
repack. If we can't access the packfile, we print a warning
for the user and tell the caller that we don't have the
object (we can then look in other packfiles, or find a loose
version, before giving up).
The warning we print to the user isn't really accomplishing
anything, and it is potentially confusing to users. In the
normal case, it is complete noise; we find the object
elsewhere, and the user does not have to care that we racily
saw a packfile index that became stale. It didn't affect the
operation at all.
A possibly more interesting case is when we later can't find
the object, and report failure to the user. In this case the
warning could be considered a clue toward that ultimate
failure. But it's not really a useful clue in practice. We
wouldn't even print it consistently (since we are racing
with another process, we might not even see the .idx file,
or we might win the race and open the packfile, completing
the operation).
This patch drops the warning entirely (not only from the
fill_pack_entry site, but also from an identical use in
pack-objects). If we did find the warning interesting in the
error case, we could stuff it away and reveal it to the user
when we later die() due to the broken object. But that
complexity just isn't worth it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Even though "git grep --quiet" is run merely to ask for the exit
status, we spawned the pager regardless. Stop doing that.
* ws/grep-quiet-no-pager:
grep: fix "--quiet" overwriting current output
An failure early in the "git clone" that started creating the
working tree and repository could have resulted in some directories
and files left without getting cleaned up.
* jk/cleanup-failed-clone:
clone: drop period from end of die_errno message
clone: initialize atexit cleanup handler earlier
"git prune" used to largely ignore broken refs when deciding which
objects are still being used, which could spread an existing small
damage and make it a larger one.
* jk/prune-with-corrupt-refs:
refs.c: drop curate_packed_refs
repack: turn on "ref paranoia" when doing a destructive repack
prune: turn on ref_paranoia flag
refs: introduce a "ref paranoia" flag
t5312: test object deletion code paths in a corrupted repository
* sb/leaks:
http: release the memory of a http pack request as well
read-cache: fix memleak
add_to_index(): free unused cache-entry
commit.c: fix a memory leak
http-push: remove unneeded cleanup
merge-recursive: fix memleaks
merge-blobs.c: fix a memleak
builtin/apply.c: fix a memleak
update-index: fix a memleak
read-cache: free cache entry in add_to_index in case of early return
"git tag -h" used to show the "--column" and "--sort" options
that are about listing in a wrong section.
* jk/tag-h-column-is-a-listing-option:
tag: fix some mis-organized options in "-h" listing
"git prune" used to largely ignore broken refs when deciding which
objects are still being used, which could spread an existing small
damage and make it a larger one.
* jk/prune-with-corrupt-refs:
refs.c: drop curate_packed_refs
repack: turn on "ref paranoia" when doing a destructive repack
prune: turn on ref_paranoia flag
refs: introduce a "ref paranoia" flag
t5312: test object deletion code paths in a corrupted repository
An failure early in the "git clone" that started creating the
working tree and repository could have resulted in some directories
and files left without getting cleaned up.
* jk/cleanup-failed-clone:
clone: drop period from end of die_errno message
clone: initialize atexit cleanup handler earlier
Even though "git grep --quiet" is run merely to ask for the exit
status, we spawned the pager regardless. Stop doing that.
* ws/grep-quiet-no-pager:
grep: fix "--quiet" overwriting current output
The expected call sequence is for the caller to use match_pathspec()
repeatedly on a set of pathspecs, accumulating the "hits" in a
separate array, and then call this function to diagnose a pathspec
that never matched anything, as that can indicate a typo from the
command line, e.g. "git commit Maekfile".
Many builtin commands use this function from builtin/ls-files.c,
which is not a very healthy arrangement. ls-files might have been
the first command to feel the need for such a helper, but the need
is shared by everybody who uses the "match and then report" pattern.
Move it to dir.c where match_pathspec() is defined.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Entries added by "git add -N" are reminder for the user so that they
don't forget to add them before committing. These entries appear in
the index even though they are not real. Their presence in the index
leads to a confusing "git status" like this:
On branch master
Changes to be committed:
new file: foo
Changes not staged for commit:
modified: foo
If you do a "git commit", "foo" will not be included even though
"status" reports it as "to be committed". This patch changes the
output to become
On branch master
Changes not staged for commit:
new file: foo
no changes added to commit
The two hunks in diff-lib.c adjust "diff-index" and "diff-files" so
that i-t-a entries appear as new files in diff-files and nothing in
diff-index.
Due to this change, diff-files may start to report "new files" for the
first time. "add -u" needs to be told about this or it will die in
denial, screaming "new files can't exist! Reality is wrong." Luckily,
it's the only one among run_diff_files() callers that needs fixing.
Now in the new world order, a hierarchy in the index that contain
i-t-a paths is written out as a tree object as if these i-t-a
entries do not exist, and comparing the index with such a tree
object that would result from writing out the hierarchy will result
in no difference. Update a test in t2203 that expected the i-t-a
entries to appear as "added to the index" in the comparison to
instead expect no output.
An earlier change eec3e7e4 (cache-tree: invalidate i-t-a paths after
generating trees, 2012-12-16) becomes an unnecessary pessimization
in the new world order---a cache-tree in the index that corresponds
to a hierarchy with i-t-a paths can now be marked as valid and
record the object name of the tree that results from writing a tree
object out of that hierarchy, as it will compare equal to that tree.
Reverting the commit is left for the future, though, as it is purely
a performance issue and no longer affects correctness.
Helped-by: Michael J Gruber <git@drmicha.warpmail.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Restructure "git push" codepath to make it easier to add new
configuration bits and then add push.followTags configuration that
turns --follow-tags option on by default.
* jk/push-config:
push: allow --follow-tags to be set by config push.followTags
cmd_push: pass "flags" pointer to config callback
cmd_push: set "atomic" bit directly
git_push_config: drop cargo-culted wt_status pointer
"git tag -h" used to show the "--column" and "--sort" options
that are about listing in a wrong section.
* jk/tag-h-column-is-a-listing-option:
tag: fix some mis-organized options in "-h" listing
oldlines is allocated earlier in the function and also freed on the
successful code path.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`old` is not used outside the loop and would get lost
once we reach the goto.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Portability fixes and workarounds for shell scripts have been added
to help BSD-derived systems.
* km/bsd-shells:
t5528: do not fail with FreeBSD shell
help.c: use SHELL_PATH instead of hard-coded "/bin/sh"
git-compat-util.h: move SHELL_PATH default into header
git-instaweb: use @SHELL_PATH@ instead of /bin/sh
git-instaweb: allow running in a working tree subdirectory
"git branch" on a detached HEAD always said "(detached from xyz)",
even when "git status" would report "detached at xyz". The HEAD is
actually at xyz and haven't been moved since it was detached in
such a case, but the user cannot read what the current value of
HEAD is when "detached from" is used.
* mg/detached-head-report:
branch: name detached HEAD analogous to status
wt-status: refactor detached HEAD analysis
If we are repacking with "-ad", we will drop any unreachable
objects. Likewise, using "-Ad --unpack-unreachable=<time>"
will drop any old, unreachable objects. In these cases, we
want to make sure the reachability we compute with "--all"
is complete. We can do this by passing GIT_REF_PARANOIA=1 in
the environment to pack-objects.
Note that "-Ad" is safe already, because it only loosens
unreachable objects. It is up to "git prune" to avoid
deleting them.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Prune should know about broken objects at the tips of refs,
so that we can feed them to our traversal rather than
ignoring them. It's better for us to abort the operation on
the broken object than it is to start deleting objects with
an incomplete view of the reachability namespace.
Note that for missing objects, aborting is the best we can
do. For a badly-named ref, we technically could use its sha1
as a reachability tip. However, the iteration code just
feeds us a null sha1, so there would be a reasonable amount
of code involved to pass down our wishes. It's not really
worth trying to do better, because this is a case that
should happen extremely rarely, and the message we provide:
fatal: unable to parse object: refs/heads/bogus:name
is probably enough to point the user in the right direction.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We do not usually end our errors with a full stop, but it
looks especially bad when you use die_errno, which adds a
colon, like:
fatal: could not create work tree dir 'foo'.: No such file or directory
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If clone fails, we generally try to clean up any directories
we've created. We do this by installing an atexit handler,
so that we don't have to manually trigger cleanup. However,
since we install this after touching the filesystem, any
errors between our initial mkdir() and our atexit() call
will result in us leaving a crufty directory around.
We can fix this by moving our atexit() call earlier. It's OK
to do it before the junk_work_tree variable is set, because
remove_junk makes sure the variable is initialized. This
means we "activate" the handler by assigning to the
junk_work_tree variable, which we now bump down to just
after we call mkdir(). We probably do not want to do it
before, because a plausible reason for mkdir() to fail is
EEXIST (i.e., we are racing with another "git init"), and we
would not want to remove their work.
OTOH, this is probably not that big a deal; we will allow
cloning into an empty directory (and skip the mkdir), which
is already racy (i.e., one clone may see the other's empty
dir and start writing into it). Still, it does not hurt to
err on the side of caution here.
Note that writing into junk_work_tree and junk_git_dir after
installing the handler is also technically racy, as we call
our handler on an async signal. Depending on the platform,
we could see a sheared write to the variables. Traditionally
we have not worried about this, and indeed we already do
this later in the function. If we want to address that, it
can come as a separate topic.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When grep is called with the --quiet option, the pager is initialized
despite not being used. When the pager is "less", anything output by
previous commands and not ended with a newline is overwritten:
$ echo -n aaa; echo bbb
aaabbb
$ echo -n aaa; git grep -q foo; echo bbb
bbb
This can be worked around, for example, by making sure STDOUT is not a
TTY or more directly by setting git's pager to "cat":
$ echo -n aaa; git grep -q foo > /dev/null; echo bbb
aaabbb
$ echo -n aaa; PAGER=cat git grep -q foo; echo bbb
aaabbb
But prevent calling the pager in the first place, which would also
save an unnecessary fork().
Signed-off-by: Wilhelm Schuermann <wimschuermann@googlemail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Description given by "grep -h" for its --exclude-standard option
was phrased poorly.
* nd/grep-exclude-standard-help-fix:
grep: correct help string for --exclude-standard
"git apply" was not very careful about reading from, removing,
updating and creating paths outside the working tree (under
--index/--cached) or the current directory (when used as a
replacement for GNU patch).
* jc/apply-beyond-symlink:
apply: do not touch a file beyond a symbolic link
apply: do not read from beyond a symbolic link
apply: do not read from the filesystem under --index
apply: reject input that touches outside the working area
Convert some magic numbers to the new GIT_SHA1 constants.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If the user enables untracked cache, then
- move worktree to an unsupported filesystem
- or simply upgrade OS
- or move the whole (portable) disk from one machine to another
- or access a shared fs from another machine
there's no guarantee that untracked cache can still function properly.
Record the worktree location and OS footprint in the cache. If it
changes, err on the safe side and disable the cache. The user can
'update-index --untracked-cache' again to make sure all conditions are
met.
This adds a new requirement that setup_git_directory* must be called
before read_cache() because we need worktree location by then, or the
cache is dropped.
This change does not cover all bases, you can fool it if you try
hard. The point is to stop accidents.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
Running "git tag -h" currently prints:
[...]
Tag creation options
[...]
--column[=<style>] show tag list in columns
--sort <type> sort tags
Tag listing options
--contains <commit> print only tags that contain the commit
--points-at <object> print only tags of the object
The "--column" and "--sort" options should go under the "Tag listing" group.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If the user has set SHELL_PATH in the Makefile then we
should respect that value and use it.
Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Various issues around "reflog expire", e.g. using --updateref when
expiring a reflog for a symbolic reference, have been corrected
and/or made saner.
* mh/expire-updateref-fixes:
reflog_expire(): never update a reference to null_sha1
reflog_expire(): ignore --updateref for symbolic references
reflog: improve and update documentation
struct ref_lock: delete the force_write member
lock_ref_sha1_basic(): do not set force_write for missing references
write_ref_sha1(): move write elision test to callers
write_ref_sha1(): remove check for lock == NULL
The prompt string "remove?" used when "git clean -i" asks the user
if a path should be removed was localizable, but the code always
expects a substring of "yes" to tell it to go ahead. Always show
[y/N] as part of this prompt to hint that the answer is not (yet)
localized.
* ja/clean-confirm-i18n:
Add hint interactive cleaning
Description given by "grep -h" for its --exclude-standard option
was phrased poorly.
* nd/grep-exclude-standard-help-fix:
grep: correct help string for --exclude-standard
Code cleanups.
* rs/simple-cleanups:
sha1_name: use strlcpy() to copy strings
pretty: use starts_with() to check for a prefix
for-each-ref: use skip_prefix() to avoid duplicate string comparison
connect: use strcmp() for string comparison
"git status" carefully names a detached HEAD "at" resp. "from" a rev or
ref depending on whether the detached HEAD has moved since. "git branch"
always uses "from", which can be confusing, because a status-aware user
would interpret this as moved detached HEAD.
Make "git branch" use the same logic and wording.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Clear the git_zstream variable at the start of git_deflate_init() etc.
so that callers don't have to do that.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Code cleanups.
* rs/simple-cleanups:
sha1_name: use strlcpy() to copy strings
pretty: use starts_with() to check for a prefix
for-each-ref: use skip_prefix() to avoid duplicate string comparison
connect: use strcmp() for string comparison
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
Revamp the "git reflog" usage documentation in the manpage and the
command help to match the current reality and improve its clarity:
* Add documentation for some options that had been left out.
* Group the subcommands and options more logically and move more
common subcommands/options higher.
* Improve some explanations.
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>
"git apply --whitespace=fix" fixed whitespace errors in the common
context lines but did so without reporting.
* jc/apply-ws-fix-expands-report:
apply: detect and mark whitespace errors in context lines when fixing
"git apply" was not very careful about reading from, removing,
updating and creating paths outside the working tree (under
--index/--cached) or the current directory (when used as a
replacement for GNU patch).
* jc/apply-beyond-symlink:
apply: do not touch a file beyond a symbolic link
apply: do not read from beyond a symbolic link
apply: do not read from the filesystem under --index
apply: reject input that touches outside the working area
For translators, specify that a [y/N] reply is needed.
Also capitalize the first word in the prompt, as all the other
interactive prompts from this command are capitalized.
Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
For each object in the input pack, we need one struct object_entry. On
x86-64, this struct is 64 bytes long. Although:
- The 8 bytes for delta_depth and base_object_no are only useful when
show_stat is set. And it's never set unless someone is debugging.
- The three fields hdr_size, type and real_type take 4 bytes each
even though they never use more than 4 bits.
By moving delta_depth and base_object_no out of struct object_entry
and make the other 3 fields one byte long instead of 4, we shrink 25%
of this struct.
On a 3.4M object repo (*) that's about 53MB. The saving is less
impressive compared to index-pack memory use for basic bookkeeping (**),
about 16%.
(*) linux-2.6.git already has 4M objects as of v3.19-rc7 so this is
not an unrealistic number of objects that we have to deal with.
(**) 3.4M * (sizeof(object_entry) + sizeof(delta_entry)) = 311MB
Brought-up-by: Matthew Sporleder <msporleder@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current help string is about --no-exclude-standard. But "git grep -h"
would show --exclude-standard instead. Flip the string. See 0a93fb8
(grep: teach --untracked and --exclude-standard options - 2011-09-27)
for more info about these options.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git apply --whitespace=fix" used to under-allocate the memory
when the fix resulted in a longer text than the original patch.
* jc/apply-ws-fix-expands:
apply: count the size of postimage correctly
apply: make update_pre_post_images() sanity check the given postlen
apply.c: typofix
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
Setting diff.submodule to 'log' made "git format-patch" produce
broken patches.
* dk/format-patch-ignore-diff-submodule:
format-patch: ignore diff.submodule setting
t4255: test am submodule with diff.submodule
"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
Use skip_prefix() to get the part after "color:" (if present) and only
compare it with "reset" instead of comparing the whole string again.
This gets rid of the duplicate "color:" part of the string constant.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If NULL is passed to ref_transaction_update()'s new_sha1 parameter,
then just verify old_sha1 (under lock) without trying to change the
new value of the reference.
Use this functionality to add a new function ref_transaction_verify(),
which checks the current value of the reference under lock but doesn't
change it.
Use ref_transaction_verify() in the implementation of "git update-ref
--stdin"'s "verify" command to avoid the awkward need to "update" the
reference to its existing value.
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>
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>
Instead, verify the reference's old value if and only if old_sha1 is
non-NULL.
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>
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>
Change the following functions' "flags" arguments from "int" to
"unsigned int":
* ref_transaction_update()
* ref_transaction_create()
* ref_transaction_delete()
* update_ref()
* delete_ref()
* lock_ref_sha1_basic()
Also change the "flags" member in "struct ref_update" to unsigned.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This will let us manipulate any transport flags which have matching
config options (there are none yet, but we will add one in
the next patch).
We could also just make "flags" a static file-scope global,
but the result is a little confusing. We end up passing it
along through do_push and push_with_options, each of which
further munge it. Having slightly-differing versions of the
flags variable available to those functions would probably
cause more confusion than it is worth. Let's just keep the
original local to cmd_push, and it can continue to pass it
through the call-stack.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This makes the code shorter and more obvious by removing an
unnecessary interim variable.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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
"git apply --whitespace=fix" used to under-allocate the memory
when the fix resulted in a longer text than the original patch.
* jc/apply-ws-fix-expands:
apply: count the size of postimage correctly
apply: make update_pre_post_images() sanity check the given postlen
apply.c: typofix
The push config callback does not expect any incoming data
via the void pointer. And if it did, it would certainly not
be a "struct wt_status". This probably got picked up
accidentally in b945901 (push: heed user.signingkey for
signed pushes, 2014-10-22), which copied the template for
the config callback from builtin/commit.c.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A few files include the same header file directly more than once.
As all these headers protect themselves against repeated inclusion
by the "#ifndef FOO_H / #define FOO_H / ... / #endif" idiom, leave
only the first inclusion and remove the later inclusion as a no-op
clean-up.
Signed-off-by: Дилян Палаузов <git-dpa@aegee.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Extending the js/push-to-deploy topic, the behaviour of "git push"
when updating the working tree and the index with an update to the
branch that is checked out can be tweaked by push-to-checkout hook.
* jc/push-to-checkout:
receive-pack: support push-to-checkout hook
receive-pack: refactor updateInstead codepath
"git push" has been taught a "--atomic" option that makes push to
update more than one ref an "all-or-none" affair.
* sb/atomic-push:
Document receive.advertiseatomic
t5543-atomic-push.sh: add basic tests for atomic pushes
push.c: add an --atomic argument
send-pack.c: add --atomic command line argument
send-pack: rename ref_update_to_be_sent to check_to_send_update
receive-pack.c: negotiate atomic push support
receive-pack.c: add execute_commands_atomic function
receive-pack.c: move transaction handling in a central place
receive-pack.c: move iterating over all commands outside execute_commands
receive-pack.c: die instead of error in case of possible future bug
receive-pack.c: shorten the execute_commands loop over all commands
Restructure "reflog expire" to fit the reflogs better with the
recently updated ref API.
Looked reasonable (except that some shortlog entries stood out like
a sore thumb).
* mh/reflog-expire: (24 commits)
refs.c: let fprintf handle the formatting
refs.c: don't expose the internal struct ref_lock in the header file
lock_any_ref_for_update(): inline function
refs.c: remove unlock_ref/close_ref/commit_ref from the refs api
reflog_expire(): new function in the reference API
expire_reflog(): treat the policy callback data as opaque
Move newlog and last_kept_sha1 to "struct expire_reflog_cb"
expire_reflog(): move rewrite to flags argument
expire_reflog(): move verbose to flags argument
expire_reflog(): pass flags through to expire_reflog_ent()
struct expire_reflog_cb: a new callback data type
Rename expire_reflog_cb to expire_reflog_policy_cb
expire_reflog(): move updateref to flags argument
expire_reflog(): move dry_run to flags argument
expire_reflog(): add a "flags" argument
expire_reflog(): extract two policy-related functions
Extract function should_expire_reflog_ent()
expire_reflog(): use a lock_file for rewriting the reflog file
expire_reflog(): return early if the reference has no reflog
expire_reflog(): rename "ref" parameter to "refname"
...
Setting diff.submodule to 'log' made "git format-patch" produce
broken patches.
* dk/format-patch-ignore-diff-submodule:
format-patch: ignore diff.submodule setting
t4255: test am submodule with diff.submodule
"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
run_setup_gently() is called before merge-file. This may result in changing
current working directory, which wasn't taken into account when opening a file
for writing.
Fix by prepending the passed prefix. Previous var is left so that error
messages keep referring to the file from the user's working directory
perspective.
Signed-off-by: Aleksander Boruch-Gruszecki <aleksander.boruchgruszecki@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Because Git tracks symbolic links as symbolic links, a path that
has a symbolic link in its leading part (e.g. path/to/dir/file,
where path/to/dir is a symbolic link to somewhere else, be it
inside or outside the working tree) can never appear in a patch
that validly applies, unless the same patch first removes the
symbolic link to allow a directory to be created there.
Detect and reject such a patch.
Things to note:
- Unfortunately, we cannot reuse the has_symlink_leading_path()
from dir.c, as that is only about the working tree, but "git
apply" can be told to apply the patch only to the index or to
both the index and to the working tree.
- We cannot directly use has_symlink_leading_path() even when we
are applying only to the working tree, as an early patch of a
valid input may remove a symbolic link path/to/dir and then a
later patch of the input may create a path path/to/dir/file, but
"git apply" first checks the input without touching either the
index or the working tree. The leading symbolic link check must
be done on the interim result we compute in-core (i.e. after the
first patch, there is no path/to/dir symbolic link and it is
perfectly valid to create path/to/dir/file).
Similarly, when an input creates a symbolic link path/to/dir and
then creates a file path/to/dir/file, we need to flag it as an
error without actually creating path/to/dir symbolic link in the
filesystem.
Instead, for any patch in the input that leaves a path (i.e. a non
deletion) in the result, we check all leading paths against the
resulting tree that the patch would create by inspecting all the
patches in the input and then the target of patch application
(either the index or the working tree).
This way, we catch a mischief or a mistake to add a symbolic link
path/to/dir and a file path/to/dir/file at the same time, while
allowing a valid patch that removes a symbolic link path/to/dir and
then adds a file path/to/dir/file.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We should reject a patch, whether it renames/copies dir/file to
elsewhere with or without modificiation, or updates dir/file in
place, if "dir/" part is actually a symbolic link to elsewhere,
by making sure that the code to read the preimage does not read
from a path that is beyond a symbolic link.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We currently read the preimage to apply a patch from the index only
when the --cached option is given. Do so also when the command is
running under the --index option. With --index, the index entry and
the working tree file for a path that is involved in a patch must be
identical, so this should not affect the result, but by reading from
the index, we will get the protection to avoid reading an unintended
path beyond a symbolic link automatically.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
By default, a patch that affects outside the working area (either a
Git controlled working tree, or the current working directory when
"git apply" is used as a replacement of GNU patch) is rejected as a
mistake (or a mischief). Git itself does not create such a patch,
unless the user bends over backwards and specifies a non-standard
prefix to "git diff" and friends.
When `git apply` is used as a "better GNU patch", the user can pass
the `--unsafe-paths` option to override this safety check. This
option has no effect when `--index` or `--cached` is in use.
The new test was stolen from Jeff King with slight enhancements.
Note that a few new tests for touching outside the working area by
following a symbolic link are still expected to fail at this step,
but will be fixed in later steps.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since ea02ffa3 (mailmap: simplify map_user() interface, 2013-01-05),
find_alignment() has been invoking commit_info_destroy() on an
uninitialized auto 'struct commit_info' (when METAINFO_SHOWN is not
set). commit_info_destroy() calls strbuf_release() for each
'commit_info' strbuf member, which randomly invokes free() on
whatever random stack value happens to reside in strbuf.buf, thus
leading to periodic crashes.
Reported-by: Dilyan Palauzov <dilyan.palauzov@aegee.org>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* sb/atomic-push:
Document receive.advertiseatomic
t5543-atomic-push.sh: add basic tests for atomic pushes
push.c: add an --atomic argument
send-pack.c: add --atomic command line argument
send-pack: rename ref_update_to_be_sent to check_to_send_update
receive-pack.c: negotiate atomic push support
receive-pack.c: add execute_commands_atomic function
receive-pack.c: move transaction handling in a central place
receive-pack.c: move iterating over all commands outside execute_commands
receive-pack.c: die instead of error in case of possible future bug
receive-pack.c: shorten the execute_commands loop over all commands
* mh/reflog-expire: (24 commits)
refs.c: let fprintf handle the formatting
refs.c: don't expose the internal struct ref_lock in the header file
lock_any_ref_for_update(): inline function
refs.c: remove unlock_ref/close_ref/commit_ref from the refs api
reflog_expire(): new function in the reference API
expire_reflog(): treat the policy callback data as opaque
Move newlog and last_kept_sha1 to "struct expire_reflog_cb"
expire_reflog(): move rewrite to flags argument
expire_reflog(): move verbose to flags argument
expire_reflog(): pass flags through to expire_reflog_ent()
struct expire_reflog_cb: a new callback data type
Rename expire_reflog_cb to expire_reflog_policy_cb
expire_reflog(): move updateref to flags argument
expire_reflog(): move dry_run to flags argument
expire_reflog(): add a "flags" argument
expire_reflog(): extract two policy-related functions
Extract function should_expire_reflog_ent()
expire_reflog(): use a lock_file for rewriting the reflog file
expire_reflog(): return early if the reference has no reflog
expire_reflog(): rename "ref" parameter to "refname"
...
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>
When the incoming patch has whitespace errors in a common context
line (i.e. a line that is expected to be found and is not modified
by the patch), "apply --whitespace=fix" corrects the whitespace
errors the line has, in addition to the whitespace error on a line
that is updated by the patch. However, we did not count and report
that we fixed whitespace errors on such lines.
[jc: This is iffy. What if the whitespace error has been fixed in
the target since the patch was written? A common context line we
see in the patch has errors, and it matches a line in the target
that has the errors already corrected, resulting in no change, which
we may not want to count after all. On the other hand, we are
reporting whitespace errors _in_ the incoming patch, so...]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Under --whitespace=fix option, match_fragment() function examines
the preimage (the common context and the removed lines in the patch)
and the file being patched and checks if they match after correcting
all whitespace errors. When they are found to match, the common
context lines in the preimage is replaced with the fixed copy,
because these lines will then be copied to the corresponding place
in the postimage by a later call to update_pre_post_images(). Lines
that are added in the postimage, under --whitespace=fix, have their
whitespace errors already fixed when apply_one_fragment() prepares
the preimage and the postimage, so in the end, application of the
patch can be done by replacing the block of text in the file being
patched that matched the preimage with what is in the postimage that
was updated by update_pre_post_images().
In the earlier days, fixing whitespace errors always resulted in
reduction of size, either collapsing runs of spaces in the indent to
a tab or removing the trailing whitespaces. These days, however,
some whitespace error fix results in extending the size.
250b3c6c (apply --whitespace=fix: avoid running over the postimage
buffer, 2013-03-22) tried to compute the final postimage size but
its math was flawed. It counted the size of the block of text in
the original being patched after fixing the whitespace errors on its
lines that correspond to the preimage. That number does not have
much to do with how big the final postimage would be.
Instead count (1) the added lines in the postimage, whose size is
the same as in the final patch result because their whitespace
errors have already been corrected, and (2) the fixed size of the
lines that are common.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git apply --whitespace=fix" used to be able to assume that fixing
errors will always reduce the size by e.g. stripping whitespaces at
the end of lines or collapsing runs of spaces into tabs at the
beginning of lines. An update to accomodate fixes that lengthens
the result by e.g. expanding leading tabs into spaces were made long
time ago but the logic miscounted the necessary space after such
whitespace fixes, leading to either under-allocation or over-usage
of already allocated space.
Illustrate this with a runtime sanity-check to protect us from
future breakage. The test was stolen from Kyle McKay who helped
to identify the problem.
Helped-by: "Kyle J. McKay" <mackyle@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git remote add $name $URL" is now allowed when "url.$URL.insteadOf"
is already defined.
* js/remote-add-with-insteadof:
Add a regression test for 'git remote add <existing> <same-url>'
git remote: allow adding remotes agreeing with url.<...>.insteadOf
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>
In cat_one_file(), "type" and "size" variables are defined in the
function scope, and then two variables of the same name are defined
in a block in one of the if/else statement, hiding the definitions
in the outer scope.
Because the values of the outer variables before the control enters
this scope, however, do not have to be preserved, we can remove
useless definitions of variables from the inner scope safely without
breaking anything.
Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The helper functions prepare_final() and prepare_initial() return a
pointer to a string that is a member of an object in the revs->pending
array. This array is later rebuilt when running prepare_revision_walk()
which potentially transforms the pointer target into a bogus string. Fix
this by maintaining a copy of the original string.
Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
This file had its own identical helper that predates
xstrdup_or_null. Let's use the global one to avoid
repetition.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The code handling %(upstream:track) and %(upstream:trackshort)
assumed that it always had a valid branch that had been sanitized
earlier in populate_value(), and thus did not check the return value
of the call to stat_tracking_info().
While there is indeed some sanitization code that basically
corresponds to stat_tracking_info() returning 0 (no base branch
set), the function can also return -1 when the base branch did exist
but has since then been deleted.
In this case, num_ours and num_theirs had undefined values and a
call to `git for-each-ref --format="%(upstream:track)"` could print
spurious values such as
[behind -111794512]
[ahead 38881640, behind 5103867]
even for repositories with one single commit.
Verify stat_tracking_info()'s return value and do not print anything
if it returns -1. This behavior also matches the documentation ("has
no effect if the ref does not have tracking information associated
with it").
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Earlier we made "rev-list --object-edge" more aggressively list the
objects at the edge commits, in order to reduce number of objects
fetched into a shallow repository, but the change affected cases
other than "fetching into a shallow repository" and made it
unusably slow (e.g. fetching into a normal repository should not
have to suffer the overhead from extra processing). Limit it to a
more specific case by introducing --objects-edge-aggressive, a new
option to rev-list.
* bc/fetch-thin-less-aggressive-in-normal-repository:
pack-objects: use --objects-edge-aggressive for shallow repos
rev-list: add an option to mark fewer edges as uninteresting
Documentation: add missing article in rev-list-options.txt
"git checkout-index --temp=$target $path" did not work correctly
for paths outside the current subdirectory in the project.
* es/checkout-index-temp:
checkout-index: fix --temp relative path mangling
t2004: demonstrate broken relative path printing
t2004: standardize file naming in symlink test
t2004: drop unnecessary write-tree/read-tree
t2004: modernize style
- "exec_cmd.h" became unnecessary at b931aa5a (Call builtin ls-tree
in git-cat-file -p, 2006-05-26), when it changed an earlier code
that delegated tree display to "ls-tree" via the run_command()
API (hence needing "exec_cmd.h") to call cmd_ls_tree() directly.
We should have removed the include in the same commit, but we
forgot to do so.
- "diff.h" was added at e5fba602 (textconv: support for cat_file,
2010-06-15), together with "userdiff.h", but "userdiff.h" can be
included without including "diff.h"; the header was unnecessary
from the beginning.
- "tag.h" and "tree.h" were necessary since 8e440259 (Use blob_,
commit_, tag_, and tree_type throughout., 2006-04-02) to check
the type of object by comparing typename with tree_type and
tag_type (pointers to extern strings).
21666f1a (convert object type handling from a string to a number,
2007-02-26) made these <type>_type strings unnecessary, and it
could have switched to include "object.h", which is necessary to
use typename(), but it forgot to do so. Because "tag.h" and
"tree.h" include "object.h", it did not need to explicitly
include "object.h" in order to start using typename() itself.
We do not even have to include "object.h" after removing these
two #includes, because "builtin.h" includes "commit.h" which in
turn includes "object.h" these days. This happened at 7b9c0a69
(git-commit-tree: make it usable from other builtins,
2008-07-01).
Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When receive.denyCurrentBranch is set to updateInstead, a push that
tries to update the branch that is currently checked out is accepted
only when the index and the working tree exactly matches the
currently checked out commit, in which case the index and the
working tree are updated to match the pushed commit. Otherwise the
push is refused.
This hook can be used to customize this "push-to-deploy" logic. The
hook receives the commit with which the tip of the current branch is
going to be updated, and can decide what kind of local changes are
acceptable and how to update the index and the working tree to match
the updated tip of the current branch.
For example, the hook can simply run `git read-tree -u -m HEAD "$1"`
in order to emulate 'git fetch' that is run in the reverse direction
with `git push`, as the two-tree form of `read-tree -u -m` is
essentially the same as `git checkout` that switches branches while
keeping the local changes in the working tree that do not interfere
with the difference between the branches.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a command line argument to the git push command to request atomic
pushes.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This adds support to send-pack to negotiate and use atomic pushes
iff the server supports it. Atomic pushes are activated by a new command
line flag --atomic.
In order to do this we also need to change the semantics for send_pack()
slightly. The existing send_pack() function actually doesn't send all the
refs back to the server when multiple refs are involved, for example
when using --all. Several of the failure modes for pushes can already be
detected locally in the send_pack client based on the information from the
initial server side list of all the refs as generated by receive-pack.
Any such refs that we thus know would fail to push are thus pruned from
the list of refs we send to the server to update.
For atomic pushes, we have to deal thus with both failures that are detected
locally as well as failures that are reported back from the server. In order
to do so we treat all local failures as push failures too.
We introduce a new status code REF_STATUS_ATOMIC_PUSH_FAILED so we can
flag all refs that we would normally have tried to push to the server
but we did not due to local failures. This is to improve the error message
back to the end user to flag that "these refs failed to update since the
atomic push operation failed."
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This adds the atomic protocol option to allow
receive-pack to inform the client that it has
atomic push capability.
This commit makes the functionality introduced
in the previous commits go live for the serving
side. The changes in documentation reflect the
protocol capabilities of the server.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This introduces the new function execute_commands_atomic which will use
one atomic transaction for all updates. The default behavior is still
the old non atomic way, one ref at a time. This is to cause as little
disruption as possible to existing clients. It is unknown if there are
client scripts that depend on the old non-atomic behavior so we make it
opt-in for now.
A later patch will add the possibility to actually use the functionality
added by this patch. For now use_atomic is always 0.
Inspired-by: Ronnie Sahlberg <sahlberg@google.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This moves all code related to transactions into the
execute_commands_non_atomic function. This includes
beginning and committing the transaction as well as
dealing with the errors which may occur during the
begin and commit phase of a transaction.
No functional changes intended.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit allows us in a later patch to easily distinguish between
the non atomic way to update the received refs and the atomic way which
is introduced in a later patch.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Discussion on the previous patch revealed we rather want to err on the
safe side. To do so we need to stop receive-pack in case of the possible
future bug when connectivity is not checked on a shallow push.
Also while touching that code we considered that removing the reported
refs may be harmful in some situations. Sound the message more like a
"This Cannot Happen, Please Investigate!" instead of giving advice to
remove refs.
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make the main "execute_commands" loop in receive-pack easier to read
by splitting out some steps into helper functions. The new helper
'should_process_cmd' checks if a ref update is unnecessary, whether
due to an error having occurred or for another reason. The helper
'warn_if_skipped_connectivity_check' warns if we have forgotten to
run a connectivity check on a ref which is shallow for the client
which would be a bug.
This will help us to duplicate less code in a later patch when we make
a second copy of the "execute_commands" loop.
No functional change intended.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff.submodule when set to log produces output which git-am cannot
handle. Ignore this setting when generating patch output.
Signed-off-by: Doug Kelly <dougk.ff7@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The get_merge_bases*() API was easy to misuse by careless
copy&paste coders, leaving object flags tainted in the commits that
needed to be traversed.
* jc/merge-bases:
get_merge_bases(): always clean-up object flags
bisect: clean flags after checking merge bases
Noticed-by: Mark Levedahl <mlevedahl@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The parse_options API expects an array of alternative usage lines
to which it automatically ads the language-appropriate "or" when
displaying. Each of these options is marked for translation with N_
and then later translated when gettext is called on each element
of the array.
Since the N_ macro just expands to its argument, if two N_-marked
strings appear next to each other without being separated by anything
else such as a comma, the preprocessor will join them into one string.
In that case two separate strings get marked for translation, but at
runtime they have been joined into a single string passed to gettext
which then fails to get translated because the combined string was
never marked for translation.
Fix this by properly separating the two N_ marked strings with
a comma and removing the embedded "\n" and " or:" that are
properly supplied by the parse_options API.
Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
checkout-index --temp only properly prints relative paths which are
descendants of the current directory. Paths in ancestor or sibling
directories (or their children) are often printed in mangled form. For
example:
mkdir a bbb &&
>file &&
>bbb/file &&
git update-index --add file bbb/file &&
cd a &&
git checkout-index --temp ../file ../bbb/file
prints:
.merge_file_ooblek le
.merge_file_igloo0 b/file
rather than the correct:
.merge_file_ooblek ../file
.merge_file_igloo0 ../bbb/file
Internally, given the above example, checkout-index prefixes each input
argument with the name of the current directory ("a/", in this case),
and then assumes that it can simply skip forward by strlen("a/") bytes
to recover the original name. This works for files in the current
directory or its descendants, but fails for files in ancestors or
siblings (or their children) due to path normalization.
For instance, given "../file", "a/" is prepended, giving "a/../file".
Path normalization folds out "a/../", resulting in "file". Attempting
to recover the original name by skipping strlen("a/") bytes gives the
incorrect "le" rather than the desired "../file".
Fix this by taking advantage of write_name_quoted_relative() to recover
the original name properly, rather than assuming that it can be
recovered by skipping strlen(prefix) bytes.
As a bonus, this also fixes a bug in which checkout-index --temp
accessed and printed memory beyond the end-of-string. For instance,
within a subdirectory named "subdirectory", and given argument
"../file", prefixing would give "subdirectory/../file", which would
become "file" after normalization. checkout-index would then attempt to
recover the original name by skipping strlen("subdirectory/") bytes of
"file", which placed it well beyond end-of-string. Despite this error,
it often appeared to give the correct result, but only due to an
accident of implementation which left an apparently correct copy of the
path in memory following the normalized value. In particular, handed
"subdirectory/../file", in-place processing by normalize_path_copy_len()
resulted in "file\0rectory/../file". When checkout-index skipped
strlen("subdirectory/") bytes, it ended up back at "../file" and thus
appeared to give the correct answer, despite being past end-of-string.
Reported-by: Russ Cox <rsc@golang.org>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When fetching into or pushing from a shallow repository, we want to
aggressively mark edges as uninteresting, since this decreases the pack
size. However, aggressively marking edges can negatively affect
performance on large non-shallow repositories with lots of refs.
Teach pack-objects a --shallow option to indicate that we're pushing
from or fetching into a shallow repository. Use
--objects-edge-aggressive only for shallow repositories and otherwise
use --objects-edge, which performs better in the general case. Update
the callers to pass the --shallow option when they are dealing with a
shallow repository.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In commit fbd4a70 (list-objects: mark more commits as edges in
mark_edges_uninteresting - 2013-08-16), we marked an increasing number
of edges uninteresting. This change, and the subsequent change to make
this conditional on --objects-edge, are used by --thin to make much
smaller packs for shallow clones.
Unfortunately, they cause a significant performance regression when
pushing non-shallow clones with lots of refs (23.322 seconds vs.
4.785 seconds with 22400 refs). Add an option to git rev-list,
--objects-edge-aggressive, that preserves this more aggressive behavior,
while leaving --objects-edge to provide more performant behavior.
Preserve the current behavior for the moment by using the aggressive
option.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git update-ref --stdin"'s verify command did not work well when
<oldvalue>, which is documented as optional, was missing.
* mh/update-ref-verify:
update-ref: fix "verify" command with missing <oldvalue>
t1400: add some more tests of "update-ref --stdin"'s verify command
When adding a remote, we make sure that the remote does not exist
already. However, this test was not quite correct: when the
url.<...>.insteadOf config variable was set to the remote name to be
added, the code would assume that the remote exists already.
Let's allow adding remotes when there is a url.<...>.insteadOf setting
when both the name and the URL agree with the remote to be added.
It might seem like a mistake to compare against remote->url[0] without
verifying that remote->url_nr >=1, but at this point a missing URL has
been filled by the name already, therefore url_nr cannot be zero.
Noticed by Anastas Dancha.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
New tag object format validation added in 2.2 showed garbage
after a tagname it reported in its error message.
* js/fsck-tag-validation:
index-pack: terminate object buffers with NUL
fsck: properly bound "invalid tag name" error message
"git branch -d" (delete) and "git branch -m" (move) learned to
honor "-f" (force) flag; unlike many other subcommands, the way to
force these have been with separate "-D/-M" options, which was
inconsistent.
* mg/branch-d-m-f:
branch: allow -f with -m and -d
t3200-branch: test -M
"git ls-tree" does not support path selection based on negative
pathspecs, but did not error out when negative pathspecs are given.
* nd/ls-tree-pathspec:
t3102: style modernization
t3102: document that ls-tree does not yet support negated pathspec
ls-tree: disable negative pathspec because it's not supported
ls-tree: remove path filtering logic in show_tree
tree.c: update read_tree_recursive callback to pass strbuf as base
"git push" into a repository with a working tree normally refuses
to modify the branch that is checked out. The command learned to
optionally do an equivalent of "git reset --hard" only when there
is no change to the working tree and the index instead, which would
be useful to "deploy" by pushing into a repository.
* js/push-to-deploy:
t5516: more tests for receive.denyCurrentBranch=updateInstead
receive-pack: add another option for receive.denyCurrentBranch
The function sometimes returned a non-freeable memory and some
other times returned a piece of memory that must be freed.
* jc/exec-cmd-system-path-leak-fix:
system_path(): always return free'able memory to the caller
"git am" learned "--message-id" option to copy the message ID of
the incoming e-mail to the log message of resulting commit.
* pb/am-message-id-footer:
git-am: add --message-id/--no-message-id
git-mailinfo: add --message-id
"git remote update --prune" to drop many refs has been optimized.
* mh/simplify-repack-without-refs:
sort_string_list(): rename to string_list_sort()
prune_remote(): iterate using for_each_string_list_item()
prune_remote(): rename local variable
repack_without_refs(): make the refnames argument a string_list
prune_remote(): sort delete_refs_list references en masse
prune_remote(): initialize both delete_refs lists in a single loop
prune_remote(): exit early if there are no stale references
Some filesystems assign filemodes in a strange way, fooling then
automatic "filemode trustability" check done during a new
repository creation.
* tb/config-core-filemode-check-on-broken-fs:
init-db: improve the filemode trustability check
"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
Git 2.0 was supposed to make the "simple" mode for the default of
"git push", but it didn't.
* jk/push-simple:
push: truly use "simple" as default, not "upstream"
"git init" (hence "git clone") initialized the per-repository
configuration file .git/config with x-bit by mistake.
* mh/config-flip-xbit-back-after-checking:
create_default_files(): don't set u+x bit on $GIT_DIR/config
"git config --get-color" did not parse its command line arguments
carefully.
* jk/colors-fix:
t4026: test "normal" color
config: fix parsing of "git config --get-color some.key -1"
docs: describe ANSI 256-color mode
"git checkout $treeish $path", when $path in the index and the
working tree already matched what is in $treeish at the $path,
still overwrote the $path unnecessarily.
* jk/checkout-from-tree:
checkout $tree: do not throw away unchanged index entries
Move expire_reflog() into refs.c and rename it to reflog_expire().
Turn the three policy functions into function pointers that are passed
into reflog_expire(). Add function prototypes and documentation to
refs.h.
[jc: squashed in $gmane/261582, drop "extern" in function definition]
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Reviewed-by: Stefan Beller <sbeller@google.com>
Tweaked-by: Ramsay Jones
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Git 2.0 was supposed to make the "simple" mode for the default of
"git push", but it didn't.
* jk/push-simple:
push: truly use "simple" as default, not "upstream"
Now that expire_reflog() doesn't actually look in the
expire_reflog_policy_cb data structure, we can make it opaque:
* Change the callers of expire_reflog() to pass it a pointer to an
entire "struct expire_reflog_policy_cb" rather than a pointer to a
"struct cmd_reflog_expire_cb".
* Change expire_reflog() to accept the argument as a "void *" and
simply pass it through to the policy functions.
* Change the policy functions, reflog_expiry_prepare(),
reflog_expiry_cleanup(), and should_expire_reflog_ent(), to accept
"void *cb_data" arguments and cast them back to "struct
expire_reflog_policy_cb" internally.
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>
These members are not needed by the policy functions.
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>
The policy objects don't care about "--rewrite". So move it to
expire_reflog()'s flags parameter.
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>
The policy objects don't care about "--verbose". So move it to
expire_reflog()'s flags parameter.
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>
Add a flags field to "struct expire_reflog_cb", and pass the flags
argument through to expire_reflog_ent(). In a moment we will start
using it to pass through flags that expire_reflog_ent() needs.
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>
Add a new data type, "struct expire_reflog_cb", for holding the data
that expire_reflog() passes to expire_reflog_ent() via
for_each_reflog_ent(). For now it only holds a pointer to a "struct
expire_reflog_policy_cb", which still contains all of the actual data.
In future commits we will move some fields from the latter to the
former.
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>
This is the first step towards separating the data needed by the
policy code from the data needed by the reflog expiration machinery.
(In a moment we will add a *new* "struct expire_reflog_cb" for the use
of expire_reflog() itself, then move fields selectively from
expire_reflog_policy_cb to expire_reflog_cb.)
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>
The policy objects don't care about "--updateref". So move it to
expire_reflog()'s flags parameter.
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>
The policy objects don't care about "--dry-run". So move it to
expire_reflog()'s flags parameter.
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>
We want to separate the options relevant to the expiry machinery from
the options affecting the expiration policy. So add a "flags" argument
to expire_reflog() to hold the former.
The argument doesn't yet do anything.
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>
Extract two functions, reflog_expiry_prepare() and
reflog_expiry_cleanup(), from expire_reflog(). This is a further step
towards separating the code for deciding on expiration policy from the
code that manages the physical deletion of reflog entries.
This change requires a couple of local variables from expire_reflog()
to be turned into fields of "struct expire_reflog_cb". More
reorganization of the callback data will follow in later commits.
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>
Extract from expire_reflog_ent() a function that is solely responsible
for deciding whether a reflog entry should be expired. By separating
this "business logic" from the mechanics of actually expiring entries,
we are working towards the goal of encapsulating reflog expiry within
the refs API, with policy decided by a callback function passed to it
by its caller.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We don't actually need the locking functionality, because we already
hold the lock on the reference itself, which is how the reflog file is
locked. But the lock_file code can do some of the bookkeeping for us,
and it is more careful than the old code here was. For example:
* It correctly handles the case that the reflog lock file already
exists for some reason or cannot be opened.
* It correctly cleans up the lockfile if the program dies.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is very little cleanup needed if the reference has no reflog. If
we move the initialization of log_file down a bit, there's even less.
So instead of jumping to the cleanup code at the end of the function,
just do the cleanup and return inline.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is our usual convention.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Prior to v1.5.4~14, expire_reflog() had to be an each_ref_fn because
it was passed to for_each_reflog(). Since then, there has been no
reason for it to implement the each_ref_fn interface. So...
* Remove the "unused" parameter (which took the place of "flags", but
was really unused).
* Declare the last parameter to be (struct cmd_reflog_expire_cb *)
rather than (void *).
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Call strbuf_complete_line() instead of open-coding it. Also remove
surrounding comments indicating the intent to complete a line since
this information is already included in the function name.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
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>
If "git update-ref --stdin" was given a "verify" command with no
"<newvalue>" at all (not even zeros), the code was mistakenly setting
have_old=0 (and leaving old_sha1 uninitialized). But this is
incorrect: this command is supposed to verify that the reference
doesn't exist. So in this case we really need old_sha1 to be set to
null_sha1 and have_old to be set to 1.
Moreover, since have_old was being set to zero, *no* check of the old
value was being done, so the new value of the reference was being set
unconditionally to the value in new_sha1. new_sha1, in turn, was set
to null_sha1 in the expectation that that was the old value and it
shouldn't be changed. But because the precondition was not being
checked, the result was that the reference was being deleted
unconditionally.
So, if <oldvalue> is missing, set have_old unconditionally and set
old_sha1 to null_sha1.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Acked-by: Brad King <brad.king@kitware.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
