Introduce the new files fetch-negotiator.{h,c}, which contains an API
behind which the details of negotiation are abstracted. Currently, only
one algorithm is available: the existing one.
This patch is written to be easily reviewed: static functions are
moved verbatim from fetch-pack.c to negotiator/default.c, and it can be
seen that the lines replaced by negotiator->X() calls are present in the
X() functions respectively.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When receiving 'ACK <object-id> continue' for a common commit, check if
the commit was already known to be common and mark it as such if not up
front. This should make future refactoring of how the information about
common commits is stored more straightforward.
No visible change intended.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reduce the number of global variables by making the priority queue and
the count of non-common commits in it local, passing them as a struct to
various functions where necessary.
This also helps in the case that fetch_pack() is invoked twice in the
same process (when tag following is required when using a transport that
does not support tag following), in that different priority queues will
now be used in each invocation, instead of reusing the possibly
non-empty one.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In negotiation using protocol v2, fetch-pack sometimes does not make
full use of the information obtained in the ref advertisement:
specifically, that if the server advertises a commit that the client
also has, the client never needs to inform the server that it has the
commit's parents, since it can just tell the server that it has the
advertised commit and it knows that the server can and will infer the
rest.
This is because, in do_fetch_pack_v2(), rev_list_insert_ref_oid() is
invoked before mark_complete_and_common_ref(). This means that if we
have a commit that is both our ref and their ref, it would be enqueued
by rev_list_insert_ref_oid() as SEEN, and since it is thus already SEEN,
mark_complete_and_common_ref() would not enqueue it.
If mark_complete_and_common_ref() were invoked first, as it is in
do_fetch_pack() for protocol v0, then mark_complete_and_common_ref()
would enqueue it with COMMON_REF | SEEN. The addition of COMMON_REF
ensures that its parents are not sent as "have" lines.
Change the order in do_fetch_pack_v2() to be consistent with
do_fetch_pack(), and to avoid sending unnecessary "have" lines.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When "ACK %s ready" is received, find_common() clears rev_list in an
attempt to stop further "have" lines from being sent [1]. It is much
more readable to explicitly break from the loop instead.
So explicitly break from the loop, and make the clearing of the rev_list
happen unconditionally.
[1] The rationale is further described in the originating commit
f2cba9299b ("fetch-pack: Finish negotation if remote replies "ACK %s
ready"", 2011-03-14).
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If tag following is required when using a transport that does not
support tag following, fetch_pack() will be invoked twice in the same
process, necessitating a clearing of the object flags used by
fetch_pack() sometime during the second invocation. This is currently
done in find_common(), which means that the invocation of
mark_complete_and_common_ref() in do_fetch_pack() is useless.
(This cannot be reproduced with Git alone, because all transports that
come with Git support tag following.)
Therefore, move this clearing from find_common() to its
parent function do_fetch_pack(), right before it calls
mark_complete_and_common_ref().
This has been occurring since the commit that introduced the clearing of
marks, 420e9af498 ("Fix tag following", 2008-03-19).
The corresponding code for protocol v2 in do_fetch_pack_v2() does not
have this problem, as the clearing of flags is done before any marking
(whether by rev_list_insert_ref_oid() or
mark_complete_and_common_ref()).
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function everything_local(), despite its name, also (1) marks
commits as COMPLETE and COMMON_REF and (2) invokes filter_refs() as
important side effects. Extract (1) into its own function
(mark_complete_and_common_ref()) and remove
(2).
The restoring of save_commit_buffer, which was introduced in a1c6d7c1a7
("fetch-pack: restore save_commit_buffer after use", 2017-12-08), is a
concern of the parse_object() call in mark_complete_and_common_ref(), so
it has been moved from the end of everything_local() to the end of
mark_complete_and_common_ref().
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"index-pack --strict" has been taught to make sure that it runs the
final object integrity checks after making the freshly indexed
packfile available to itself.
* jk/index-pack-maint:
index-pack: correct install_packed_git() args
index-pack: handle --strict checks of non-repo packs
prepare_commit_graft: treat non-repository as a noop
Work around zsh segfaulting when loading git-completion.zsh
* sg/completion-zsh-workaround:
completion: correct zsh detection when run from git-completion.zsh
Finishing touches to a topic that already is in 'maint'.
* jk/submodule-fsck-loose-fixup:
fsck: avoid looking at NULL blob->object
t7415: don't bother creating commit for symlink test
Mention that this feature works with some commands (merge and cherry-pick,
implying that it also works with commands that build on these like rebase
-m and rebase -i). Explicitly mentioning two commands hopefully implies
that it may not always work with other commands (am, and rebase without
flags that imply either -m or -i).
Also, since the directory rename detection from this cycle was
specifically added in merge-recursive and not diffcore-rename, remove the
'in "diff" family" phrase from the note. (Folks have requested in the
past that `git diff` detect directory renames and somehow simplify its
output, so it may be helpful to avoid implying that diff has any new
capability here.)
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
v2.18.0-rc0~90^2 (completion: reduce overhead of clearing cached
--options, 2018-04-18) worked around a bug in bash's "set" builtin on
MacOS by using compgen instead. It was careful to avoid breaking zsh
by guarding this workaround with
if [[ -n ${ZSH_VERSION-}} ]]
Alas, this interacts poorly with git-completion.zsh's bash emulation:
ZSH_VERSION='' . "$script"
Correct it by instead using a new GIT_SOURCING_ZSH_COMPLETION shell
variable to detect whether git-completion.bash is being sourced from
git-completion.zsh. This way, the zsh variant is used both when run
from zsh directly and when run via git-completion.zsh.
Reproduction recipe:
1. cd git/contrib/completion && cp git-completion.zsh _git
2. Put the following in a new ~/.zshrc file:
autoload -U compinit; compinit
autoload -U bashcompinit; bashcompinit
fpath=(~/src/git/contrib/completion $fpath)
3. Open zsh and "git <TAB>".
With this patch:
Triggers nice git-completion.bash based tab completion
Without:
contrib/completion/git-completion.bash:354: read-only variable: QISUFFIX
zsh:12: command not found: ___main
zsh:15: _default: function definition file not found
_dispatch:70: bad math expression: operand expected at `/usr/bin/g...'
Segmentation fault
Reported-by: Rick van Hattem <wolph@wol.ph>
Reported-by: Dave Borowitz <dborowitz@google.com>
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function does not start taking the repository object as a
parameter before v2.18 track. Make the topic mergeable to v2.17
maintenance track by dropping it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The output shall behave more similar to ordinary file merges' output to provide
a more consistent user experience.
Signed-off-by: Leif Middelschulte <Leif.Middelschulte@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 159e7b080b (fsck: detect gitmodules files,
2018-05-02) taught fsck to look at the content of
.gitmodules files. If the object turns out not to be a blob
at all, we just complain and punt on checking the content.
And since this was such an obvious and trivial code path, I
didn't even bother to add a test.
Except it _does_ do one non-trivial thing, which is call the
report() function, which wants us to pass a pointer to a
"struct object". Which we don't have (we have only a "struct
object_id"). So we erroneously pass a NULL object to
report(), which gets dereferenced and causes a segfault.
It seems like we could refactor report() to just take the
object_id itself. But we pass the object pointer along to
a callback function, and indeed this ends up in
builtin/fsck.c's objreport() which does want to look at
other parts of the object (like the type).
So instead, let's just use lookup_unknown_object() to get
the real "struct object", and pass that.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Early versions of the fsck .gitmodules detection code
actually required a tree to be at the root of a commit for
it to be checked for .gitmodules. What we ended up with in
159e7b080b (fsck: detect gitmodules files, 2018-05-02),
though, finds a .gitmodules file in _any_ tree (see that
commit for more discussion).
As a result, there's no need to create a commit in our
tests. Let's drop it in the name of simplicity. And since
that was the only thing referencing $tree, we can pull our
tree creation out of a command substitution.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git remote update" can take both a single remote nickname and a
nickname for remote groups, but only one of them was documented.
* nd/remote-update-doc:
remote: doc typofix
remote.txt: update documentation for 'update' command
"git pull -recurse-submodules --rebase", when the submodule
repository's history did not have anything common between ours and
the upstream's, failed to execute. We need to fetch from them to
continue even in such a case.
* jt/submodule-pull-recurse-rebase:
submodule: do not pass null OID to setup_revisions
When v2.18.0-rc0~10^2~1 (refspec: consolidate ref-prefix generation
logic, 2018-05-16) factored out the ref-prefix generation code for
reuse, it left out the 'if (!item->exact_sha1)' test in the original
ref-prefix generation code. As a result, fetches by SHA-1 generate
ref-prefixes as though the SHA-1 being fetched were an abbreviated ref
name:
$ GIT_TRACE_PACKET=1 bin-wrappers/git -c protocol.version=2 \
fetch origin 12039e008f
[...]
packet: fetch> ref-prefix 12039e008f
packet: fetch> ref-prefix refs/12039e008f9a4e3394f3f94f8ea897785cb09448
packet: fetch> ref-prefix refs/tags/12039e008f9a4e3394f3f94f8ea897785cb09448
packet: fetch> ref-prefix refs/heads/12039e008f9a4e3394f3f94f8ea897785cb09448
packet: fetch> ref-prefix refs/remotes/12039e008f9a4e3394f3f94f8ea897785cb09448
packet: fetch> ref-prefix refs/remotes/12039e008f9a4e3394f3f94f8ea897785cb09448/HEAD
packet: fetch> 0000
If there is another ref name on the command line or the object being
fetched is already available locally, then that's mostly harmless.
But otherwise, we error out with
fatal: no matching remote head
since the server did not send any refs we are interested in. Filter
out the exact_sha1 refspecs to avoid this.
This patch adds a test to check this behavior that notices another
behavior difference between protocol v0 and v2 in the process. Add a
NEEDSWORK comment to clear it up.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Quite a many tests assumed that newly created refs are made as
loose refs using the files backend, which have been updated to use
proper plumbing like rev-parse and update-ref, to avoid breakage
once we start using different ref backends.
* cc/tests-without-assuming-ref-files-backend:
t990X: use '.git/objects' as 'deep inside .git' path
t: make many tests depend less on the refs being files
"git rev-parse Y..." etc. misbehaved when given endpoints were
not committishes.
* en/rev-parse-invalid-range:
rev-parse: check lookup'ed commit references for NULL
The import-tars script (in contrib/) has been taught to handle
tarballs with overly long paths that use PAX extended headers.
* pa/import-tars-long-names:
import-tars: read overlong names from pax extended header
The list of commands with their various attributes were spread
across a few places in the build procedure, but it now is getting a
bit more consolidated to allow more automation.
* nd/command-list:
completion: allow to customize the completable command list
completion: add and use --list-cmds=alias
completion: add and use --list-cmds=nohelpers
Move declaration for alias.c to alias.h
completion: reduce completable command list
completion: let git provide the completable command list
command-list.txt: documentation and guide line
help: use command-list.txt for the source of guides
help: add "-a --verbose" to list all commands with synopsis
git: support --list-cmds=list-<category>
completion: implement and use --list-cmds=main,others
git --list-cmds: collect command list in a string_list
git.c: convert --list-* to --list-cmds=*
Remove common-cmds.h
help: use command-list.h for common command list
generate-cmds.sh: export all commands to command-list.h
generate-cmds.sh: factor out synopsis extract code
Commit 73c3f0f704 (index-pack: check .gitmodules files with
--strict, 2018-05-04) added a call to add_packed_git(), with
the intent that the newly-indexed objects would be available
to the process when we run fsck_finish(). But that's not
what add_packed_git() does. It only allocates the struct,
and you must install_packed_git() on the result. So that
call was effectively doing nothing (except leaking a
struct).
But wait, we passed all of the tests! Does that mean we
don't need the call at all?
For normal cases, no. When we run "index-pack --stdin"
inside a repository, we write the new pack into the object
directory. If fsck_finish() needs to access one of the new
objects, then our initial lookup will fail to find it, but
we'll follow up by running reprepare_packed_git() and
looking again. That logic was meant to handle somebody else
repacking simultaneously, but it ends up working for us
here.
But there is a case that does need this, that we were not
testing. You can run "git index-pack foo.pack" on any file,
even when it is not inside the object directory. Or you may
not even be in a repository at all! This case fails without
doing the proper install_packed_git() call.
We can make this work by adding the install call.
Note that we should be prepared to handle add_packed_git()
failing. We can just silently ignore this case, though. If
fsck_finish() later needs the objects and they're not
available, it will complain itself. And if it doesn't
(because we were able to resolve the whole fsck in the first
pass), then it actually isn't an interesting error at all.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The parse_commit_buffer() function consults lookup_commit_graft()
to see if we need to rewrite parents. The latter will look
at $GIT_DIR/info/grafts. If you're outside of a repository,
then this will trigger a BUG() as of b1ef400eec (setup_git_env:
avoid blind fall-back to ".git", 2016-10-20).
It's probably uncommon to actually parse a commit outside of
a repository, but you can see it in action with:
cd /not/a/git/repo
git index-pack --strict /some/file.pack
This works fine without --strict, but the fsck checks will
try to parse any commits, triggering the BUG(). We can fix
that by teaching the graft code to behave as if there are no
grafts when we aren't in a repository.
Arguably index-pack (and fsck) are wrong to consider grafts
at all. So another solution is to disable grafts entirely
for those commands. But given that the graft feature is
deprecated anyway, it's not worth even thinking through the
ramifications that might have.
There is one other corner case I considered here. What
should:
cd /not/a/git/repo
export GIT_GRAFT_FILE=/file/with/grafts
git index-pack --strict /some/file.pack
do? We don't have a repository, but the user has pointed us
directly at a graft file, which we could respect. I believe
this case did work that way prior to b1ef400eec. However,
fixing it now would be pretty invasive. Back then we would
just call into setup_git_env() even without a repository.
But these days it actually takes a git_dir argument. So
there would be a fair bit of refactoring of the setup code
involved.
Given the obscurity of this case, plus the fact that grafts
are deprecated and probably shouldn't work under index-pack
anyway, it's not worth pursuing further. This patch at least
un-breaks the common case where you're _not_ using grafts,
but we BUG() anyway trying to even find that out.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When parse_refspec() function was created at 3eec3700 ("refspec:
factor out parsing a single refspec", 2018-05-16) to take a caller
supplied piece of memory to fill parsed refspec_item, it forgot that
a refspec without colon must set item->dst to NULL to let the users
of refspec know that the result of the fetch does not get stored in
an ref on our side.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
For consistency, use backquotes when referring to environment
variables, as is done in other man pages.
Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit b344e1614b (git remote update: Fallback to remote if group does
not exist - 2009-04-06) lets "git remote update" accept individual
remotes as well. Previously this command only accepted remote
groups. The commit updates the command syntax but not the actual
document of this subcommand. Update it.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the 2.18 cycle, directory rename detection was merged, then reverted,
then reworked in such a way to fix another prominent bug in addition to
the original problem causing it to be reverted. When the reworked series
was merged, we ended up with two nearly duplicate release notes. Remove
the second copy, but preserve the information about the extra bug fix.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a mention of the security mailing list to the README, and to
Documentation/SubmittingPatches.. 2caa7b8d27 ("git manpage: note
git-security@googlegroups.com", 2018-03-08) already added it to the
man page, but for developers either the README, or the documentation
on how to contribute (SubmittingPatches) may be the first place to
look.
Use the same wording as we already have on the git-scm.com website and
in the man page for the README, while the wording is adjusted in
SubmittingPatches to match the surrounding document better.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use names instead of numbers for the AsciiDoc attributes that are used
for the footnotes. We will add more footnotes in subsequent commits,
and attributes should ideally all be unique. Having named attributes
will help ensure uniqueness, and we won't have to re-number the
attributes if we add a footnote earlier in the document.
In addition it also clarifies that the attribute name/number is not
related to the number the footnote will get in the output.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
