git.git
5 years agobuiltin/index-pack: add option to specify hash algorithm
brian m. carlson [Fri, 19 Jun 2020 17:55:52 +0000 (17:55 +0000)]
builtin/index-pack: add option to specify hash algorithm

git index-pack is usually run in a repository, but need not be. Since
packs don't contains information on the algorithm in use, instead
relying on context, add an option to index-pack to tell it which one
we're using in case someone runs it outside of a repository.  Since
using --stdin necessarily implies a repository, don't allow specifying
an object format if it's provided to prevent users from passing an
option that won't work.  Add documentation for this option.

Signed-off-by: brian m. carlson <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoremote-curl: detect algorithm for dumb HTTP by size
brian m. carlson [Fri, 19 Jun 2020 17:55:51 +0000 (17:55 +0000)]
remote-curl: detect algorithm for dumb HTTP by size

When reading the info/refs file for a repository, we have no explicit
way to detect which hash algorithm is in use because the file doesn't
provide one. Detect the hash algorithm in use by the size of the first
object ID.

If we have an empty repository, we don't know what the hash algorithm is
on the remote side, so default to whatever the local side has
configured.  Without doing this, we cannot clone an empty repository
since we don't know its hash algorithm.  Test this case appropriately,
since we currently have no tests for cloning an empty repository with
the dumb HTTP protocol.

We anonymize the URL like elsewhere in the function in case the user has
decided to include a secret in the URL.

Signed-off-by: brian m. carlson <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agostrbuf: remove unreferenced strbuf_write_fd method.
Randall S. Becker [Fri, 19 Jun 2020 20:23:20 +0000 (16:23 -0400)]
strbuf: remove unreferenced strbuf_write_fd method.

strbuf_write_fd was only used in bugreport.c. Since that file now uses
write_in_full, this method is no longer needed. In addition, strbuf_write_fd
did not guard against exceeding MAX_IO_SIZE for the platform, nor
provided error handling in the event of a failure if only partial data
was written to the file descriptor. Since already write_in_full has this
capability and is in general use, it should be used instead. The change
impacts strbuf.c and strbuf.h.

Signed-off-by: Randall S. Becker <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agobugreport.c: replace strbuf_write_fd with write_in_full
Randall S. Becker [Fri, 19 Jun 2020 20:23:19 +0000 (16:23 -0400)]
bugreport.c: replace strbuf_write_fd with write_in_full

The strbuf_write_fd method did not provide checks for buffers larger
than MAX_IO_SIZE. Replacing with write_in_full ensures the entire
buffer will always be written to disk or report an error and die.

Signed-off-by: Randall S. Becker <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agopull: plug minor memory leak after using is_descendant_of()
René Scharfe [Fri, 19 Jun 2020 13:14:19 +0000 (15:14 +0200)]
pull: plug minor memory leak after using is_descendant_of()

cmd_pull() builds a commit_list to pass a single potential ancestor to
is_descendant_of().  The latter leaves the list intact.  Release the
allocated memory after the call.

Leaking in cmd_*() isn't a big deal, but sets a bad example for other
users of is_descendant_of().

Signed-off-by: René Scharfe <redacted>
Acked-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit-reach: plug minor memory leak after using is_descendant_of()
René Scharfe [Fri, 19 Jun 2020 13:13:46 +0000 (15:13 +0200)]
commit-reach: plug minor memory leak after using is_descendant_of()

ref_newer() builds a commit_list to pass a single potential ancestor to
is_descendant_of().  The latter leaves the list intact.  Release the
allocated memory after the call.

Signed-off-by: René Scharfe <redacted>
Acked-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agorefs: implement reference transaction hook
Patrick Steinhardt [Fri, 19 Jun 2020 06:56:14 +0000 (08:56 +0200)]
refs: implement reference transaction hook

The low-level reference transactions used to update references are
currently completely opaque to the user. While certainly desirable in
most usecases, there are some which might want to hook into the
transaction to observe all queued reference updates as well as observing
the abortion or commit of a prepared transaction.

One such usecase would be to have a set of replicas of a given Git
repository, where we perform Git operations on all of the repositories
at once and expect the outcome to be the same in all of them. While
there exist hooks already for a certain subset of Git commands that
could be used to implement a voting mechanism for this, many others
currently don't have any mechanism for this.

The above scenario is the motivation for the new "reference-transaction"
hook that reaches directly into Git's reference transaction mechanism.
The hook receives as parameter the current state the transaction was
moved to ("prepared", "committed" or "aborted") and gets via its
standard input all queued reference updates. While the exit code gets
ignored in the "committed" and "aborted" states, a non-zero exit code in
the "prepared" state will cause the transaction to be aborted
prematurely.

Given the usecase described above, a voting mechanism can now be
implemented via this hook: as soon as it gets called, it will take all
of stdin and use it to cast a vote to a central service. When all
replicas of the repository agree, the hook will exit with zero,
otherwise it will abort the transaction by returning non-zero. The most
important upside is that this will catch _all_ commands writing
references at once, allowing to implement strong consistency for
reference updates via a single mechanism.

In order to test the impact on the case where we don't have any
"reference-transaction" hook installed in the repository, this commit
introduce two new performance tests for git-update-refs(1). Run against
an empty repository, it produces the following results:

  Test                         origin/master     HEAD
  --------------------------------------------------------------------
  1400.2: update-ref           2.70(2.10+0.71)   2.71(2.10+0.73) +0.4%
  1400.3: update-ref --stdin   0.21(0.09+0.11)   0.21(0.07+0.14) +0.0%

The performance test p1400.2 creates, updates and deletes a branch a
thousand times, thus averaging runtime of git-update-refs over 3000
invocations. p1400.3 instead calls `git-update-refs --stdin` three times
and queues a thousand creations, updates and deletes respectively.

As expected, p1400.3 consistently shows no noticeable impact, as for
each batch of updates there's a single call to access(3P) for the
negative hook lookup. On the other hand, for p1400.2, one can see an
impact caused by this patchset. But doing five runs of the performance
tests where each one was run with GIT_PERF_REPEAT_COUNT=10, the overhead
ranged from -1.5% to +1.1%. These inconsistent performance numbers can
be explained by the overhead of spawning 3000 processes. This shows that
the overhead of assembling the hook path and executing access(3P) once
to check if it's there is mostly outweighed by the operating system's
overhead.

Signed-off-by: Patrick Steinhardt <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot4014: do not use "slave branch" nomenclature
Paolo Bonzini [Fri, 19 Jun 2020 09:32:10 +0000 (11:32 +0200)]
t4014: do not use "slave branch" nomenclature

Git branches have been qualified as topic branches, integration branches,
development branches, feature branches, release branches and so on.
Git has a branch that is the master *for* development, but it is not
the master *of* any "slave branch": Git does not have slave branches,
and has never had, except for a single testcase that claims otherwise. :)

Independent of any future change to the naming of the "master" branch,
removing this sole appearance of the term is a strict improvement: it
avoids divisive language, and talking about "feature branch" clarifies
which developer workflow the test is trying to emulate.

Reported-by: Till Maas <redacted>
Signed-off-by: Paolo Bonzini <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agobuiltin/diff: update usage comment
Denton Liu [Thu, 18 Jun 2020 10:43:34 +0000 (06:43 -0400)]
builtin/diff: update usage comment

A comment in cmd_diff() states that if one tree-ish and no blobs are
provided, (the "N=1, M=0" case), it will provide a diff between the tree
and the cache. This is incorrect because a diff happens between the
tree-ish and the working tree. Remove the `--cached` in the comment so
that the correct behavior is shown. Add a new section describing the
"N=1, M=0, --cached" behavior.

Next, describe the "N=0, M=0, --cached" case, similar to the above since
it is undocumented.

Finally, fix some spacing issues. Add spaces between each section for
consistency and readability. Also, change tabs within the comment into
spaces.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agowt-status: show sparse checkout status as well
Elijah Newren [Thu, 18 Jun 2020 20:49:57 +0000 (20:49 +0000)]
wt-status: show sparse checkout status as well

Some of the early feedback of folks trying out sparse-checkouts at
$dayjob is that sparse checkouts can sometimes be disorienting; users
can forget that they had a sparse-checkout and then wonder where files
went.  Add some output to 'git status' in the form of a simple line that
states:

    You are in a sparse checkout with 35% of files present.

where, obviously, the exact figure changes depending on what percentage
of files from the index do not have the SKIP_WORKTREE bit set.

Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoThe third batch
Junio C Hamano [Thu, 18 Jun 2020 04:28:15 +0000 (21:28 -0700)]
The third batch

Also let's update the DEF_VER in GIT-VERSION-GEN that presuably
is not looked at by anybody ;-)

Signed-off-by: Junio C Hamano <redacted>
5 years agoMerge branch 'es/advertise-contribution-doc'
Junio C Hamano [Thu, 18 Jun 2020 04:54:06 +0000 (21:54 -0700)]
Merge branch 'es/advertise-contribution-doc'

Doc updates.

* es/advertise-contribution-doc:
  docs: mention MyFirstContribution in more places

5 years agoMerge branch 'dl/python-2.7-is-the-floor-version'
Junio C Hamano [Thu, 18 Jun 2020 04:54:05 +0000 (21:54 -0700)]
Merge branch 'dl/python-2.7-is-the-floor-version'

Document that we do not support Python 2.6 or older.

* dl/python-2.7-is-the-floor-version:
  CodingGuidelines: specify Python 2.7 is the oldest version

5 years agoMerge branch 'dl/t-readme-spell-git-correctly'
Junio C Hamano [Thu, 18 Jun 2020 04:54:05 +0000 (21:54 -0700)]
Merge branch 'dl/t-readme-spell-git-correctly'

Doc updates.

* dl/t-readme-spell-git-correctly:
  t/README: avoid poor-man's small caps GIT

5 years agoMerge branch 'js/fuzz-commit-graph-leakfix'
Junio C Hamano [Thu, 18 Jun 2020 04:54:04 +0000 (21:54 -0700)]
Merge branch 'js/fuzz-commit-graph-leakfix'

Leakfix.

* js/fuzz-commit-graph-leakfix:
  fuzz-commit-graph: properly free graph struct

5 years agoMerge branch 'en/do-match-pathspec-fix'
Junio C Hamano [Thu, 18 Jun 2020 04:54:03 +0000 (21:54 -0700)]
Merge branch 'en/do-match-pathspec-fix'

Use of negative pathspec, while collecting paths including
untracked ones in the working tree, was broken.

* en/do-match-pathspec-fix:
  dir: fix treatment of negated pathspecs

5 years agoMerge branch 'js/msvc-build-fix'
Junio C Hamano [Thu, 18 Jun 2020 04:54:02 +0000 (21:54 -0700)]
Merge branch 'js/msvc-build-fix'

Workaround breakage in MSVC build, where "curl-config --cflags"
gives settings appropriate for GCC build.

* js/msvc-build-fix:
  msvc: fix "REG_STARTEND" issue

5 years agoMerge branch 'en/sparse-checkout'
Junio C Hamano [Thu, 18 Jun 2020 04:54:02 +0000 (21:54 -0700)]
Merge branch 'en/sparse-checkout'

The behaviour of "sparse-checkout" in the state "git clone
--no-checkout" left was changed accidentally in 2.27, which has
been corrected.

* en/sparse-checkout:
  sparse-checkout: avoid staging deletions of all files

5 years agoMerge branch 'js/reflog-anonymize-for-clone-and-fetch'
Junio C Hamano [Thu, 18 Jun 2020 04:54:01 +0000 (21:54 -0700)]
Merge branch 'js/reflog-anonymize-for-clone-and-fetch'

The reflog entries for "git clone" and "git fetch" did not
anonymize the URL they operated on.

* js/reflog-anonymize-for-clone-and-fetch:
  clone/fetch: anonymize URLs in the reflog

5 years agoMerge branch 'tb/t5318-cleanup'
Junio C Hamano [Thu, 18 Jun 2020 04:54:00 +0000 (21:54 -0700)]
Merge branch 'tb/t5318-cleanup'

Code cleanup.

* tb/t5318-cleanup:
  t5318: test that '--stdin-commits' respects '--[no-]progress'
  t5318: use 'test_must_be_empty'

5 years agoMerge branch 'jk/diff-memuse-optim-with-stat-unmatch'
Junio C Hamano [Thu, 18 Jun 2020 04:54:00 +0000 (21:54 -0700)]
Merge branch 'jk/diff-memuse-optim-with-stat-unmatch'

Reduce memory usage during "diff --quiet" in a worktree with too
many stat-unmatched paths.

* jk/diff-memuse-optim-with-stat-unmatch:
  diff: discard blob data from stat-unmatched pairs

5 years agocommit-graph: minimize commit_graph_data_slab access
Abhishek Kumar [Wed, 17 Jun 2020 09:14:11 +0000 (14:44 +0530)]
commit-graph: minimize commit_graph_data_slab access

In an earlier patch, multiple struct acccesses to `graph_pos` and
`generation` were auto-converted to multiple method calls.

Since the values are fixed and commit-slab access costly, we would be
better off with storing the values as a local variable and reusing it.

Signed-off-by: Abhishek Kumar <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit: move members graph_pos, generation to a slab
Abhishek Kumar [Wed, 17 Jun 2020 09:14:10 +0000 (14:44 +0530)]
commit: move members graph_pos, generation to a slab

We remove members `graph_pos` and `generation` from the struct commit.
The default assignments in init_commit_node() are no longer valid,
which is fine as the slab helpers return appropriate default values and
the assignments are removed.

We will replace existing use of commit->generation and commit->graph_pos
by commit_graph_data_slab helpers using
`contrib/coccinelle/commit.cocci'.

Signed-off-by: Abhishek Kumar <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit-graph: introduce commit_graph_data_slab
Abhishek Kumar [Wed, 17 Jun 2020 09:14:09 +0000 (14:44 +0530)]
commit-graph: introduce commit_graph_data_slab

The struct commit is used in many contexts. However, members
`generation` and `graph_pos` are only used for commit-graph related
operations and otherwise waste memory.

This wastage would have been more pronounced as we transition to
generation number v2, which uses 64-bit generation number instead of
current 32-bits.

As they are often accessed together, let's introduce struct
commit_graph_data and move them to a commit_graph_data slab.

While the overall test suite runs just as fast as master,
(series: 26m48s, master: 27m34s, faster by 2.87%), certain commands
like `git merge-base --is-ancestor` were slowed by 40% as discovered
by Szeder Gábor [1]. After minimizing commit-slab access, the slow down
persists but is closer to 20%.

Derrick Stolee believes the slow down is attributable to the underlying
algorithm rather than the slowness of commit-slab access [2] and we will
follow-up in a later series.

[1]: https://lore.kernel.org/git/20200607195347.GA8232@szeder.dev/
[2]: https://lore.kernel.org/git/13db757a-9412-7f1e-805c-8a028c4ab2b1@gmail.com/

Signed-off-by: Abhishek Kumar <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoobject: drop parsed_object_pool->commit_count
Abhishek Kumar [Wed, 17 Jun 2020 09:14:08 +0000 (14:44 +0530)]
object: drop parsed_object_pool->commit_count

14ba97f8 (alloc: allow arbitrary repositories for alloc functions,
2018-05-15) introduced parsed_object_pool->commit_count to keep count of
commits per repository and was used to assign commit->index.

However, commit-slab code requires commit->index values to be unique
and a global count would be correct, rather than a per-repo count.

Let's introduce a static counter variable, `parsed_commits_count` to
keep track of parsed commits so far.

As commit_count has no use anymore, let's also drop it from the struct.

Signed-off-by: Abhishek Kumar <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit-reach: use fast logic in repo_in_merge_base
Derrick Stolee [Wed, 17 Jun 2020 17:24:29 +0000 (17:24 +0000)]
commit-reach: use fast logic in repo_in_merge_base

The repo_is_descendant_of() method is aware of the existence of the
commit-graph file. It checks for generation_numbers_enabled() before
deciding on using can_all_from_reach() or repo_in_merge_bases()
depending on the situation. The reason here is that can_all_from_reach()
uses a depth-first search that is limited by the minimum generation
number of the target commits, and that algorithm can be very slow when
generation numbers are not present. The alternative uses
paint_down_to_common() which will walk the entire merge-base boundary,
which is typically slower.

This method is used by commands like "git tag --contains" and "git
branch --contains" for very fast results when a commit-graph file
exists. Unfortunately, it is _not_ used in commands like "git merge-base
--is-ancestor" which is doing an even simpler request.

This issue was raised recently [1] with respect to a change to how
generation numbers are stored, but was also reported much earlier [2]
before commit-reach.c existed to simplify these reachability queries.

[1] https://lore.kernel.org/git/20200607195347.GA8232@szeder.dev/
[2] https://lore.kernel.org/git/87608bawoa.fsf@evledraar.gmail.com/

The root cause is that builtin/merge-base.c has a method
handle_is_ancestor() that calls in_merge_bases(), an older version of
repo_in_merge_bases(). It would be better if we have every caller to
in_merge_bases() use the logic in can_all_from_reach() when possible.

This is where things get a little tricky: repo_is_descendant_of() calls
repo_in_merge_bases() in the non-generation numbers enabled case! If we
simply update repo_in_merge_bases() to call repo_is_descendant_of()
instead of repo_in_merge_bases_many(), then we will get a recursive call
loop. Thankfully, this is caught by the test suite in the default mode
(i.e. GIT_TEST_COMMIT_GRAPH=0).

The trick, then, is to make the non-generation number case for
repo_is_descendant_of() call repo_in_merge_bases_many() directly,
skipping the non-_many version. This allows us to take advantage of this
faster code path, when possible.

The easiest way to measure the performance impact is to test the
following command on the Linux kernel repository:

git merge-base --is-ancestor <A> <B>

  | A    | B    | Time Before | Time After |
  |------|------|-------------|------------|
  | v3.0 | v5.7 | 0.459s      | 0.028s     |
  | v4.0 | v5.7 | 0.267s      | 0.021s     |
  | v5.0 | v5.7 | 0.074s      | 0.013s     |

Note that each of these samples return success. The old code performed
the same operation when <A> and <B> are swapped. However,
can_all_from_reach() will return immediately if the generation numbers
show that <A> has larger generation number than <B>. Thus, the time for
the swapped case is universally 0.004s in each case.

Reported-by: Ævar Arnfjörð Bjarmason <redacted>
Reported-by: SZEDER Gábor <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit-reach: create repo_is_descendant_of()
Derrick Stolee [Wed, 17 Jun 2020 17:24:28 +0000 (17:24 +0000)]
commit-reach: create repo_is_descendant_of()

The next change will make repo_in_merge_bases() depend on the logic in
is_descendant_of(), but we need to make the method independent of
the_repository first.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: fix a sparse '0 as NULL pointer' warning
Ramsay Jones [Mon, 15 Jun 2020 23:00:20 +0000 (00:00 +0100)]
upload-pack: fix a sparse '0 as NULL pointer' warning

Signed-off-by: Ramsay Jones <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agobranch: don't mix --edit-description
Denton Liu [Mon, 15 Jun 2020 11:53:20 +0000 (07:53 -0400)]
branch: don't mix --edit-description

`git branch` accepts `--edit-description` in conjunction with other
arguments. However, `--edit-description` is its own mode, similar to
`--set-upstream-to`, which is also made mutually exclusive with other
modes. Prevent `--edit-description` from being mixed with other modes.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot3200: test for specific errors
Denton Liu [Mon, 15 Jun 2020 11:53:19 +0000 (07:53 -0400)]
t3200: test for specific errors

In the "--set-upstream-to" and "--unset-upstream" tests, specific error
conditions are being tested. However, there is no way of ensuring that a
test case is failing because of some specific error.

Check stderr of failing commands to ensure that they are failing in the
expected way.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot3200: rename "expected" to "expect"
Denton Liu [Mon, 15 Jun 2020 11:53:18 +0000 (07:53 -0400)]
t3200: rename "expected" to "expect"

Clean up style of test by changing some filenames from "expected" to
"expect", which follows typical test convention.

Also, change a space-indent into a tab-indent.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoclean: optimize and document cases where we recurse into subdirectories
Elijah Newren [Thu, 11 Jun 2020 06:59:33 +0000 (06:59 +0000)]
clean: optimize and document cases where we recurse into subdirectories

Commit 6b1db43109 ("clean: teach clean -d to preserve ignored paths",
2017-05-23) added the following code block (among others) to git-clean:
    if (remove_directories)
        dir.flags |= DIR_SHOW_IGNORED_TOO | DIR_KEEP_UNTRACKED_CONTENTS;
The reason for these flags is well documented in the commit message, but
isn't obvious just from looking at the code.  Add some explanations to
the code to make it clearer.

Further, it appears git-2.26 did not correctly handle this combination
of flags from git-clean.  With both these flags and without
DIR_SHOW_IGNORED_TOO_MODE_MATCHING set, git is supposed to recurse into
all untracked AND ignored directories.  git-2.26.0 clearly was not doing
that.  I don't know the full reasons for that or whether git < 2.27.0
had additional unknown bugs because of that misbehavior, because I don't
feel it's worth digging into.  As per the huge changes and craziness
documented in commit 8d92fb2927 ("dir: replace exponential algorithm
with a linear one", 2020-04-01), the old algorithm was a mess and was
thrown out.  What I can say is that git-2.27.0 correctly recurses into
untracked AND ignored directories with that combination.

However, in clean's case we don't need to recurse into ignored
directories; that is just a waste of time.  Thus, when git-2.27.0
started correctly handling those flags, we got a performance regression
report.  Rather than relying on other bugs in fill_directory()'s former
logic to provide the behavior of skipping ignored directories, make use
of the DIR_SHOW_IGNORED_TOO_MODE_MATCHING value specifically added in
commit eec0f7f2b7 ("status: add option to show ignored files
differently", 2017-10-30) for this purpose.

Reported-by: Brian Malehorn <redacted>
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoclean: consolidate handling of ignored parameters
Elijah Newren [Thu, 11 Jun 2020 06:59:32 +0000 (06:59 +0000)]
clean: consolidate handling of ignored parameters

I spent a long time trying to figure out how and whether the code worked
with different values of ignore, ignore_only, and remove_directories.
After lots of time setting up lots of testcases, sifting through lots of
print statements, and walking through the debugger, I finally realized
that one piece of code related to how it was all setup was found in
clean.c rather than dir.c.  Make a change that would have made it easier
for me to do the extra testing by putting this handling in one spot.

Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agodir, clean: avoid disallowed behavior
Elijah Newren [Thu, 11 Jun 2020 06:59:31 +0000 (06:59 +0000)]
dir, clean: avoid disallowed behavior

dir.h documented quite clearly that DIR_SHOW_IGNORED and
DIR_SHOW_IGNORED_TOO are mutually exclusive, with a big comment to this
effect by the definition of both enum values.  However, a command like
   git clean -fx $DIR
would set both values for dir.flags.  I _think_ it happened to work
because:
  * As dir.h points out, DIR_KEEP_UNTRACKED_CONTENTS only takes effect
    if DIR_SHOW_IGNORED_TOO is set.
  * As coded, I believe DIR_SHOW_IGNORED would just happen to take
    precedence over DIR_SHOW_IGNORED_TOO in the code as currently
    constructed.
Which is a long way of saying "we just got lucky".

Fix clean.c to avoid setting these mutually exclusive values at the same
time, and add a check to dir.c that will throw a BUG() to prevent anyone
else from making this mistake.

Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agodir: fix a few confusing comments
Elijah Newren [Thu, 11 Jun 2020 06:59:30 +0000 (06:59 +0000)]
dir: fix a few confusing comments

Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agogit-sparse-checkout: clarify interactions with submodules
Elijah Newren [Wed, 10 Jun 2020 23:16:49 +0000 (23:16 +0000)]
git-sparse-checkout: clarify interactions with submodules

Ignoring the sparse-checkout feature momentarily, if one has a submodule and
creates local branches within it with unpushed changes and maybe adds some
untracked files to it, then we would want to avoid accidentally removing such
a submodule.  So, for example with git.git, if you run
   git checkout v2.13.0
then the sha1collisiondetection/ submodule is NOT removed even though it
did not exist as a submodule until v2.14.0.  Similarly, if you only had
v2.13.0 checked out previously and ran
   git checkout v2.14.0
the sha1collisiondetection/ submodule would NOT be automatically
initialized despite being part of v2.14.0.  In both cases, git requires
submodules to be initialized or deinitialized separately.  Further, we
also have special handling for submodules in other commands such as
clean, which requires two --force flags to delete untracked submodules,
and some commands have a --recurse-submodules flag.

sparse-checkout is very similar to checkout, as evidenced by the similar
name -- it adds and removes files from the working copy.  However, for
the same avoid-data-loss reasons we do not want to remove a submodule
from the working copy with checkout, we do not want to do it with
sparse-checkout either.  So submodules need to be separately initialized
or deinitialized; changing sparse-checkout rules should not
automatically trigger the removal or vivification of submodules.

I believe the previous wording in git-sparse-checkout.txt about
submodules was only about this particular issue.  Unfortunately, the
previous wording could be interpreted to imply that submodules should be
considered active regardless of sparsity patterns.  Update the wording
to avoid making such an implication.  It may be helpful to consider two
example situations where the differences in wording become important:

In the future, we want users to be able to run commands like
   git clone --sparse=moduleA --recurse-submodules $REPO_URL
and have sparsity paths automatically set up and have submodules *within
the sparsity paths* be automatically initialized.  We do not want all
submodules in any path to be automatically initialized with that
command.

Similarly, we want to be able to do things like
   git -c sparse.restrictCmds grep --recurse-submodules $REV $PATTERN
and search through $REV for $PATTERN within the recorded sparsity
patterns.  We want it to recurse into submodules within those sparsity
patterns, but do not want to recurse into directories that do not match
the sparsity patterns in search of a possible submodule.

Signed-off-by: Elijah Newren <redacted>
Reviewed-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoMerge branch 'hn/refs-cleanup'
Junio C Hamano [Fri, 12 Jun 2020 20:57:13 +0000 (13:57 -0700)]
Merge branch 'hn/refs-cleanup'

Preliminary clean-ups around refs API, plus file format
specification documentation for the reftable backend.

* hn/refs-cleanup:
  reftable: define version 2 of the spec to accomodate SHA256
  reftable: clarify how empty tables should be written
  reftable: file format documentation
  refs: improve documentation for ref iterator
  t: use update-ref and show-ref to reading/writing refs
  refs.h: clarify reflog iteration order

5 years agoIntroduce CMake support for configuring Git
Sibi Siddharthan [Fri, 12 Jun 2020 18:29:19 +0000 (18:29 +0000)]
Introduce CMake support for configuring Git

At the moment, the recommended way to configure Git's builds is to
simply run `make`. If that does not work, the recommended strategy is to
look at the top of the `Makefile` to see whether any "Makefile knob" has
to be turned on/off, e.g. `make NO_OPENSSL=YesPlease`.

Alternatively, Git also has an `autoconf` setup which allows configuring
builds via `./configure [<option>...]`.

Both of these options are fine if the developer works on Unix or Linux.
But on Windows, we have to jump through hoops to configure a build
(read: we force the user to install a full Git for Windows SDK, which
occupies around two gigabytes (!) on disk and downloads about three
quarters of a gigabyte worth of Git objects).

The build infrastructure for Git is written around being able to run
make, which is not supported natively on Windows.
To help Windows developers a CMake build script is introduced here.

With a working support CMake, developers on Windows need only install
CMake, configure their build, load the generated Visual Studio solution
and immediately start modifying the code and build their own version of
Git. Likewise, developers on other platforms can use the convenient GUI
tools provided by CMake to configure their build.

So let's start building CMake support for Git.

This is only the first step, and to make it easier to review, it only
allows for configuring builds on the platform that is easiest to
configure for: Linux.

The CMake script checks whether the headers are present(eg. libgen.h),
whether the functions are present(eg. memmem), whether the funtions work
properly (eg. snprintf) and generate the required compile definitions
for the platform. The script also searches for the required libraries,
if it fails to find the required libraries the respective executables
won't be built.(eg. If libcurl is not found then git-remote-http won't
be built). This will help building Git easier.

With a CMake script an out of source build of git is possible resulting
in a clean source tree.

Note: this patch asks for the minimum version v3.14 of CMake (which is
not all that old as of time of writing) because that is the first
version to offer a platform-independent way to generate hardlinks as
part of the build. This is needed to generate all those hardlinks for
the built-in commands of Git.

Signed-off-by: Sibi Siddharthan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agolib-submodule-update: prepend "git" to $command
Denton Liu [Thu, 11 Jun 2020 17:41:49 +0000 (13:41 -0400)]
lib-submodule-update: prepend "git" to $command

Since all invocations of test_submodule_forced_switch() are git
commands, automatically prepend "git" before invoking
test_submodule_switch_common().

Similarly, many invocations of test_submodule_switch() are also git
commands so automatically prepend "git" before invoking
test_submodule_switch_common() as well.

Finally, for invocations of test_submodule_switch() that invoke a custom
function, rename the old function to test_submodule_switch_func().

This is necessary because in a future commit, we will be adding some
logic that needs to distinguish between an invocation of a plain git
comamnd and an invocation of a test helper function.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoDocumentation: usage for diff combined commits
Chris Torek [Fri, 12 Jun 2020 16:20:00 +0000 (16:20 +0000)]
Documentation: usage for diff combined commits

Document the usage for producing combined commits with "git diff".
This includes updating the synopsis section.

While here, add the three-dot notation to the synopsis.

Make "git diff -h" print the same usage summary as the manual
page synopsis, minus the "A..B" form, which is now discouraged.

Signed-off-by: Chris Torek <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agogit diff: improve range handling
Chris Torek [Fri, 12 Jun 2020 16:19:59 +0000 (16:19 +0000)]
git diff: improve range handling

When git diff is given a symmetric difference A...B, it chooses
some merge base from the two specified commits (as documented).

This fails, however, if there is *no* merge base: instead, you
see the differences between A and B, which is certainly not what
is expected.

Moreover, if additional revisions are specified on the command
line ("git diff A...B C"), the results get a bit weird:

 * If there is a symmetric difference merge base, this is used
   as the left side of the diff.  The last final ref is used as
   the right side.
 * If there is no merge base, the symmetric status is completely
   lost.  We will produce a combined diff instead.

Similar weirdness occurs if you use, e.g., "git diff C A...B D".
Likewise, using multiple two-dot ranges, or tossing extra
revision specifiers into the command line with two-dot ranges,
or mixing two and three dot ranges, all produce nonsense.

To avoid all this, add a routine to catch the range cases and
verify that that the arguments make sense.  As a side effect,
produce a warning showing *which* merge base is being used when
there are multiple choices; die if there is no merge base.

Signed-off-by: Chris Torek <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: refactor common code into do_got_oid()
Christian Couder [Thu, 11 Jun 2020 12:05:18 +0000 (14:05 +0200)]
upload-pack: refactor common code into do_got_oid()

As 'upload-pack.c' is now using 'struct upload_pack_data'
thoroughly, let's refactor some common code into a new
do_got_oid() function.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: move oldest_have to upload_pack_data
Christian Couder [Thu, 11 Jun 2020 12:05:17 +0000 (14:05 +0200)]
upload-pack: move oldest_have to upload_pack_data

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's move the 'oldest_have' static variable
into this struct.

It is used by both protocol v0 and protocol v2 code.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: pass upload_pack_data to got_oid()
Christian Couder [Thu, 11 Jun 2020 12:05:16 +0000 (14:05 +0200)]
upload-pack: pass upload_pack_data to got_oid()

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's pass that struct to got_oid(), so that
this function can use all the fields of the struct.

This will be used in followup commits to move a static variable
into 'upload_pack_data'.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: pass upload_pack_data to ok_to_give_up()
Christian Couder [Thu, 11 Jun 2020 12:05:15 +0000 (14:05 +0200)]
upload-pack: pass upload_pack_data to ok_to_give_up()

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's pass that struct to ok_to_give_up(), so
that this function can use all the fields of the struct.

This will be used in followup commits to move a static variable
into 'upload_pack_data'.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: pass upload_pack_data to send_acks()
Christian Couder [Thu, 11 Jun 2020 12:05:14 +0000 (14:05 +0200)]
upload-pack: pass upload_pack_data to send_acks()

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's pass that struct to send_acks(), so
that this function can use all the fields of the struct.

This will be used in followup commits to move a static variable
into 'upload_pack_data'.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: pass upload_pack_data to process_haves()
Christian Couder [Thu, 11 Jun 2020 12:05:13 +0000 (14:05 +0200)]
upload-pack: pass upload_pack_data to process_haves()

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's pass that struct to process_haves(), so
that this function can use all the fields of the struct.

This will be used in followup commits to move a static variable
into 'upload_pack_data'.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: change allow_unadvertised_object_request to an enum
Christian Couder [Thu, 11 Jun 2020 12:05:12 +0000 (14:05 +0200)]
upload-pack: change allow_unadvertised_object_request to an enum

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's change allow_unadvertised_object_request,
which is now part of 'upload_pack_data', from an 'unsigned int'
to an enum.

This will make it clear which values this variable can take.

While at it let's change this variable name to 'allow_uor' to
make it shorter.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: move allow_unadvertised_object_request to upload_pack_data
Christian Couder [Thu, 11 Jun 2020 12:05:11 +0000 (14:05 +0200)]
upload-pack: move allow_unadvertised_object_request to upload_pack_data

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's move the 'allow_unadvertised_object_request'
static variable into this struct.

It is used by code common to protocol v0 and protocol v2.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: move extra_edge_obj to upload_pack_data
Christian Couder [Thu, 11 Jun 2020 12:05:10 +0000 (14:05 +0200)]
upload-pack: move extra_edge_obj to upload_pack_data

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's move the 'extra_edge_obj' static variable
into this struct.

It is used by code common to protocol v0 and protocol v2.

While at it let's properly initialize and clear 'extra_edge_obj'
in the appropriate 'upload_pack_data' initialization and
clearing functions.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: move shallow_nr to upload_pack_data
Christian Couder [Thu, 11 Jun 2020 12:05:09 +0000 (14:05 +0200)]
upload-pack: move shallow_nr to upload_pack_data

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's move the 'shallow_nr' static variable
into this struct.

It is used by code common to protocol v0 and protocol v2.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: pass upload_pack_data to send_unshallow()
Christian Couder [Thu, 11 Jun 2020 12:05:08 +0000 (14:05 +0200)]
upload-pack: pass upload_pack_data to send_unshallow()

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's pass that struct to send_unshallow(), so
that this function can use all the fields of the struct.

This will be used in followup commits to move static variables
into 'upload_pack_data'.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: pass upload_pack_data to deepen_by_rev_list()
Christian Couder [Thu, 11 Jun 2020 12:05:07 +0000 (14:05 +0200)]
upload-pack: pass upload_pack_data to deepen_by_rev_list()

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's pass that struct to deepen_by_rev_list(),
so that this function can use all the fields of the struct.

This will be used in followup commits to move static variables
into 'upload_pack_data'.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: pass upload_pack_data to deepen()
Christian Couder [Thu, 11 Jun 2020 12:05:06 +0000 (14:05 +0200)]
upload-pack: pass upload_pack_data to deepen()

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's pass that struct to deepen(), so that
this function can use all the fields of the struct.

This will be used in followup commits to move static variables
into 'upload_pack_data'.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: pass upload_pack_data to send_shallow_list()
Christian Couder [Thu, 11 Jun 2020 12:05:05 +0000 (14:05 +0200)]
upload-pack: pass upload_pack_data to send_shallow_list()

As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
more thoroughly, let's pass that struct to send_shallow_list(),
so that this function can use all the fields of the struct.

This will be used in followup commits to move static variables
into 'upload_pack_data'.

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: send part of packfile response as uri
Jonathan Tan [Wed, 10 Jun 2020 20:57:23 +0000 (13:57 -0700)]
upload-pack: send part of packfile response as uri

Teach upload-pack to send part of its packfile response as URIs.

An administrator may configure a repository with one or more
"uploadpack.blobpackfileuri" lines, each line containing an OID, a pack
hash, and a URI. A client may configure fetch.uriprotocols to be a
comma-separated list of protocols that it is willing to use to fetch
additional packfiles - this list will be sent to the server. Whenever an
object with one of those OIDs would appear in the packfile transmitted
by upload-pack, the server may exclude that object, and instead send the
URI. The client will then download the packs referred to by those URIs
before performing the connectivity check.

Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agofetch-pack: support more than one pack lockfile
Jonathan Tan [Wed, 10 Jun 2020 20:57:22 +0000 (13:57 -0700)]
fetch-pack: support more than one pack lockfile

Whenever a fetch results in a packfile being downloaded, a .keep file is
generated, so that the packfile can be preserved (from, say, a running
"git repack") until refs are written referring to the contents of the
packfile.

In a subsequent patch, a successful fetch using protocol v2 may result
in more than one .keep file being generated. Therefore, teach
fetch_pack() and the transport mechanism to support multiple .keep
files.

Implementation notes:

 - builtin/fetch-pack.c normally does not generate .keep files, and thus
   is unaffected by this or future changes. However, it has an
   undocumented "--lock-pack" feature, used by remote-curl.c when
   implementing the "fetch" remote helper command. In keeping with the
   remote helper protocol, only one "lock" line will ever be written;
   the rest will result in warnings to stderr. However, in practice,
   warnings will never be written because the remote-curl.c "fetch" is
   only used for protocol v0/v1 (which will not generate multiple .keep
   files). (Protocol v2 uses the "stateless-connect" command, not the
   "fetch" command.)

 - connected.c has an optimization in that connectivity checks on a ref
   need not be done if the target object is in a pack known to be
   self-contained and connected. If there are multiple packfiles, this
   optimization can no longer be done.

Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoupload-pack: refactor reading of pack-objects out
Jonathan Tan [Wed, 10 Jun 2020 20:57:21 +0000 (13:57 -0700)]
upload-pack: refactor reading of pack-objects out

Subsequent patches will change how the output of pack-objects is
processed, so extract that processing into its own function.

Currently, at most 1 character can be buffered (in the "buffered" local
variable). One of those patches will require a larger buffer, so replace
that "buffered" local variable with a buffer array.

Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoDocumentation: add Packfile URIs design doc
Jonathan Tan [Wed, 10 Jun 2020 20:57:20 +0000 (13:57 -0700)]
Documentation: add Packfile URIs design doc

Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoDocumentation: order protocol v2 sections
Jonathan Tan [Wed, 10 Jun 2020 20:57:19 +0000 (13:57 -0700)]
Documentation: order protocol v2 sections

The current C Git implementation expects Git servers to follow a
specific order of sections when transmitting protocol v2 responses, but
this is not explicit in the documentation. Make the order explicit.

Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agohttp-fetch: support fetching packfiles by URL
Jonathan Tan [Wed, 10 Jun 2020 20:57:18 +0000 (13:57 -0700)]
http-fetch: support fetching packfiles by URL

Teach http-fetch the ability to download packfiles directly, given a
URL, and to verify them.

The http_pack_request suite has been augmented with a function that
takes a URL directly. With this function, the hash is only used to
determine the name of the temporary file.

Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agohttp-fetch: refactor into function
Jonathan Tan [Wed, 10 Jun 2020 20:57:17 +0000 (13:57 -0700)]
http-fetch: refactor into function

cmd_main() in http-fetch.c will grow in a future patch, so refactor the
HTTP walking part into its own function.

Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agohttp: refactor finish_http_pack_request()
Jonathan Tan [Wed, 10 Jun 2020 20:57:16 +0000 (13:57 -0700)]
http: refactor finish_http_pack_request()

finish_http_pack_request() does multiple tasks, including some
housekeeping on a struct packed_git - (1) closing its index, (2)
removing it from a list, and (3) installing it. These concerns are
independent of fetching a pack through HTTP: they are there only because
(1) the calling code opens the pack's index before deciding to fetch it,
(2) the calling code maintains a list of packfiles that can be fetched,
and (3) the calling code fetches it in order to make use of its objects
in the same process.

In preparation for a subsequent commit, which adds a feature that does
not need any of this housekeeping, remove (1), (2), and (3) from
finish_http_pack_request(). (2) and (3) are now done by a helper
function, and (1) is the responsibility of the caller (in this patch,
done closer to the point where the pack index is opened).

Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agohttp: use --stdin when indexing dumb HTTP pack
Jonathan Tan [Wed, 10 Jun 2020 20:57:15 +0000 (13:57 -0700)]
http: use --stdin when indexing dumb HTTP pack

When Git fetches a pack using dumb HTTP, (among other things) it invokes
index-pack on a ".pack.temp" packfile, specifying the filename as an
argument.

A future commit will require the aforementioned invocation of index-pack
to also generate a "keep" file. To use this, we either have to use
index-pack's naming convention (because --keep requires the pack's
filename to end with ".pack") or to pass the pack through stdin. Of the
two, it is simpler to pass the pack through stdin.

Thus, teach http to pass --stdin to index-pack. As a bonus, the code is
now simpler.

Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoworktree: make "move" refuse to move atop missing registered worktree
Eric Sunshine [Wed, 10 Jun 2020 06:30:49 +0000 (02:30 -0400)]
worktree: make "move" refuse to move atop missing registered worktree

"git worktree add" takes special care to avoid creating a new worktree
at a location already registered to an existing worktree even if that
worktree is missing (which can happen, for instance, if the worktree
resides on removable media). "git worktree move", however, is not so
careful when validating the destination location and will happily move
the source worktree atop the location of a missing worktree. This leads
to the anomalous situation of multiple worktrees being associated with
the same path, which is expressly forbidden by design. For example:

    $ git clone foo.git
    $ cd foo
    $ git worktree add ../bar
    $ git worktree add ../baz
    $ rm -rf ../bar
    $ git worktree move ../baz ../bar
    $ git worktree list
    .../foo beefd00f [master]
    .../bar beefd00f [bar]
    .../bar beefd00f [baz]
    $ git worktree remove ../bar
    fatal: validation failed, cannot remove working tree:
        '.../bar' does not point back to '.git/worktrees/bar'

Fix this shortcoming by enhancing "git worktree move" to perform the
same additional validation of the destination directory as done by "git
worktree add".

While at it, add a test to verify that "git worktree move" won't move a
worktree atop an existing (non-worktree) path -- a restriction which has
always been in place but was never tested.

Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoworktree: generalize candidate worktree path validation
Eric Sunshine [Wed, 10 Jun 2020 06:30:48 +0000 (02:30 -0400)]
worktree: generalize candidate worktree path validation

"git worktree add" checks that the specified path is a valid location
for a new worktree by ensuring that the path does not already exist and
is not already registered to another worktree (a path can be registered
but missing, for instance, if it resides on removable media). Since "git
worktree add" is not the only command which should perform such
validation ("git worktree move" ought to also), generalize the the
validation function for use by other callers, as well.

Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoworktree: prune linked worktree referencing main worktree path
Eric Sunshine [Wed, 10 Jun 2020 06:30:47 +0000 (02:30 -0400)]
worktree: prune linked worktree referencing main worktree path

"git worktree prune" detects when multiple entries are associated with
the same path and prunes the duplicates, however, it does not detect
when a linked worktree points at the path of the main worktree.
Although "git worktree add" disallows creating a new worktree with the
same path as the main worktree, such a case can arise outside the
control of Git even without the user mucking with .git/worktree/<id>/
administrative files. For instance:

    $ git clone foo.git
    $ git -C foo worktree add ../bar
    $ rm -rf bar
    $ mv foo bar
    $ git -C bar worktree list
    .../bar deadfeeb [master]
    .../bar deadfeeb [bar]

Help the user recover from such corruption by extending "git worktree
prune" to also detect when a linked worktree is associated with the path
of the main worktree.

Reported-by: Jonathan Müller <redacted>
Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoworktree: prune duplicate entries referencing same worktree path
Eric Sunshine [Wed, 10 Jun 2020 06:30:46 +0000 (02:30 -0400)]
worktree: prune duplicate entries referencing same worktree path

A fundamental restriction of linked working trees is that there must
only ever be a single worktree associated with a particular path, thus
"git worktree add" explicitly disallows creation of a new worktree at
the same location as an existing registered worktree. Nevertheless,
users can still "shoot themselves in the foot" by mucking with
administrative files in .git/worktree/<id>/. Worse, "git worktree move"
is careless[1] and allows a worktree to be moved atop a registered but
missing worktree (which can happen, for instance, if the worktree is on
removable media). For instance:

    $ git clone foo.git
    $ cd foo
    $ git worktree add ../bar
    $ git worktree add ../baz
    $ rm -rf ../bar
    $ git worktree move ../baz ../bar
    $ git worktree list
    .../foo beefd00f [master]
    .../bar beefd00f [bar]
    .../bar beefd00f [baz]

Help users recover from this form of corruption by teaching "git
worktree prune" to detect when multiple worktrees are associated with
the same path.

[1]: A subsequent commit will fix "git worktree move" validation to be
     more strict.

Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoworktree: make high-level pruning re-usable
Eric Sunshine [Wed, 10 Jun 2020 06:30:45 +0000 (02:30 -0400)]
worktree: make high-level pruning re-usable

The low-level logic for removing a worktree is well encapsulated in
delete_git_dir(). However, high-level details related to pruning a
worktree -- such as dealing with verbosity and dry-run mode -- are not
encapsulated. Factor out this high-level logic into its own function so
it can be re-used as new worktree corruption detectors are added.

Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoworktree: give "should be pruned?" function more meaningful name
Eric Sunshine [Wed, 10 Jun 2020 06:30:44 +0000 (02:30 -0400)]
worktree: give "should be pruned?" function more meaningful name

Readers of the name prune_worktree() are likely to expect the function
to actually prune a worktree, however, it only answers the question
"should this worktree be pruned?". Give it a name more reflective of its
true purpose to avoid such confusion.

Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot/t3430: avoid undefined git diff behavior
Chris Torek [Tue, 9 Jun 2020 19:00:21 +0000 (19:00 +0000)]
t/t3430: avoid undefined git diff behavior

The autosquash-and-exec test used "git diff HEAD^!" to mean
"git diff HEAD^ HEAD".  Use these directly instead of relying
on the undefined but actual-current behavior of "HEAD^!".

Signed-off-by: Chris Torek <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoreftable: define version 2 of the spec to accomodate SHA256
Han-Wen Nienhuys [Wed, 20 May 2020 17:36:12 +0000 (17:36 +0000)]
reftable: define version 2 of the spec to accomodate SHA256

Version appends a hash ID to the file header, making it slightly larger.

This commit also changes "SHA-1" into "object ID" in many places.

Signed-off-by: Han-Wen Nienhuys <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoreftable: clarify how empty tables should be written
Han-Wen Nienhuys [Wed, 20 May 2020 17:36:11 +0000 (17:36 +0000)]
reftable: clarify how empty tables should be written

The format allows for some ambiguity, as a lone footer also starts
with a valid file header. However, the current JGit code will barf on
this. This commit codifies this behavior into the standard.

Signed-off-by: Han-Wen Nienhuys <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoreftable: file format documentation
Jonathan Nieder [Wed, 20 May 2020 17:36:10 +0000 (17:36 +0000)]
reftable: file format documentation

Shawn Pearce explains:

Some repositories contain a lot of references (e.g. android at 866k,
rails at 31k). The reftable format provides:

- Near constant time lookup for any single reference, even when the
  repository is cold and not in process or kernel cache.
- Near constant time verification if a SHA-1 is referred to by at least
  one reference (for allow-tip-sha1-in-want).
- Efficient lookup of an entire namespace, such as `refs/tags/`.
- Support atomic push `O(size_of_update)` operations.
- Combine reflog storage with ref storage.

This file format spec was originally written in July, 2017 by Shawn
Pearce.  Some refinements since then were made by Shawn and by Han-Wen
Nienhuys based on experiences implementing and experimenting with the
format.  (All of this was in the context of our work at Google and
Google is happy to contribute the result to the Git project.)

Imported from JGit[1]'s current version (c217d33ff,
"Documentation/technical/reftable: improve repo layout", 2020-02-04)
of Documentation/technical/reftable.md and converted to asciidoc by
running

  pandoc -t asciidoc -f markdown reftable.md >reftable.txt

using pandoc 2.2.1.  The result required the following additional
minor changes:

- removed the [TOC] directive to add a table of contents, since
  asciidoc does not support it
- replaced git-scm.com/docs links with linkgit: directives that link
  to other pages within Git's documentation

[1] https://eclipse.googlesource.com/jgit/jgit

Signed-off-by: Jonathan Nieder <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoThe second batch
Junio C Hamano [Tue, 9 Jun 2020 00:50:01 +0000 (17:50 -0700)]
The second batch

Signed-off-by: Junio C Hamano <redacted>
5 years agoMerge branch 'jt/curl-verbose-on-trace-curl'
Junio C Hamano [Tue, 9 Jun 2020 01:06:32 +0000 (18:06 -0700)]
Merge branch 'jt/curl-verbose-on-trace-curl'

Rewrite support for GIT_CURL_VERBOSE in terms of GIT_TRACE_CURL.

Looking good.

* jt/curl-verbose-on-trace-curl:
  http, imap-send: stop using CURLOPT_VERBOSE
  t5551: test that GIT_TRACE_CURL redacts password

5 years agoMerge branch 'cc/upload-pack-data'
Junio C Hamano [Tue, 9 Jun 2020 01:06:32 +0000 (18:06 -0700)]
Merge branch 'cc/upload-pack-data'

Code clean-up.

* cc/upload-pack-data:
  upload-pack: use upload_pack_data fields in receive_needs()
  upload-pack: pass upload_pack_data to create_pack_file()
  upload-pack: remove static variable 'stateless_rpc'
  upload-pack: pass upload_pack_data to check_non_tip()
  upload-pack: pass upload_pack_data to send_ref()
  upload-pack: move symref to upload_pack_data
  upload-pack: use upload_pack_data writer in receive_needs()
  upload-pack: pass upload_pack_data to receive_needs()
  upload-pack: pass upload_pack_data to get_common_commits()
  upload-pack: use 'struct upload_pack_data' in upload_pack()
  upload-pack: move 'struct upload_pack_data' around
  upload-pack: move {want,have}_obj to upload_pack_data
  upload-pack: remove unused 'wants' from upload_pack_data

5 years agoMerge branch 'cb/bisect-helper-parser-fix'
Junio C Hamano [Tue, 9 Jun 2020 01:06:31 +0000 (18:06 -0700)]
Merge branch 'cb/bisect-helper-parser-fix'

The code to parse "git bisect start" command line was lax in
validating the arguments.

* cb/bisect-helper-parser-fix:
  bisect--helper: avoid segfault with bad syntax in `start --term-*`

5 years agoMerge branch 'js/checkout-p-new-file'
Junio C Hamano [Tue, 9 Jun 2020 01:06:31 +0000 (18:06 -0700)]
Merge branch 'js/checkout-p-new-file'

"git checkout -p" did not handle a newly added path at all.

* js/checkout-p-new-file:
  checkout -p: handle new files correctly

5 years agoMerge branch 'dl/remote-curl-deadlock-fix'
Junio C Hamano [Tue, 9 Jun 2020 01:06:30 +0000 (18:06 -0700)]
Merge branch 'dl/remote-curl-deadlock-fix'

On-the-wire protocol v2 easily falls into a deadlock between the
remote-curl helper and the fetch-pack process when the server side
prematurely throws an error and disconnects.  The communication has
been updated to make it more robust.

* dl/remote-curl-deadlock-fix:
  stateless-connect: send response end packet
  pkt-line: define PACKET_READ_RESPONSE_END
  remote-curl: error on incomplete packet
  pkt-line: extern packet_length()
  transport: extract common fetch_pack() call
  remote-curl: remove label indentation
  remote-curl: fix typo

5 years agoMerge branch 'bc/filter-process'
Junio C Hamano [Tue, 9 Jun 2020 01:06:30 +0000 (18:06 -0700)]
Merge branch 'bc/filter-process'

Code simplification and test coverage enhancement.

* bc/filter-process:
  t2060: add a test for switch with --orphan and --discard-changes
  builtin/checkout: simplify metadata initialization

5 years agoMerge branch 'vs/complete-stash-show-p-fix'
Junio C Hamano [Tue, 9 Jun 2020 01:06:29 +0000 (18:06 -0700)]
Merge branch 'vs/complete-stash-show-p-fix'

The command line completion script (in contrib/) tried to complete
"git stash -p" as if it were "git stash push -p", but it was too
aggressive and also affected "git stash show -p", which has been
corrected.

* vs/complete-stash-show-p-fix:
  completion: don't override given stash subcommand with -p

5 years agoMerge branch 'rs/fsck-duplicate-names-in-trees'
Junio C Hamano [Tue, 9 Jun 2020 01:06:28 +0000 (18:06 -0700)]
Merge branch 'rs/fsck-duplicate-names-in-trees'

The check in "git fsck" to ensure that the tree objects are sorted
still had corner cases it missed unsorted entries.

* rs/fsck-duplicate-names-in-trees:
  fsck: detect more in-tree d/f conflicts
  t1450: demonstrate undetected in-tree d/f conflict
  t1450: increase test coverage of in-tree d/f detection
  fsck: fix a typo in a comment

5 years agoMerge branch 'es/bugreport-shell'
Junio C Hamano [Tue, 9 Jun 2020 01:06:28 +0000 (18:06 -0700)]
Merge branch 'es/bugreport-shell'

"git bugreport" learns to report what shell is in use.

* es/bugreport-shell:
  bugreport: include user interactive shell
  help: add shell-path to --build-options

5 years agoMerge branch 'tb/commit-graph-no-check-oids'
Junio C Hamano [Tue, 9 Jun 2020 01:06:27 +0000 (18:06 -0700)]
Merge branch 'tb/commit-graph-no-check-oids'

Clean-up the commit-graph codepath.

* tb/commit-graph-no-check-oids:
  commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flag
  t5318: reorder test below 'graph_read_expect'
  commit-graph.c: simplify 'fill_oids_from_commits'
  builtin/commit-graph.c: dereference tags in builtin
  builtin/commit-graph.c: extract 'read_one_commit()'
  commit-graph.c: peel refs in 'add_ref_to_set'
  commit-graph.c: show progress of finding reachable commits
  commit-graph.c: extract 'refs_cb_data'

5 years agoMerge branch 'cb/t4210-illseq-auto-detect'
Junio C Hamano [Tue, 9 Jun 2020 01:06:27 +0000 (18:06 -0700)]
Merge branch 'cb/t4210-illseq-auto-detect'

As FreeBSD is not the only platform whose regexp library reports
a REG_ILLSEQ error when fed invalid UTF-8, add logic to detect that
automatically and skip the affected tests.

* cb/t4210-illseq-auto-detect:
  t4210: detect REG_ILLSEQ dynamically and skip affected tests
  t/helper: teach test-regex to report pattern errors (like REG_ILLSEQ)

5 years agoMerge branch 'ds/line-log-on-bloom'
Junio C Hamano [Tue, 9 Jun 2020 01:06:26 +0000 (18:06 -0700)]
Merge branch 'ds/line-log-on-bloom'

"git log -L..." now takes advantage of the "which paths are touched
by this commit?" info stored in the commit-graph system.

* ds/line-log-on-bloom:
  line-log: integrate with changed-path Bloom filters
  line-log: try to use generation number-based topo-ordering
  line-log: more responsive, incremental 'git log -L'
  t4211-line-log: add tests for parent oids
  line-log: remove unused fields from 'struct line_log_data'

5 years agodocs: mention MyFirstContribution in more places
Emily Shaffer [Mon, 8 Jun 2020 21:11:32 +0000 (14:11 -0700)]
docs: mention MyFirstContribution in more places

While the MyFirstContribution guide exists and has received some use and
positive reviews, it is still not as discoverable as it could be. Add a
reference to it from the GitHub pull request template, where many
brand-new contributors may look. Also add a reference to it in
SubmittingPatches, which is the central source of guidance for patch
contribution.

Signed-off-by: Emily Shaffer <redacted>
Reviewed-by: Philippe Blain <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoworktree: factor out repeated string literal
Eric Sunshine [Mon, 8 Jun 2020 06:23:49 +0000 (02:23 -0400)]
worktree: factor out repeated string literal

For each worktree removed by "git worktree prune", it reports the reason
for the removal. All reasons share the common prefix "Removing
worktrees/%s:". As new removal reasons are added, this prefix needs to
be duplicated, which is error-prone and potentially cumbersome.
Therefore, factor out the common prefix.

Although this change seems to increase the "sentence lego quotient", it
should be reasonably safe, as the reason for removal is a distinct
clause, not strictly related to the prefix. Moreover, the "worktrees" in
"Removing worktrees/%s:" is a path literal which ought not be localized,
so by factoring it out, we can more easily avoid exposing that path
fragment to translators.

Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit-graph: simplify write_commit_graph_file() #2
SZEDER Gábor [Fri, 5 Jun 2020 13:00:32 +0000 (13:00 +0000)]
commit-graph: simplify write_commit_graph_file() #2

Unify the 'chunk_ids' and 'chunk_sizes' arrays into an array of
'struct chunk_info'.  This will allow more cleanups in the following
patches.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit-graph: simplify write_commit_graph_file() #1
SZEDER Gábor [Fri, 5 Jun 2020 13:00:31 +0000 (13:00 +0000)]
commit-graph: simplify write_commit_graph_file() #1

In write_commit_graph_file() one block of code fills the array of
chunk IDs, another block of code fills the array of chunk offsets,
then the chunk IDs and offsets are written to the Chunk Lookup table,
and finally a third block of code writes the actual chunks.  In case
of optional chunks like Extra Edge List and Base Graphs List there is
also a condition checking whether that chunk is necessary/desired, and
that same condition is repeated in all those three blocks of code.
This patch series is about to add more optional chunks, so there would
be even more repeated conditions.

Those chunk offsets are relative to the beginning of the file, so they
inherently depend on the size of the Chunk Lookup table, which in turn
depends on the number of chunks that are to be written to the
commit-graph file.  IOW at the time we set the first chunk's ID we
can't yet know its offset, because we don't yet know how many chunks
there are.

Simplify this by initially filling an array of chunk sizes, not
offsets, and calculate the offsets based on the chunk sizes only
later, while we are writing the Chunk Lookup table.  This way we can
fill the arrays of chunk IDs and sizes in one go, eliminating one set
of repeated conditions.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit-graph: simplify parse_commit_graph() #2
SZEDER Gábor [Fri, 5 Jun 2020 13:00:30 +0000 (13:00 +0000)]
commit-graph: simplify parse_commit_graph() #2

The Chunk Lookup table stores the chunks' starting offset in the
commit-graph file, not their sizes.  Consequently, the size of a chunk
can only be calculated by subtracting its offset from the offset of
the subsequent chunk (or that of the terminating label).  This is
currenly implemented in a bit complicated way: as we iterate over the
entries of the Chunk Lookup table, we check the id of each chunk and
store its starting offset, then we check the id of the last seen chunk
and calculate its size using its previously saved offset.  At the
moment there is only one chunk for which we calculate its size, but
this patch series will add more, and the repeated chunk id checks are
not that pretty.

Instead let's read ahead the offset of the next chunk on each
iteration, so we can calculate the size of each chunk right away,
right where we store its starting offset.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit-graph: simplify parse_commit_graph() #1
SZEDER Gábor [Fri, 5 Jun 2020 13:00:29 +0000 (13:00 +0000)]
commit-graph: simplify parse_commit_graph() #1

While we iterate over all entries of the Chunk Lookup table we make
sure that we don't attempt to read past the end of the mmap-ed
commit-graph file, and check in each iteration that the chunk ID and
offset we are about to read is still within the mmap-ed memory region.
However, these checks in each iteration are not really necessary,
because the number of chunks in the commit-graph file is already known
before this loop from the just parsed commit-graph header.

So let's check that the commit-graph file is large enough for all
entries in the Chunk Lookup table before we start iterating over those
entries, and drop those per-iteration checks.  While at it, take into
account the size of everything that is necessary to have a valid
commit-graph file, i.e. the size of the header, the size of the
mandatory OID Fanout chunk, and the size of the signature in the
trailer as well.

Note that this necessitates the change of the error message as well,
and, consequently, have to update the 'detect incorrect chunk count'
test in 't5318-commit-graph.sh' as well.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit-graph: clean up #includes
SZEDER Gábor [Fri, 5 Jun 2020 13:00:28 +0000 (13:00 +0000)]
commit-graph: clean up #includes

Our CodingGuidelines says that it's sufficient to include one of
'git-compat-util.h' and 'cache.h', but both 'commit-graph.c' and
'commit-graph.h' include both.  Let's include only 'git-compat-util.h'
to loose a bunch of unnecessary dependencies; but include 'hash.h',
because 'commit-graph.h' does require the definition of 'struct
object_id'.

'commit-graph.h' explicitly includes 'repository.h' and
'string-list.h', but only needs the declaration of a few structs from
them.  Drop these includes and forward-declare the necessary structs
instead.

'commit-graph.c' includes 'dir.h', but doesn't actually use anything
from there, so let's drop that #include as well.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agodiff.h: drop diff_tree_oid() & friends' return value
SZEDER Gábor [Fri, 5 Jun 2020 13:00:27 +0000 (13:00 +0000)]
diff.h: drop diff_tree_oid() & friends' return value

ll_diff_tree_oid() has only ever returned 0 [1], so it's return value
is basically useless.  It's only caller diff_tree_oid() has only ever
returned the return value of ll_diff_tree_oid() as-is [2], so its
return value is just as useless.  Most of diff_tree_oid()'s callers
simply ignore its return value, except:

  - diff_root_tree_oid() is a thin wrapper around diff_tree_oid() and
    returns with its return value, but all of diff_root_tree_oid()'s
    callers ignore its return value.

  - rev_compare_tree() and rev_same_tree_as_empty() do look at the
    return value in a condition, but, since the return value is always
    0, the former's < 0 condition is never fulfilled, while the
    latter's >= 0 condition is always fulfilled.

So let's drop the return value of ll_diff_tree_oid(), diff_tree_oid()
and diff_root_tree_oid(), and drop those conditions from
rev_compare_tree() and rev_same_tree_as_empty() as well.

[1] ll_diff_tree_oid() and its ancestors have been returning only 0
    ever since it was introduced as diff_tree() in 9174026cfe (Add
    "diff-tree" program to show which files have changed between two
    trees., 2005-04-09).
[2] diff_tree_oid() traces back to diff-tree.c:main() in 9174026cfe as
    well.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit-slab: add a function to deep free entries on the slab
SZEDER Gábor [Fri, 5 Jun 2020 13:00:26 +0000 (13:00 +0000)]
commit-slab: add a function to deep free entries on the slab

clear_##slabname() frees only the memory allocated for a commit slab
itself, but entries in the commit slab might own additional memory
outside the slab that should be freed as well.  We already have (at
least) one such commit slab, and this patch series is about to add one
more.

To free all additional memory owned by entries on the commit slab the
user of such a slab could iterate over all commits it knows about,
peek whether there is a valid entry associated with each commit, and
free the additional memory, if any.  Or it could rely on intimate
knowledge about the internals of the commit slab implementation, and
could itself iterate directly through all entries in the slab, and
free the additional memory.  Or it could just leak the additional
memory...

Introduce deep_clear_##slabname() to allow releasing memory owned by
commit slab entries by invoking the 'void free_fn(elemtype *ptr)'
function specified as parameter for each entry in the slab.

Use it in get_shallow_commits() in 'shallow.c' to replace an
open-coded iteration over a commit slab's entries.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit-graph-format.txt: all multi-byte numbers are in network byte order
SZEDER Gábor [Fri, 5 Jun 2020 13:00:25 +0000 (13:00 +0000)]
commit-graph-format.txt: all multi-byte numbers are in network byte order

The commit-graph format specifies that "All 4-byte numbers are in
network order", but the commit-graph contains 8-byte integers as well
(file offsets in the Chunk Lookup table), and their byte order is
unspecified.

Clarify that all multi-byte integers are in network byte order.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocommit-graph: fix parsing the Chunk Lookup table
SZEDER Gábor [Fri, 5 Jun 2020 13:00:24 +0000 (13:00 +0000)]
commit-graph: fix parsing the Chunk Lookup table

The commit-graph file format specifies that the chunks may be in any
order.  However, if the OID Lookup chunk happens to be the last one in
the file, then any command attempting to access the commit-graph data
will fail with:

  fatal: invalid commit position. commit-graph is likely corrupt

In this case the error is wrong, the commit-graph file does conform to
the specification, but the parsing of the Chunk Lookup table is a bit
buggy, and leaves the field holding the number of commits in the
commit-graph zero-initialized.

The number of commits in the commit-graph is determined while parsing
the Chunk Lookup table, by dividing the size of the OID Lookup chunk
with the hash size.  However, the Chunk Lookup table doesn't actually
store the size of the chunks, but it stores their starting offset.
Consequently, the size of a chunk can only be calculated by
subtracting the starting offsets of that chunk from the offset of the
subsequent chunk, or in case of the last chunk from the offset
recorded in the terminating label.  This is currenly implemented in a
bit complicated way: as we iterate over the entries of the Chunk
Lookup table, we check the ID of each chunk and store its starting
offset, then we check the ID of the last seen chunk and calculate its
size using its previously saved offset if necessary (at the moment
it's only necessary for the OID Lookup chunk).  Alas, while parsing
the Chunk Lookup table we only interate through the "real" chunks, but
never look at the terminating label, thus don't even check whether
it's necessary to calulate the size of the last chunk.  Consequently,
if the OID Lookup chunk is the last one, then we don't calculate its
size and turn don't run the piece of code determining the number of
commits in the commit graph, leaving the field holding that number
unchanged (i.e. zero-initialized), eventually triggering the sanity
check in load_oid_from_graph().

Fix this by iterating through all entries in the Chunk Lookup table,
including the terminating label.

Note that this is the minimal fix, suitable for the maintenance track.
A better fix would be to simplify how the chunk sizes are calculated,
but that is a more invasive change, less suitable for 'maint', so that
will be done in later patches.

This additional flexibility of scanning more chunks breaks a test for
"git commit-graph verify" so alter that test to mutate the commit-graph
to have an even lower chunk count.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agotree-walk.c: don't match submodule entries for 'submod/anything'
SZEDER Gábor [Fri, 5 Jun 2020 13:00:23 +0000 (13:00 +0000)]
tree-walk.c: don't match submodule entries for 'submod/anything'

Submodules should be handled the same as regular directories with
respect to the presence of a trailing slash, i.e. commands like:

  git diff rev1 rev2 -- $path
  git rev-list HEAD -- $path

should produce the same output whether $path is 'submod' or 'submod/'.
This has been fixed in commit 74b4f7f277 (tree-walk.c: ignore trailing
slash on submodule in tree_entry_interesting(), 2014-01-23).

Unfortunately, that commit had the unintended side effect to handle
'submod/anything' the same as 'submod' and 'submod/' as well, e.g.:

  $ git log --oneline --name-only -- sha1collisiondetection/whatever
  4125f78222 sha1dc: update from upstream
  sha1collisiondetection
  07a20f569b Makefile: fix unaligned loads in sha1dc with UBSan
  sha1collisiondetection
  23e37f8e9d sha1dc: update from upstream
  sha1collisiondetection
  86cfd61e6b sha1dc: optionally use sha1collisiondetection as a submodule
  sha1collisiondetection

Fix this by rejecting submodules as partial pathnames when their
trailing slash is followed by anything.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoCodingGuidelines: specify Python 2.7 is the oldest version
Denton Liu [Sun, 7 Jun 2020 10:21:06 +0000 (06:21 -0400)]
CodingGuidelines: specify Python 2.7 is the oldest version

In 0b4396f068 (git-p4: make python2.7 the oldest supported version,
2019-12-13), git-p4 was updated to only support 2.7 and newer. Since
Python 2.6 is pretty much ancient history, update CodingGuidelines to
show that 2.7 is the oldest version supported.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>
git clone https://git.99rst.org/PROJECT