Denton Liu [Tue, 29 Oct 2019 17:01:52 +0000 (10:01 -0700)]
submodule: teach set-url subcommand
Currently, in the event that a submodule's upstream URL changes, users
have to manually alter the URL in the .gitmodules file then run
`git submodule sync`. Let's make that process easier.
Teach submodule the set-url subcommand which will automatically change
the `submodule.$name.url` property in the .gitmodules file and then run
`git submodule sync` to complete the process.
Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
William Baker [Wed, 16 Oct 2019 19:35:47 +0000 (19:35 +0000)]
t7519-status-fsmonitor: improve comments
The comments for the staging/unstaging test did not accurately
describe the scenario being tested. It is not essential that
the test files being staged/unstaged appear at the end of the
index. All that is required is that the test files are not
flagged with CE_FSMONITOR_VALID and have a position in the
index greater than the number of entries in the index after
unstaging.
The comment for this test has been updated to be more
accurate with respect to the scenario that's being tested.
Signed-off-by: William Baker <William.Baker@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Prarit Bhargava [Tue, 29 Oct 2019 12:09:14 +0000 (08:09 -0400)]
pretty: add "%aL" etc. to show local-part of email addresses
In many projects the number of contributors is low enough that users know
each other and the full email address doesn't need to be displayed.
Displaying only the author's username saves a lot of columns on the screen.
Existing 'e/E' (as in "%ae" and "%aE") placeholders would show the
author's address as "prarit@redhat.com", which would waste columns to show
the same domain-part for all contributors when used in a project internal
to redhat. Introduce 'l/L' placeholders that strip '@' and domain part from
the e-mail address.
Signed-off-by: Prarit Bhargava <prarit@redhat.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Philippe Blain [Sun, 27 Oct 2019 17:16:25 +0000 (17:16 +0000)]
worktree: teach "add" to ignore submodule.recurse config
"worktree add" internally calls "reset --hard", but if
submodule.recurse is set, reset tries to recurse into
initialized submodules, which makes start_command try to
cd into non-existing submodule paths and die.
Fix that by making sure that the call to reset in "worktree add"
does not recurse.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Philippe Blain [Mon, 28 Oct 2019 13:05:46 +0000 (13:05 +0000)]
help: add gitsubmodules to the list of guides
The guide "gitsubmodules" was added in d480345 (submodules: overhaul
documentation, 2017-06-22), but it was not added to
command-list.txt when commit 1b81d8c (help: use command-list.txt
for the source of guides, 2018-05-20) taught "git help" to obtain the
guide list from this file.
Add it now, and capitalize the first word of the description of
gitsubmodules, as was done in 1b81d8c (help: use command-list.txt
for the source of guides, 2018-05-20) for the other guides.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ever since worktrees were introduced, the `git_path()` function _really_
needed to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is
specific to the worktree, and therefore so is its reflog). However, the
wrong path is returned for `logs/HEAD.lock`.
This does not matter as long as the Git executable is doing the asking,
as the path for that `logs/HEAD.lock` file is constructed from
`git_path("logs/HEAD")` by appending the `.lock` suffix.
However, Git GUI just learned to use `--git-path` instead of appending
relative paths to what `git rev-parse --git-dir` returns (and as a
consequence not only using the correct hooks directory, but also using
the correct paths in worktrees other than the main one). While it does
not seem as if Git GUI in particular is asking for `logs/HEAD.lock`,
let's be safe rather than sorry.
Side note: Git GUI _does_ ask for `index.lock`, but that is already
resolved correctly, due to `update_common_dir()` preferring to leave
unknown paths in the (worktree-specific) git directory.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Without this, you cannot use `--run=<...>` to skip that part, and a run
with `--run=0` (which is a common way to determine the test case number
corresponding to a given test case title).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Mon, 28 Oct 2019 10:52:41 +0000 (11:52 +0100)]
builtin/blame.c: remove '--indent-heuristic' from usage string
The indent heuristic is our default diff heuristic since 33de716387
(diff: enable indent heuristic by default, 2017-05-08), but the usage
string of 'git blame' still mentions it as "experimental heuristic".
We could simply update the short help associated with the option, but
according to the comment above the option's declaration it was "only
included here to get included in the "-h" output". That made sense
while the feature was still experimental and we wanted to give it more
exposure, but nowadays it's unnecessary.
So let's rather remove the '--indent-heuristic' option from 'git
blame's usage string. Note that 'git blame' will still accept this
option, as it is parsed in parse_revision_opt().
Astute readers may notice that this patch removes a comment mentioning
"the following two options", but it only removes one option. The
reason is that the comment is outdated: that other options was
'--compaction-heuristic', and it has already been removed in 3cde4e02ee (diff: retire "compaction" heuristics, 2016-12-23), but
that commit forgot to update this comment.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Miriam Rubio [Mon, 28 Oct 2019 16:55:23 +0000 (17:55 +0100)]
clone: rename static function `dir_exists()`.
builtin/clone.c has a static function dir_exists() that
checks if a given path exists on the filesystem. It returns
true (and it is correct for it to return true) when the
given path exists as a non-directory (e.g. a regular file).
This is confusing. What the caller wants to check, and what
this function wants to return, is if the path exists, so
rename it to path_exists().
Signed-off-by: Miriam Rubio <mirucam@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Mihail Atanassov [Mon, 28 Oct 2019 22:01:22 +0000 (22:01 +0000)]
Documentation/git-bisect.txt: add --no-ff to merge command
The hotfix application example uses `git merge --no-commit` to apply
temporary changes to the working tree during a bisect operation. In some
situations this can be a fast-forward and `merge` will apply the hotfix
branch's commits regardless of `--no-commit` (as documented in the `git
merge` manual).
In the pathological case this will make a `git bisect run` invocation
loop indefinitely between the first bisect step and the fast-forwarded
post-merge HEAD.
Add `--no-ff` to the merge command to avoid this issue.
Signed-off-by: Mihail Atanassov <m.atanassov92@gmail.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jiang Xin [Mon, 28 Oct 2019 05:18:55 +0000 (13:18 +0800)]
Merge tag 'v2.24.0-rc1' of github.com:git/git into master
Git 2.24-rc1
* tag 'v2.24.0-rc1' of github.com:git/git:
Git 2.24-rc1
repo-settings: read an int for index.version
ci: fix GCC install in the Travis CI GCC OSX job
Eleventh batch
ci(osx): use new location of the `perforce` cask
t7419: change test_must_fail to ! for grep
t4014: make output-directory tests self-contained
ci(visual-studio): actually run the tests in parallel
ci(visual-studio): use strict compile flags, and optimization
userdiff: fix some corner cases in dts regex
test-progress: fix test failures on big-endian systems
completion: clarify installation instruction for zsh
grep: avoid leak of chartables in PCRE2
grep: make PCRE2 aware of custom allocator
grep: make PCRE1 aware of custom allocator
remote-curl: pass on atomic capability to remote side
diff-highlight: fix a whitespace nit
fsmonitor: don't fill bitmap with entries to be removed
Jeff King [Fri, 18 Oct 2019 05:02:08 +0000 (01:02 -0400)]
fsck: accept an oid instead of a "struct tree" for fsck_tree()
We don't actually look at the tree struct in fsck_tree() beyond its oid
and type (which is obviously OBJ_TREE). Just taking an oid gives our
callers more flexibility to avoid creating a struct, and makes it clear
that we are fscking just what is in the buffer, not any pre-parsed bits
from the struct.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 05:01:48 +0000 (01:01 -0400)]
fsck: accept an oid instead of a "struct commit" for fsck_commit()
We don't actually look at the commit struct in fsck_commit() beyond its
oid and type (which is obviously OBJ_COMMIT). Just taking an oid gives
our callers more flexibility to avoid creating or parsing a struct, and
makes it clear that we are fscking just what is in the buffer, not any
pre-parsed bits from the struct.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 05:01:26 +0000 (01:01 -0400)]
fsck: accept an oid instead of a "struct tag" for fsck_tag()
We don't actually look at the tag struct in fsck_tag() beyond its oid
and type (which is obviously OBJ_TAG). Just taking an oid gives our
callers more flexibility to avoid creating or parsing a struct, and
makes it clear that we are fscking just what is in the buffer, not any
pre-parsed bits from the struct.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 05:00:59 +0000 (01:00 -0400)]
fsck: rename vague "oid" local variables
In fsck_commit() and fsck_tag(), we have local "oid" variables used for
parsing parent and tagged-object oids. Let's give these more specific
names in preparation for the functions taking an "oid" parameter for the
object we're checking.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 05:00:04 +0000 (01:00 -0400)]
fsck: don't require an object struct for fsck_ident()
The only thing we do with the struct is pass its oid and type to
report(). We can just take those explicitly, which gives our callers
more flexibility.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 04:59:54 +0000 (00:59 -0400)]
fsck: drop blob struct from fsck_finish()
Since fsck_blob() no longer requires us to have a "struct blob", we
don't need to create one. Which also means we don't need to worry about
handling the case that lookup_blob() returns NULL (we'll still catch
wrongly-identified blobs when we read the actual object contents and
type from disk).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 04:59:29 +0000 (00:59 -0400)]
fsck: accept an oid instead of a "struct blob" for fsck_blob()
We don't actually need any information from the object struct except its
oid (and the type, of course, but that's implicitly OBJ_BLOB). This
gives our callers more flexibility to drop the object structs, too.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 04:59:15 +0000 (00:59 -0400)]
fsck: don't require an object struct for report()
The report() function really only cares about the oid and type of the
object, not the full object struct. Let's convert it to take those two
items separately, which gives our callers more flexibility.
This makes some already-long lines even longer. I've mostly left them,
as our eventual goal is to shrink these down as we continue refactoring
(e.g., "&item->object" becomes "&item->object.oid, item->object.type",
but will eventually shrink down to "oid, type").
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 04:58:40 +0000 (00:58 -0400)]
fsck: only provide oid/type in fsck_error callback
None of the callbacks actually care about having a "struct object";
they're happy with just the oid and type information. So let's give
ourselves more flexibility to avoid having a "struct object" by just
passing the broken-down fields.
Note that the callback already takes a "type" field for the fsck message
type. We'll rename that to "msg_type" (and use "object_type" for the
object type) to make the distinction explicit.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 04:58:07 +0000 (00:58 -0400)]
fsck: don't require object structs for display functions
Our printable_type() and describe_object() functions take whole object
structs, but they really only care about the oid and type. Let's take
those individually in order to give our callers more flexibility.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 04:57:37 +0000 (00:57 -0400)]
fsck: use oids rather than objects for object_name API
We don't actually care about having object structs; we only need to look
up decorations by oid. Let's accept this more limited form, which will
give our callers more flexibility.
Note that the decoration API we rely on uses object structs itself (even
though it only looks at their oids). We can solve this by switching to
a kh_oid_map (we could also use the hashmap oidmap, but it's more
awkward for the simple case of just storing a void pointer).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 04:56:13 +0000 (00:56 -0400)]
fsck: unify object-name code
Commit 90cf590f53 (fsck: optionally show more helpful info for broken
links, 2016-07-17) added a system for decorating objects with names. The
code is split across builtin/fsck.c (which gives the initial names) and
fsck.c (which adds to the names as it traverses the object graph). This
leads to some duplication, where both sites have near-identical
describe_object() functions (the difference being that the one in
builtin/fsck.c uses a circular array of buffers to allow multiple calls
in a single printf).
Let's provide a unified object_name API for fsck. That lets us drop the
duplication, as well as making the interface boundaries more clear
(which will let us refactor the implementation more in a future patch).
We'll leave describe_object() in builtin/fsck.c as a thin wrapper around
the new API, as it relies on a static global to make its many callers a
bit shorter.
We'll also convert the bare add_decoration() calls in builtin/fsck.c to
put_object_name(). This fixes two minor bugs:
1. We leak many small strings. add_decoration() has a last-one-wins
approach: it updates the decoration to the new string and returns
the old one. But we ignore the return value, leaking the old
string. This is quite common to trigger, since we look at reflogs:
the tip of any ref will be described both by looking at the actual
ref, as well as the latest reflog entry. So we'd always end up
leaking one of those strings.
2. The last-one-wins approach gives us lousy names. For instance, we
first look at all of the refs, and then all of the reflogs. So
rather than seeing "refs/heads/master", we're likely to overwrite
it with "HEAD@{12345678}". We're generally better off using the
first name we find.
And indeed, the test in t1450 expects this ugly HEAD@{} name. After
this patch, we've switched to using fsck_put_object_name()'s
first-one-wins semantics, and we output the more human-friendly
"refs/tags/julius" (and the test is updated accordingly).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 04:54:12 +0000 (00:54 -0400)]
fsck: require an actual buffer for non-blobs
The fsck_object() function takes in a buffer, but also a "struct
object". The rules for using these vary between types:
- for a commit, we'll use the provided buffer; if it's NULL, we'll
fall back to get_commit_buffer(), which loads from either an
in-memory cache or from disk. If the latter fails, we'd die(), which
is non-ideal for fsck.
- for a tag, a NULL buffer will fall back to loading the object from
disk (and failure would lead to an fsck error)
- for a tree, we _never_ look at the provided buffer, and always use
tree->buffer
- for a blob, we usually don't look at the buffer at all, unless it
has been marked as a .gitmodule file. In that case we check the
buffer given to us, or assume a NULL buffer is a very large blob
(and complain about it)
This is much more complex than it needs to be. It turns out that nobody
ever feeds a NULL buffer that isn't a blob:
- git-fsck calls fsck_object() only from fsck_obj(). That in turn is
called by one of:
- fsck_obj_buffer(), which is a callback to verify_pack(), which
unpacks everything except large blobs into a buffer (see
pack-check.c, lines 131-141).
- fsck_loose(), which hits a BUG() on non-blobs with a NULL buffer
(builtin/fsck.c, lines 639-640)
And in either case, we'll have just called parse_object_buffer()
anyway, which would segfault on a NULL buffer for commits or tags
(not for trees, but it would install a NULL tree->buffer which would
later cause a segfault)
- git-index-pack asserts that the buffer is non-NULL unless the object
is a blob (see builtin/index-pack.c, line 832)
- git-unpack-objects always writes a non-NULL buffer into its
obj_buffer hash, which is then fed to fsck_object(). (There is
actually a funny thing here where it does not store blob buffers at
all, nor does it call fsck on them; it does check any needed blobs
via fsck_finish() though).
Let's make the rules simpler, which reduces the amount of code and gives
us more flexibility in refactoring the fsck code. The new rules are:
- only blobs are allowed to pass a NULL buffer
- we always use the provided buffer, never pulling information from
the object struct
We don't have to adjust any callers, because they were already adhering
to these. Note that we do drop a few fsck identifiers for missing tags,
but that was all dead code (because nobody passed a NULL tag buffer).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 04:51:19 +0000 (00:51 -0400)]
fsck: stop checking tag->tagged
Way back in 92d4c85d24 (fsck-cache: fix SIGSEGV on bad tag object,
2005-05-03), we added an fsck check that the "tagged" field of a tag
struct isn't NULL. But that was mainly protecting the printing code for
"--tags", and that code wasn't moved along with the check as part of ba002f3b28 (builtin-fsck: move common object checking code to fsck.c,
2008-02-25).
It could also serve to detect type mismatch problems (where a tag points
to object X as a commit, but really X is a blob), but it couldn't do so
reliably (we'd call lookup_commit(X), but it will only notice the
problem if we happen to have previously called lookup_blob(X) in the
same process). And as of a commit earlier in this series, we'd consider
that a parse error and complain about the object even before getting to
this point anyway.
So let's drop this "tag->tagged" check. It's not helping anything, and
getting rid of it makes the function conceptually cleaner, as it really
is just checking the buffer we feed it. In fact, we can get rid of our
one-line wrapper and just unify fsck_tag() and fsck_tag_buffer().
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 04:49:10 +0000 (00:49 -0400)]
fsck: stop checking commit->parent counts
In 4516338243 (builtin-fsck: reports missing parent commits,
2008-02-25), we added code to check that fsck found the same number of
parents from parsing the commit itself as we see in the commit struct we
got from parse_commit_buffer(). Back then the rationale was that the
normal commit parser might skip some bad parents.
But earlier in this series, we started treating that reliably as a
parsing error, meaning that we'd complain about it before we even hit
the code in fsck.c.
Let's drop this code, which now makes fsck_commit_buffer() completely
independent of any parsed values in the commit struct (that's
conceptually cleaner, and also opens up more refactoring options).
Note that we can also drop the MISSING_PARENT and MISSING_GRAFT fsck
identifiers. This is no loss, as these would not trigger reliably
anyway. We'd hit them only when lookup_commit() failed, which occurs
only if we happen to have seen the object with another type already in
the same process. In most cases, we'd actually run into the problem
during the connectivity walk, not here.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 18 Oct 2019 04:48:20 +0000 (00:48 -0400)]
fsck: stop checking commit->tree value
We check in fsck_commit_buffer() that commit->tree isn't NULL, which in
turn generally comes from a previous parse by parse_commit(). But this
isn't really accomplishing anything. The two things we might care about
are:
- was there a syntactically valid "tree <oid>" line in the object? But
we've just done our own parse in fsck_commit_buffer() to check this.
- does it point to a valid tree object? But checking the "tree"
pointer here doesn't actually accomplish that; it just shows that
lookup_tree() didn't return NULL, which only means that we haven't
yet seen that oid as a non-tree in this process.
A real connectivity check would exhaustively walk all graph links,
and we do that already in a separate function.
So this code isn't helping anything. And it makes the fsck code slightly
more confusing and rigid (e.g., it requires that any commit structs have
already been parsed). Let's drop it.
As a bit of history, the presence of this code looks like a leftover
from early fsck code (which did rely on parse_commit() to do most of the
parsing). The check comes from ff5ebe39b0 (Port fsck-cache to use
parsing functions, 2005-04-18), but we later added an explicit walk in 355885d531 (add generic, type aware object chain walker, 2008-02-25).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 25 Oct 2019 21:20:20 +0000 (17:20 -0400)]
commit, tag: don't set parsed bit for parse failures
If we can't parse a commit, then parse_commit() will return an error
code. But it _also_ sets the "parsed" flag, which tells us not to bother
trying to re-parse the object. That means that subsequent parses have no
idea that the information in the struct may be bogus. I.e., doing this:
parse_commit(commit);
...
if (parse_commit(commit) < 0)
die("commit is broken");
will never trigger the die(). The second parse_commit() will see the
"parsed" flag and quietly return success.
There are two obvious ways to fix this:
1. Stop setting "parsed" until we've successfully parsed.
2. Keep a second "corrupt" flag to indicate that we saw an error (and
when the parsed flag is set, return 0/-1 depending on the corrupt
flag).
This patch does option 1. The obvious downside versus option 2 is that
we might continually re-parse a broken object. But in practice,
corruption like this is rare, and we typically die() or return an error
in the caller. So it's OK not to worry about optimizing for corruption.
And it's much simpler: we don't need to use an extra bit in the object
struct, and callers which check the "parsed" flag don't need to learn
about the corrupt bit, too.
There's no new test here, because this case is already covered in t5318.
Note that we do need to update the expected message there, because we
now detect the problem in the return from "parse_commit()", and not with
a separate check for a NULL tree. In fact, we can now ditch that
explicit tree check entirely, as we're covered robustly by this change
(and the previous recent change to treat a NULL tree as a parse error).
We'll also give tags the same treatment. I don't know offhand of any
cases where the problem can be triggered (it implies somebody ignoring a
parse error earlier in the process), but consistently returning an error
should cause the least surprise.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When this function is passed a path with a trailing slash, it runs right
over the end of that path.
Let's fix this.
Co-authored-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The first line in 'git commit-graph's usage string indicates that this
command can be invoked without specifying a subcommand. However, this
is not the case:
brian m. carlson [Mon, 28 Oct 2019 00:59:05 +0000 (00:59 +0000)]
t4044: update test to work with SHA-256
This test produces pseudo-collisions and tests git diff's behavior with
them, and is therefore sensitive to the hash in use. Update the test to
compute the collisions for both SHA-1 and SHA-256 using appropriate
constants. Move the heredocs inside the setup block so that all of the
setup code can be tested for failure.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
brian m. carlson [Mon, 28 Oct 2019 00:59:03 +0000 (00:59 +0000)]
t4038: abstract away SHA-1 specific constants
Compute several object IDs that exist in expected output, since we don't
care about the specific object IDs, only that the format of the output
is syntactically correct.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
brian m. carlson [Mon, 28 Oct 2019 00:59:02 +0000 (00:59 +0000)]
t4034: abstract away SHA-1-specific constants
Adjust the test so that it computes variables for object IDs instead of
using hard-coded hashes. Move some expected result heredocs around so
that they can use computed variables.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
brian m. carlson [Mon, 28 Oct 2019 00:59:01 +0000 (00:59 +0000)]
t4027: make hash-size independent
Instead of hard-coding the length of an object ID, look this value up
using the translation tables. Similarly, compute input data for invalid
submodule entries using the tables as well.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
brian m. carlson [Mon, 28 Oct 2019 00:58:56 +0000 (00:58 +0000)]
t1305: avoid comparing extensions
A repository using a hash other than SHA-1 will need to have an
extension in the config file. Ignore any extensions when comparing
config files, since they don't usefully contribute to the goal of the
test.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
brian m. carlson [Mon, 28 Oct 2019 00:58:55 +0000 (00:58 +0000)]
rev-parse: add a --show-object-format option
Add an option to print the object format used for input, output, or
storage. This allows shell scripts to discover the hash algorithm in
use.
Since the transition plan allows for multiple input algorithms, document
that we may provide multiple results for input, and the format that the
results may take. While we don't support this now, documenting it early
means that script authors can future-proof their scripts for when we do.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Robert Luberda [Sun, 27 Oct 2019 09:14:26 +0000 (10:14 +0100)]
gitweb: correctly store previous rev in javascript-actions mode
Without this change, the setting
$feature{'javascript-actions'}{'default'} = [1];
in gitweb.conf breaks gitweb's blame page: clicking on line numbers
displayed in the second column on the page has no effect.
For comparison, with javascript-actions disabled, clicking on line
numbers loads the previous version of the line.
Addresses https://bugs.debian.org/741883.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Robert Luberda <robert@debian.org> Acked-by: Jakub Narębski <jnareb@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Derrick Stolee [Thu, 24 Oct 2019 13:40:42 +0000 (13:40 +0000)]
commit-graph: fix writing first commit-graph during fetch
The previous commit includes a failing test for an issue around
fetch.writeCommitGraph and fetching in a repo with a submodule. Here, we
fix that bug and set the test to "test_expect_success".
The problem arises with this set of commands when the remote repo at
<url> has a submodule. Note that --recurse-submodules is not needed to
demonstrate the bug.
$ git clone <url> test
$ cd test
$ git -c fetch.writeCommitGraph=true fetch origin
Computing commit graph generation numbers: 100% (12/12), done.
BUG: commit-graph.c:886: missing parent <hash1> for commit <hash2>
Aborted (core dumped)
As an initial fix, I converted the code in builtin/fetch.c that calls
write_commit_graph_reachable() to instead launch a "git commit-graph
write --reachable --split" process. That code worked, but is not how we
want the feature to work long-term.
That test did demonstrate that the issue must be something to do with
internal state of the 'git fetch' process.
The write_commit_graph() method in commit-graph.c ensures the commits we
plan to write are "closed under reachability" using close_reachable().
This method walks from the input commits, and uses the UNINTERESTING
flag to mark which commits have already been visited. This allows the
walk to take O(N) time, where N is the number of commits, instead of
O(P) time, where P is the number of paths. (The number of paths can be
exponential in the number of commits.)
However, the UNINTERESTING flag is used in lots of places in the
codebase. This flag usually means some barrier to stop a commit walk,
such as in revision-walking to compare histories. It is not often
cleared after the walk completes because the starting points of those
walks do not have the UNINTERESTING flag, and clear_commit_marks() would
stop immediately.
This is happening during a 'git fetch' call with a remote. The fetch
negotiation is comparing the remote refs with the local refs and marking
some commits as UNINTERESTING.
I tested running clear_commit_marks_many() to clear the UNINTERESTING
flag inside close_reachable(), but the tips did not have the flag, so
that did nothing.
It turns out that the calculate_changed_submodule_paths() method is at
fault. Thanks, Peff, for pointing out this detail! More specifically,
for each submodule, the collect_changed_submodules() runs a revision
walk to essentially do file-history on the list of submodules. That
revision walk marks commits UNININTERESTING if they are simplified away
by not changing the submodule.
Instead, I finally arrived on the conclusion that I should use a flag
that is not used in any other part of the code. In commit-reach.c, a
number of flags were defined for commit walk algorithms. The REACHABLE
flag seemed like it made the most sense, and it seems it was not
actually used in the file. The REACHABLE flag was used in early versions
of commit-reach.c, but was removed by 4fbcca4 (commit-reach: make
can_all_from_reach... linear, 2018-07-20).
Add the REACHABLE flag to commit-graph.c and use it instead of
UNINTERESTING in close_reachable(). This fixes the bug in manual
testing.
Reported-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Jeff King <peff@peff.net> Helped-by: Szeder Gábor <szeder.dev@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
While dogfooding, Johannes found a bug in the fetch.writeCommitGraph
config behavior. His example initially happened during a clone with
--recurse-submodules, we found that this happens with the first fetch
after cloning a repository that contains a submodule:
$ git clone <url> test
$ cd test
$ git -c fetch.writeCommitGraph=true fetch origin
Computing commit graph generation numbers: 100% (12/12), done.
BUG: commit-graph.c:886: missing parent <hash1> for commit <hash2>
Aborted (core dumped)
In the repo I had cloned, there were really 60 commits to scan, but
only 12 were in the list to write when calling
compute_generation_numbers(). A commit in the list expects to see a
parent, but that parent is not in the list.
A follow-up will fix the bug, but first we create a test that
demonstrates the problem. This test must be careful about an existing
commit-graph file, since GIT_TEST_COMMIT_GRAPH=1 will cause the repo we
are cloning to already have one. This then prevents the incremtnal
commit-graph write during the first 'git fetch'.
Helped-by: Jeff King <peff@peff.net> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Szeder Gábor <szeder.dev@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Heba Waly [Thu, 24 Oct 2019 11:29:11 +0000 (11:29 +0000)]
documentation: remove empty doc files
Remove empty and redundant documentation files from the
Documentation/technical/ directory.
The empty doc files included only TODO messages with no documentation for
years. Instead an approach is being taken to keep all doc beside the code
in the relevant header files.
Having empty doc files is confusing and disappointing to anybody looking
for information, besides having the documentation in header files makes it
easier for developers to find the information they are looking for.
Some of the content which could have gone here already exists elsewhere:
- api-object-access.txt -> sha1-file.c and object.h have some details.
- api-quote.txt -> quote.h has some details.
- api-xdiff-interface.txt -> xdiff-interface.h has some details.
- api-grep.txt -> grep.h does not have enough documentation at the moment.
Signed-off-by: Heba Waly <heba.waly@gmail.com> Reviewed-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This will cause a "unable to load config blob object", because the
fetch_config_from_gitmodules() invocation in cmd_fetch() will attempt to
load ".gitmodules" (which Git knows to exist because the client has the
tree of HEAD) while fetch_if_missing is set to 0.
fetch_if_missing is set to 0 too early - ".gitmodules" here should be
lazily fetched. Git must set fetch_if_missing to 0 before the fetch
because as part of the fetch, packfile negotiation happens (and we do
not want to fetch any missing objects when checking existence of
objects), but we do not need to set it so early. Move the setting of
fetch_if_missing to the earliest possible point in cmd_fetch(), right
before any fetching happens.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Derrick Stolee [Wed, 23 Oct 2019 20:38:57 +0000 (20:38 +0000)]
repo-settings: read an int for index.version
Several config options were combined into a repo_settings struct in
ds/feature-macros, including a move of the "index.version" config
setting in 7211b9e (repo-settings: consolidate some config settings,
2019-08-13).
Unfortunately, that file looked like a lot of boilerplate and what is
clearly a factor of copy-paste overload, the config setting is parsed
with repo_config_ge_bool() instead of repo_config_get_int(). This means
that a setting "index.version=4" would not register correctly and would
revert to the default version of 3.
I caught this while incorporating v2.24.0-rc0 into the VFS for Git
codebase, where we really care that the index is in version 4.
This was not caught by the codebase because the version checks placed
in t1600-index.sh did not test the "basic" scenario enough. Here, we
modify the test to include these normal settings to not be overridden by
features.manyFiles or GIT_INDEX_VERSION. While the "default" version is
3, this is demoted to version 2 in do_write_index() when not necessary.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Denton Liu [Wed, 23 Oct 2019 23:32:38 +0000 (16:32 -0700)]
apply: respect merge.conflictStyle in --3way
Before, when doing a 3-way merge, the merge.conflictStyle option was not
respected and the "merge" style was always used, even if "diff3" was
specified.
Call git_xmerge_config() at the end of git_apply_config() so that the
merge.conflictStyle config is read.
Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Denton Liu [Wed, 23 Oct 2019 23:32:36 +0000 (16:32 -0700)]
t4108: demonstrate bug in apply
Currently, apply does not respect the merge.conflictStyle setting.
Demonstrate this by making the 'apply with --3way' test case generic and
extending it to show that the configuration of
merge.conflictStyle = diff3 causes a breakage.
Change print_sanitized_conflicted_diff() to also sanitize `|||||||`
conflict markers.
Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Denton Liu [Wed, 23 Oct 2019 23:32:33 +0000 (16:32 -0700)]
t4108: use `test_config` instead of `git config`
Since `git config` leaves the configurations set even after the test
case completes, use `test_config` instead so that the configurations are
reset once the test case finishes.
Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Denton Liu [Wed, 23 Oct 2019 23:32:31 +0000 (16:32 -0700)]
t4108: remove git command upstream of pipe
Before, the output of `git diff HEAD` would always be piped to
sanitize_conflicted_diff(). However, since the Git command was upstream
of the pipe, in case the Git command fails, the return code would be
lost. Rewrite into separate statements so that the return code is no
longer lost.
Since only the command `git diff HEAD` was being piped to
sanitize_conflicted_diff(), move the command into the function and rename
it to print_sanitized_conflicted_diff().
Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Denton Liu [Wed, 23 Oct 2019 23:32:28 +0000 (16:32 -0700)]
t4108: replace create_file with test_write_lines
Since the locally defined create_file() duplicates the functionality of
the test_write_lines() helper function, remove create_file() and replace
all instances with test_write_lines(). While we're at it, move
redirection operators to the end of the command which is the more
conventional place to put it.
Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Thu, 24 Oct 2019 00:20:40 +0000 (02:20 +0200)]
ci: fix GCC install in the Travis CI GCC OSX job
A few days ago Travis CI updated their existing OSX images, including
the Homebrew database in the xcode10.1 OSX image that we use. Since
then installing dependencies in the 'osx-gcc' job fails when it tries
to link gcc@8:
+ brew link gcc@8
Error: No such keg: /usr/local/Cellar/gcc@8
GCC8 is still installed but not linked to '/usr/local' in the updated
image, as it was before this update, but now we have to link it by
running 'brew link gcc'. So let's do that then, and fall back to
linking gcc@8 if it doesn't, just to be sure.
Our builds on Azure Pipelines are unaffected by this issue. The OSX
image over there doesn't contain the gcc@8 package, so we have to
'brew install' it, which already takes care of linking it to
'/usr/local'. After that the 'brew link gcc' command added by this
patch fails, but the ||-chained fallback 'brew link gcc@8' command
succeeds with an "already linked" warning.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Heba Waly [Wed, 23 Oct 2019 05:30:52 +0000 (05:30 +0000)]
config: move documentation to config.h
Move the documentation from Documentation/technical/api-config.txt into
config.h as it's easier for the developers to find the usage information
beside the code instead of looking for it in another doc file, also
documentation/technical/api-config.txt is removed because the information
it has is now redundant and it'll be hard to keep it up to date and
syncronized with the documentation in config.h
Signed-off-by: Heba Waly <heba.waly@gmail.com> Reviewed-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Wed, 23 Oct 2019 05:43:11 +0000 (14:43 +0900)]
Merge branch 'cb/pcre2-chartables-leakfix'
Leakfix.
* cb/pcre2-chartables-leakfix:
grep: avoid leak of chartables in PCRE2
grep: make PCRE2 aware of custom allocator
grep: make PCRE1 aware of custom allocator
SZEDER Gábor [Mon, 21 Oct 2019 16:00:43 +0000 (18:00 +0200)]
path.c: don't call the match function without value in trie_find()
'logs/refs' is not a working tree-specific path, but since commit b9317d55a3 (Make sure refs/rewritten/ is per-worktree, 2019-03-07)
'git rev-parse --git-path' has been returning a bogus path if a
trailing '/' is present:
We use a trie data structure to efficiently decide whether a path
belongs to the common dir or is working tree-specific. As it happens b9317d55a3 triggered a bug that is as old as the trie implementation
itself, added in 4e09cf2acf (path: optimize common dir checking,
2015-08-31).
- According to the comment describing trie_find(), it should only
call the given match function 'fn' for a "/-or-\0-terminated
prefix of the key for which the trie contains a value". This is
not true: there are three places where trie_find() calls the match
function, but one of them is missing the check for value's
existence.
- b9317d55a3 added two new keys to the trie: 'logs/refs/rewritten'
and 'logs/refs/worktree', next to the already existing
'logs/refs/bisect'. This resulted in a trie node with the path
'logs/refs/', which didn't exist before, and which doesn't have a
value attached. A query for 'logs/refs/' finds this node and then
hits that one callsite of the match function which doesn't check
for the value's existence, and thus invokes the match function
with NULL as value.
- When the match function check_common() is invoked with a NULL
value, it returns 0, which indicates that the queried path doesn't
belong to the common directory, ultimately resulting the bogus
path shown above.
Add the missing condition to trie_find() so it will never invoke the
match function with a non-existing value. check_common() will then no
longer have to check that it got a non-NULL value, so remove that
condition.
I believe that there are no other paths that could cause similar bogus
output. AFAICT the only other key resulting in the match function
being called with a NULL value is 'co' (because of the keys 'common'
and 'config'). However, as they are not in a directory that belongs
to the common directory the resulting working tree-specific path is
expected.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Mon, 21 Oct 2019 16:00:42 +0000 (18:00 +0200)]
path.c: clarify two field names in 'struct common_dir'
An array of 'struct common_dir' instances is used to specify whether
various paths in $GIT_DIR are specific to a worktree, or are common,
i.e. belong to main worktree. The names of two fields in this
struct are somewhat confusing or ambigious:
- The path is recorded in the struct's 'dirname' field, even though
several entries are regular files e.g. 'gc.pid', 'packed-refs',
etc.
Rename this field to 'path' to reduce confusion.
- The field 'exclude' tells whether the path is excluded... from
where? Excluded from the common dir or from the worktree? It
means the former, but it's ambigious.
Rename this field to 'is_common' to make it unambigious what it
means. This, however, means the exact opposite of what 'exclude'
meant, so we have to negate the field's value in all entries as
well.
The diff is best viewed with '--color-words'.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Mon, 21 Oct 2019 16:00:41 +0000 (18:00 +0200)]
path.c: mark 'logs/HEAD' in 'common_list' as file
'logs/HEAD', i.e. HEAD's reflog, is a file, but its entry in
'common_list' has the 'is_dir' bit set.
Unset that bit to make it consistent with what 'logs/HEAD' is supposed
to be.
This doesn't make a difference in behavior: check_common() is the only
function that looks at the 'is_dir' bit, and that function either
returns 0, or '!exclude', which for 'logs/HEAD' results in 0 as well.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Mon, 21 Oct 2019 16:00:40 +0000 (18:00 +0200)]
path.c: clarify trie_find()'s in-code comment
A fairly long comment describes trie_find()'s behavior and shows
examples, but it's slightly incomplete/inaccurate. Update this
comment to specify how trie_find() handles a negative return value
from the given match function.
Furthermore, update the list of examples to include not only two but
three levels of path components. This makes the examples slightly
more complicated, but it can illustrate the behavior in more corner
cases.
Finally, basically everything refers to the data stored for a key as
"value", with two confusing exceptions:
- The type definition of the match function calls its corresponding
parameter 'data'.
Rename that parameter to 'value'. (check_common(), the only
function of this type already calls it 'value').
- The table of examples above trie_find() has a "val from node"
column, which has nothing to do with the value stored in the trie:
it's a "prefix of the key for which the trie contains a value"
that led to that node.
Rename that column header to "prefix to node".
Note that neither the original nor the updated description and
examples correspond 100% to the current implementation, because the
implementation is a bit buggy, but the comment describes the desired
behavior. The bug will be fixed in the last patch of this series.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Mon, 21 Oct 2019 16:00:39 +0000 (18:00 +0200)]
Documentation: mention more worktree-specific exceptions
If a directory in $GIT_DIR is overridden when $GIT_COMMON_DIR is set,
then usually all paths within that directory are overridden as well.
There are a couple of exceptions, though, and two of them, namely
'refs/rewritten' and 'logs/HEAD' are not mentioned in
'gitrepository-layout'. Document them as well.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
William Baker [Mon, 21 Oct 2019 18:40:03 +0000 (18:40 +0000)]
multi-pack-index: add [--[no-]progress] option.
Add the --[no-]progress option to git multi-pack-index.
Pass the MIDX_PROGRESS flag to the subcommand functions
when progress should be displayed by multi-pack-index.
The progress feature was added to 'verify' in 144d703
("multi-pack-index: report progress during 'verify'", 2018-09-13)
but some subcommands were not updated to display progress, and
the ability to opt-out was overlooked.
Signed-off-by: William Baker <William.Baker@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
