git
3 years agogit-apply: allow simultaneous --cached and --3way options
Jerry Zhang [Thu, 8 Apr 2021 02:13:44 +0000 (19:13 -0700)] 
git-apply: allow simultaneous --cached and --3way options

"git apply" does not allow "--cached" and "--3way" to be used
together, since "--3way" writes conflict markers into the working
tree.

Allow "git apply" to accept "--cached" and "--3way" at the same
time.  When a single file auto-resolves cleanly, the result is
placed in the index at stage #0 and the command exits with 0 status.

For a file that has a conflict which cannot be cleanly
auto-resolved, the original contents from common ancestor (stage
conflict at the content level, and the command exists with non-zero
status, because there is no place (like the working tree) to leave a
half-resolved merge for the user to resolve.

The user can use `git diff` to view the contents of the conflict, or
`git checkout -m -- .` to regenerate the conflict markers in the
working directory.

Don't attempt rerere in this case since it depends on conflict
markers written to file for its database storage and lookup. There
would be two main changes required to get rerere working:

1. Allow the rerere api to accept in memory object rather than
   files, which would allow us to pass in the conflict markers
   contained in the result from ll_merge().

2. Rerere can't write to the working directory, so it would have to
   apply the result to cache stage #0 directly. A flag would be
   needed to control this.

Signed-off-by: Jerry Zhang <jerry@skydio.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoThe seventh batch
Junio C Hamano [Wed, 7 Apr 2021 23:44:08 +0000 (16:44 -0700)] 
The seventh batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'ab/fsck-api-cleanup'
Junio C Hamano [Wed, 7 Apr 2021 23:54:09 +0000 (16:54 -0700)] 
Merge branch 'ab/fsck-api-cleanup'

Fsck API clean-up.

* ab/fsck-api-cleanup:
  fetch-pack: use new fsck API to printing dangling submodules
  fetch-pack: use file-scope static struct for fsck_options
  fetch-pack: don't needlessly copy fsck_options
  fsck.c: move gitmodules_{found,done} into fsck_options
  fsck.c: add an fsck_set_msg_type() API that takes enums
  fsck.c: pass along the fsck_msg_id in the fsck_error callback
  fsck.[ch]: move FOREACH_FSCK_MSG_ID & fsck_msg_id from *.c to *.h
  fsck.c: give "FOREACH_MSG_ID" a more specific name
  fsck.c: undefine temporary STR macro after use
  fsck.c: call parse_msg_type() early in fsck_set_msg_type()
  fsck.h: re-order and re-assign "enum fsck_msg_type"
  fsck.h: move FSCK_{FATAL,INFO,ERROR,WARN,IGNORE} into an enum
  fsck.c: refactor fsck_msg_type() to limit scope of "int msg_type"
  fsck.c: rename remaining fsck_msg_id "id" to "msg_id"
  fsck.c: remove (mostly) redundant append_msg_id() function
  fsck.c: rename variables in fsck_set_msg_type() for less confusion
  fsck.h: use "enum object_type" instead of "int"
  fsck.h: use designed initializers for FSCK_OPTIONS_{DEFAULT,STRICT}
  fsck.c: refactor and rename common config callback

3 years agoMerge branch 'cc/downcase-opt-help'
Junio C Hamano [Wed, 7 Apr 2021 23:54:09 +0000 (16:54 -0700)] 
Merge branch 'cc/downcase-opt-help'

A few option description strings started with capital letters,
which were corrected.

* cc/downcase-opt-help:
  column, range-diff: downcase option description

3 years agoMerge branch 'js/security-md'
Junio C Hamano [Wed, 7 Apr 2021 23:54:09 +0000 (16:54 -0700)] 
Merge branch 'js/security-md'

SECURITY.md that is facing individual contributors and end users
has been introduced.  Also a procedure to follow when preparing
embargoed releases has been spelled out.

* js/security-md:
  Document how we do embargoed releases
  SECURITY: describe how to report vulnerabilities

3 years agoMerge branch 'ps/pack-bitmap-optim'
Junio C Hamano [Wed, 7 Apr 2021 23:54:09 +0000 (16:54 -0700)] 
Merge branch 'ps/pack-bitmap-optim'

Optimize "rev-list --use-bitmap-index --objects" corner case that
uses negative tags as the stopping points.

* ps/pack-bitmap-optim:
  pack-bitmap: avoid traversal of objects referenced by uninteresting tag

3 years agoMerge branch 'zh/commit-trailer'
Junio C Hamano [Wed, 7 Apr 2021 23:54:08 +0000 (16:54 -0700)] 
Merge branch 'zh/commit-trailer'

"git commit" learned "--trailer <key>[=<value>]" option; together
with the interpret-trailers command, this will make it easier to
support custom trailers.

* zh/commit-trailer:
  commit: add --trailer option

3 years agoMerge branch 'js/cmake-vsbuild'
Junio C Hamano [Wed, 7 Apr 2021 23:54:08 +0000 (16:54 -0700)] 
Merge branch 'js/cmake-vsbuild'

CMake update for vsbuild.

* js/cmake-vsbuild:
  cmake(install): include vcpkg dlls
  cmake: add a preparatory work-around to accommodate `vcpkg`
  cmake(install): fix double .exe suffixes
  cmake: support SKIP_DASHED_BUILT_INS

3 years agoMerge branch 'ds/clarify-hashwrite'
Junio C Hamano [Wed, 7 Apr 2021 23:54:08 +0000 (16:54 -0700)] 
Merge branch 'ds/clarify-hashwrite'

The hashwrite() API uses a buffering mechanism to avoid calling
write(2) too frequently. This logic has been refactored to be
easier to understand.

* ds/clarify-hashwrite:
  csum-file: make hashwrite() more readable

3 years agoMerge branch 'ah/plugleaks'
Junio C Hamano [Wed, 7 Apr 2021 23:54:08 +0000 (16:54 -0700)] 
Merge branch 'ah/plugleaks'

Plug or annotate remaining leaks that trigger while running the
very basic set of tests.

* ah/plugleaks:
  transport: also free remote_refs in transport_disconnect()
  parse-options: don't leak alias help messages
  parse-options: convert bitfield values to use binary shift
  init-db: silence template_dir leak when converting to absolute path
  init: remove git_init_db_config() while fixing leaks
  worktree: fix leak in dwim_branch()
  clone: free or UNLEAK further pointers when finished
  reset: free instead of leaking unneeded ref
  symbolic-ref: don't leak shortened refname in check_symref()

3 years agobash completion: complete CHERRY_PICK_HEAD
Ævar Arnfjörð Bjarmason [Wed, 7 Apr 2021 10:50:51 +0000 (12:50 +0200)] 
bash completion: complete CHERRY_PICK_HEAD

When e.g. in a failed cherry pick we did not recognize
CHERRY_PICK_HEAD as we do e.g. REBASE_HEAD in a failed rebase let's
rectify that.

When REBASE_HEAD was added in fbd7a232370 (rebase: introduce and use
pseudo-ref REBASE_HEAD, 2018-02-11) a completion was added for it, but
no corresponding completion existed for CHERRY_PICK_HEAD added in
d7e5c0cbfb0 (Introduce CHERRY_PICK_HEAD, 2011-02-19).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogit-apply: try threeway first when "--3way" is used
Jerry Zhang [Tue, 6 Apr 2021 23:25:32 +0000 (16:25 -0700)] 
git-apply: try threeway first when "--3way" is used

The apply_fragments() method of "git apply"
can silently apply patches incorrectly if
a file has repeating contents. In these
cases a three-way merge is capable of applying
it correctly in more situations, and will
show a conflict rather than applying it
incorrectly. However, because the patches
apply "successfully" using apply_fragments(),
git will never fall back to the merge, even
if the "--3way" flag is used, and the user has
no way to ensure correctness by forcing the
three-way merge method.

Change the behavior so that when "--3way" is used,
git will always try the three-way merge first and
will only fall back to apply_fragments() in cases
where blobs are not available or some other error
(but not in the case of a merge conflict).

Since user-facing results will be different,
this has backwards compatibility implications
for users depending on the old behavior. In
addition, the three-way merge will be slower
than direct patch application.

Signed-off-by: Jerry Zhang <jerry@skydio.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomaintenance: simplify prefetch logic
Derrick Stolee [Tue, 6 Apr 2021 18:47:46 +0000 (18:47 +0000)] 
maintenance: simplify prefetch logic

The previous logic filled a string list with the names of each remote,
but instead we could simply run the appropriate 'git fetch' data
directly in the remote iterator. Do this for reduced code size, but also
because it sets up an upcoming change to use the remote's refspec. This
data is accessible from the 'struct remote' data that is now accessible
in fetch_remote().

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogit-send-email: improve --validate error output
Ævar Arnfjörð Bjarmason [Tue, 6 Apr 2021 14:00:37 +0000 (16:00 +0200)] 
git-send-email: improve --validate error output

Improve the output we emit on --validate error to:

 * Say "FILE:LINE" instead of "FILE: LINE", to match "grep -n",
   compiler error messages etc.

 * Don't say "patch contains a" after just mentioning the filename,
   just leave it at "FILE:LINE: is longer than[...]. The "contains a"
   sounded like we were talking about the file in general, when we're
   actually checking it line-by-line.

 * Don't just say "rejected by sendemail-validate hook", but combine
   that with the system_or_msg() output to say what exit code the hook
   died with.

I had an aborted attempt to make the line length checker note all
lines that were longer than the limit. I didn't think that was worth
the effort, but I've left in the testing change to check that we die
as soon as we spot the first long line.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogit-send-email: refactor duplicate $? checks into a function
Ævar Arnfjörð Bjarmason [Tue, 6 Apr 2021 14:00:36 +0000 (16:00 +0200)] 
git-send-email: refactor duplicate $? checks into a function

Refactor the duplicate checking of $? into a function. There's an
outstanding series[1] wanting to add a third use of system() in this
file, let's not copy this boilerplate anymore when that happens.

1. http://lore.kernel.org/git/87y2esg22j.fsf@evledraar.gmail.com

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogit-send-email: test full --validate output
Ævar Arnfjörð Bjarmason [Tue, 6 Apr 2021 14:00:35 +0000 (16:00 +0200)] 
git-send-email: test full --validate output

Change the tests that grep substrings out of the output to use a full
test_cmp, in preparation for improving the output.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotest-bloom: fix missing 'bloom' from usage string
Christian Couder [Tue, 6 Apr 2021 02:35:14 +0000 (04:35 +0200)] 
test-bloom: fix missing 'bloom' from usage string

Like 'get_murmur3' and 'generate_filter', 'get_filter_for_commit' is a
subcommand of `test-tool bloom` not of `test-tool` itself.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomacOS: precompose startup_info->prefix
Torsten Bögershausen [Sun, 4 Apr 2021 17:14:14 +0000 (19:14 +0200)] 
macOS: precompose startup_info->prefix

The "prefix" was precomposed for macOS in commit 5c327502 (MacOS:
precompose_argv_prefix(), 2021-02-03).

However, this commit forgot to update "startup_info->prefix" after
precomposing.

Move the (possible) precomposition towards the end of
setup_git_directory_gently(), so that precompose_string_if_needed()
can use git_config_get_bool("core.precomposeunicode") correctly.

Keep prefix, startup_info->prefix and GIT_PREFIX_ENVIRONMENT all in sync.

And as a result, the prefix no longer needs to be precomposed in git.c

Reported-by: Dmitry Torilov <d.torilov@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoprecompose_utf8: make precompose_string_if_needed() public
Torsten Bögershausen [Sun, 4 Apr 2021 06:17:45 +0000 (08:17 +0200)] 
precompose_utf8: make precompose_string_if_needed() public

commit 5c327502 (MacOS: precompose_argv_prefix(), 2021-02-03) uses
the function precompose_string_if_needed() internally.  It is only
used from precompose_argv_prefix() and therefore static in
compat/precompose_utf8.c

Expose this function, it will be used in the next commit.

While there, allow passing a NULL pointer, which will return NULL.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agouser-manual.txt: assign preface an id and a title
Firmin Martin [Sun, 4 Apr 2021 04:07:39 +0000 (06:07 +0200)] 
user-manual.txt: assign preface an id and a title

Two among the three warnings raised by "make git.info" are related to the fact
that the preface has not id in user-manual.txt.

    user-manual.texi:15: warning: empty menu entry name in `* : idm4.'
    user-manual.texi:141: warning: @unnumbered missing argument

This causes asciidoc creating an empty preface and an empty title tag in
user-manual.xml which turns to be an empty node in user-manual.texi and
git.info. Consequently, one can notice in user-manual.texi and git.info
a node named "idm4" in the menu and the navigation bar. In emacs, the
first entry of the menu in the git info page is even displayed as empty.

This fix will name "Introduction" the preface and assign it an id.
The result can be seen in the files: user-manual.{xml, texi, html, pdf}
and git.info.

For future reference, the diff between old and new user-manual.xml,
user-manual.texi, git.info, user-manual.html (converted through
html2markdown) and user-manual.pdf (converted through pdftotext) are
attached.

    --- before/user-manual.xml 2021-04-04 03:58:47.758008722 +0200
    +++ after/user-manual.xml 2021-04-04 03:56:40.520551163 +0200
    @@ -7,8 +7,8 @@
     <bookinfo>
         <title>Git User Manual</title>
     </bookinfo>
    -<preface>
    -<title></title>
    +<preface id="_introduction">
    +<title>Introduction</title>
     <simpara>Git is a fast distributed revision control system.</simpara>
     <simpara>This manual is designed to be readable by someone with basic UNIX
     command-line skills, but no previous knowledge of Git.</simpara>

    --- before/user-manual.texi 2021-04-04 03:58:47.490005652 +0200
    +++ after/user-manual.texi 2021-04-04 03:56:40.520551163 +0200
    @@ -7,12 +7,12 @@
     * Git: (git).           A fast distributed revision control system
     @end direntry

    -@node Top, idm4, , (dir)
    +@node Top, Introduction, , (dir)
     @documentlanguage en
     @top Git User Manual

     @menu
    -* : idm4.
    +* Introduction::
     * Repositories and Branches::
     * Exploring Git history::
     * Developing with Git::
    @@ -137,8 +137,8 @@
     @end detailmenu
     @end menu

    -@node idm4, Repositories and Branches, Top, Top
    -@unnumbered
    +@node Introduction, Repositories and Branches, Top, Top
    +@unnumbered Introduction

     Git is a fast distributed revision control system.

    @@ -178,7 +178,7 @@
     Finally, see @ref{Notes and todo list for this manual} for ways that you can help make this manual more
     complete.

    -@node Repositories and Branches, Exploring Git history, idm4, Top
    +@node Repositories and Branches, Exploring Git history, Introduction, Top
     @chapter Repositories and Branches

     @menu

    --- before/git.info 2021-04-04 03:58:46.557994966 +0200
    +++ after/git.info 2021-04-04 03:56:40.520551163 +0200
    @@ -7,14 +7,14 @@
     END-INFO-DIR-ENTRY

    -File: git.info,  Node: Top,  Next: idm4,  Up: (dir)
    +File: git.info,  Node: Top,  Next: Introduction,  Up: (dir)

     Git User Manual
     ***************

     * Menu:

    -* : idm4.
    +* Introduction::
     * Repositories and Branches::
     * Exploring Git history::
     * Developing with Git::
    @@ -137,7 +137,10 @@

    -File: git.info,  Node: idm4,  Next: Repositories and Branches,  Prev: Top,  Up: Top
    +File: git.info,  Node: Introduction,  Next: Repositories and Branches,  Prev: Top,  Up: Top
    +
    +Introduction
    +************

     Git is a fast distributed revision control system.

    @@ -174,7 +177,7 @@
     that you can help make this manual more complete.

    -File: git.info,  Node: Repositories and Branches,  Next: Exploring Git history,  Prev: idm4,  Up: Top
    +File: git.info,  Node: Repositories and Branches,  Next: Exploring Git history,  Prev: Introduction,  Up: Top

     1 Repositories and Branches
     ***************************
    @@ -5471,207 +5474,207 @@
    ...
     Tag Table:
     Node: Top\7f212
    -Node: idm4\7f3164
    -Node: Repositories and Branches\7f4465
    ...
    +Node: Introduction\7f3179
    +Node: Repositories and Branches\7f4515
    +Node: How to get a Git repository\7f5128
    ...
    End Tag Table

    --- before/user-manual.html.md 2021-04-04 05:20:55.378695854 +0200
    +++ after/user-manual.html.md 2021-04-04 05:21:11.282850802 +0200
    @@ -4,6 +4,8 @@

      **Table of Contents**

    +Introduction
    +
     1\. Repositories and Branches

    @@ -278,7 +280,7 @@

     Todo list

    -#
    +# Introduction

     Git is a fast distributed revision control system.

    --- before/user-manual.pdf.txt 2021-04-04 05:28:20.367036836 +0200
    +++ after/user-manual.pdf.txt 2021-04-04 05:30:01.680026312 +0200
    @@ -487,6 +487,7 @@

     vii

    +Introduction
     Git is a fast distributed revision control system.
     This manual is designed to be readable by someone with basic UNIX command-line skills, but no previous knowledge of Git.
     Chapter 1 and Chapter 2 explain how to fetch and study a project using git—read these chapters to learn how to build and test a

Signed-off-by: Firmin Martin <firminmartin24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoThe sixth batch
Junio C Hamano [Fri, 2 Apr 2021 21:43:31 +0000 (14:43 -0700)] 
The sixth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'zh/format-patch-fractional-reroll-count'
Junio C Hamano [Fri, 2 Apr 2021 21:43:14 +0000 (14:43 -0700)] 
Merge branch 'zh/format-patch-fractional-reroll-count'

"git format-patch -v<n>" learned to allow a reroll count that is
not an integer.

* zh/format-patch-fractional-reroll-count:
  format-patch: allow a non-integral version numbers

3 years agoMerge branch 'jh/simple-ipc'
Junio C Hamano [Fri, 2 Apr 2021 21:43:14 +0000 (14:43 -0700)] 
Merge branch 'jh/simple-ipc'

A simple IPC interface gets introduced to build services like
fsmonitor on top.

* jh/simple-ipc:
  t0052: add simple-ipc tests and t/helper/test-simple-ipc tool
  simple-ipc: add Unix domain socket implementation
  unix-stream-server: create unix domain socket under lock
  unix-socket: disallow chdir() when creating unix domain sockets
  unix-socket: add backlog size option to unix_stream_listen()
  unix-socket: eliminate static unix_stream_socket() helper function
  simple-ipc: add win32 implementation
  simple-ipc: design documentation for new IPC mechanism
  pkt-line: add options argument to read_packetized_to_strbuf()
  pkt-line: add PACKET_READ_GENTLE_ON_READ_ERROR option
  pkt-line: do not issue flush packets in write_packetized_*()
  pkt-line: eliminate the need for static buffer in packet_write_gently()

3 years agoMerge branch 'mt/parallel-checkout-part-1'
Junio C Hamano [Fri, 2 Apr 2021 21:43:14 +0000 (14:43 -0700)] 
Merge branch 'mt/parallel-checkout-part-1'

Preparatory API changes for parallel checkout.

* mt/parallel-checkout-part-1:
  entry: add checkout_entry_ca() taking preloaded conv_attrs
  entry: move conv_attrs lookup up to checkout_entry()
  entry: extract update_ce_after_write() from write_entry()
  entry: make fstat_output() and read_blob_entry() public
  entry: extract a header file for entry.c functions
  convert: add classification for conv_attrs struct
  convert: add get_stream_filter_ca() variant
  convert: add [async_]convert_to_working_tree_ca() variants
  convert: make convert_attrs() and convert structs public

3 years agogit-send-email: replace "map" in void context with "for"
Ævar Arnfjörð Bjarmason [Fri, 2 Apr 2021 11:34:33 +0000 (13:34 +0200)] 
git-send-email: replace "map" in void context with "for"

While using "map" instead of "for" or "map" instead of "grep" and
vice-versa makes for interesting trivia questions when interviewing
Perl programmers, it doesn't make for very readable code. Let's
refactor this loop initially added in 8fd5bb7f44b (git send-email: add
--annotate option, 2008-11-11) to be a for-loop instead.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMakefile: add QUIET_GEN to "tags" and "TAGS" targets
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 02:18:34 +0000 (04:18 +0200)] 
Makefile: add QUIET_GEN to "tags" and "TAGS" targets

Don't show the very verbose $(FIND_SOURCE_FILES) command on every
"make TAGS" invocation.

Let's use "generate into temporary and rename to the final file,
after seeing the command that generated the output finished
successfully" pattern, to avoid leaving a file with an incorrect
output generated by a failed command.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomidx.c: improve cache locality in midx_pack_order_cmp()
Jeff King [Tue, 30 Mar 2021 15:04:36 +0000 (11:04 -0400)] 
midx.c: improve cache locality in midx_pack_order_cmp()

There is a lot of pointer dereferencing in the pre-image version of
'midx_pack_order_cmp()', which this patch gets rid of.

Instead of comparing the pack preferred-ness and then the pack id, both
of these checks are done at the same time by using the high-order bit of
the pack id to represent whether it's preferred. Then the pack id and
offset are compared as usual.

This produces the same result so long as there are less than 2^31 packs,
which seems like a likely assumption to make in practice.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-revindex: write multi-pack reverse indexes
Taylor Blau [Tue, 30 Mar 2021 15:04:32 +0000 (11:04 -0400)] 
pack-revindex: write multi-pack reverse indexes

Implement the writing half of multi-pack reverse indexes. This is
nothing more than the format describe a few patches ago, with a new set
of helper functions that will be used to clear out stale .rev files
corresponding to old MIDXs.

Unfortunately, a very similar comparison function as the one implemented
recently in pack-revindex.c is reimplemented here, this time accepting a
MIDX-internal type. An effort to DRY these up would create more
indirection and overhead than is necessary, so it isn't pursued here.

Currently, there are no callers which pass the MIDX_WRITE_REV_INDEX
flag, meaning that this is all dead code. But, that won't be the case
for long, since subsequent patches will introduce the multi-pack bitmap,
which will begin passing this field.

(In midx.c:write_midx_internal(), the two adjacent if statements share a
conditional, but are written separately since the first one will
eventually also handle the MIDX_WRITE_BITMAP flag, which does not yet
exist.)

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-write.c: extract 'write_rev_file_order'
Taylor Blau [Tue, 30 Mar 2021 15:04:29 +0000 (11:04 -0400)] 
pack-write.c: extract 'write_rev_file_order'

Existing callers provide the reverse index code with an array of 'struct
pack_idx_entry *'s, which is then sorted by pack order (comparing the
offsets of each object within the pack).

Prepare for the multi-pack index to write a .rev file by providing a way
to write the reverse index without an array of pack_idx_entry (which the
MIDX code does not have).

Instead, callers can invoke 'write_rev_index_positions()', which takes
an array of uint32_t's. The ith entry in this array specifies the ith
object's (in index order) position within the pack (in pack order).

Expose this new function for use in a later patch, and rewrite the
existing write_rev_file() in terms of this new function.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-revindex: read multi-pack reverse indexes
Taylor Blau [Tue, 30 Mar 2021 15:04:26 +0000 (11:04 -0400)] 
pack-revindex: read multi-pack reverse indexes

Implement reading for multi-pack reverse indexes, as described in the
previous patch.

Note that these functions don't yet have any callers, and won't until
multi-pack reachability bitmaps are introduced in a later patch series.
In the meantime, this patch implements some of the infrastructure
necessary to support multi-pack bitmaps.

There are three new functions exposed by the revindex API:

  - load_midx_revindex(): loads the reverse index corresponding to the
    given multi-pack index.

  - midx_to_pack_pos() and pack_pos_to_midx(): these convert between the
    multi-pack index and pseudo-pack order.

load_midx_revindex() and pack_pos_to_midx() are both relatively
straightforward.

load_midx_revindex() needs a few functions to be exposed from the midx
API. One to get the checksum of a midx, and another to get the .rev's
filename. Similar to recent changes in the packed_git struct, three new
fields are added to the multi_pack_index struct: one to keep track of
the size, one to keep track of the mmap'd pointer, and another to point
past the header and at the reverse index's data.

pack_pos_to_midx() simply reads the corresponding entry out of the
table.

midx_to_pack_pos() is the trickiest, since it needs to find an object's
position in the psuedo-pack order, but that order can only be recovered
in the .rev file itself. This mapping can be implemented with a binary
search, but note that the thing we're binary searching over isn't an
array of values, but rather a permuted order of those values.

So, when comparing two items, it's helpful to keep in mind the
difference. Instead of a traditional binary search, where you are
comparing two things directly, here we're comparing a (pack, offset)
tuple with an index into the multi-pack index. That index describes
another (pack, offset) tuple, and it is _those_ two tuples that are
compared.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoDocumentation/technical: describe multi-pack reverse indexes
Taylor Blau [Tue, 30 Mar 2021 15:04:23 +0000 (11:04 -0400)] 
Documentation/technical: describe multi-pack reverse indexes

As a prerequisite to implementing multi-pack bitmaps, motivate and
describe the format and ordering of the multi-pack reverse index.

The subsequent patch will implement reading this format, and the patch
after that will implement writing it while producing a multi-pack index.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomidx: make some functions non-static
Taylor Blau [Tue, 30 Mar 2021 15:04:20 +0000 (11:04 -0400)] 
midx: make some functions non-static

In a subsequent commit, pack-revindex.c will become responsible for
sorting a list of objects in the "MIDX pack order" (which will be
defined in the following patch). To do so, it will need to be know the
pack identifier and offset within that pack for each object in the MIDX.

The MIDX code already has functions for doing just that
(nth_midxed_offset() and nth_midxed_pack_int_id()), but they are
statically declared.

Since there is no reason that they couldn't be exposed publicly, and
because they are already doing exactly what the caller in
pack-revindex.c will want, expose them publicly so that they can be
reused there.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomidx: keep track of the checksum
Taylor Blau [Tue, 30 Mar 2021 15:04:17 +0000 (11:04 -0400)] 
midx: keep track of the checksum

write_midx_internal() uses a hashfile to write the multi-pack index, but
discards its checksum. This makes sense, since nothing that takes place
after writing the MIDX cares about its checksum.

That is about to change in a subsequent patch, when the optional
reverse index corresponding to the MIDX will want to include the MIDX's
checksum.

Store the checksum of the MIDX in preparation for that.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomidx: don't free midx_name early
Taylor Blau [Tue, 30 Mar 2021 15:04:14 +0000 (11:04 -0400)] 
midx: don't free midx_name early

A subsequent patch will need to refer back to 'midx_name' later on in
the function. In fact, this variable is already free()'d later on, so
this makes the later free() no longer redundant.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomidx: allow marking a pack as preferred
Taylor Blau [Tue, 30 Mar 2021 15:04:11 +0000 (11:04 -0400)] 
midx: allow marking a pack as preferred

When multiple packs in the multi-pack index contain the same object, the
MIDX machinery must make a choice about which pack it associates with
that object. Prior to this patch, the lowest-ordered[1] pack was always
selected.

Pack selection for duplicate objects is relatively unimportant today,
but it will become important for multi-pack bitmaps. This is because we
can only invoke the pack-reuse mechanism when all of the bits for reused
objects come from the reuse pack (in order to ensure that all reused
deltas can find their base objects in the same pack).

To encourage the pack selection process to prefer one pack over another
(the pack to be preferred is the one a caller would like to later use as
a reuse pack), introduce the concept of a "preferred pack". When
provided, the MIDX code will always prefer an object found in a
preferred pack over any other.

No format changes are required to store the preferred pack, since it
will be able to be inferred with a corresponding MIDX bitmap, by looking
up the pack associated with the object in the first bit position (this
ordering is described in detail in a subsequent commit).

[1]: the ordering is specified by MIDX internals; for our purposes we
can consider the "lowest ordered" pack to be "the one with the
most-recent mtime.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobuiltin/clone.c: add --reject-shallow option
Li Linchao [Thu, 1 Apr 2021 10:46:59 +0000 (10:46 +0000)] 
builtin/clone.c: add --reject-shallow option

In some scenarios, users may want more history than the repository
offered for cloning, which happens to be a shallow repository, can
give them. But because users don't know it is a shallow repository
until they download it to local, we may want to refuse to clone
this kind of repository, without creating any unnecessary files.

The '--depth=x' option cannot be used as a solution; the source may
be deep enough to give us 'x' commits when cloned, but the user may
later need to deepen the history to arbitrary depth.

Teach '--reject-shallow' option to "git clone" to abort as soon as
we find out that we are cloning from a shallow repository.

Signed-off-by: Li Linchao <lilinchao@oschina.cn>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoref-filter: fix NULL check for parse object failure
Jeff King [Thu, 1 Apr 2021 08:32:24 +0000 (04:32 -0400)] 
ref-filter: fix NULL check for parse object failure

After we run parse_object_buffer() to get an object's contents, we try
to check that the return value wasn't NULL. However, since our "struct
object" is a pointer-to-pointer, and we assign like:

  *obj = parse_object_buffer(...);

it's not correct to check:

  if (!obj)

That will always be true, since our double pointer will continue to
point to the single pointer (which is itself NULL). This is a regression
that was introduced by aa46a0da30 (ref-filter: use oid_object_info() to
get object, 2018-07-17); since that commit we'll segfault on a parse
failure, as we try to look at the NULL object pointer.

There are many ways a parse could fail, but most of them are hard to set
up in the tests (it's easy to make a bogus object, but update-ref will
refuse to point to it). The test here uses a tag which points to a wrong
object type. A parse of just the broken tag object will succeed, but
seeing both tag objects in the same process will lead to a parse error
(since we'll see the pointed-to object as both types).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobuiltin/pack-objects.c: respect 'pack.preferBitmapTips'
Taylor Blau [Thu, 1 Apr 2021 01:32:14 +0000 (21:32 -0400)] 
builtin/pack-objects.c: respect 'pack.preferBitmapTips'

When writing a new pack with a bitmap, it is sometimes convenient to
indicate some reference prefixes which should receive priority when
selecting which commits to receive bitmaps.

A truly motivated caller could accomplish this by setting
'pack.islandCore', (since all commits in the core island are similarly
marked as preferred) but this requires callers to opt into using delta
islands, which they may or may not want to do.

Introduce a new multi-valued configuration, 'pack.preferBitmapTips' to
allow callers to specify a list of reference prefixes. All references
which have a prefix contained in 'pack.preferBitmapTips' will mark their
tips as "preferred" in the same way as commits are marked as preferred
for selection by 'pack.islandCore'.

The choice of the verb "prefer" is intentional: marking the NEEDS_BITMAP
flag on an object does *not* guarantee that that object will receive a
bitmap. It merely guarantees that that commit will receive a bitmap over
any *other* commit in the same window by bitmap_writer_select_commits().

The test this patch adds reflects this quirk, too. It only tests that
a commit (which didn't receive bitmaps by default) is selected for
bitmaps after changing the value of 'pack.preferBitmapTips' to include
it. Other commits may lose their bitmaps as a byproduct of how the
selection process works (bitmap_writer_select_commits() ignores the
remainder of a window after seeing a commit with the NEEDS_BITMAP flag).

This configuration will aide in selecting important references for
multi-pack bitmaps, since they do not respect the same pack.islandCore
configuration. (They could, but doing so may be confusing, since it is
packs--not bitmaps--which are influenced by the delta-islands
configuration).

In a fork network repository (one which lists all forks of a given
repository as remotes), for example, it is useful to set
pack.preferBitmapTips to 'refs/remotes/<root>/heads' and
'refs/remotes/<root>/tags', where '<root>' is an opaque identifier
referring to the repository which is at the base of the fork chain.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot/helper/test-bitmap.c: initial commit
Taylor Blau [Thu, 1 Apr 2021 01:32:11 +0000 (21:32 -0400)] 
t/helper/test-bitmap.c: initial commit

Add a new 'bitmap' test-tool which can be used to list the commits that
have received bitmaps.

In theory, a determined tester could run 'git rev-list --test-bitmap
<commit>' to check if '<commit>' received a bitmap or not, since
'--test-bitmap' exits with a non-zero code when it can't find the
requested commit.

But this is a dubious behavior to rely on, since arguably 'git
rev-list' could continue its object walk outside of which commits are
covered by bitmaps.

This will be used to test the behavior of 'pack.preferBitmapTips', which
will be added in the following patch.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-bitmap: add 'test_bitmap_commits()' helper
Taylor Blau [Thu, 1 Apr 2021 01:32:07 +0000 (21:32 -0400)] 
pack-bitmap: add 'test_bitmap_commits()' helper

The next patch will add a 'bitmap' test-tool which prints the list of
commits that have bitmaps computed.

The test helper could implement this itself, but it would need access to
the 'bitmaps' field of the 'pack_bitmap' struct. To avoid exposing this
private detail, implement the entirety of the helper behind a
test_bitmap_commits() function in pack-bitmap.c.

There is some precedence for this with test_bitmap_walk() which is used
to implement the '--test-bitmap' flag in 'git rev-list' (and is also
implemented in pack-bitmap.c).

A caller will be added in the next patch.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosequencer: fix edit handling for cherry-pick and revert messages
Elijah Newren [Wed, 31 Mar 2021 06:52:20 +0000 (06:52 +0000)] 
sequencer: fix edit handling for cherry-pick and revert messages

save_opts() should save any non-default values.  It was intended to do
this, but since most options in struct replay_opts default to 0, it only
saved non-zero values.  Unfortunately, this does not always work for
options.edit.  Roughly speaking, options.edit had a default value of 0
for cherry-pick but a default value of 1 for revert.  Make save_opts()
record a value whenever it differs from the default.

options.edit was also overly simplistic; we had more than two cases.
The behavior that previously existed was as follows:

                       Non-conflict commits    Right after Conflict
    revert             Edit iff isatty(0)      Edit (ignore isatty(0))
    cherry-pick        No edit                 See above
    Specify --edit     Edit (ignore isatty(0)) See above
    Specify --no-edit  (*)                     See above

    (*) Before stopping for conflicts, No edit is the behavior.  After
        stopping for conflicts, the --no-edit flag is not saved so see
        the first two rows.

However, the expected behavior is:

                       Non-conflict commits    Right after Conflict
    revert             Edit iff isatty(0)      Edit iff isatty(0)
    cherry-pick        No edit                 Edit iff isatty(0)
    Specify --edit     Edit (ignore isatty(0)) Edit (ignore isatty(0))
    Specify --no-edit  No edit                 No edit

In order to get the expected behavior, we need to change options.edit
to a tri-state: unspecified, false, or true.  When specified, we follow
what it says.  When unspecified, we need to check whether the current
commit being created is resolving a conflict as well as consulting
options.action and isatty(0).  While at it, add a should_edit() utility
function that compresses options.edit down to a boolean based on the
additional information for the non-conflict case.

continue_single_pick() is the function responsible for resuming after
conflict cases, regardless of whether there is one commit being picked
or many.  Make this function stop assuming edit behavior in all cases,
so that it can correctly handle !isatty(0) and specific requests to not
edit the commit message.

Reported-by: Renato Botelho <garga@freebsd.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoThe fifth batch
Junio C Hamano [Tue, 30 Mar 2021 21:33:34 +0000 (14:33 -0700)] 
The fifth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'jc/doc-format-patch-clarify'
Junio C Hamano [Tue, 30 Mar 2021 21:35:38 +0000 (14:35 -0700)] 
Merge branch 'jc/doc-format-patch-clarify'

Explain pieces of the format-patch output upfront before the rest
of the documentation starts referring to them.

* jc/doc-format-patch-clarify:
  format-patch: give an overview of what a "patch" message is

3 years agoMerge branch 'ab/detox-gettext-tests'
Junio C Hamano [Tue, 30 Mar 2021 21:35:38 +0000 (14:35 -0700)] 
Merge branch 'ab/detox-gettext-tests'

Testfix.

* ab/detox-gettext-tests:
  mktag tests: fix broken "&&" chain

3 years agoMerge branch 'hx/pack-objects-chunk-comment'
Junio C Hamano [Tue, 30 Mar 2021 21:35:37 +0000 (14:35 -0700)] 
Merge branch 'hx/pack-objects-chunk-comment'

Comment update.

* hx/pack-objects-chunk-comment:
  pack-objects: fix comment of reused_chunk.difference

3 years agoMerge branch 'rf/send-email-hookspath'
Junio C Hamano [Tue, 30 Mar 2021 21:35:37 +0000 (14:35 -0700)] 
Merge branch 'rf/send-email-hookspath'

"git send-email" learned to honor the core.hooksPath configuration.

* rf/send-email-hookspath:
  git-send-email: Respect core.hooksPath setting

3 years agoMerge branch 'ab/remove-rebase-usebuiltin'
Junio C Hamano [Tue, 30 Mar 2021 21:35:37 +0000 (14:35 -0700)] 
Merge branch 'ab/remove-rebase-usebuiltin'

Remove the final hint that we used to have a scripted "git rebase".

* ab/remove-rebase-usebuiltin:
  rebase: remove transitory rebase.useBuiltin setting & env

3 years agoMerge branch 'cs/http-use-basic-after-failed-negotiate'
Junio C Hamano [Tue, 30 Mar 2021 21:35:37 +0000 (14:35 -0700)] 
Merge branch 'cs/http-use-basic-after-failed-negotiate'

When accessing a server with a URL like https://user:pass@site/, we
did not to fall back to the basic authentication with the
credential material embedded in the URL after the "Negotiate"
authentication failed.  Now we do.

* cs/http-use-basic-after-failed-negotiate:
  remote-curl: fall back to basic auth if Negotiate fails

3 years agoMerge branch 'ab/diff-no-index-tests'
Junio C Hamano [Tue, 30 Mar 2021 21:35:37 +0000 (14:35 -0700)] 
Merge branch 'ab/diff-no-index-tests'

More test coverage over "diff --no-index".

* ab/diff-no-index-tests:
  diff --no-index tests: test mode normalization
  diff --no-index tests: add test for --exit-code

3 years agoMerge branch 'ab/read-tree'
Junio C Hamano [Tue, 30 Mar 2021 21:35:37 +0000 (14:35 -0700)] 
Merge branch 'ab/read-tree'

Code simplification by removing support for a caller that is long gone.

* ab/read-tree:
  tree.h API: simplify read_tree_recursive() signature
  tree.h API: expose read_tree_1() as read_tree_at()
  archive: stop passing "stage" through read_tree_recursive()
  ls-files: refactor away read_tree()
  ls-files: don't needlessly pass around stage variable
  tree.c API: move read_tree() into builtin/ls-files.c
  ls-files tests: add meaningful --with-tree tests
  show tests: add test for "git show <tree>"

3 years agoMerge branch 'bs/asciidoctor-installation-hints'
Junio C Hamano [Tue, 30 Mar 2021 21:35:36 +0000 (14:35 -0700)] 
Merge branch 'bs/asciidoctor-installation-hints'

Doc update.

* bs/asciidoctor-installation-hints:
  INSTALL: note on using Asciidoctor to build doc

3 years agoMerge branch 'mt/checkout-remove-nofollow'
Junio C Hamano [Tue, 30 Mar 2021 21:35:36 +0000 (14:35 -0700)] 
Merge branch 'mt/checkout-remove-nofollow'

When "git checkout" removes a path that does not exist in the
commit it is checking out, it wasn't careful enough not to follow
symbolic links, which has been corrected.

* mt/checkout-remove-nofollow:
  checkout: don't follow symlinks when removing entries
  symlinks: update comment on threaded_check_leading_path()

3 years agop2000: add sparse-index repos
Derrick Stolee [Tue, 30 Mar 2021 13:11:04 +0000 (13:11 +0000)] 
p2000: add sparse-index repos

p2000-sparse-operations.sh compares different Git commands in
repositories with many files at HEAD but using sparse-checkout to focus
on a small portion of those files.

Add extra copies of the repository that use the sparse-index format so
we can track how that affects the performance of different commands.

At this point in time, the sparse-index is 100% overhead from the CPU
front, and this is measurable in these tests:

Test
---------------------------------------------------------------
2000.2: git status (full-index-v3)              0.59(0.51+0.12)
2000.3: git status (full-index-v4)              0.59(0.52+0.11)
2000.4: git status (sparse-index-v3)            1.40(1.32+0.12)
2000.5: git status (sparse-index-v4)            1.41(1.36+0.08)
2000.6: git add -A (full-index-v3)              2.32(1.97+0.19)
2000.7: git add -A (full-index-v4)              2.17(1.92+0.14)
2000.8: git add -A (sparse-index-v3)            2.31(2.21+0.15)
2000.9: git add -A (sparse-index-v4)            2.30(2.20+0.13)
2000.10: git add . (full-index-v3)              2.39(2.02+0.20)
2000.11: git add . (full-index-v4)              2.20(1.94+0.16)
2000.12: git add . (sparse-index-v3)            2.36(2.27+0.12)
2000.13: git add . (sparse-index-v4)            2.33(2.21+0.16)
2000.14: git commit -a -m A (full-index-v3)     2.47(2.12+0.20)
2000.15: git commit -a -m A (full-index-v4)     2.26(2.00+0.17)
2000.16: git commit -a -m A (sparse-index-v3)   3.01(2.92+0.16)
2000.17: git commit -a -m A (sparse-index-v4)   3.01(2.94+0.15)

Note that there is very little difference between the v3 and v4 index
formats when the sparse-index is enabled. This is primarily due to the
fact that the relative file sizes are the same, and the command time is
mostly taken up by parsing tree objects to expand the sparse index into
a full one.

With the current file layout, the index file sizes are given by this
table:

       |  full index | sparse index |
       +-------------+--------------+
    v3 |     108 MiB |      1.6 MiB |
    v4 |      80 MiB |      1.2 MiB |

Future updates will improve the performance of Git commands when the
index is sparse.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosparse-index: loose integration with cache_tree_verify()
Derrick Stolee [Tue, 30 Mar 2021 13:11:03 +0000 (13:11 +0000)] 
sparse-index: loose integration with cache_tree_verify()

The cache_tree_verify() method is run when GIT_TEST_CHECK_CACHE_TREE
is enabled, which it is by default in the test suite. The logic must
be adjusted for the presence of these directory entries.

For now, leave the test as a simple check for whether the directory
entry is sparse. Do not go any further until needed.

This allows us to re-enable GIT_TEST_CHECK_CACHE_TREE in
t1092-sparse-checkout-compatibility.sh. Further,
p2000-sparse-operations.sh uses the test suite and hence this is enabled
for all tests. We need to integrate with it before we run our
performance tests with a sparse-index.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocache-tree: integrate with sparse directory entries
Derrick Stolee [Tue, 30 Mar 2021 13:11:02 +0000 (13:11 +0000)] 
cache-tree: integrate with sparse directory entries

The cache-tree extension was previously disabled with sparse indexes.
However, the cache-tree is an important performance feature for commands
like 'git status' and 'git add'. Integrate it with sparse directory
entries.

When writing a sparse index, completely clear and recalculate the cache
tree. By starting from scratch, the only integration necessary is to
check if we hit a sparse directory entry and create a leaf of the
cache-tree that has an entry_count of one and no subtrees.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosparse-checkout: disable sparse-index
Derrick Stolee [Tue, 30 Mar 2021 13:11:01 +0000 (13:11 +0000)] 
sparse-checkout: disable sparse-index

We use 'git sparse-checkout init --cone --sparse-index' to toggle the
sparse-index feature. It makes sense to also disable it when running
'git sparse-checkout disable'. This is particularly important because it
removes the extensions.sparseIndex config option, allowing other tools
to use this Git repository again.

This does mean that 'git sparse-checkout init' will not re-enable the
sparse-index feature, even if it was previously enabled.

While testing this feature, I noticed that the sparse-index was not
being written on the first run, but by a second. This was caught by the
call to 'test-tool read-cache --table'. This requires adjusting some
assignments to core_apply_sparse_checkout and pl.use_cone_patterns in
the sparse_checkout_init() logic.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosparse-checkout: toggle sparse index from builtin
Derrick Stolee [Tue, 30 Mar 2021 13:11:00 +0000 (13:11 +0000)] 
sparse-checkout: toggle sparse index from builtin

The sparse index extension is used to signal that index writes should be
in sparse mode. This was only updated using GIT_TEST_SPARSE_INDEX=1.

Add a '--[no-]sparse-index' option to 'git sparse-checkout init' that
specifies if the sparse index should be used. It also updates the index
to use the correct format, either way. Add a warning in the
documentation that the use of a repository extension might reduce
compatibility with third-party tools. 'git sparse-checkout init' already
sets extension.worktreeConfig, which places most sparse-checkout users
outside of the scope of most third-party tools.

Update t1092-sparse-checkout-compatibility.sh to use this CLI instead of
GIT_TEST_SPARSE_INDEX=1.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosparse-index: add index.sparse config option
Derrick Stolee [Tue, 30 Mar 2021 13:10:59 +0000 (13:10 +0000)] 
sparse-index: add index.sparse config option

When enabled, this config option signals that index writes should
attempt to use sparse-directory entries.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosparse-index: check index conversion happens
Derrick Stolee [Tue, 30 Mar 2021 13:10:58 +0000 (13:10 +0000)] 
sparse-index: check index conversion happens

Add a test case that uses test_region to ensure that we are truly
expanding a sparse index to a full one, then converting back to sparse
when writing the index. As we integrate more Git commands with the
sparse index, we will convert these commands to check that we do _not_
convert the sparse index to a full index and instead stay sparse the
entire time.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agounpack-trees: allow sparse directories
Derrick Stolee [Tue, 30 Mar 2021 13:10:57 +0000 (13:10 +0000)] 
unpack-trees: allow sparse directories

The index_pos_by_traverse_info() currently throws a BUG() when a
directory entry exists exactly in the index. We need to consider that it
is possible to have a directory in a sparse index as long as that entry
is itself marked with the skip-worktree bit.

The 'pos' variable is assigned a negative value if an exact match is not
found. Since a directory name can be an exact match, it is no longer an
error to have a nonnegative 'pos' value.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosubmodule: sparse-index should not collapse links
Derrick Stolee [Tue, 30 Mar 2021 13:10:56 +0000 (13:10 +0000)] 
submodule: sparse-index should not collapse links

A submodule is stored as a "Git link" that actually points to a commit
within a submodule. Submodules are populated or not depending on
submodule configuration, not sparse-checkout. To ensure that the
sparse-index feature integrates correctly with submodules, we should not
collapse a directory if there is a Git link within its range.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosparse-index: convert from full to sparse
Derrick Stolee [Tue, 30 Mar 2021 13:10:55 +0000 (13:10 +0000)] 
sparse-index: convert from full to sparse

If we have a full index, then we can convert it to a sparse index by
replacing directories outside of the sparse cone with sparse directory
entries. The convert_to_sparse() method does this, when the situation is
appropriate.

For now, we avoid converting the index to a sparse index if:

 1. the index is split.
 2. the index is already sparse.
 3. sparse-checkout is disabled.
 4. sparse-checkout does not use cone mode.

Finally, we currently limit the conversion to when the
GIT_TEST_SPARSE_INDEX environment variable is enabled. A mode using Git
config will be added in a later change.

The trickiest thing about this conversion is that we might not be able
to mark a directory as a sparse directory just because it is outside the
sparse cone. There might be unmerged files within that directory, so we
need to look for those. Also, if there is some strange reason why a file
is not marked with CE_SKIP_WORKTREE, then we should give up on
converting that directory. There is still hope that some of its
subdirectories might be able to convert to sparse, so we keep looking
deeper.

The conversion process is assisted by the cache-tree extension. This is
calculated from the full index if it does not already exist. We then
abandon the cache-tree as it no longer applies to the newly-sparse
index. Thus, this cache-tree will be recalculated in every
sparse-full-sparse round-trip until we integrate the cache-tree
extension with the sparse index.

Some Git commands use the index after writing it. For example, 'git add'
will update the index, then write it to disk, then read its entries to
report information. To keep the in-memory index in a full state after
writing, we re-expand it to a full one after the write. This is wasteful
for commands that only write the index and do not read from it again,
but that is only the case until we make those commands "sparse aware."

We can compare the behavior of the sparse-index in
t1092-sparse-checkout-compability.sh by using GIT_TEST_SPARSE_INDEX=1
when operating on the 'sparse-index' repo. We can also compare the two
sparse repos directly, such as comparing their indexes (when expanded to
full in the case of the 'sparse-index' repo). We also verify that the
index is actually populated with sparse directory entries.

The 'checkout and reset (mixed)' test is marked for failure when
comparing a sparse repo to a full repo, but we can compare the two
sparse-checkout cases directly to ensure that we are not changing the
behavior when using a sparse index.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosparse-index: add 'sdir' index extension
Derrick Stolee [Tue, 30 Mar 2021 13:10:54 +0000 (13:10 +0000)] 
sparse-index: add 'sdir' index extension

The index format does not currently allow for sparse directory entries.
This violates some expectations that older versions of Git or
third-party tools might not understand. We need an indicator inside the
index file to warn these tools to not interact with a sparse index
unless they are aware of sparse directory entries.

Add a new _required_ index extension, 'sdir', that indicates that the
index may contain sparse directory entries. This allows us to continue
to use the differences in index formats 2, 3, and 4 before we create a
new index version 5 in a later change.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosparse-checkout: hold pattern list in index
Derrick Stolee [Tue, 30 Mar 2021 13:10:53 +0000 (13:10 +0000)] 
sparse-checkout: hold pattern list in index

As we modify the sparse-checkout definition, we perform index operations
on a pattern_list that only exists in-memory. This allows easy backing
out in case the index update fails.

However, if the index write itself cares about the sparse-checkout
pattern set, we need access to that in-memory copy. Place a pointer to
a 'struct pattern_list' in the index so we can access this on-demand.
This will be used in the next change which uses the sparse-checkout
definition to filter out directories that are outside the sparse cone.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agounpack-trees: ensure full index
Derrick Stolee [Tue, 30 Mar 2021 13:10:52 +0000 (13:10 +0000)] 
unpack-trees: ensure full index

The next change will translate full indexes into sparse indexes at write
time. The existing logic provides a way for every sparse index to be
expanded to a full index at read time. However, there are cases where an
index is written and then continues to be used in-memory to perform
further updates.

unpack_trees() is frequently called after such a write. In particular,
commands like 'git reset' do this double-update of the index.

Ensure that we have a full index when entering unpack_trees(), but only
when command_requires_full_index is true. This is always true at the
moment, but we will later relax that after unpack_trees() is updated to
handle sparse directory entries.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotest-tool: don't force full index
Derrick Stolee [Tue, 30 Mar 2021 13:10:51 +0000 (13:10 +0000)] 
test-tool: don't force full index

We will use 'test-tool read-cache --table' to check that a sparse
index is written as part of init_repos. Since we will no longer always
expand a sparse index into a full index, add an '--expand' parameter
that adds a call to ensure_full_index() so we can compare a sparse index
directly against a full index, or at least what the in-memory index
looks like when expanded in this way.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotest-read-cache: print cache entries with --table
Derrick Stolee [Tue, 30 Mar 2021 13:10:50 +0000 (13:10 +0000)] 
test-read-cache: print cache entries with --table

This table is helpful for discovering data in the index to ensure it is
being written correctly, especially as we build and test the
sparse-index. This table includes an output format similar to 'git
ls-tree', but should not be compared to that directly. The biggest
reasons are that 'git ls-tree' includes a tree entry for every
subdirectory, even those that would not appear as a sparse directory in
a sparse-index. Further, 'git ls-tree' does not use a trailing directory
separator for its tree rows.

This does not print the stat() information for the blobs. That will be
added in a future change with another option. The tests that are added
in the next few changes care only about the object types and IDs.
However, this future need for full index information justifies the need
for this test helper over extending a user-facing feature, such as 'git
ls-files'.

To make the option parsing slightly more robust, wrap the string
comparisons in a loop adapted from test-dir-iterator.c.

Care must be taken with the final check for the 'cnt' variable. We
continue the expectation that the numerical value is the final argument.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot1092: compare sparse-checkout to sparse-index
Derrick Stolee [Tue, 30 Mar 2021 13:10:49 +0000 (13:10 +0000)] 
t1092: compare sparse-checkout to sparse-index

Add a new 'sparse-index' repo alongside the 'full-checkout' and
'sparse-checkout' repos in t1092-sparse-checkout-compatibility.sh. Also
add run_on_sparse and test_sparse_match helpers. These helpers will be
used when the sparse index is implemented.

Add the GIT_TEST_SPARSE_INDEX environment variable to enable the
sparse-index by default. This can be enabled across all tests, but that
will only affect cases where the sparse-checkout feature is enabled.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosparse-index: implement ensure_full_index()
Derrick Stolee [Tue, 30 Mar 2021 13:10:48 +0000 (13:10 +0000)] 
sparse-index: implement ensure_full_index()

We will mark an in-memory index_state as having sparse directory entries
with the sparse_index bit. These currently cannot exist, but we will add
a mechanism for collapsing a full index to a sparse one in a later
change. That will happen at write time, so we must first allow parsing
the format before writing it.

Commands or methods that require a full index in order to operate can
call ensure_full_index() to expand that index in-memory. This requires
parsing trees using that index's repository.

Sparse directory entries have a specific 'ce_mode' value. The macro
S_ISSPARSEDIR(ce->ce_mode) can check if a cache_entry 'ce' has this type.
This ce_mode is not possible with the existing index formats, so we don't
also verify all properties of a sparse-directory entry, which are:

 1. ce->ce_mode == 0040000
 2. ce->flags & CE_SKIP_WORKTREE is true
 3. ce->name[ce->namelen - 1] == '/' (ends in dir separator)
 4. ce->oid references a tree object.

These are all semi-enforced in ensure_full_index() to some extent. Any
deviation will cause a warning at minimum or a failure in the worst
case.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosparse-index: add guard to ensure full index
Derrick Stolee [Tue, 30 Mar 2021 13:10:47 +0000 (13:10 +0000)] 
sparse-index: add guard to ensure full index

Upcoming changes will introduce modifications to the index format that
allow sparse directories. It will be useful to have a mechanism for
converting those sparse index files into full indexes by walking the
tree at those sparse directories. Name this method ensure_full_index()
as it will guarantee that the index is fully expanded.

This method is not implemented yet, and instead we focus on the
scaffolding to declare it and call it at the appropriate time.

Add a 'command_requires_full_index' member to struct repo_settings. This
will be an indicator that we need the index in full mode to do certain
index operations. This starts as being true for every command, then we
will set it to false as some commands integrate with sparse indexes.

If 'command_requires_full_index' is true, then we will immediately
expand a sparse index to a full one upon reading from disk. This
suffices for now, but we will want to add more callers to
ensure_full_index() later.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot1092: clean up script quoting
Derrick Stolee [Tue, 30 Mar 2021 13:10:46 +0000 (13:10 +0000)] 
t1092: clean up script quoting

This test was introduced in 19a0acc83e4 (t1092: test interesting
sparse-checkout scenarios, 2021-01-23), but it contains issues with quoting
that were not noticed until starting this follow-up series. The old
mechanism would drop quoting such as in

   test_all_match git commit -m "touch README.md"

The above happened to work because README.md is a file in the
repository, so 'git commit -m touch REAMDE.md' would succeed by
accident.

Other cases included quoting for no good reason, so clean that up now.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot/perf: add performance test for sparse operations
Derrick Stolee [Tue, 30 Mar 2021 13:10:45 +0000 (13:10 +0000)] 
t/perf: add performance test for sparse operations

Create a test script that takes the default performance test (the Git
codebase) and multiplies it by 256 using four layers of duplicated
trees of width four. This results in nearly one million blob entries in
the index. Then, we can clone this repository with sparse-checkout
patterns that demonstrate four copies of the initial repository. Each
clone will use a different index format or mode so peformance can be
tested across the different options.

Note that the initial repo is stripped of submodules before doing the
copies. This preserves the expected data shape of the sparse index,
because directories containing submodules are not collapsed to a sparse
directory entry.

Run a few Git commands on these clones, especially those that use the
index (status, add, commit).

Here are the results on my Linux machine:

Test
--------------------------------------------------------------
2000.2: git status (full-index-v3)             0.37(0.30+0.09)
2000.3: git status (full-index-v4)             0.39(0.32+0.10)
2000.4: git add -A (full-index-v3)             1.42(1.06+0.20)
2000.5: git add -A (full-index-v4)             1.26(0.98+0.16)
2000.6: git add . (full-index-v3)              1.40(1.04+0.18)
2000.7: git add . (full-index-v4)              1.26(0.98+0.17)
2000.8: git commit -a -m A (full-index-v3)     1.42(1.11+0.16)
2000.9: git commit -a -m A (full-index-v4)     1.33(1.08+0.16)

It is perhaps noteworthy that there is an improvement when using index
version 4. This is because the v3 index uses 108 MiB while the v4
index uses 80 MiB. Since the repeated portions of the directories are
very short (f3/f1/f2, for example) this ratio is less pronounced than in
similarly-sized real repositories.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosparse-index: design doc and format update
Derrick Stolee [Tue, 30 Mar 2021 13:10:44 +0000 (13:10 +0000)] 
sparse-index: design doc and format update

This begins a long effort to update the index format to allow sparse
directory entries. This should result in a significant improvement to
Git commands when HEAD contains millions of files, but the user has
selected many fewer files to keep in their sparse-checkout definition.

Currently, the index format is only updated in the presence of
extensions.sparseIndex instead of increasing a file format version
number. This is temporary, and index v5 is part of the plan for future
work in this area.

The design document details many of the reasons for embarking on this
work, and also the plan for completing it safely.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot/helper/test-read-midx.c: add '--show-objects'
Taylor Blau [Tue, 30 Mar 2021 15:04:07 +0000 (11:04 -0400)] 
t/helper/test-read-midx.c: add '--show-objects'

The 'read-midx' helper is used in places like t5319 to display basic
information about a multi-pack-index.

In the next patch, the MIDX writing machinery will learn a new way to
choose from which pack an object is selected when multiple copies of
that object exist.

To disambiguate which pack introduces an object so that this feature can
be tested, add a '--show-objects' option which displays additional
information about each object in the MIDX.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobuiltin/multi-pack-index.c: display usage on unrecognized command
Taylor Blau [Tue, 30 Mar 2021 15:04:04 +0000 (11:04 -0400)] 
builtin/multi-pack-index.c: display usage on unrecognized command

When given a sub-command that it doesn't understand, 'git
multi-pack-index' dies with the following message:

    $ git multi-pack-index bogus
    fatal: unrecognized subcommand: bogus

Instead of 'die()'-ing, we can display the usage text, which is much
more helpful:

    $ git.compile multi-pack-index bogus
    error: unrecognized subcommand: bogus
    usage: git multi-pack-index [<options>] write
       or: git multi-pack-index [<options>] verify
       or: git multi-pack-index [<options>] expire
       or: git multi-pack-index [<options>] repack [--batch-size=<size>]

        --object-dir <file>   object directory containing set of packfile and pack-index pairs
        --progress            force progress reporting

While we're at it, clean up some duplication between the "no sub-command"
and "unrecognized sub-command" conditionals.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobuiltin/multi-pack-index.c: don't enter bogus cmd_mode
Taylor Blau [Tue, 30 Mar 2021 15:04:01 +0000 (11:04 -0400)] 
builtin/multi-pack-index.c: don't enter bogus cmd_mode

Even before the recent refactoring, 'git multi-pack-index' calls
'trace2_cmd_mode()' before verifying that the sub-command is recognized.

Push this call down into the individual sub-commands so that we don't
enter a bogus command mode.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobuiltin/multi-pack-index.c: split sub-commands
Taylor Blau [Tue, 30 Mar 2021 15:03:57 +0000 (11:03 -0400)] 
builtin/multi-pack-index.c: split sub-commands

Handle sub-commands of the 'git multi-pack-index' builtin (e.g.,
"write", "repack", etc.) separately from one another. This allows
sub-commands with unique options, without forcing cmd_multi_pack_index()
to reject invalid combinations itself.

This comes at the cost of some duplication and boilerplate. Luckily, the
duplication is reduced to a minimum, since common options are shared
among sub-commands due to a suggestion by Ævar. (Sub-commands do have to
retain the common options, too, since this builtin accepts common
options on either side of the sub-command).

Roughly speaking, cmd_multi_pack_index() parses options (including
common ones), and stops at the first non-option, which is the
sub-command. It then dispatches to the appropriate sub-command, which
parses the remaining options (also including common options).

Unknown options are kept by the sub-commands in order to detect their
presence (and complain that too many arguments were given).

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobuiltin/multi-pack-index.c: define common usage with a macro
Taylor Blau [Tue, 30 Mar 2021 15:03:54 +0000 (11:03 -0400)] 
builtin/multi-pack-index.c: define common usage with a macro

Factor out the usage message into pieces corresponding to each mode.
This avoids options specific to one sub-command from being shared with
another in the usage.

A subsequent commit will use these #define macros to have usage
variables for each sub-command without duplicating their contents.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobuiltin/multi-pack-index.c: don't handle 'progress' separately
Taylor Blau [Tue, 30 Mar 2021 15:03:51 +0000 (11:03 -0400)] 
builtin/multi-pack-index.c: don't handle 'progress' separately

Now that there is a shared 'flags' member in the options structure,
there is no need to keep track of whether to force progress or not,
since ultimately the decision of whether or not to show a progress meter
is controlled by a bit in the flags member.

Manipulate that bit directly, and drop the now-unnecessary 'progress'
field while we're at it.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobuiltin/multi-pack-index.c: inline 'flags' with options
Taylor Blau [Tue, 30 Mar 2021 15:03:47 +0000 (11:03 -0400)] 
builtin/multi-pack-index.c: inline 'flags' with options

Subcommands of the 'git multi-pack-index' command (e.g., 'write',
'verify', etc.) will want to optionally change a set of shared flags
that are eventually passed to the MIDX libraries.

Right now, options and flags are handled separately. That's fine, since
the options structure is never passed around. But a future patch will
make it so that common options shared by all sub-commands are defined in
a common location. That means that "flags" would have to become a global
variable.

Group it with the options structure so that we reduce the number of
global variables we have overall.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocolumn, range-diff: downcase option description
Chinmoy Chakraborty [Mon, 29 Mar 2021 14:46:34 +0000 (14:46 +0000)] 
column, range-diff: downcase option description

It is customary not to begin the help text for each option given to
the parse-options API with a capital letter. Various (sub)commands'
option arrays don't follow the guideline provided by the parse_options
Documentation regarding the descriptions.

Downcase the first word of some option descriptions for "column"
and "range-diff".

Signed-off-by: Chinmoy Chakraborty <chinmoy12c@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocmake(install): include vcpkg dlls
Dennis Ameling [Mon, 29 Mar 2021 12:41:45 +0000 (12:41 +0000)] 
cmake(install): include vcpkg dlls

Our CMake configuration generates not only build definitions, but also
install definitions: After building Git using `msbuild git.sln`, the
built artifacts can be installed via `msbuild INSTALL.vcxproj`.

To specify _where_ the files should be installed, the
`-DCMAKE_INSTALL_PREFIX=<path>` option can be used when running CMake.

However, this process would really only install the files that were just
built. On Windows, we need more than that: We also need the `.dll` files
of the dependencies (such as libcurl). The `vcpkg` ecosystem, which we
use to obtain those dependencies, can be asked to install said `.dll`
files really easily, so let's do that.

This requires more than just the built `vcpkg` artifacts in the CI build
definition; We now clone the `vcpkg` repository so that the relevant
CMake scripts are available, in particular the ones related to defining
the toolchain.

Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocmake: add a preparatory work-around to accommodate `vcpkg`
Johannes Schindelin [Mon, 29 Mar 2021 12:41:44 +0000 (12:41 +0000)] 
cmake: add a preparatory work-around to accommodate `vcpkg`

We are about to add support for installing the `.dll` files of Git's
dependencies (such as libcurl) in the CMake configuration. The `vcpkg`
ecosystem from which we get said dependencies makes that relatively
easy: simply turn on `X_VCPKG_APPLOCAL_DEPS_INSTALL`.

However, current `vcpkg` introduces a limitation if one does that:
While it is totally cool with CMake to specify multiple targets within
one invocation of `install(TARGETS ...) (at least according to
https://cmake.org/cmake/help/latest/command/install.html#command:install),
`vcpkg`'s parser insists on a single target per `install(TARGETS ...)`
invocation.

Well, that's easily accomplished: Let's feed the targets individually to
the `install(TARGETS ...)` function in a `foreach()` look.

This also has the advantage that we do not have to manually cull off the
two entries from the `${PROGRAMS_BUILT}` array before scheduling the
remainder to be installed into `libexec/git-core`. Instead, we iterate
through the array and decide for each entry where it wants to go.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofetch-pack: use new fsck API to printing dangling submodules
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:51 +0000 (15:15 +0200)] 
fetch-pack: use new fsck API to printing dangling submodules

Refactor the check added in 5476e1efde (fetch-pack: print and use
dangling .gitmodules, 2021-02-22) to make use of us now passing the
"msg_id" to the user defined "error_func". We can now compare against
the FSCK_MSG_GITMODULES_MISSING instead of parsing the generated
message.

Let's also replace register_found_gitmodules() with directly
manipulating the "gitmodules_found" member. A recent commit moved it
into "fsck_options" so we could do this here.

I'm sticking this callback in fsck.c. Perhaps in the future we'd like
to accumulate such callbacks into another file (maybe fsck-cb.c,
similar to parse-options-cb.c?), but while we've got just the one
let's just put it into fsck.c.

A better alternative in this case would be some library some more
obvious library shared by fetch-pack.c ad builtin/index-pack.c, but
there isn't such a thing.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofetch-pack: use file-scope static struct for fsck_options
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:50 +0000 (15:15 +0200)] 
fetch-pack: use file-scope static struct for fsck_options

Change code added in 5476e1efde (fetch-pack: print and use dangling
.gitmodules, 2021-02-22) so that we use a file-scoped "static struct
fsck_options" instead of defining one in the "fsck_gitmodules_oids()"
function.

We use this pattern in all of
builtin/{fsck,index-pack,mktag,unpack-objects}.c. It's odd to see
fetch-pack be the odd one out. One might think that we're using other
fsck_options structs in fetch-pack, or doing on fsck twice there, but
we're not.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofetch-pack: don't needlessly copy fsck_options
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:49 +0000 (15:15 +0200)] 
fetch-pack: don't needlessly copy fsck_options

Change the behavior of the .gitmodules validation added in
5476e1efde (fetch-pack: print and use dangling .gitmodules,
2021-02-22) so we're using one "fsck_options".

I found that code confusing to read. One might think that not setting
up the error_func earlier means that we're relying on the "error_func"
not being set in some code in between the two hunks being modified
here.

But we're not, all we're doing in the rest of "cmd_index_pack()" is
further setup by calling fsck_set_msg_types(), and assigning to
do_fsck_object.

So there was no reason in 5476e1efde to make a shallow copy of the
fsck_options struct before setting error_func. Let's just do this
setup at the top of the function, along with the "walk" assignment.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.c: move gitmodules_{found,done} into fsck_options
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:48 +0000 (15:15 +0200)] 
fsck.c: move gitmodules_{found,done} into fsck_options

Move the gitmodules_{found,done} static variables added in
159e7b080bf (fsck: detect gitmodules files, 2018-05-02) into the
fsck_options struct. It makes sense to keep all the context in the
same place.

This requires changing the recently added register_found_gitmodules()
function added in 5476e1efde (fetch-pack: print and use dangling
.gitmodules, 2021-02-22) to take fsck_options. That function will be
removed in a subsequent commit, but as it'll require the new
gitmodules_found attribute of "fsck_options" we need this intermediate
step first.

An earlier version of this patch removed the small amount of
duplication we now have between FSCK_OPTIONS_{DEFAULT,STRICT} with a
FSCK_OPTIONS_COMMON macro. I don't think such de-duplication is worth
it for this amount of copy/pasting.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.c: add an fsck_set_msg_type() API that takes enums
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:47 +0000 (15:15 +0200)] 
fsck.c: add an fsck_set_msg_type() API that takes enums

Change code I added in acf9de4c94e (mktag: use fsck instead of custom
verify_tag(), 2021-01-05) to make use of a new API function that takes
the fsck_msg_{id,type} types, instead of arbitrary strings that
we'll (hopefully) parse into those types.

At the time that the fsck_set_msg_type() API was introduced in
0282f4dced0 (fsck: offer a function to demote fsck errors to warnings,
2015-06-22) it was only intended to be used to parse user-supplied
data.

For things that are purely internal to the C code it makes sense to
have the compiler check these arguments, and to skip the sanity
checking of the data in fsck_set_msg_type() which is redundant to
checks we get from the compiler.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.c: pass along the fsck_msg_id in the fsck_error callback
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:46 +0000 (15:15 +0200)] 
fsck.c: pass along the fsck_msg_id in the fsck_error callback

Change the fsck_error callback to also pass along the
fsck_msg_id. Before this change the only way to get the message id was
to parse it back out of the "message".

Let's pass it down explicitly for the benefit of callers that might
want to use it, as discussed in [1].

Passing the msg_type is now redundant, as you can always get it back
from the msg_id, but I'm not changing that convention. It's really
common to need the msg_type, and the report() function itself (which
calls "fsck_error") needs to call fsck_msg_type() to discover
it. Let's not needlessly re-do that work in the user callback.

1. https://lore.kernel.org/git/87blcja2ha.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.[ch]: move FOREACH_FSCK_MSG_ID & fsck_msg_id from *.c to *.h
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:45 +0000 (15:15 +0200)] 
fsck.[ch]: move FOREACH_FSCK_MSG_ID & fsck_msg_id from *.c to *.h

Move the FOREACH_FSCK_MSG_ID macro and the fsck_msg_id enum it helps
define from fsck.c to fsck.h. This is in preparation for having
non-static functions take the fsck_msg_id as an argument.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.c: give "FOREACH_MSG_ID" a more specific name
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:44 +0000 (15:15 +0200)] 
fsck.c: give "FOREACH_MSG_ID" a more specific name

Rename the FOREACH_MSG_ID macro to FOREACH_FSCK_MSG_ID in preparation
for moving it over to fsck.h. It's good convention to name macros
in *.h files in such a way as to clearly not clash with any other
names in other files.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.c: undefine temporary STR macro after use
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:43 +0000 (15:15 +0200)] 
fsck.c: undefine temporary STR macro after use

In f417eed8cde (fsck: provide a function to parse fsck message IDs,
2015-06-22) the "STR" macro was introduced, but that short macro name
was not undefined after use as was done earlier in the same series for
the MSG_ID macro in c99ba492f1c (fsck: introduce identifiers for fsck
messages, 2015-06-22).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.c: call parse_msg_type() early in fsck_set_msg_type()
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:42 +0000 (15:15 +0200)] 
fsck.c: call parse_msg_type() early in fsck_set_msg_type()

There's no reason to defer the calling of parse_msg_type() until after
we've checked if the "id < 0". This is not a hot codepath, and
parse_msg_type() itself may die on invalid input.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.h: re-order and re-assign "enum fsck_msg_type"
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:41 +0000 (15:15 +0200)] 
fsck.h: re-order and re-assign "enum fsck_msg_type"

Change the values in the "enum fsck_msg_type" from being manually
assigned to using default C enum values.

This means we end up with a FSCK_IGNORE=0, which was previously
defined as "2".

I'm confident that nothing relies on these values, we always compare
them for equality. Let's not omit "0" so it won't be assumed that
we're using these as a boolean somewhere.

This also allows us to re-structure the fields to mark which are
"private" v.s. "public". See the preceding commit for a rationale for
not simply splitting these into two enums, namely that this is used
for both the private and public fsck API.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.h: move FSCK_{FATAL,INFO,ERROR,WARN,IGNORE} into an enum
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:40 +0000 (15:15 +0200)] 
fsck.h: move FSCK_{FATAL,INFO,ERROR,WARN,IGNORE} into an enum

Move the FSCK_{FATAL,INFO,ERROR,WARN,IGNORE} defines into a new
fsck_msg_type enum.

These defines were originally introduced in:

 - ba002f3b28a (builtin-fsck: move common object checking code to
   fsck.c, 2008-02-25)
 - f50c4407305 (fsck: disallow demoting grave fsck errors to warnings,
   2015-06-22)
 - efaba7cc77f (fsck: optionally ignore specific fsck issues
   completely, 2015-06-22)
 - f27d05b1704 (fsck: allow upgrading fsck warnings to errors,
   2015-06-22)

The reason these were defined in two different places is because we
use FSCK_{IGNORE,INFO,FATAL} only in fsck.c, but FSCK_{ERROR,WARN} are
used by external callbacks.

Untangling that would take some more work, since we expose the new
"enum fsck_msg_type" to both. Similar to "enum object_type" it's not
worth structuring the API in such a way that only those who need
FSCK_{ERROR,WARN} pass around a different type.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.c: refactor fsck_msg_type() to limit scope of "int msg_type"
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:39 +0000 (15:15 +0200)] 
fsck.c: refactor fsck_msg_type() to limit scope of "int msg_type"

Refactor "if options->msg_type" and other code added in
0282f4dced0 (fsck: offer a function to demote fsck errors to warnings,
2015-06-22) to reduce the scope of the "int msg_type" variable.

This is in preparation for changing its type in a subsequent commit,
only using it in the "!options->msg_type" scope makes that change

This also brings the code in line with the fsck_set_msg_type()
function (also added in 0282f4dced0), which does a similar check for
"!options->msg_type". Another minor benefit is getting rid of the
style violation of not having braces for the body of the "if".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.c: rename remaining fsck_msg_id "id" to "msg_id"
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:38 +0000 (15:15 +0200)] 
fsck.c: rename remaining fsck_msg_id "id" to "msg_id"

Rename the remaining variables of type fsck_msg_id from "id" to
"msg_id". This change is relatively small, and is worth the churn for
a later change where we have different id's in the "report" function.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.c: remove (mostly) redundant append_msg_id() function
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:37 +0000 (15:15 +0200)] 
fsck.c: remove (mostly) redundant append_msg_id() function

Remove the append_msg_id() function in favor of calling
prepare_msg_ids(). We already have code to compute the camel-cased
msg_id strings in msg_id_info, let's use it.

When the append_msg_id() function was added in 71ab8fa840f (fsck:
report the ID of the error/warning, 2015-06-22) the prepare_msg_ids()
function didn't exist. When prepare_msg_ids() was added in
a46baac61eb (fsck: factor out msg_id_info[] lazy initialization code,
2018-05-26) this code wasn't moved over to lazy initialization.

This changes the behavior of the code to initialize all the messages
instead of just camel-casing the one we need on the fly. Since the
common case is that we're printing just one message this is mostly
redundant work.

But that's OK in this case, reporting this fsck issue to the user
isn't performance-sensitive. If we were somehow doing so in a tight
loop (in a hopelessly broken repository?) this would help, since we'd
save ourselves from re-doing this work for identical messages, we
could just grab the prepared string from msg_id_info after the first
invocation.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.c: rename variables in fsck_set_msg_type() for less confusion
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:36 +0000 (15:15 +0200)] 
fsck.c: rename variables in fsck_set_msg_type() for less confusion

Rename variables in a function added in 0282f4dced0 (fsck: offer a
function to demote fsck errors to warnings, 2015-06-22).

It was needlessly confusing that it took a "msg_type" argument, but
then later declared another "msg_type" of a different type.

Let's rename that to "severity", and rename "id" to "msg_id" and
"msg_id" to "msg_id_str" etc. This will make a follow-up change
smaller.

While I'm at it properly indent the fsck_set_msg_type() argument list.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsck.h: use "enum object_type" instead of "int"
Ævar Arnfjörð Bjarmason [Sun, 28 Mar 2021 13:15:35 +0000 (15:15 +0200)] 
fsck.h: use "enum object_type" instead of "int"

Change the fsck_walk_func to use an "enum object_type" instead of an
"int" type. The types are compatible, and ever since this was added in
355885d5315 (add generic, type aware object chain walker, 2008-02-25)
we've used entries from object_type (OBJ_BLOB etc.).

So this doesn't really change anything as far as the generated code is
concerned, it just gives the compiler more information and makes this
easier to read.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>