Fix performance regression in v1.8.4.x and later.
* jk/mark-edges-uninteresting:
list-objects: only look at cmdline trees with edge_hint
t/perf: time rev-list with UNINTERESTING commits
Fix a handful of bugs around interpreting $branch@{upstream}
notation and its lookalike, when $branch part has interesting
characters, e.g. "@", and ":".
* jk/interpret-branch-name-fix:
interpret_branch_name: find all possible @-marks
interpret_branch_name: avoid @{upstream} past colon
interpret_branch_name: always respect "namelen" parameter
interpret_branch_name: rename "cp" variable to "at"
interpret_branch_name: factor out upstream handling
"git clone" would fail to clone from a repository that has a ref
directly under "refs/", e.g. "refs/stash", because different
validation paths do different things on such a refname. Loosen the
client side's validation to allow such a ref.
* jk/allow-fetch-onelevel-refname:
fetch-pack: do not filter out one-level refs
"git log --left-right A...B" lost the "leftness" of commits
reachable from A when A is a tag as a side effect of a recent
bugfix. This is a regression in 1.8.4.x series.
* jc/revision-range-unpeel:
revision: propagate flag bits from tags to pointees
revision: mark contents of an uninteresting tree uninteresting
The current basedir compare aborts early in order to avoid futile
recursive searches. However, a match may still be found by another
pathspec. This can cause an error while checking out files from a branch
when using multiple pathspecs:
$ git checkout master -- 'a/*.txt' 'b/*.txt'
error: pathspec 'a/*.txt' did not match any file(s) known to git.
Signed-off-by: Andy Spencer <andy753421@gmail.com>
Acked-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some versions of touch (such as /usr/ucb/touch on Solaris)
do not know about the "-r" option. This would make sense as
a feature of test-chmtime, but fortunately this fix is even
easier.
The test does not care about the timestamp of the .keep file it
creates at all, only that it exists. For such a use case, with or
without portability issues around "-r", "touch" should not be used
in the first place.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t7501.9 tries to check that "git commit" will fail when the
index is unchanged. It relies on previous tests not to have
modified the index. When it was originally written, this was
always the case. However, commit c65dc35 (t7501: test the
right kind of breakage, 2012-03-30) changed earlier tests (4
and 5) to leave a modification in the index.
We never noticed, however, because t7501.7, between the two,
clears the index state as a side effect. However, that test
depends on the PERL prerequisite, and so it does not always
run. Therefore if NO_PERL is set, we do not run the
intervening test, the index is left unclean, and t7501.9
fails.
We could fix this by moving t7501.9 up in the script.
However, this patch instead leaves it in place and adds a
"git reset" before the commit. This makes the test more
explicit about its preconditions, and will future-proof it
against any other changes in the test state.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We do ignore trailing slash on a directory, so pathspec "abc/" matches
directory "abc". A submodule is also a directory. Apply the same logic
to it. This makes "git log submodule-path" and "git log submodule-path/"
produce the same output.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The p4 server can enforce file locking, so that only one user
can edit a file at a time. Git p4 is unable to submit changes
to locked files. Currently it exits poorly. Ideally it would
notice the locked condition and clean up nicely.
Add a bunch of tests that describe the problem, hoping that
fixes appear in the future.
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 9d7d446 (git p4: submit files with wildcards, 2012-04-29)
fixed problems with handling files that had p4 wildcard
characters, like "@" and "*". But it missed one case, that of
RCS keyword scrubbing, which uses "p4 fstat" to extract type
information. Fix it by calling wildcard_encode() on the raw
filename.
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Generating the submit template for p4 uses tempfile.mkstemp(),
which by default puts files in /tmp. For a test that fails,
possibly on purpose, this is not cleaned up. Run with TMPDIR
pointing into the trash directory so the temp files go away
with the test results.
To do this required some other minor changes. First, the editor
is launched using system(editor + " " + template_file), using
shell expansion to build the command string. This doesn't work
if editor has a space in it. And is generally unwise as it's
easy to fool the shell into doing extra work. Exec the args
directly, without shell expansion.
Second, without shell expansion, the trick of "P4EDITOR=:" used
in the tests doesn't work. Use a real command, true, as the
non-interactive editor for testing.
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The tests use author@example.com as the canonical submitter,
but he does not have an entry in the p4 users database.
This causes the generated change description to complain
that the git and p4 users disagree. The complaint message
is still valid, but isn't useful in tests. It was introduced
in 848de9c (git-p4: warn if git authorship won't be retained,
2011-05-13).
Fix t9813 to use @example.com instead of @localhost due to
change in p4_add_user(). Move the function into the git p4
test library so author can be added at initialization time.
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit e9df0f9 (git p4: cygwin p4 client does not mark read-only,
2013-01-26) fixed a problem with "test -w" on cygwin, but mistakenly
marked the new test as failing. Fix this.
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There was no test where p4 deleted a file with a wildcard
character. Make sure git p4 applies the wildcard decoding
properly when importing a delete that includes a wildcard.
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Damien Gérard highlights an interesting problem. Some p4
repositories end up with symlinks that have an empty target. It
is not possible to create this with current p4, but they do
indeed exist.
The effect in git p4 is that "p4 print" on the symlink returns an
empty string, confusing the curret symlink-handling code.
Such broken repositories cause problems in p4 as well, even with
no git involved. In p4, syncing to a change that includes a
bogus symlink causes errors:
//depot/empty-symlink - updating /home/me/p4/empty-symlink
rename: /home/me/p4/empty-symlink: No such file or directory
and leaves no symlink.
In git, replicate the p4 behavior by ignoring these bad symlinks.
If, in a later p4 revision, the symlink happens to point to
something non-null, the symlink will be replaced properly.
Add a big test for all this too.
This happens to be a regression introduced by 1292df1 (git-p4:
Fix occasional truncation of symlink contents., 2013-08-08) and
appeared first in 1.8.5. But it shows up only in p4 repositories
of dubious character, so can wait for a proper release.
Tested-by: Damien Gérard <damien@iwi.me>
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
While this happens to work, there was no test to make sure
that the basic importing of a symlink from p4 to git functioned.
Add a simple test to create a symlink in p4 and import it into git,
then verify that the symlink exists and has the correct target.
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since 9d57c4a (git p4: implement view spec wildcards with "p4
where", 2013-08-30), all the wildcard types should be supported.
Change must-fail tests to mark that they now pass.
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We time a straight "rev-list --all" and its "--object"
counterpart, both going all the way to the root. However, we
do not time a partial history walk. This patch adds an
extreme case: a walk over a very small slice of history, but
with a very large set of UNINTERESTING tips. This is similar
to the connectivity check run by git on a small fetch, or
the walk done by any pre-receive hooks that want to check
incoming commits.
This test reveals a performance regression in git v1.8.4.2,
caused by fbd4a70 (list-objects: mark more commits as edges
in mark_edges_uninteresting, 2013-08-16):
Test fbd4a703^ fbd4a703
------------------------------------------------------------------------------------------
0001.1: rev-list --all 0.69(0.67+0.02) 0.69(0.68+0.01) +0.0%
0001.2: rev-list --all --objects 3.47(3.44+0.02) 3.48(3.44+0.03) +0.3%
0001.4: rev-list $commit --not --all 0.04(0.04+0.00) 0.04(0.04+0.00) +0.0%
0001.5: rev-list --objects $commit --not --all 0.04(0.03+0.00) 0.27(0.24+0.02) +575.0%
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fetching from a shallow-cloned repository used to be forbidden,
primarily because the codepaths involved were not carefully vetted
and we did not bother supporting such usage. This attempts to allow
object transfer out of a shallow-cloned repository in a controlled
way (i.e. the receiver become a shallow repository with truncated
history).
* nd/shallow-clone: (31 commits)
t5537: fix incorrect expectation in test case 10
shallow: remove unused code
send-pack.c: mark a file-local function static
git-clone.txt: remove shallow clone limitations
prune: clean .git/shallow after pruning objects
clone: use git protocol for cloning shallow repo locally
send-pack: support pushing from a shallow clone via http
receive-pack: support pushing to a shallow clone via http
smart-http: support shallow fetch/clone
remote-curl: pass ref SHA-1 to fetch-pack as well
send-pack: support pushing to a shallow clone
receive-pack: allow pushes that update .git/shallow
connected.c: add new variant that runs with --shallow-file
add GIT_SHALLOW_FILE to propagate --shallow-file to subprocesses
receive/send-pack: support pushing from a shallow clone
receive-pack: reorder some code in unpack()
fetch: add --update-shallow to accept refs that update .git/shallow
upload-pack: make sure deepening preserves shallow roots
fetch: support fetching from a shallow repository
clone: support remote shallow repository
...
With the previous fix 895c5ba3 (revision: do not peel tags used in
range notation, 2013-09-19), handle_revision_arg() that processes
command line arguments for the "git log" family of commands no
longer directly places the object pointed by the tag in the pending
object array when it sees a tag object. We used to place pointee
there after copying the flag bits like UNINTERESTING and
SYMMETRIC_LEFT.
This change meant that any flag that is relevant to later history
traversal must now be propagated to the pointed objects (most often
these are commits) while starting the traversal, which is partly
done by handle_commit() that is called from prepare_revision_walk().
We did propagate UNINTERESTING, but did not do so for others, most
notably SYMMETRIC_LEFT. This caused "git log --left-right v1.0..."
(where "v1.0" is a tag) to start losing the "leftness" from the
commit the tag points at.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git rev-list --objects ^A^{tree} B^{tree}" ought to mean "I want a
list of objects inside B's tree, but please exclude the objects that
appear inside A's tree".
we see the top-level tree marked as uninteresting (i.e. ^A^{tree} in
the above example) and call mark_tree_uninteresting() on it; this
unfortunately prevents us from recursing into the tree and marking
the objects in the tree as uninteresting.
The reason why "git log ^A A" yields an empty set of commits,
i.e. we do not have a similar issue for commits, is because we call
mark_parents_uninteresting() after seeing an uninteresting commit.
The uninteresting-ness of the commit itself does not prevent its
parents from being marked as uninteresting.
Introduce mark_tree_contents_uninteresting() and structure the code
in handle_commit() in such a way that it makes it the responsibility
of the callchain leading to this function to mark commits, trees and
blobs as uninteresting, and also make it the responsibility of the
helpers called from this function to mark objects that are reachable
from them.
Note that this is a very old bug that probably dates back to the day
when "rev-list --objects" was introduced. The line to clear
tree->object.parsed at the end of mark_tree_contents_uninteresting()
can be removed when this fix is merged to the codebase after
6e454b9a (clear parsed flag when we free tree buffers, 2013-06-05).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we parse a string like "foo@{upstream}", we look for
the first "@"-sign, and check to see if it is an upstream
mark. However, since branch names can contain an @, we may
also see "@foo@{upstream}". In this case, we check only the
first @, and ignore the second. As a result, we do not find
the upstream.
We can solve this by iterating through all @-marks in the
string, and seeing if any is a legitimate upstream or
empty-at mark.
Another strategy would be to parse from the right-hand side
of the string. However, that does not work for the
"empty_at" case, which allows "@@{upstream}". We need to
find the left-most one in this case (and we then recurse as
"HEAD@{upstream}").
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
get_sha1() cannot currently parse a valid object name like
"HEAD:@{upstream}" (assuming that such an oddly named file
exists in the HEAD commit). It takes two passes to parse the
string:
1. It first considers the whole thing as a ref, which
results in looking for the upstream of "HEAD:".
2. It finds the colon, parses "HEAD" as a tree-ish, and then
finds the path "@{upstream}" in the tree.
For a path that looks like a normal reflog (e.g.,
"HEAD:@{yesterday}"), the first pass is a no-op. We try to
dwim_ref("HEAD:"), that returns zero refs, and we proceed
with colon-parsing.
For "HEAD:@{upstream}", though, the first pass ends up in
interpret_upstream_mark, which tries to find the branch
"HEAD:". When it sees that the branch does not exist, it
actually dies rather than returning an error to the caller.
As a result, we never make it to the second pass.
One obvious way of fixing this would be to teach
interpret_upstream_mark to simply report "no, this isn't an
upstream" in such a case. However, that would make the
error-reporting for legitimate upstream cases significantly
worse. Something like "bogus@{upstream}" would simply report
"unknown revision: bogus@{upstream}", while the current code
diagnoses a wide variety of possible misconfigurations (no
such branch, branch exists but does not have upstream, etc).
However, we can take advantage of the fact that a branch
name cannot contain a colon. Therefore even if we find an
upstream mark, any prefix with a colon must mean that
the upstream mark we found is actually a pathname, and
should be disregarded completely. This patch implements that
logic.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
interpret_branch_name gets passed a "name" buffer to parse,
along with a "namelen" parameter representing its length. If
"namelen" is zero, we fallback to the NUL-terminated
string-length of "name".
However, it does not necessarily follow that if we have
gotten a non-zero "namelen", it is the NUL-terminated
string-length of "name". E.g., when get_sha1() is parsing
"foo:bar", we will be asked to operate only on the first
three characters.
Yet in interpret_branch_name and its helpers, we use string
functions like strchr() to operate on "name", looking past
the length we were given. This can result in us mis-parsing
object names. We should instead be limiting our search to
"namelen" bytes.
There are three distinct types of object names this patch
addresses:
- The intrepret_empty_at helper uses strchr to find the
next @-expression after our potential empty-at. In an
expression like "@:foo@bar", it erroneously thinks that
the second "@" is relevant, even if we were asked only
to look at the first character. This case is easy to
trigger (and we test it in this patch).
- When finding the initial @-mark for @{upstream}, we use
strchr. This means we might treat "foo:@{upstream}" as
the upstream for "foo:", even though we were asked only
to look at "foo". We cannot test this one in practice,
because it is masked by another bug (which is fixed in
the next patch).
- The interpret_nth_prior_checkout helper did not receive
the name length at all. This turns out not to be a
problem in practice, though, because its parsing is so
limited: it always starts from the far-left of the
string, and will not tolerate a colon (which is
currently the only way to get a smaller-than-strlen
"namelen"). However, it's still worth fixing to make the
code more obviously correct, and to future-proof us
against callers with more exotic buffers.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently fetching a one-level ref like "refs/foo" does not
work consistently. The outer "git fetch" program filters the
list of refs, checking each against check_refname_format.
Then it feeds the result to do_fetch_pack to actually
negotiate the haves/wants and get the pack. The fetch-pack
code does its own filter, and it behaves differently.
The fetch-pack filter looks for refs in "refs/", and then
feeds everything _after_ the slash (i.e., just "foo") into
check_refname_format. But check_refname_format is not
designed to look at a partial refname. It complains that the
ref has only one component, thinking it is at the root
(i.e., alongside "HEAD"), when in reality we just fed it a
partial refname.
As a result, we omit a ref like "refs/foo" from the pack
request, even though "git fetch" then tries to store the
resulting ref. If we happen to get the object anyway (e.g.,
because the ref is contained in another ref we are
fetching), then the fetch succeeds. But if it is a unique
object, we fail when trying to update "refs/foo".
We can fix this by just passing the whole refname into
check_refname_format; we know the part we were omitting is
"refs/", which is acceptable in a refname. This at least
makes the checks consistent with each other.
This problem happens most commonly with "refs/stash", which
is the only one-level ref in wide use. However, our test
does not use "refs/stash", as we may later want to restrict
it specifically (not because it is one-level, but because
of the semantics of stashes).
We may also want to do away with the multiple levels of
filtering (which can cause problems when they are out of
sync), or even forbid one-level refs entirely. However,
those decisions can come later; this fixes the most
immediate problem, which is the mismatch between the two.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Finishing touches to avoid casting unnecessary detail in stone.
* sb/diff-orderfile-config:
diff test: reading a directory as a file need not error out
With a submodule that was initialized in an old fashioned way
without gitlinks, switching branches in the superproject between
the one with and without the submodule may leave the submodule
working tree with its embedded repository behind, as there may be
unexpendable state there. Document and warn users about this.
* jl/submodule-mv-checkout-caveat:
rm: better document side effects when removing a submodule
mv: better document side effects when moving a submodule
The implementation of 'git stash $cmd "stash@{...}"' did not quote
the stash argument properly and left it split at IFS whitespace.
* ow/stash-with-ifs:
stash: handle specifying stashes with $IFS
Just like we give a reasonable default for "less" via the LESS
environment variable, specify a reasonable default for "lv" via the
"LV" environment variable when spawning the pager.
* jn/pager-lv-default-env:
pager: set LV=-c alongside LESS=FRSX
"git mv A B/", when B does not exist as a directory, should error
out, but it didn't.
* mm/mv-file-to-no-such-dir-with-slash:
mv: let 'git mv file no-such-dir/' error out on Windows, too
mv: let 'git mv file no-such-dir/' error out
"git rev-parse <revs> -- <paths>" did not implement the usual
disambiguation rules the commands in the "git log" family used in
the same way.
* jk/rev-parse-double-dashes:
rev-parse: be more careful with munging arguments
rev-parse: correctly diagnose revision errors before "--"
"git cat-file --batch=", an admittedly useless command, did not
behave very well.
* jk/cat-file-regression-fix:
cat-file: handle --batch format with missing type/size
cat-file: pass expand_data to print_object_or_die
Commit 43eb920 switched one of the sub-repository in this
test to matching to prepare for a world where the default
becomes "simple". However, the main repository needs a
similar change.
We did not notice any test failure when merged with b2ed944
(push: switch default from "matching" to "simple", 2013-01-04)
because t5531.6 is trying to provoke a failure of "git push"
due to a submodule check. When combined with b2ed944 the
push still fails, but for the wrong reason (because our
upstream setup does not exist, not because of the submodule).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is no guarantee that strbuf_read_file must error out for
directories. On some operating systems (e.g., Debian GNU/kFreeBSD
wheezy), reading a directory gives its raw content:
$ head -c5 < / | cat -A
^AM-|^_^@^L$
As a result, 'git diff -O/' succeeds instead of erroring out on
these systems, causing t4056.5 "orderfile is a directory" to fail.
On some weird OS it might even make sense to pass a directory to the
-O option and this is not a common user mistake that needs catching.
Remove the test.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is no reason to have a hardcoded upper limit of the number of
parents for an octopus merge, created via the graft mechanism.
* js/lift-parent-count-limit:
Remove the line length limit for graft files
The basic test used to leave unnecessary trash directories in the
t/ directory.
* jk/test-framework-updates:
t0000: drop "known breakage" test
t0000: simplify HARNESS_ACTIVE hack
t0000: set TEST_OUTPUT_DIRECTORY for sub-tests
"git merge-base --octopus" used to leave cleaning up suboptimal
result to the caller, but now it does the clean-up itself.
* bm/merge-base-octopus-dedup:
merge-base --octopus: reduce the result from get_octopus_merge_bases()
merge-base: separate "--independent" codepath into its own helper
Using the same username and password during the tests would not
catch a potential breakage of sending one when we should be sending
the other.
* jk/http-auth-tests-robustify:
use distinct username/password for http auth tests
Teach "cat-file --batch" to show delta-base object name for a
packed object that is represented as a delta.
* jk/oi-delta-base:
cat-file: provide %(deltabase) batch format
sha1_object_info_extended: provide delta base sha1s
"git add -A" (no other arguments) in a totally empty working tree
used to emit an error.
* nd/add-empty-fix:
add: don't complain when adding empty project root
Fetching 'frotz' branch with "git fetch", while having
'frotz/nitfol' remote-tracking branch from an earlier fetch, would
error out, primarily because the command has not been told to
remove anything on our side. In such a case, "git fetch --prune"
can be used to remove 'frotz/nitfol' to make room to fetch and
store 'frotz' remote-tracking branch.
* tm/fetch-prune:
fetch --prune: Run prune before fetching
fetch --prune: always print header url
Allow "git diff -O<file>" to be configured with a new configuration
variable.
* sb/diff-orderfile-config:
diff: add diff.orderfile configuration variable
diff: let "git diff -O" read orderfile from any file and fail properly
t4056: add new tests for "git diff -O"
read_sha1_file() that is the workhorse to read the contents given
an object name honoured object replacements, but there is no
corresponding mechanism to sha1_object_info() that is used to
obtain the metainfo (e.g. type & size) about the object, leading
callers to weird inconsistencies.
* cc/replace-object-info:
replace info: rename 'full' to 'long' and clarify in-code symbols
Documentation/git-replace: describe --format option
builtin/replace: unset read_replace_refs
t6050: add tests for listing with --format
builtin/replace: teach listing using short, medium or full formats
sha1_file: perform object replacement in sha1_object_info_extended()
t6050: show that git cat-file --batch fails with replace objects
sha1_object_info_extended(): add an "unsigned flags" parameter
sha1_file.c: add lookup_replace_object_extended() to pass flags
replace_object: don't check read_replace_refs twice
rename READ_SHA1_FILE_REPLACE flag to LOOKUP_REPLACE_OBJECT
