summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* t7527: test status with untracked-cache and fsmonitor--daemonJeff Hostetler2022-03-261-0/+115
| | | | | | | | | Create 2x2 test matrix with the untracked-cache and fsmonitor--daemon features and a series of edits and verify that status output is identical. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor: force update index after large responsesJeff Hostetler2022-03-261-1/+54
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Measure the time taken to apply the FSMonitor query result to the index and the untracked-cache. Set the `FSMONITOR_CHANGED` bit on `istate->cache_changed` when FSMonitor returns a very large repsonse to ensure that the index is written to disk. Normally, when the FSMonitor response includes a tracked file, the index is always updated. Similarly, the index might be updated when the response alters the untracked-cache (when enabled). However, in cases where neither of those cause the index to be considered changed, the FSMonitor response is wasted. Subsequent Git commands will make requests with the same token and receive the same response. If that response is very large, performance may suffer. It would be more efficient to force update the index now (and the token in the index extension) in order to reduce the size of the response received by future commands. This was observed on Windows after a large checkout. On Windows, the kernel emits events for the files that are changed as they are changed. However, it might delay events for the containing directories until the system is more idle (or someone scans the directory (so it seems)). The first status following a checkout would get the list of files. The subsequent status commands would get the list of directories as the events trickled out. But they would never catch up because the token was not advanced because the index wasn't updated. This list of directories caused `wt_status_collect_untracked()` to unnecessarily spend time actually scanning them during each command. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor--daemon: use a cookie file to sync with file systemJeff Hostetler2022-03-262-1/+241
| | | | | | | | | | | | | | | | | | | | | | | | Teach fsmonitor--daemon client threads to create a cookie file inside the .git directory and then wait until FS events for the cookie are observed by the FS listener thread. This helps address the racy nature of file system events by blocking the client response until the kernel has drained any event backlog. This is especially important on MacOS where kernel events are only issued with a limited frequency. See the `latency` argument of `FSeventStreamCreate()`. The kernel only signals every `latency` seconds, but does not guarantee that the kernel queue is completely drained, so we may have to wait more than one interval. If we increase the latency, the system is more likely to drop events. We avoid these issues by having each client thread create a unique cookie file and then wait until it is seen in the event stream. Co-authored-by: Kevin Willford <Kevin.Willford@microsoft.com> Co-authored-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor--daemon: periodically truncate list of modified filesJeff Hostetler2022-03-261-0/+88
| | | | | | | | | | | | | | | | | | | | | | | Teach fsmonitor--daemon to periodically truncate the list of modified files to save some memory. Clients will ask for the set of changes relative to a token that they found in the FSMN index extension in the index. (This token is like a point in time, but different). Clients will then update the index to contain the response token (so that subsequent commands will be relative to this new token). Therefore, the daemon can gradually truncate the in-memory list of changed paths as they become obsolete (older than the previous token). Since we may have multiple clients making concurrent requests with a skew of tokens and clients may be racing to the talk to the daemon, we lazily truncate the list. We introduce a 5 minute delay and truncate batches 5 minutes after they are considered obsolete. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t/perf/p7519: add fsmonitor--daemon test casesJeff Hostetler2022-03-261-4/+34
| | | | | | | | Repeat all of the fsmonitor perf tests using `git fsmonitor--daemon` and the "Simple IPC" interface. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t/perf/p7519: speed up test on WindowsJeff Hostetler2022-03-261-8/+16
| | | | | | | | | Change p7519 to use `test_seq` and `xargs` rather than a `for` loop to touch thousands of files. This takes minutes off of test runs on Windows because of process creation overhead. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t/perf/p7519: fix coding styleJeff Hostetler2022-03-261-4/+4
| | | | | Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t/helper/test-chmtime: skip directories on WindowsJeff Hostetler2022-03-261-0/+15
| | | | | | | | | | | | | | | | Teach `test-tool.exe chmtime` to ignore errors when setting the mtime on a directory on Windows. NEEDSWORK: The Windows version of `utime()` (aka `mingw_utime()`) does not properly handle directories because it uses `_wopen()`. It should be converted to using `CreateFileW()` and backup semantics at a minimum. Since I'm already in the middle of a large patch series, I did not want to destabilize other callers of `utime()` right now. The problem has only been observed in the t/perf/p7519 test when the test repo contains an empty directory on disk. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t/perf: avoid copying builtin fsmonitor files into test repoJeff Hostetler2022-03-261-1/+1
| | | | | | | | | | | | | | | | | | | | | | Do not copy any of the various fsmonitor--daemon files from the .git directory of the (GIT_PREF_REPO or GIT_PERF_LARGE_REPO) source repo into the test's trash directory. When perf tests start, they copy the contents of the source repo into the test's trash directory. If fsmonitor is running in the source repo, there may be control files, such as the IPC socket and/or fsmonitor cookie files. These should not be copied into the test repo. Unix domain sockets cannot be copied in the manner used by the test setup, so if present, the test setup fails. Cookie files are harmless, but we should avoid them. The builtin fsmonitor keeps all such control files/sockets in .git/fsmonitor--daemon*, so it is simple to exclude them. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t7527: create test for fsmonitor--daemonJeff Hostetler2022-03-261-0/+494
| | | | | Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t/helper/fsmonitor-client: create IPC client to talk to FSMonitor DaemonJeff Hostetler2022-03-264-0/+119
| | | | | | | Create an IPC client to send query and flush commands to the daemon. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* help: include fsmonitor--daemon feature flag in version infoJeff Hostetler2022-03-262-0/+11
| | | | | | | | | | | | | | | | | | | | | | | Add the "feature: fsmonitor--daemon" message to the output of `git version --build-options`. The builtin FSMonitor is only available on certain platforms and even then only when certain Makefile flags are enabled, so print a message in the verbose version output when it is available. This can be used by test scripts for prereq testing. Granted, tests could just try `git fsmonitor--daemon status` and look for a 128 exit code or grep for a "not supported" message on stderr, but these methods are rather obscure. The main advantage is that the feature message will automatically appear in bug reports and other support requests. This concept was also used during the development of Scalar for similar reasons. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor--daemon: implement handle_client callbackJeff Hostetler2022-03-261-2/+309
| | | | | | | | | Teach fsmonitor--daemon to respond to IPC requests from client Git processes and respond with a list of modified pathnames relative to the provided token. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* compat/fsmonitor/fsm-listen-darwin: implement FSEvent listener on MacOSJeff Hostetler2022-03-261-0/+383
| | | | | | | | | | Implement file system event listener on MacOS using FSEvent, CoreFoundation, and CoreServices. Co-authored-by: Kevin Willford <Kevin.Willford@microsoft.com> Co-authored-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* compat/fsmonitor/fsm-listen-darwin: add MacOS header files for FSEventJeff Hostetler2022-03-262-0/+116
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Include MacOS system declarations to allow us to use FSEvent and CoreFoundation APIs. We need different versions of the declarations for GCC vs. clang because of compiler and header file conflicts. While it is quite possible to #include Apple's CoreServices.h when compiling C source code with clang, trying to build it with GCC currently fails with this error: In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/System/... ...Library/Frameworks/Security.framework/Headers/AuthSession.h:32, from /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/System/... ...Library/Frameworks/Security.framework/Headers/Security.h:42, from /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/System/... ...Library/Frameworks/CoreServices.framework/Frameworks/... ...OSServices.framework/Headers/CSIdentity.h:43, from /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/System/... ...Library/Frameworks/CoreServices.framework/Frameworks/... ...OSServices.framework/Headers/OSServices.h:29, from /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/System/... ...Library/Frameworks/CoreServices.framework/Frameworks/... ...LaunchServices.framework/Headers/IconsCore.h:23, from /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/System/... ...Library/Frameworks/CoreServices.framework/Frameworks/... ...LaunchServices.framework/Headers/LaunchServices.h:23, from /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/System/... ...Library/Frameworks/CoreServices.framework/Headers/CoreServices.h:45, /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/System/... ...Library/Frameworks/Security.framework/Headers/Authorization.h:193:7: error: variably modified 'bytes' at file scope 193 | char bytes[kAuthorizationExternalFormLength]; | ^~~~~ The underlying reason is that GCC (rightfully) objects that an `enum` value such as `kAuthorizationExternalFormLength` is not a constant (because it is not, the preprocessor has no knowledge of it, only the actual C compiler does) and can therefore not be used to define the size of a C array. This is a known problem and tracked in GCC's bug tracker: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93082 In the meantime, let's not block things and go the slightly ugly route of declaring/defining the FSEvents constants, data structures and functions that we need, so that we can avoid above-mentioned issue. Let's do this _only_ for GCC, though, so that the CI/PR builds (which build both with clang and with GCC) can guarantee that we _are_ using the correct data types. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* compat/fsmonitor/fsm-listen-win32: implement FSMonitor backend on WindowsJeff Hostetler2022-03-261-0/+565
| | | | | | | | | | Teach the win32 backend to register a watch on the working tree root directory (recursively). Also watch the <gitdir> if it is not inside the working tree. And to collect path change notifications into batches and publish. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor--daemon: create token-based changed path cacheJeff Hostetler2022-03-262-2/+268
| | | | | | | | | | | | | | | | | | | | Teach fsmonitor--daemon to build a list of changed paths and associate them with a token-id. This will be used by the platform-specific backends to accumulate changed paths in response to filesystem events. The platform-specific file system listener thread receives file system events containing one or more changed pathnames (with whatever bucketing or grouping that is convenient for the file system). These paths are accumulated (without locking) by the file system layer into a `fsmonitor_batch`. When the file system layer has drained the kernel event queue, it will "publish" them to our token queue and make them visible to concurrent client worker threads. The token layer is free to combine and/or de-dup paths within these batches for efficient presentation to clients. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor--daemon: define token-idsJeff Hostetler2022-03-261-1/+115
| | | | | | | | Teach fsmonitor--daemon to create token-ids and define the overall token naming scheme. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor--daemon: add pathname classificationJeff Hostetler2022-03-262-0/+168
| | | | | | | | | | | | | | | | | | | Teach fsmonitor--daemon to classify relative and absolute pathnames and decide how they should be handled. This will be used by the platform-specific backend to respond to each filesystem event. When we register for filesystem notifications on a directory, we get events for everything (recursively) in the directory. We want to report to clients changes to tracked and untracked paths within the working directory proper. We do not want to report changes within the .git directory, for example. This classification will be used in a later commit by the different backends to classify paths as events are received. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor--daemon: implement 'start' commandJeff Hostetler2022-03-261-2/+107
| | | | | | | | | | | | | | | | | | | | | Implement 'git fsmonitor--daemon start' command. This command starts an instance of 'git fsmonitor--daemon run' in the background using the new 'start_bg_command()' function. We avoid the fork-and-call technique on Unix systems in favor of a fork-and-exec technique. This gives us more uniform Trace2 child-* events. It also makes our usage more consistent with Windows usage. On Windows, teach 'git fsmonitor--daemon run' to optionally call 'FreeConsole()' to release handles to the inherited Win32 console (despite being passed invalid handles for stdin/out/err). Without this, command prompts and powershell terminal windows could hang in "exit" until the last background child process exited or released their Win32 console handle. (This was not seen with git-bash shells because they don't have a Win32 console attached to them.) Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor--daemon: implement 'run' commandJeff Hostetler2022-03-262-1/+261
| | | | | | | | | | | | | | | Implement `run` command to try to begin listening for file system events. This version defines the thread structure with a single fsmonitor_fs_listen thread to watch for file system events and a simple IPC thread pool to watch for connection from Git clients over a well-known named pipe or Unix domain socket. This commit does not actually do anything yet because the platform backends are still just stubs. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* compat/fsmonitor/fsm-listen-darwin: stub in backend for DarwinJeff Hostetler2022-03-263-0/+33
| | | | | | | | Stub in empty implementation of fsmonitor--daemon backend for Darwin (aka MacOS). Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* compat/fsmonitor/fsm-listen-win32: stub in backend for WindowsJeff Hostetler2022-03-266-0/+101
| | | | | | | Stub in empty filesystem listener backend for fsmonitor--daemon on Windows. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor--daemon: implement 'stop' and 'status' commandsJeff Hostetler2022-03-261-0/+51
| | | | | | | | | | | | Implement `stop` and `status` client commands to control and query the status of a `fsmonitor--daemon` server process (and implicitly start a server process if necessary). Later commits will implement the actual server and monitor the file system. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor--daemon: add a built-in fsmonitor daemonJeff Hostetler2022-03-265-0/+50
| | | | | | | | | | | | | | | | Create a built-in file system monitoring daemon that can be used by the existing `fsmonitor` feature (protocol API and index extension) to improve the performance of various Git commands, such as `status`. The `fsmonitor--daemon` feature builds upon the `Simple IPC` API and provides an alternative to hook access to existing fsmonitors such as `watchman`. This commit merely adds the new command without any functionality. Co-authored-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor: document builtin fsmonitorJeff Hostetler2022-03-263-17/+126
| | | | | | | | | | | | | Document how `core.fsmonitor` can be set to a boolean to enable or disable the builtin FSMonitor. Update references to `core.fsmonitor` and `core.fsmonitorHookVersion` and pointers to `Watchman` to refer to it. Create `git-fsmonitor--daemon` manual page and describe its features. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor: use IPC to query the builtin FSMonitor daemonJeff Hostetler2022-03-261-2/+36
| | | | | | | | | Use simple IPC to directly communicate with the new builtin file system monitor daemon when `core.fsmonitor` is set to true. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor: config settings are repository-specificJeff Hostetler2022-03-2612-49/+196
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move fsmonitor config settings to a new and opaque `struct fsmonitor_settings` structure. Add a lazily-loaded pointer to this into `struct repo_settings` Create an `enum fsmonitor_mode` type in `struct fsmonitor_settings` to represent the state of fsmonitor. This lets us represent which, if any, fsmonitor provider (hook or IPC) is enabled. Create `fsm_settings__get_*()` getters to lazily look up fsmonitor- related config settings. Get rid of the `core_fsmonitor` global variable. Move the code to lookup the existing `core.fsmonitor` config value into the fsmonitor settings. Create a hook pathname variable in `struct fsmonitor-settings` and only set it when in hook mode. Extend the definition of `core.fsmonitor` to be either a boolean or a hook pathname. When true, the builtin FSMonitor is used. When false or unset, no FSMonitor (neither builtin nor hook) is used. The existing `core_fsmonitor` global variable was used to store the pathname to the fsmonitor hook *and* it was used as a boolean to see if fsmonitor was enabled. This dual usage and global visibility leads to confusion when we add the IPC-based provider. So lets hide the details in fsmonitor-settings.c and let it decide which provider to use in the case of multiple settings. This avoids cluttering up repo-settings.c with these private details. A future commit in builtin-fsmonitor series will add the ability to disqualify worktrees for various reasons, such as being mounted from a remote volume, where fsmonitor should not be started. Having the config settings hidden in fsmonitor-settings.c allows such worktree restrictions to override the config values used. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor-ipc: create client routines for git-fsmonitor--daemonJeff Hostetler2022-03-263-0/+220
| | | | | | | | | | | Create fsmonitor_ipc__*() client routines to spawn the built-in file system monitor daemon and send it an IPC request using the `Simple IPC` API. Stub in empty fsmonitor_ipc__*() functions for unsupported platforms. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fsmonitor: enhance existing comments, clarify trivial response handlingJeff Hostetler2022-03-261-23/+41
| | | | | Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* The eighth batchJunio C Hamano2022-02-261-0/+29
| | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'tb/coc-plc-update'Junio C Hamano2022-02-261-1/+1
|\ | | | | | | | | | | | | Document Taylor as a new member of Git PLC at SFC. Welcome. * tb/coc-plc-update: CODE_OF_CONDUCT.md: update PLC members list
| * CODE_OF_CONDUCT.md: update PLC members listTaylor Blau2022-02-181-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As part of our code of conduct, we maintain a list of active members on the Project Leadership Committee, which serves a couple of purposes. The details are in 3f9ef874a7 (CODE_OF_CONDUCT: mention individual project-leader emails, 2019-09-26), but the gist is as follows: - It makes it clear that people with a CoC complaint may contact members individually as opposed to the general PLC list (in case the subject of their complaint has to do with one of the committee members). - It also serves as the de-facto list of people on the PLC, which isn't committed anywhere else in the tree. As of [1], Peff is no longer a member of Git's Project Leadership Committee. Let's update the list of active members accordingly [2]. This also gives us a convenient opportunity to thank Peff for his many years of service on the PLC, during which he helped the Git community in more ways than we can easily list here. [1]: https://lore.kernel.org/git/YboaAe4LWySOoAe7@coredump.intra.peff.net/ [2]: https://lore.kernel.org/git/CAP8UFD2XxP9r3PJ4GQjxUbV=E1ASDq1NDgB-h+S=v-bZQ7DYwQ@mail.gmail.com/ Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'en/ort-inner-merge-conflict-report'Junio C Hamano2022-02-261-0/+5
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | Messages "ort" merge backend prepares while dealing with conflicted paths were unnecessarily confusing since it did not differentiate inner merges and outer merges. * en/ort-inner-merge-conflict-report: merge-ort: make informational messages from recursive merges clearer
| * | merge-ort: make informational messages from recursive merges clearerElijah Newren2022-02-181-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is another simple change with a long explanation... merge-recursive and merge-ort are both based on the same recursive idea: if there is more than one merge base, merge the merge bases (which may require first merging the merge bases of the merges bases, etc.). The depth of the inner merge is recorded via a variable called "call_depth", which we'll bring up again later. Naturally, the inner merges themselves can have conflicts and various messages generated about those files. merge-recursive immediately prints to stdout as it goes, at the risk of printing multiple conflict notices for the same path separated far apart from each other with many intervenining conflict notices for other paths between them. And this is true even if there are no inner merges involved. An example of this was given in [1] and apparently caused some confusion: CONFLICT (rename/add): Rename A->B in HEAD. B added in otherbranch ...dozens of conflicts for OTHER paths... CONFLICT (content): Merge conflicts in B In contrast, merge-ort collects messages and stores them by path so that it can print them grouped by path. Thus, the same case handled by merge-ort would have output of the form: CONFLICT (rename/add): Rename A->B in HEAD. B added in otherbranch CONFLICT (content): Merge conflicts in B ...dozens of conflicts for OTHER paths... This is generally helpful, but does make a separate bug more problematic. In particular, while merge-recursive might report the following for a recursive merge: Auto-merging dir.c Auto-merging midx.c CONFLICT (content): Merge conflict in midx.c Auto-merging diff.c Auto-merging dir.c CONFLICT (content): Merge conflict in dir.c merge-ort would instead report: Auto-merging diff.c Auto-merging dir.c Auto-merging dir.c CONFLICT (content): Merge conflict in dir.c Auto-merging midx.c CONFLICT (content): Merge conflict in midx.c The fact that messages for the same file are together is probably helpful in general, but with the indentation missing for the inner merge it unfortunately serves to confuse. This probably would lead users to wonder: * Why is Git reporting that "dir.c" is being merged twice? * If midx.c has conflicts, why do I not see any when I open up the file and why are no conflicts shown in the index? Fix this output confusion by changing the output to clearly differentiate the messages for outer merges from the ones for inner merges, changing the above output from merge-ort to: Auto-merging diff.c From inner merge: Auto-merging dir.c Auto-merging dir.c CONFLICT (content): Merge conflict in dir.c From inner merge: Auto-merging midx.c From inner merge: CONFLICT (content): Merge conflict in midx.c (Note: the number of spaces after the 'From inner merge:' is 2*call_depth). One other thing to note here, that I didn't notice until typing up this commit message, is that merge-recursive does not print any messages from the inner merges by default; the extra verbosity has to be requested. merge-ort currently has no verbosity controls and always prints these. We may also want to change that, but for now, just make the output clearer with these extra markings and indentation. [1] https://lore.kernel.org/git/CAGyf7-He4in8JWUh9dpAwvoPkQz9hr8nCBpxOxhZEd8+jtqTpg@mail.gmail.com/ Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'rs/pcre-invalid-utf8-fix-fix'Junio C Hamano2022-02-261-1/+1
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Workaround we have for versions of PCRE2 before their version 10.36 were in effect only for their versions newer than 10.36 by mistake, which has been corrected. * rs/pcre-invalid-utf8-fix-fix: grep: fix triggering PCRE2_NO_START_OPTIMIZE workaround
| * | | grep: fix triggering PCRE2_NO_START_OPTIMIZE workaroundRené Scharfe2022-02-171-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | PCRE2 bug 2642 was fixed in version 10.36. Our 95ca1f987e (grep/pcre2: better support invalid UTF-8 haystacks, 2021-01-24) worked around it on older versions by setting the flag PCRE2_NO_START_OPTIMIZE. 797c359978 (grep/pcre2: use compile-time PCREv2 version test, 2021-02-18) switched it around to set the flag on 10.36 and higher instead, while it claimed to use "the same test done at compile-time". Switch the condition back to apply the workaround on PCRE2 versions _before_ 10.36. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'ds/core-untracked-cache-config'Junio C Hamano2022-02-261-1/+3
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Setting core.untrackedCache to true failed to add the untracked cache extension to the index. * ds/core-untracked-cache-config: dir: force untracked cache with core.untrackedCache
| * | | | dir: force untracked cache with core.untrackedCacheDerrick Stolee2022-02-171-1/+3
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The GIT_FORCE_UNTRACKED_CACHE environment variable writes the untracked cache more frequently than the core.untrackedCache config variable. This is due to how read_directory() handles the creation of an untracked cache. Before this change, Git would not create the untracked cache extension for an index that did not already have one. Users would need to run a command such as 'git update-index --untracked-cache' before the index would actually contain an untracked cache. In particular, users noticed that the untracked cache would not appear even with core.untrackedCache=true. Some users reported setting GIT_FORCE_UNTRACKED_CACHE=1 in their engineering system environment to ensure the untracked cache would be created. The decision to not write the untracked cache without an environment variable tracks back to fc9ecbeb9 (dir.c: don't flag the index as dirty for changes to the untracked cache, 2018-02-05). The motivation of that change is that writing the index is expensive, and if the untracked cache is the only thing that needs to be written, then it is more expensive than the benefit of the cache. However, this also means that the untracked cache never gets populated, so the user who enabled it via config does not actually get the extension until running 'git update-index --untracked-cache' manually or using the environment variable. We have had a version of this change in the microsoft/git fork for a few major releases now. It has been working well to get users into a good state. Yes, that first index write is slow, but the remaining index writes are much faster than they would be without this change. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'ab/diff-free-more'Junio C Hamano2022-02-265-9/+5
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Leakfixes. * ab/diff-free-more: diff.[ch]: have diff_free() free options->parseopts diff.[ch]: have diff_free() call clear_pathspec(opts.pathspec)
| * | | | diff.[ch]: have diff_free() free options->parseoptsÆvar Arnfjörð Bjarmason2022-02-161-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The "struct option" added in 4a288478394 (diff.c: prepare to use parse_options() for parsing, 2019-01-27) would be free'd in the case of diff_setup_done() being called. But not all codepaths that allocate it reach that, e.g. "t6427-diff3-conflict-markers.sh" will now free memory that it didn't free before. By using FREE_AND_NULL() here (which diff_setup_done() also does) we ensure that we free the memory, and that we won't have double-free's. Before this running: ./t6427-diff3-conflict-markers.sh -vixd --run=7 Would report: SUMMARY: LeakSanitizer: 7823 byte(s) leaked in 6 allocation(s). But now we'll report: SUMMARY: LeakSanitizer: 703 byte(s) leaked in 5 allocation(s). I.e. the largest leak in that particular test has now been addressed. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | diff.[ch]: have diff_free() call clear_pathspec(opts.pathspec)Ævar Arnfjörð Bjarmason2022-02-165-9/+4
| | |_|/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Have the diff_free() function call clear_pathspec(). Since the diff_flush() function calls this all its callers can be simplified to rely on it instead. When I added the diff_free() function in e900d494dcf (diff: add an API for deferred freeing, 2021-02-11) I simply missed this, or wasn't interested in it. Let's consolidate this now. This means that any future callers (and I've got revision.c in mind) that embed a "struct diff_options" can simply call diff_free() instead of needing know that it has an embedded pathspec. This does fix a bunch of leaks, but I can't mark any test here as passing under the SANITIZE=leak testing mode because in 886e1084d78 (builtin/: add UNLEAKs, 2017-10-01) an UNLEAK(rev) was added, which plasters over the memory leak. E.g. "t4011-diff-symlink.sh" would report fewer leaks with this fix, but because of the UNLEAK() reports none. I'll eventually loop around to removing that UNLEAK(rev) annotation as I'll fix deeper issues with the revisions API leaking. This is one small step on the way there, a new freeing function in revisions.c will want to call this diff_free(). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'ab/date-mode-release'Junio C Hamano2022-02-2620-54/+112
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Plug (some) memory leaks around parse_date_format(). * ab/date-mode-release: date API: add and use a date_mode_release() date API: add basic API docs date API: provide and use a DATE_MODE_INIT date API: create a date.h, split from cache.h cache.h: remove always unused show_date_human() declaration
| * | | | date API: add and use a date_mode_release()Ævar Arnfjörð Bjarmason2022-02-165-1/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix a memory leak in the parse_date_format() function by providing a new date_mode_release() companion function. By using this in "t/helper/test-date.c" we can mark the "t0006-date.sh" test as passing when git is compiled with SANITIZE=leak, and whitelist it to run under "GIT_TEST_PASSING_SANITIZE_LEAK=true" by adding "TEST_PASSES_SANITIZE_LEAK=true" to the test itself. The other tests that expose this memory leak (i.e. take the "mode->type == DATE_STRFTIME" branch in parse_date_format()) are "t6300-for-each-ref.sh" and "t7004-tag.sh". The former is due to an easily fixed leak in "ref-filter.c", and brings the failures in "t6300-for-each-ref.sh" down from 51 to 48. Fixing the remaining leaks will have to wait until there's a release_revisions() in "revision.c", as they have to do with leaks via "struct rev_info". There is also a leak in "builtin/blame.c" due to its call to parse_date_format() to parse the "blame.date" configuration. However as it declares a file-level "static struct date_mode blame_date_mode" to track the data, LSAN will not report it as a leak. It's possible to get valgrind(1) to complain about it with e.g.: valgrind --leak-check=full --show-leak-kinds=all ./git -P -c blame.date=format:%Y blame README.md But let's focus on things LSAN complains about, and are thus observable with "TEST_PASSES_SANITIZE_LEAK=true". We should get to fixing memory leaks in "builtin/blame.c", but as doing so would require some re-arrangement of cmd_blame() let's leave it for some other time. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | date API: add basic API docsÆvar Arnfjörð Bjarmason2022-02-161-2/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add basic API doc comments to date.h, and while doing so move the the parse_date_format() function adjacent to show_date(). This way all the "struct date_mode" functions are grouped together. Documenting the rest is one of our #leftoverbits. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | date API: provide and use a DATE_MODE_INITÆvar Arnfjörð Bjarmason2022-02-164-4/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Provide and use a DATE_MODE_INIT macro. Most of the users of struct date_mode" use it via pretty.h's "struct pretty_print_context" which doesn't have an initialization macro, so we're still bound to being initialized to "{ 0 }" by default. But we can change the couple of callers that directly declared a variable on the stack to instead use the initializer, and thus do away with the "mode.local = 0" added in add00ba2de9 (date: make "local" orthogonal to date format, 2015-09-03). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | date API: create a date.h, split from cache.hÆvar Arnfjörð Bjarmason2022-02-1618-48/+68
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move the declaration of the date.c functions from cache.h, and adjust the relevant users to include the new date.h header. The show_ident_date() function belonged in pretty.h (it's defined in pretty.c), its two users outside of pretty.c didn't strictly need to include pretty.h, as they get it indirectly, but let's add it to them anyway. Similarly, the change to "builtin/{fast-import,show-branch,tag}.c" isn't needed as far as the compiler is concerned, but since they all use the "DATE_MODE()" macro we now define in date.h, let's have them include it. We could simply include this new header in "cache.h", but as this change shows these functions weren't common enough to warrant including in it in the first place. By moving them out of cache.h changes to this API will no longer cause a (mostly) full re-build of the project when "make" is run. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | cache.h: remove always unused show_date_human() declarationÆvar Arnfjörð Bjarmason2022-02-161-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There has never been a show_date_human() function on the "master" branch in git.git. This declaration was added in b841d4ff438 (Add `human` format to test-tool, 2019-01-28). A look at the ML history reveals that it was leftover cruft from an earlier version of that commit[1]. 1. https://lore.kernel.org/git/20190118061805.19086-5-ischis2@cox.net/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'jc/name-rev-stdin'Junio C Hamano2022-02-261-1/+1
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Finishing touches to an earlier "name-rev --annotate-stdin" series. * jc/name-rev-stdin: name-rev: replace --stdin with --annotate-stdin in synopsis
| * | | | | name-rev: replace --stdin with --annotate-stdin in synopsisJohn Cai2022-02-161-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 34ae3b70 (name-rev: deprecate --stdin in favor of --annotate-stdin, 2022-01-05) added --annotate-stdin to replace --stdin as a clearer flag name. Since --stdin is to be deprecated, we should replace --stdin in the output from "git name-rev -h". Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>