440c705ea6 (cat-file: add --batch-command mode, 2022-02-18) added
the new option and operating mode without listing it to the synopsis
section. Fix it.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 455923e0a1 ("ls-tree: introduce "--format" option", 2022-03-23)
introduced `--format` and the various placeholders it can take, such as
%(objectname) and %(objectsize).
At some point when that patch was being developed, those placeholders
had shorter names, e.g., %(name) and %(size), which can be seen in the
commit message of 455923e0a1. One instance of "%(size:padded)" also
managed to enter the documentation in the final version of the patch.
Correct it to "%(objectsize:padded)".
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The missing space at the end of the line makes the closing square
bracket sticking to the dash in the next line
Found during localisation v2.36.0 round 1
Signed-off-by: Fangyi Zhou <me@fangyi.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Asciidoc renders `--` as em-dash. This is not appropriate for command
names. It also breaks linkgit links to these commands.
Fix git-credential-cache--daemon and git-fsmonitor--daemon. The latter
was added 3248486920 (fsmonitor: document builtin fsmonitor, 2022-03-25)
and included several links. A check for broken links in the HTML docs
turned this up.
Manually inspecting the other Documentation/git-*--*.txt files turned up
the issue in git-credential-cache--daemon.
While here, quote `git credential-cache--daemon` in the synopsis to
match the vast majority of our other documentation.
Signed-off-by: Todd Zullinger <tmz@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git ls-tree" learns "--oid-only" option, similar to "--name-only",
and more generalized "--format" option.
source: <cover.1648026472.git.dyroneteng@gmail.com>
* tl/ls-tree-oid-only:
ls-tree: `-l` should not imply recursive listing
A couple of fix-up to a topic that is now in 'master'.
source: <pull.1193.git.1648663716891.gitgitgadget@gmail.com>
* ns/core-fsyncmethod:
configure.ac: fix HAVE_SYNC_FILE_RANGE definition
A micro fix to a topic earlier merged to 'master'
source: <patch-1.1-05949221e3f-20220319T002715Z-avarab@gmail.com>
* ab/make-optim-noop:
contrib/scalar: fix 'all' target in Makefile
Documentation/Makefile: fix "make info" regression in dad9cd7d51
Tests that affect the repo in stateful ways are easier to write if we
can run setup steps outside of the measured portion of perf iteration.
This change adds a "--setup 'setup-script'" parameter to test_perf. To
make invocations easier to understand, I also moved the prerequisites to
a new --prereq parameter.
The setup facility will be used in the upcoming perf tests for batch
mode, but it already helps in some existing tests, like t5302 and t7820.
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add basic performance tests for git commands that can add data to the
object database. We cover:
* git add
* git stash
* git update-index (via git stash)
* git unpack-objects
* git commit --all
We cover all currently available fsync methods as well.
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add test cases to exercise batch mode for:
* 'git add'
* 'git stash'
* 'git update-index'
* 'git unpack-objects'
These tests ensure that the added data winds up in the object database.
In this change we introduce a new test helper lib-unique-files.sh. The
goal of this library is to create a tree of files that have different
oids from any other files that may have been created in the current test
repo. This helps us avoid missing validation of an object being added
due to it already being in the repo.
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Several tests use awk to parse OIDs from the output of 'git ls-files
--stage' and 'git ls-tree'. Introduce helpers to centralize these uses
of awk.
Update t5317-pack-objects-filter-objects.sh to use the new ls-files
helper so that it has some usages to review. Other updates are left for
the future.
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Git for Windows has defaulted to core.fsyncObjectFiles=true since
September 2017. We turn on syncing of loose object files with batch mode
in upstream Git so that we can get broad coverage of the new code
upstream.
We don't actually do fsyncs in the most of the test suite, since
GIT_TEST_FSYNC is set to 0. However, we do exercise all of the
surrounding batch mode code since GIT_TEST_FSYNC merely makes the
maybe_fsync wrapper always appear to succeed.
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The unpack-objects functionality is used by fetch, push, and fast-import
to turn the transfered data into object database entries when there are
fewer objects than the 'unpacklimit' setting.
By enabling an odb-transaction when unpacking objects, we can take advantage
of batched fsyncs.
Here are some performance numbers to justify batch mode for
unpack-objects, collected on a WSL2 Ubuntu VM.
Fsync Mode | Time for 90 objects (ms)
-------------------------------------
Off | 170
On,fsync | 760
On,batch | 230
Note that the default unpackLimit is 100 objects, so there's a 3x
benefit in the worst case. The non-batch mode fsync scales linearly
with the number of objects, so there are significant benefits even with
smaller numbers of objects.
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The update-index functionality is used internally by 'git stash push' to
setup the internal stashed commit.
This change enables odb-transactions for update-index infrastructure to
speed up adding new objects to the object database by leveraging the
batch fsync functionality.
There is some risk with this change, since under batch fsync, the object
files will be in a tmp-objdir until update-index is complete, so callers
using the --stdin option will not see them until update-index is done.
This risk is mitigated by flushing the ODB transaction prior to
reporting any verbose output so that objects will be visible to callers
that are synchronizing with update-index by snooping its output.
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The add_files_to_cache function is invoked internally by
builtin/commit.c and builtin/checkout.c for their flags that stage
modified files before doing the larger operation. These commands
can benefit from batched fsyncing.
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Take advantage of the odb transaction infrastructure around writing the
cached tree to the object database.
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When adding many objects to a repo with `core.fsync=loose-object`,
the cost of fsync'ing each object file can become prohibitive.
One major source of the cost of fsync is the implied flush of the
hardware writeback cache within the disk drive. This commit introduces
a new `core.fsyncMethod=batch` option that batches up hardware flushes.
It hooks into the bulk-checkin odb-transaction functionality, takes
advantage of tmp-objdir, and uses the writeout-only support code.
When the new mode is enabled, we do the following for each new object:
1a. Create the object in a tmp-objdir.
2a. Issue a pagecache writeback request and wait for it to complete.
At the end of the entire transaction when unplugging bulk checkin:
1b. Issue an fsync against a dummy file to flush the log and hardware
writeback cache, which should by now have seen the tmp-objdir writes.
2b. Rename all of the tmp-objdir files to their final names.
3b. When updating the index and/or refs, we assume that Git will issue
another fsync internal to that operation. This is not the default
today, but the user now has the option of syncing the index and there
is a separate patch series to implement syncing of refs.
On a filesystem with a singular journal that is updated during name
operations (e.g. create, link, rename, etc), such as NTFS, HFS+, or XFS
we would expect the fsync to trigger a journal writeout so that this
sequence is enough to ensure that the user's data is durable by the time
the git command returns. This sequence also ensures that no object files
appear in the main object store unless they are fsync-durable.
Batch mode is only enabled if core.fsync includes loose-objects. If
the legacy core.fsyncObjectFiles setting is enabled, but core.fsync does
not include loose-objects, we will use file-by-file fsyncing.
In step (1a) of the sequence, the tmp-objdir is created lazily to avoid
work if no loose objects are ever added to the ODB. We use a tmp-objdir
to maintain the invariant that no loose-objects are visible in the main
ODB unless they are properly fsync-durable. This is important since
future ODB operations that try to create an object with specific
contents will silently drop the new data if an object with the target
hash exists without checking that the loose-object contents match the
hash. Only a full git-fsck would restore the ODB to a functional state
where dataloss doesn't occur.
In step (1b) of the sequence, we issue a fsync against a dummy file
created specifically for the purpose. This method has a little higher
cost than using one of the input object files, but makes adding new
callers of this mechanism easier, since we don't need to figure out
which object file is "last" or risk sharing violations by caching the fd
of the last object file.
_Performance numbers_:
Linux - Hyper-V VM running Kernel 5.11 (Ubuntu 20.04) on a fast SSD.
Mac - macOS 11.5.1 running on a Mac mini on a 1TB Apple SSD.
Windows - Same host as Linux, a preview version of Windows 11.
Adding 500 files to the repo with 'git add' Times reported in seconds.
object file syncing | Linux | Mac | Windows
--------------------|-------|-------|--------
disabled | 0.06 | 0.35 | 0.61
fsync | 1.88 | 11.18 | 2.47
batch | 0.15 | 0.41 | 1.53
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make it clearer in the naming and documentation of the plug_bulk_checkin
and unplug_bulk_checkin APIs that they can be thought of as
a "transaction" to optimize operations on the object database. These
transactions may be nested so that subsystems like the cache-tree
writing code can optimize their operations without caring whether the
top-level code has a transaction active.
Add a flush_odb_transaction API that will be used in update-index to
make objects visible even if a transaction is active. The flush call may
also be useful in future cases if we hold a transaction active around
calling hooks.
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit prepares for adding batch-fsync to the bulk-checkin
infrastructure.
The bulk-checkin infrastructure is currently used to batch up addition
of large blobs to a packfile. When a blob is larger than
big_file_threshold, we unconditionally add it to a pack. If bulk
checkins are 'plugged', we allow multiple large blobs to be added to a
single pack until we reach the packfile size limit; otherwise, we simply
make a new packfile for each large blob. The 'unplug' call tells us when
the series of blob additions is done so that we can finish the packfiles
and make their objects available to subsequent operations.
Stated another way, bulk-checkin allows callers to define a transaction
that adds multiple objects to the object database, where the object
database can optimize its internal operations within the transaction
boundary.
Batched fsync will fit into bulk-checkin by taking advantage of the
plug/unplug functionality to determine the appropriate time to fsync
and make newly-added objects available in the primary object database.
* Rename 'state' variable to 'bulk_checkin_packfile', since we will
later be adding 'bulk_fsync_objdir'. This also makes the variable
easier to find in the debugger, since the name is more unique.
* Rename finish_bulk_checkin to flush_bulk_checkin_packfile and call it
unconditionally from unplug_bulk_checkin. Internally it will
conditionally do a flush if there's any work to do.
* Move the 'plugged' data member of 'bulk_checkin_state' into a separate
static variable. Doing this avoids resetting the variable in
finish_bulk_checkin when zeroing the 'bulk_checkin_state'. As-is, we
seem to unintentionally disable the plugging functionality the first
time a new packfile must be created due to packfile size limits. While
disabling the plugging state only results in suboptimal behavior for
the current code, it would be fatal for the bulk-fsync functionality
later in this patch series.
The net effect of these changes is to make a clear separation between
the portion of the bulk-checkin infrastructure that is related to the
packfile (nearly all of it at present) and the part that is related to
other future optimizations of the ODB.
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Perforce has a file type "utf8" which represents a text file with
explicit BOM. utf8-encoded files *without* BOM are stored as
regular file type "text". The "utf8" file type behaves like text
in all but one important way: it is stored, internally, without
the leading 3 BOM bytes.
git-p4 has historically imported utf8-with-BOM files (files stored,
in Perforce, as type "utf8") the same way as regular text files -
losing the BOM in the process.
Under most circumstances this issue has little functional impact,
as most systems consider the BOM to be optional and redundant, but
this *is* a correctness failure, and can have lead to practical
issues for example when BOMs are explicitly included in test files,
for example in a file encoding test suite.
Fix the handling of utf8-with-BOM files when importing changes from
p4 to git, and introduce a test that checks it is working correctly.
Signed-off-by: Tao Klerks <tao@klerks.biz>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
With the --branch argument of the "sync" subcommand, git-p4 enables
you to import a perforce branch/path to an arbitrary git ref, using
a full ref path, or to refs/remotes/p4/* or refs/heads/p4/*,
depending on --import-local, using a short ref name.
However, when you later want to explicitly sync such a given ref to
pick up subsequent p4 changes, it only works if the ref was placed
in the p4 path *and* has only one path component (no "/").
This limitation results from a bad assumption in the
existing-branch sync logic, and also means you cannot individually
sync branches detected by --detect-branches, as these also get a
"/" in their names.
Fix "git p4 sync --branch", when called with an existing ref, so
that it works correctly regardless of whether the ref is in the p4
path or not, and (in the case of refs in the p4 path) regardless of
whether it has a "/" in its short name or not.
Also add tests to validate that these branch-specific syncs work
as expected.
Signed-off-by: Tao Klerks <tao@klerks.biz>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add extra ':' to second 'all' target definition to allow 'scalar' to build.
Without this fix, the 'all:' and 'all::' targets together cause a build
failure when 'scalar' build is enabled with 'INCLUDE_SCALAR':
Makefile:14: *** target file `all' has both : and :: entries. Stop.
Signed-off-by: Victoria Dye <vdye@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a regression in my dad9cd7d51 (Makefile: move ".SUFFIXES" rule to
shared.mak, 2022-03-03). As explained in the GNU make documentation
for the $* variable, available at:
info make --index-search='$*'
This rule relied on ".texi" being in the default list of suffixes, as
seen at:
make -f/dev/null -p | grep -v -e ^# -e ^$|grep -F .SUFFIXES
The documentation explains what was going on here:
In an explicit rule, there is no stem; so '$*' cannot be determined
in that way. Instead, if the target name ends with a recognized
suffix (*note Old-Fashioned Suffix Rules: Suffix Rules.), '$*' is
set to the target name minus the suffix. For example, if the
target name is 'foo.c', then '$*' is set to 'foo', since '.c' is a
suffix. GNU 'make' does this bizarre thing only for compatibility
with other implementations of 'make'. You should generally avoid
using '$*' except in implicit rules or static pattern rules.
If the target name in an explicit rule does not end with a
recognized suffix, '$*' is set to the empty string for that rule.
I.e. this rule added back in 5cefc33bff (Documentation: add
gitman.info target, 2007-12-10) was resolving gitman.texi from
gitman.info. We can instead just use the more obvious $< variable
referring to the prerequisite.
This was the only use of $* in our Makefiles in an explicit rule, the
three remaining ones are all implicit rules, and therefore didn't
depend on the ".SUFFIXES" list.
Reported-by: Adam Dinwoodie <adam@dinwoodie.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Tested-by: Adam Dinwoodie <adam@dinwoodie.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove include "lockfile.h" from builtin/apply.c, which is orphaned
since 6d058c8826 (apply: move lockfile into `apply_state`, 2017-10-05)
Signed-off-by: Garrit Franke <garrit@slashdev.space>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove include "strvec.h" from serve.c, which is orphaned since
f0a35c9ce5 (serve: drop "keys" strvec, 2021-09-15)
Signed-off-by: Garrit Franke <garrit@slashdev.space>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If sync_file_range is not available when building the configure script,
there is a cosmetic bug when running that script reporting
"HAVE_SYNC_FILE_RANGE: command not found". Remove that error message by
defining HAVE_SYNC_FILE_RANGE to an empty string, rather than generating
a script where that appears as a bare command.
Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
05cd988dce (wrapper: add a helper to generate numbers from a CSPRNG,
2022-01-17), configure openssl as the source for entropy in NON-STOP
but doesn't add the needed header or link options.
Since the only system that is configured to use openssl as a source
of entropy is NON-STOP, add the header unconditionally, and -lcrypto
to the list of external libraries.
An additional change is required to make sure a NO_OPENSSL=1 build
will be able to work as well (tested on Linux with a modified value
of CSPRNG_METHOD = openssl), and the more complex logic that allows
for compatibility with APPLE_COMMON_CRYPTO or allowing for simpler
ways to link (without libssl) has been punted for now.
Reported-by: Randall Becker <rsbecker@nexbridge.com>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Using a named enum allows casting an integer to the enum type in both
GDB and LLDB:
$ gdb -q -ex 'b wt-status.c:44' -ex r --args ./git status
(gdb) p (enum color_wt_status) slot
$1 = WT_STATUS_ONBRANCH
$ lldb -o 'b wt-status.c:44' -o r -- ./git status
(lldb) p (color_wt_status) slot
(color_wt_status) $0 = WT_STATUS_ONBRANCH
In LLDB, it's also required to cast in the reversed direction, i.e.
cast an enum constant into its corresponding integer:
(lldb) p (int) color_wt_status::WT_STATUS_ONBRANCH
(int) $1 = 8
Name the enum listing the different RECURSE_SUBMODULES_* modes, to make
debugging easier. For example, when stepping through a part of the code
where an int is compared with a constant in this enum, it allows casting
the int to the enum type or vice-versa, after quickly checking where the
enum constant is declared and learning the enum name.
As to not make this patch a debug-only change, convert the
'fetch_recurse' member of 'struct submodule' to use the newly named
enum.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Reviewed-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 9c4d58ff2c (ls-tree: split up "fast path" callbacks, 2022-03-23), a
refactoring of the various read_tree_at() callbacks caused us to
unconditionally recurse into directories if `-l` (long format) was
passed on the command line, regardless of whether or not we also pass
the `-r` (recursive) flag.
Fix this by making show_tree_long() return the value of `recurse`,
rather than always returning 1. This value is interpreted by
read_tree_at() to be a signal on whether or not to recurse.
Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the test 'cherry-pick after renaming branch', stop checking for
the presence of a file (opos) because we are going to "grep" in it in
the same test and the lack of it will be noticed as a failure anyway.
In the test 'revert after renaming branch', instead of allowing any
random contents as long as a known phrase is not there in it, we can
expect the exact outcome---after the successful revert of "added", the
contents of file "spoo" should become identical to what was in file
"oops" in the "initial" commit. This test also contains 'test -f' that
verifies presence of a file, but we have a helper function to do the same
thing. Replace it with appropriate helper function 'test_path_is_file'
for better readability and better error messages.
In both tests, we will not notice when "git rev-parse" starts segfaulting
without emitting any output. The 'test' command will end up being just
"test =", which yields success. Use the 'test_cmp_rev' helper to make
sure we will notice such a breakage.
Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
dc6b1d92ca (wt-status: use settings from git_diff_ui_config, 2018-05-04)
disabled diffopt.break_opt for diffstats shown by git status and in
commit templates. For git status there isn't even a way to enable it.
Make the commit summary (shown after the commit) consistent by disabling
it there as well.
Reported-by: Laurent Lyaudet <laurent.lyaudet@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git worktree list --porcelain" did not c-quote pathnames and lock
reasons with unsafe bytes correctly, which is worked around by
introducing NUL terminated output format with "-z".
* pw/worktree-list-with-z:
worktree: add -z option for list subcommand
Built-in fsmonitor (part 2).
* jh/builtin-fsmonitor-part2: (30 commits)
t7527: test status with untracked-cache and fsmonitor--daemon
fsmonitor: force update index after large responses
fsmonitor--daemon: use a cookie file to sync with file system
fsmonitor--daemon: periodically truncate list of modified files
t/perf/p7519: add fsmonitor--daemon test cases
t/perf/p7519: speed up test on Windows
t/perf/p7519: fix coding style
t/helper/test-chmtime: skip directories on Windows
t/perf: avoid copying builtin fsmonitor files into test repo
t7527: create test for fsmonitor--daemon
t/helper/fsmonitor-client: create IPC client to talk to FSMonitor Daemon
help: include fsmonitor--daemon feature flag in version info
fsmonitor--daemon: implement handle_client callback
compat/fsmonitor/fsm-listen-darwin: implement FSEvent listener on MacOS
compat/fsmonitor/fsm-listen-darwin: add MacOS header files for FSEvent
compat/fsmonitor/fsm-listen-win32: implement FSMonitor backend on Windows
fsmonitor--daemon: create token-based changed path cache
fsmonitor--daemon: define token-ids
fsmonitor--daemon: add pathname classification
fsmonitor--daemon: implement 'start' command
...
Give hint when branch tracking cannot be established because fetch
refspecs from multiple remote repositories overlap.
* tk/ambiguous-fetch-refspec:
tracking branches: add advice to ambiguous refspec error
"git fetch --refetch" learned to fetch everything without telling
the other side what we already have, which is useful when you
cannot trust what you have in the local object store.
* rc/fetch-refetch:
docs: mention --refetch fetch option
fetch: after refetch, encourage auto gc repacking
t5615-partial-clone: add test for fetch --refetch
fetch: add --refetch option
builtin/fetch-pack: add --refetch option
fetch-pack: add refetch
fetch-negotiator: add specific noop initializer
"git am" can read from the standard input when no mailbox is given
on the command line, but the end-user gets no indication when it
happens, making Git appear stuck.
* jc/mailsplit-warn-on-tty:
am/apply: warn if we end up reading patches from terminal
