kernel/git - git - PowerEL Git System

Commit Graph

Author	SHA1	Message	Date
Elijah Newren	6c34239de6	unpack-trees: also allow get_progress() to work on a different index commit `b0a5a12a60` ("unpack-trees: allow check_updates() to work on a different index", 2020-03-27) allowed check_updates() to work on a different index, but it called get_progress() which was hardcoded to work on o->result much like check_updates() had been. Update it to also accept an index parameter and have check_updates() pass that parameter along so that both are working on the same index. Noticed-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Derrick Stolee	0eeb3be4c4	unpack-trees: avoid array out-of-bounds error The loop in warn_conflicted_path() that checks for the count of entries with the same path uses "i+count" for the array entry. However, the loop only verifies that the value of count is below the array size. Fix this by adding i to the condition. I hit this condition during a test of the in-tree sparse-checkout feature, so it is exercised by the end of the series. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> [jc: readability fix] Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Derrick Stolee	ace224ac5f	sparse-checkout: stop blocking empty workdirs Remove the error condition when updating the sparse-checkout leaves an empty working directory. This behavior was added in `9e1afb167` (sparse checkout: inhibit empty worktree, 2009-08-20). The comment was added in `a7bc906f2` (Add explanation why we do not allow to sparse checkout to empty working tree, 2011-09-22) in response to a "dubious" comment in `84563a624` (unpack-trees.c: cosmetic fix, 2010-12-22). With the recent "cone mode" and "git sparse-checkout init [--cone]" command, it is common to set a reasonable sparse-checkout pattern set of /* !/*/ which matches only files at root. If the repository has no such files, then their "git sparse-checkout init" command will fail. Now that we expect this to be a common pattern, we should not have the commands fail on an empty working directory. If it is a confusing result, then the user can recover with "git sparse-checkout disable" or "git sparse-checkout set". This is especially simple when using cone mode. Reported-by: Lars Schneider <larsxschneider@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Jonathan Tan	db7ed7418b	promisor-remote: accept 0 as oid_nr in function There are 3 callers to promisor_remote_get_direct() that first check if the number of objects to be fetched is equal to 0. Fold that check into promisor_remote_get_direct(), and in doing so, be explicit as to what promisor_remote_get_direct() does if oid_nr is 0 (it returns 0, success, immediately). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	681c637b4a	unpack-trees: failure to set SKIP_WORKTREE bits always just a warning Setting and clearing of the SKIP_WORKTREE bit is not only done when users run 'sparse-checkout'; other commands such as 'checkout' also run through unpack_trees() which has logic for handling this special bit. As such, we need to consider how they handle special cases. A couple comparison points should help explain the rationale for changing how unpack_trees() handles these bits: Ignoring sparse checkouts for a moment, if you are switching branches and have dirty changes, it is only considered an error that will prevent the branch switching from being successful if the dirty file happens to be one of the paths with different contents. SKIP_WORKTREE has always been considered advisory; for example, if rebase or merge need or even want to materialize a path as part of their work, they have always been allowed to do so regardless of the SKIP_WORKTREE setting. This has been used for unmerged paths, but it was often used for paths it wasn't needed just because it made the code simpler. It was a best-effort consideration, and when it materialized paths contrary to the SKIP_WORKTREE setting, it was never required to even print a warning message. In the past if you trying to run e.g. 'git checkout' and: 1) you had a path that was materialized and had some dirty changes 2) the path was listed in $GITDIR/info/sparse-checkout 3) this path did not different between the current and target branches then despite the comparison points above, the inability to set SKIP_WORKTREE was treated as a hard error that would abort the checkout operation. This is completely inconsistent with how SKIP_WORKTREE is handled elsewhere, and rather annoying for users as leaving the paths materialized in the working copy (with a simple warning) should present no problem at all. Downgrade any errors from inability to toggle the SKIP_WORKTREE bit to a warning and allow the operations to continue. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	ebb568b9e2	unpack-trees: provide warnings on sparse updates for unmerged paths too When sparse-checkout runs to update the list of sparsity patterns, it gives warnings if it can't remove paths from the working tree because those files have dirty changes. Add a similar warning for unmerged paths as well. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	22ab0b37d8	unpack-trees: make sparse path messages sound like warnings The messages for problems with sparse paths are phrased as errors that cause the operation to abort, even though we are not making the operation abort. Reword the messages to make sense in their new context. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	6271d77cb1	unpack-trees: split display_error_msgs() into two display_error_msgs() is never called to show messages of both ERROR_* and WARNING_* types at the same time; it is instead called multiple times, separately for each type. Since we want to display these types differently, make two slightly different versions of this function. A subsequent commit will further modify unpack_trees() and how it calls the new display_warning_msgs(). Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	1ac83f42da	unpack-trees: rename ERROR_* fields meant for warnings to WARNING_* We want to treat issues with setting the SKIP_WORKTREE bit as a warning rather than an error; rename the enum values to reflect this intent as a simple step towards that goal. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	cd002c1561	unpack-trees: move ERROR_WOULD_LOSE_SUBMODULE earlier A minor change, but we want to convert the sparse messages to warnings and this allows us to group warnings and errors. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	7af7a25853	unpack-trees: add a new update_sparsity() function Previously, the only way to update the SKIP_WORKTREE bits for various paths was invoking `git read-tree -mu HEAD` or calling the same code that this codepath invoked. This however had a number of problems if the index or working directory were not clean. First, let's consider the case: Flipping SKIP_WORKTREE -> !SKIP_WORKTREE (materializing files) If the working tree was clean this was fine, but if there were files or directories or symlinks or whatever already present at the given path then the operation would abort with an error. Let's label this case for later discussion: A) There is an untracked path in the way Now let's consider the opposite case: Flipping !SKIP_WORKTREE -> SKIP_WORKTREE (removing files) If the index and working tree was clean this was fine, but if there were any unclean paths we would run into problems. There are three different cases to consider: B) The path is unmerged C) The path has unstaged changes D) The path has staged changes (differs from HEAD) If any path fell into case B or C, then the whole operation would be aborted with an error. With sparse-checkout, the whole operation would be aborted for case D as well, but for its predecessor of using `git read-tree -mu HEAD` directly, any paths that fell into case D would be removed from the working copy and the index entry for that path would be reset to match HEAD -- which looks and feels like data loss to users (only a few are even aware to ask whether it can be recovered, and even then it requires walking through loose objects trying to match up the right ones). Refusing to remove files that have unsaved user changes is good, but refusing to work on any other paths is very problematic for users. If the user is in the middle of a rebase or has made modifications to files that bring in more dependencies, then for their build to work they need to update the sparse paths. This logic has been preventing them from doing so. Sometimes in response, the user will stage the files and re-try, to no avail with sparse-checkout or to the horror of losing their changes if they are using its predecessor of `git read-tree -mu HEAD`. Add a new update_sparsity() function which will not error out in any of these cases but behaves as follows for the special cases: A) Leave the file in the working copy alone, clear the SKIP_WORKTREE bit, and print a warning (thus leaving the path in a state where status will report the file as modified, which seems logical). B) Do NOT mark this path as SKIP_WORKTREE, and leave it as unmerged. C) Do NOT mark this path as SKIP_WORKTREE and print a warning about the dirty path. D) Mark the path as SKIP_WORKTREE, but do not revert the version stored in the index to match HEAD; leave the contents alone. I tried a different behavior for A (leave the SKIP_WORKTREE bit set), but found it very surprising and counter-intuitive (e.g. the user sees it is present along with all the other files in that directory, tries to stage it, but git add ignores it since the SKIP_WORKTREE bit is set). A & C seem like optimal behavior to me. B may be as well, though I wonder if printing a warning would be an improvement. Some might be slightly surprised by D at first, but given that it does the right thing with `git commit` and even `git commit -a` (`git add` ignores entries that are marked SKIP_WORKTREE and thus doesn't delete them, and `commit -a` is similar), it seems logical to me. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	30e89c12f0	unpack-trees: pull sparse-checkout pattern reading into a new function Create a populate_from_existing_patterns() function for reading the path_patterns from $GIT_DIR/info/sparse-checkout so that we can re-use it elsewhere. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	3cc7c50402	unpack-trees: do not mark a dirty path with SKIP_WORKTREE If a path is dirty, removing from the working tree risks losing data. As such, we want to make sure any such path is not marked with SKIP_WORKTREE. While the current callers of this code detect this case and re-populate with a previous set of sparsity patterns, we want to allow some paths to be marked with SKIP_WORKTREE while others are left unmarked without it being considered an error. The reason this shouldn't be considered an error is that SKIP_WORKTREE has always been an advisory-only setting; merge and rebase for example were free to materialize paths and clear the SKIP_WORKTREE bit in order to accomplish their work even though they kept the SKIP_WORKTREE bit set for other paths. Leaving dirty working files in the working tree is thus a natural extension of what we have already been doing. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	b0a5a12a60	unpack-trees: allow check_updates() to work on a different index check_updates() previously assumed it was working on o->result. We want to use this function in combination with a different index_state, so take the intended index_state as a parameter. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	fa0bde45cd	unpack-trees: simplify pattern_list freeing commit `e091228e17` ("sparse-checkout: update working directory in-process", 2019-11-21) allowed passing a pre-defined set of patterns to unpack_trees(). However, if o->pl was NULL, it would still read the existing patterns and use those. If those patterns were read into a data structure that was allocated, naturally they needed to be free'd. However, despite the same function being responsible for knowing about both the allocation and the free'ing, the logic for tracking whether to free the pattern_list was hoisted to an outer function with an additional flag in unpack_trees_options. Put the logic back in the relevant function and discard the now unnecessary flag. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	d61633ae18	unpack-trees: simplify verify_absent_sparse() verify_absent_sparse() was introduced in commit `08402b0409` ("merge-recursive: distinguish "removed" and "overwritten" messages", 2010-08-11), and has always had exactly one caller which always passes error_type == ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN. This function then checks whether error_type is this value, and if so, sets it instead to ERROR_WOULD_LOSE_ORPHANED_OVERWRITTEN. It has been nearly a decade and no other caller has been created, and no other value has ever been passed, so just pass the expected value to begin with. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	d7dc1e1668	unpack-trees: remove unused error type commit `08402b0409` ("merge-recursive: distinguish "removed" and "overwritten" messages", 2010-08-11) split ERROR_WOULD_LOSE_UNTRACKED into both ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN ERROR_WOULD_LOSE_UNTRACKED_REMOVED and also split ERROR_WOULD_LOSE_ORPHANED into both ERROR_WOULD_LOSE_ORPHANED_OVERWRITTEN ERROR_WOULD_LOSE_ORPHANED_REMOVED However, despite the split only three of these four types were used. ERROR_WOULD_LOSE_ORPHANED_REMOVED was not put into use when it was introduced and nothing else has used it in the intervening decade either. Remove it. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	031ba55b6b	unpack-trees: fix minor typo in comment Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
brian m. carlson	13e7ed6a3a	builtin/checkout: compute checkout metadata for checkouts Provide commit metadata for checkout code paths that use unpack_trees and friends. When we're checking out a commit, use the commit information, but don't provide commit information if we're checking out from the index, since there need not be any particular commit associated with the index, and even if there is one, we can't know what it is. Signed-off-by: brian m. carlson <bk2204@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Philippe Blain	e84704f15c	unpack-trees: check for missing submodule directory in merged_entry Using `git checkout --recurse-submodules` to switch between a branch with no submodules and a branch with initialized nested submodules currently causes a fatal error: $ git checkout --recurse-submodules branch-with-nested-submodules fatal: exec '--super-prefix=submodule/nested/': cd to 'nested' failed: No such file or directory error: Submodule 'nested' could not be updated. error: Submodule 'submodule/nested' cannot checkout new HEAD. error: Submodule 'submodule' could not be updated. M submodule Switched to branch 'branch-with-nested-submodules' The checkout succeeds but the worktree and index of the first level submodule are left empty: $ cd submodule $ git -c status.submoduleSummary=1 status HEAD detached at b3ce885 Changes to be committed: (use "git restore --staged <file>..." to unstage) deleted: .gitmodules deleted: first.t deleted: nested fatal: not a git repository: 'nested/.git' Submodule changes to be committed: * nested 1e96f59...0000000: $ git ls-files -s $ # empty $ ls -A .git The reason for the fatal error during the checkout is that a child git process tries to cd into the yet unexisting nested submodule directory. The sequence is the following: 1. The main git process (the one running in the superproject) eventually reaches write_entry() in entry.c, which creates the first level submodule directory and then calls submodule_move_head() in submodule.c, which spawns `git read-tree` in the submodule directory. 2. The first child git process (the one in the submodule of the superproject) eventually calls check_submodule_move_head() at unpack_trees.c:2021, which calls submodule_move_head in dry-run mode, which spawns `git read-tree` in the nested submodule directory. 3. The second child git process tries to chdir() in the yet unexisting nested submodule directory in start_command() at run-command.c:829 and dies before exec'ing. The reason why check_submodule_move_head() is reached in the first child and not in the main process is that it is inside an if(submodule_from_ce()) construct, and submodule_from_ce() returns a valid struct submodule pointer, whereas it returns a null pointer in the main git process. The reason why submodule_from_ce() returns a null pointer in the main git process is because the call to cache_lookup_path() in config_from() (called from submodule_from_path() in submodule_from_ce()) returns a null pointer since the hashmap "for_path" in the submodule_cache of the_repository is not yet populated. It is not populated because both repo_get_oid(repo, GITMODULES_INDEX, &oid) and repo_get_oid(repo, GITMODULES_HEAD, &oid) in config_from_gitmodules() at submodule-config.c:639-640 return -1, as at this stage of the operation, neither the HEAD of the superproject nor its index contain any .gitmodules file. In contrast, in the first child the hashmap is populated because repo_get_oid(repo, GITMODULES_HEAD, &oid) returns 0 as the HEAD of the first level submodule, i.e. .git/modules/submodule/HEAD, points to a commit where .gitmodules is present and records 'nested' as a submodule. Fix this bug by checking that the submodule directory exists before calling check_submodule_move_head() in merged_entry() in the `if(!old)` branch, i.e. if going from a commit with no submodule to a commit with a submodule present. Also protect the other call to check_submodule_move_head() in merged_entry() the same way as it is safer, even though the `else if (!(old->ce_flags & CE_CONFLICTED))` branch of the code is not at play in the present bug. The other calls to check_submodule_move_head() in other functions in unpack_trees.c are all already protected by calls to lstat() somewhere in the program flow so we don't need additional protection for them. All commands in the unpack_trees machinery are affected, i.e. checkout, reset and read-tree when called with the --recurse-submodules flag. This bug was first reported in [1]. [1] https://lore.kernel.org/git/7437BB59-4605-48EC-B05E-E2BDB2D9DABC@gmail.com/ Reported-by: Philippe Blain <levraiphilippeblain@gmail.com> Reported-by: Damien Robert <damien.olivier.robert@gmail.com> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Philippe Blain	16f2b6bb36	unpack-trees: remove outdated description for verify_clean_submodule The function verify_clean_submodule() learned to verify if a submodule working tree is clean in `a7bc845a9a` (unpack-trees: check if we can perform the operation for submodules, 2017-03-14), but the commented description above it was not updated to reflect that, such that this description has been outdated since then. Since Git has now learned to optionnally recursively check out submodules during a superproject checkout, remove this outdated description. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Derrick Stolee	f998a3f1e5	sparse-checkout: fix cone mode behavior mismatch The intention of the special "cone mode" in the sparse-checkout feature is to always match the same patterns that are matched by the same sparse-checkout file as when cone mode is disabled. When a file path is given to "git sparse-checkout set" in cone mode, then the cone mode improperly matches the file as a recursive path. When setting the skip-worktree bits, files were not expecting the MATCHED_RECURSIVE response, and hence these were left out of the matched cone. Fix this bug by checking for MATCHED_RECURSIVE in addition to MATCHED and add a test that prevents regression. Reported-by: Finn Bryant <finnbryant@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Jeff King	d20bc01a51	avoid computing zero offsets from NULL pointer The Undefined Behavior Sanitizer in clang-11 seems to have learned a new trick: it complains about computing offsets from a NULL pointer, even if that offset is 0. This causes numerous test failures. For example, from t1090: unpack-trees.c:1355:41: runtime error: applying zero offset to null pointer ... not ok 6 - in partial clone, sparse checkout only fetches needed blobs The code in question looks like this: struct cache_entry **cache_end = cache + nr; ... while (cache != cache_end) and we sometimes pass in a NULL and 0 for "cache" and "nr". This is conceptually fine, as "cache_end" would be equal to "cache" in this case, and we wouldn't enter the loop at all. But computing even a zero offset violates the C standard. And given the fact that UBSan is noticing this behavior, this might be a potential problem spot if the compiler starts making unexpected assumptions based on undefined behavior. So let's just avoid it, which is pretty easy. In some cases we can just switch to iterating with a numeric index (as we do in sequencer.c here). In other cases (like the cache_end one) the use of an end pointer is more natural; we can keep that by just explicitly checking for the NULL/0 case when assigning the end pointer. Note that there are two ways you can write this latter case, checking for the pointer: cache_end = cache ? cache + nr : cache; or the size: cache_end = nr ? cache + nr : cache; For the case of a NULL/0 ptr/len combo, they are equivalent. But writing it the second way (as this patch does) has the property that if somebody were to incorrectly pass a NULL pointer with a non-zero length, we'd continue to notice and segfault, rather than silently pretending the length was zero. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Matheus Tavares	d7992421e1	submodule-config: add skip_if_read option to repo_read_gitmodules() Currently, submodule-config.c doesn't have an externally accessible function to read gitmodules only if it wasn't already read. But this exact behavior is internally implemented by gitmodules_read_check(), to perform a lazy load. Let's merge this function with repo_read_gitmodules() adding a 'skip_if_read' which allows both internal and external callers to access this functionality. This simplifies a little the code. The added option will also be used in the following patch. Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Derrick Stolee via GitGitGadget	4c6c7971e0	unpack-trees: correctly compute result count The clear_ce_flags_dir() method processes the cache entries within a common directory. The returned int is the number of cache entries processed by that directory. When using the sparse-checkout feature in cone mode, we can skip the pattern matching for entries in the directories that are entirely included or entirely excluded. `eb42feca` (unpack-trees: hash less in cone mode, 2019-11-21) introduced this performance feature. The old mechanism relied on the counts returned by calling clear_ce_flags_1(), but the new mechanism calculated the number of rows by subtracting "cache_end" from "cache" to find the size of the range. However, the equation is wrong because it divides by sizeof(struct cache_entry ). This is not how pointer arithmetic works! A coverity build of Git for Windows in preparation for the 2.25.0 release found this issue with the warning, "Pointer differences, such as cache_end - cache, are automatically scaled down by the size (8 bytes) of the pointed-to type (struct cache_entry ). Most likely, the division by sizeof(struct cache_entry *) is extraneous and should be eliminated." This warning is correct. This leaves us with the question "how did this even work?" The problem that occurs with this incorrect pointer arithmetic is a performance-only bug, and a very slight one at that. Since the entry count returned by clear_ce_flags_dir() is reduced by a factor of 8, the loop in clear_ce_flags_1() will re-process entries from those directories. By inserting global counters into unpack-tree.c and tracing them with trace2_data_intmax() (in a private change, for testing), I was able to see count how many times the loop inside clear_ce_flags_1() processed an entry and how many times clear_ce_flags_dir() was called. Each of these are reduced by at least a factor of 8 with the current change. A factor larger than 8 happens when multiple levels of directories are repeated. Specifically, in the Linux kernel repo, the command git sparse-checkout set LICENSES restricts the working directory to only the files at root and in the LICENSES directory. Here are the measured counts: clear_ce_flags_1 loop blocks: Before: 11,520 After: 1,621 clear_ce_flags_dir calls: Before: 7,048 After: 606 While these are dramatic counts, the time spent in clear_ce_flags_1() is under one millisecond in each case, so the improvement is not measurable as an end-to-end time. Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Emily Shaffer	573117dfa5	unpack-trees: watch for out-of-range index position It's possible in a case where the index file contains a tree extension but no blobs within that tree exist for index_pos_by_traverse_info() to segfault. If the name_entry passed into index_pos_by_traverse_info() has no blobs inside, AND is alphabetically later than all blobs currently in the index file, index_pos_by_traverse_info() will segfault. For example, an index file which looks something like this: aaa#0 bbb/aaa#0 [Extensions] TREE: zzz In this example, 'index_name_pos(..., "zzz/", ...)' will return '-4', indicating that "zzz/" could be inserted at position 3. However, when the checks which ensure that the insertion position of "zzz/" look for a blob at that position beginning with "zzz/", the index cache is accessed out of range, causing a segfault. This kind of index state is not typically generated during user operations, and is in fact an edge case of the state being checked for in the conditional where it was added. However, since the entry for the BUG() line is ambiguous, tell some additional context to help Git developers debug the failure later. When we know the name of the dir we were trying to look up, it becomes possible to examine the index file in a hex util to determine what went wrong; the position gives a hint about where to start looking. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	26f924d50e	unpack-trees: exit check_updates() early if updates are not wanted check_updates() has a lot of code that repeatedly checks whether o->update or o->dry_run are set. (Note that o->dry_run is a near-synonym for !o->update, but not quite as per commit `2c9078d05b` ("unpack-trees: add the dry_run flag to unpack_trees_options", 2011-05-25).) In fact, this function almost turns into a no-op whenever the condition !o->update \|\| o->dry_run is met. Simplify the code by checking this condition at the beginning of the function, and when it is true, do the few things that are relevant and return early. There are a few things that make the conversion not quite obvious: * The fact that check_updates() does not actually turn into a no-op when updates are not wanted may be slightly surprising. However, commit `33ecf7eb61` (Discard "deleted" cache entries after using them to update the working tree, 2008-02-07) put the discarding of unused cache entries in check_updates() so we still need to keep the call to remove_marked_cache_entries(). It's possible this call belongs in another function, but it is certainly needed as tests will fail if it is removed. * The original called remove_scheduled_dirs() unconditionally. Technically, commit `7847892716` (unlink_entry(): introduce schedule_dir_for_removal(), 2009-02-09) should have made that call conditional, but it didn't matter in practice because remove_scheduled_dirs() becomes a no-op when all the calls to unlink_entry() are skipped. As such, we do not need to call it. * When (o->dry_run && o->update), the original would have two calls to git_attr_set_direction() surrounding a bunch of skipped updates. These two calls to git_attr_set_direction() cancel each other out and thus can be omitted when o->dry_run is true just as they already are when !o->update. * The code would previously call setup_collided_checkout_detection() and report_collided_checkout() even when o->dry_run. However, this was just an expensive no-op because setup_collided_checkout_detection() merely cleared the CE_MATCHED flag for each cache entry, and report_collided_checkout() reported which ones had it set. Since a dry-run would skip all the checkout_entry() calls, CE_MATCHED would never get set and thus no collisions would be reported. Since we can't detect the collisions anyway without doing updates, skipping the collisions detection setup and reporting is an optimization. * The code previously would call get_progress() and display_progress() even when (!o->update \|\| o->dry_run). This served to show how long it took to skip all the updates, which is somewhat useless. Since we are skipping the updates, we can skip showing how long it takes to skip them. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Johannes Schindelin	cc756edda6	unpack-trees: let merged_entry() pass through do_add_entry()'s errors A `git clone` will end with exit code 0 when `merged_entry()` returns a positive value during a call of `unpack_trees()` to `traverse_trees()`. The reason is that `unpack_trees()` will interpret a positive value not to be an error. The problem is, however, that `add_index_entry()` (which is called by `merged_entry()` can report an error, and we really should fail the entire clone in such a case. Let's fix this problem, in preparation for a Windows-specific patch disallowing `mkdir()` with directory names that contain a trailing space (which is illegal on NTFS): we want `git clone` to abort when a path cannot be checked out due to that condition. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>	5 years ago
Derrick Stolee	e091228e17	sparse-checkout: update working directory in-process The sparse-checkout builtin used 'git read-tree -mu HEAD' to update the skip-worktree bits in the index and to update the working directory. This extra process is overly complex, and prone to failure. It also requires that we write our changes to the sparse-checkout file before trying to update the index. Remove this extra process call by creating a direct call to unpack_trees() in the same way 'git read-tree -mu HEAD' does. In addition, provide an in-memory list of patterns so we can avoid reading from the sparse-checkout file. This allows us to test a proposed change to the file before writing to it. An earlier version of this patch included a bug when the 'set' command failed due to the "Sparse checkout leaves no entry on working directory" error. It would not rollback the index.lock file, so the replay of the old sparse-checkout specification would fail. A test in t1091 now covers that scenario. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Derrick Stolee	4dcd4def3c	unpack-trees: add progress to clear_ce_flags() When a large repository has many sparse-checkout patterns, the process for updating the skip-worktree bits can take long enough that a user gets confused why nothing is happening. Update the clear_ce_flags() method to write progress. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Derrick Stolee	eb42feca97	unpack-trees: hash less in cone mode The sparse-checkout feature in "cone mode" can use the fact that the recursive patterns are "connected" to the root via parent patterns to decide if a directory is entirely contained in the sparse-checkout or entirely removed. In these cases, we can skip hashing the paths within those directories and simply set the skipworktree bit to the correct value. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Derrick Stolee	96cc8ab531	sparse-checkout: use hashmaps for cone patterns The parent and recursive patterns allowed by the "cone mode" option in sparse-checkout are restrictive enough that we can avoid using the regex parsing. Everything is based on prefix matches, so we can use hashsets to store the prefixes from the sparse-checkout file. When checking a path, we can strip path entries from the path and check the hashset for an exact match. As a test, I created a cone-mode sparse-checkout file for the Linux repository that actually includes every file. This was constructed by taking every folder in the Linux repo and creating the pattern pairs here: /$folder/ !/$folder/*/ This resulted in a sparse-checkout file sith 8,296 patterns. Running 'git read-tree -mu HEAD' on this file had the following performance: core.sparseCheckout=false: 0.21 s (0.00 s) core.sparseCheckout=true: 3.75 s (3.50 s) core.sparseCheckoutCone=true: 0.23 s (0.01 s) The times in parentheses above correspond to the time spent in the first clear_ce_flags() call, according to the trace2 performance traces. While this example is contrived, it demonstrates how these patterns can slow the sparse-checkout feature. Helped-by: Eric Wong <e@80x24.org> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Jeff Hostetler	e6152e35ff	trace2: add region in clear_ce_flags When Git updates the working directory with the sparse-checkout feature enabled, the unpack_trees() method calls clear_ce_flags() to update the skip-wortree bits on the cache entries. This check can be expensive, depending on the patterns used. Add trace2 regions around the method, including some flag information, so we can get granular performance data during experiments. This data will be used to measure improvements to the pattern-matching algorithms for sparse-checkout. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Utsav Shah	679f2f9fdd	unpack-trees: skip stat on fsmonitor-valid files The index might be aware that a file hasn't modified via fsmonitor, but unpack-trees did not pay attention to it and checked via ie_match_stat which can be inefficient on certain filesystems. This significantly slows down commands that run oneway_merge, like checkout and reset --hard. This patch makes oneway_merge check whether a file is considered unchanged through fsmonitor and skips ie_match_stat on it. unpack-trees also now correctly copies over fsmonitor validity state from the source index. Finally, for correctness, we force a refresh of fsmonitor state in tweak_fsmonitor. After this change, commands like stash (that use reset --hard internally) go from 8s or more to ~2s on a 250k file repository on a mac. Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Kevin Willford <Kevin.Willford@microsoft.com> Signed-off-by: Utsav Shah <utsav@dropbox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Elijah Newren	15beaaa3d1	Fix spelling errors in code comments Reported-by: Jens Schleusener <Jens.Schleusener@fossies.org> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
René Scharfe	2fe44394c8	treewide: remove duplicate #include directives Found with "git grep '^#include ' '*.c' \| sort \| uniq -d". Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Derrick Stolee	468ce99b77	unpack-trees: rename 'is_excluded_from_list()' The first consumer of pattern-matching filenames was the .gitignore feature. In that context, storing a list of patterns as a 'struct exclude_list' makes sense. However, the sparse-checkout feature then adopted these structures and methods, but with the opposite meaning: these patterns match the files that should be included! Now that this library is renamed to use 'struct pattern_list' and 'struct pattern', we can now rename the method used by the sparse-checkout feature to determine which paths should appear in the working directory. The method is_excluded_from_list() is only used by the sparse-checkout logic in unpack-trees and list-objects-filter. The confusing part is that it returned 1 for "excluded" (i.e. it matches the list of exclusions) but that really manes that the path matched the list of patterns for _inclusion_ in the working directory. Rename the method to be path_matches_pattern_list() and have it return an explicit 'enum pattern_match_result'. Here, the values MATCHED = 1, UNMATCHED = 0, and UNDECIDED = -1 agree with the previous integer values. This shift allows future consumers to better understand what the retur values mean, and provides more type checking for handling those values. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Derrick Stolee	65edd96aec	treewide: rename 'exclude' methods to 'pattern' The first consumer of pattern-matching filenames was the .gitignore feature. In that context, storing a list of patterns as a 'struct exclude_list' makes sense. However, the sparse-checkout feature then adopted these structures and methods, but with the opposite meaning: these patterns match the files that should be included! It would be clearer to rename this entire library as a "pattern matching" library, and the callers apply exclusion/inclusion logic accordingly based on their needs. This commit renames several methods defined in dir.h to make more sense with the renamed 'struct exclude_list' to 'struct pattern_list' and 'struct exclude' to 'struct path_pattern': * last_exclude_matching() -> last_matching_pattern() * parse_exclude() -> parse_path_pattern() In addition, the word 'exclude' was replaced with 'pattern' in the methods below: * add_exclude_list() * add_excludes_from_file_to_list() * add_excludes_from_file() * add_excludes_from_blob_to_list() * add_exclude() * clear_exclude_list() A few methods with the word "exclude" remain. These will be handled seperately. In particular, the method "is_excluded()" is concretely about the .gitignore file relative to a specific directory. This is the important boundary between library and consumer: is_excluded() cares about .gitignore, but is_excluded() calls last_matching_pattern() to make that decision. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Derrick Stolee	caa3d55444	treewide: rename 'struct exclude_list' to 'struct pattern_list' The first consumer of pattern-matching filenames was the .gitignore feature. In that context, storing a list of patterns as a 'struct exclude_list' makes sense. However, the sparse-checkout feature then adopted these structures and methods, but with the opposite meaning: these patterns match the files that should be included! It would be clearer to rename this entire library as a "pattern matching" library, and the callers apply exclusion/inclusion logic accordingly based on their needs. This commit renames 'struct exclude_list' to 'struct pattern_list' and renames several variables called 'el' to 'pl'. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Jeff King	5aa02f9868	tree-walk: harden make_traverse_path() length computations The make_traverse_path() function isn't very careful about checking its output buffer boundaries. In fact, it doesn't even _know_ the size of the buffer it's writing to, and just assumes that the caller used traverse_path_len() correctly. And even then we assume that our traverse_info.pathlen components are all correct, and just blindly write into the buffer. Let's improve this situation a bit: - have the caller pass in their allocated buffer length, which we'll check against our own computations - check for integer underflow as we do our backwards-insertion of pathnames into the buffer - check that we do not run out items in our list to traverse before we've filled the expected number of bytes None of these should be triggerable in practice (especially since our switch to size_t everywhere in a previous commit), but it doesn't hurt to check our assumptions. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Jeff King	c43ab06259	tree-walk: add a strbuf wrapper for make_traverse_path() All but one of the callers of make_traverse_path() allocate a new heap buffer to store the path. Let's give them an easy way to write to a strbuf, which saves them from computing the length themselves (which is especially tricky when they want to add to the path). It will also make it easier for us to change the make_traverse_path() interface in a future patch to improve its bounds-checking. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Jeff King	b3b3cbcbf2	tree-walk: accept a raw length for traverse_path_len() We take a "struct name_entry", but only care about the length of the path name. Let's just take that length directly, making it easier to use the function from callers that sometimes do not have a name_entry at all. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Jeff King	37806080d7	tree-walk: use size_t consistently We store and manipulate the cumulative traverse_info.pathlen as an "int", which can overflow when we are fed ridiculously long pathnames (e.g., ones at the edge of 2GB or 4GB, even if the individual tree entry names are smaller than that). The results can be confusing, though after some prodding I was not able to use this integer overflow to cause an under-allocated buffer. Let's consistently use size_t to generate and store these, and make sure our addition doesn't overflow. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Jeff King	9055384710	tree-walk: drop oid from traverse_info As the previous commit shows, the presence of an oid in each level of the traverse_info is confusing and ultimately not necessary. Let's drop it to make it clear that it will not always be set (as well as convince us that it's unused, and let the compiler catch any merges with other branches that do add new uses). Since the oid is part of name_entry, we'll actually stop embedding a name_entry entirely, and instead just separately hold the pathname, its length, and the mode. This makes the resulting code slightly more verbose as we have to pass those elements around individually. But it also makes it more clear what each code path is going to use (and in most of the paths, we really only care about the pathname itself). A few of these conversions are noisier than they need to be, as they also take the opportunity to rename "len" to "namelen" for clarity (especially where we also have "pathlen" or "ce_len" alongside). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Nguyễn Thái Ngọc Duy	5e57580733	tree-walk.c: remove the_repo from fill_tree_descriptor() While at there, clean up the_repo usage in builtin/merge-tree.c a tiny bit. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Christian Couder	b14ed5adaf	Use promisor_remote_get_direct() and has_promisor_remote() Instead of using the repository_format_partial_clone global and fetch_objects() directly, let's use has_promisor_remote() and promisor_remote_get_direct(). This way all the configured promisor remotes will be taken into account, not only the one specified by extensions.partialClone. Also when cloning or fetching using a partial clone filter, remote.origin.promisor will be set to "true" instead of setting extensions.partialClone to "origin". This makes it possible to use many promisor remote just by fetching from them. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Nguyễn Thái Ngọc Duy	328c6cb853	doc: promote "git switch" The new command "git switch" is added to avoid the confusion of one-command-do-all "git checkout" for new users. They are also helpful to avoid ambiguation context. For these reasons, promote it everywhere possible. This includes documentation, suggestions/advice from other commands... The "Checking out files" progress line in unpack-trees.c is also updated to "Updating files" to be neutral to both git-checkout and git-switch. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Jonathan Tan	0f4a4fb1c4	sha1-file: support OBJECT_INFO_FOR_PREFETCH Teach oid_object_info_extended() to support a new flag that inhibits fetching of missing objects. This is equivalent to setting fetch_is_missing to 0, calling oid_object_info_extended(), then setting fetch_if_missing to whatever it was before. Update unpack-trees.c to use this new flag instead of repeatedly setting fetch_if_missing. This new flag complicates things slightly in that there are now 2 ways to do the same thing. But this eliminates the need to repeatedly set a global variable, and more importantly, allows prefetching to be done in parallel (in the future); hence, this patch. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Nguyễn Thái Ngọc Duy	b165fac8c1	unpack-trees: rename "gently" flag to "quiet" The gently flag was added in `17e4642667` (Add flag to make unpack_trees() not print errors. - 2008-02-07) to suppress error messages. The name "gently" does not quite express that. Granted, being quiet is gentle but it could mean not performing some other actions. Rename the flag to "quiet" to be more on point. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Nguyễn Thái Ngọc Duy	191e9d2c2d	unpack-trees: keep gently check inside add_rejected_path This basically follows the footsteps of `6a143aa2b2` (checkout -m: attempt merge when deletion of path was staged - 2014-08-12) where there gently check is moved inside reject_merge() so that callers do not accidentally forget it. add_rejected_path() has the same usage pattern. All call sites check gently first, then decide to call add_rejected_path() if needed. Move the check inside. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago

1 2 3 4 5 ...

467 Commits (5602b500c3cd9ac308bf9af0d5f0a79bd2195346)