summaryrefslogtreecommitdiff
path: root/builtin
AgeCommit message (Collapse)Author
2019-10-07Merge branch 'tg/stash-refresh-index'Junio C Hamano
"git stash" learned to write refreshed index back to disk. * tg/stash-refresh-index: stash: make sure to write refreshed cache merge: use refresh_and_write_cache factor out refresh_and_write_cache function
2019-10-07hashmap_entry: remove first member requirement from docsEric Wong
Comments stating that "struct hashmap_entry" must be the first member in a struct are no longer valid. Suggested-by: Phillip Wood <phillip.wood123@gmail.com> Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap: remove type arg from hashmap_{get,put,remove}_entryEric Wong
Since these macros already take a `keyvar' pointer of a known type, we can rely on OFFSETOF_VAR to get the correct offset without relying on non-portable `__typeof__' and `offsetof'. Argument order is also rearranged, so `keyvar' and `member' are sequential as they are used as: `keyvar->member' Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07OFFSETOF_VAR macro to simplify hashmap iteratorsEric Wong
While we cannot rely on a `__typeof__' operator being portable to use with `offsetof'; we can calculate the pointer offset using an existing pointer and the address of a member using pointer arithmetic for compilers without `__typeof__'. This allows us to simplify usage of hashmap iterator macros by not having to specify a type when a pointer of that type is already given. In the future, list iterator macros (e.g. list_for_each_entry) may also be implemented using OFFSETOF_VAR to save hackers the trouble of using container_of/list_entry macros and without relying on non-portable `__typeof__'. v3: use `__typeof__' to avoid clang warnings Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap: introduce hashmap_free_entriesEric Wong
`hashmap_free_entries' behaves like `container_of' and passes the offset of the hashmap_entry struct to the internal `hashmap_free_' function, allowing the function to free any struct pointer regardless of where the hashmap_entry field is located. `hashmap_free' no longer takes any arguments aside from the hashmap itself. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap: use *_entry APIs for iterationEric Wong
Inspired by list_for_each_entry in the Linux kernel. Once again, these are somewhat compromised usability-wise by compilers lacking __typeof__ support. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_cmp_fn takes hashmap_entry paramsEric Wong
Another step in eliminating the requirement of hashmap_entry being the first member of a struct. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_get{,_from_hash} return "struct hashmap_entry *"Eric Wong
Update callers to use hashmap_get_entry, hashmap_get_entry_from_hash or container_of as appropriate. This is another step towards eliminating the requirement of hashmap_entry being the first field in a struct. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_put takes "struct hashmap_entry *"Eric Wong
This is less error-prone than "void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_get takes "const struct hashmap_entry *"Eric Wong
This is less error-prone than "const void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_add takes "struct hashmap_entry *"Eric Wong
This is less error-prone than "void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_entry_init takes "struct hashmap_entry *"Eric Wong
C compilers do type checking to make life easier for us. So rely on that and update all hashmap_entry_init callers to take "struct hashmap_entry *" to avoid future bugs while improving safety and readability. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-06stash apply: report status correctly even in a worktree's subdirectoryJohannes Schindelin
When Git wants to spawn a child Git process inside a worktree's subdirectory while `GIT_DIR` is set, we need to take care of specifying the work tree's top-level directory explicitly because it cannot be discovered: the current directory is _not_ the top-level directory of the work tree, and neither is it inside the parent directory of `GIT_DIR`. This fixes the problem where `git stash apply` would report pretty much everything deleted or untracked when run inside a worktree's subdirectory. To make sure that we do not introduce the "reverse problem", i.e. when `GIT_WORK_TREE` is defined but `GIT_DIR` is not, we simply make sure that both are set. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-05fetch: let --jobs=<n> parallelize --multiple, tooJohannes Schindelin
So far, `--jobs=<n>` only parallelizes submodule fetches/clones, not `--multiple` fetches, which is unintuitive, given that the option's name does not say anything about submodules in particular. Let's change that. With this patch, also fetches from multiple remotes are parallelized. For backwards-compatibility (and to prepare for a use case where submodule and multiple-remote fetches may need different parallelization limits), the config setting `submodule.fetchJobs` still only controls the submodule part of `git fetch`, while the newly-introduced setting `fetch.parallel` controls both (but can be overridden for submodules with `submodule.fetchJobs`). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-03treewide: remove duplicate #include directivesRené Scharfe
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>
2019-10-03push: do not pretend to return `int` from `die_push_simple()`Johannes Schindelin
This function is marked as `NORETURN`, and it indeed does not want to return anything. So let's not declare it with the return type `int`. This fixes the following warning when building with MSVC: C4646: function declared with 'noreturn' has non-void return type Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-03fast-export: handle nested tagsElijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-03fast-export: allow user to request tags be marked with --mark-tagsElijah Newren
Add a new option, --mark-tags, which will output mark identifiers with each tag object. This improves the incremental export story with --export-marks since it will allow us to record that annotated tags have been exported, and it is also needed as a step towards supporting nested tags. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-03fast-export: add support for --import-marks-if-existsElijah Newren
fast-import has support for both an --import-marks flag and an --import-marks-if-exists flag; the latter of which will not die() if the file does not exist. fast-export only had support for an --import-marks flag; add an --import-marks-if-exists flag for consistency. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-03push: add trace2 instrumentationJosh Steadmon
Add trace2 regions in transport.c and builtin/push.c to better track time spent in various phases of pushing: * Listing refs * Checking submodules * Pushing submodules * Pushing refs Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-03fetch: add trace2 instrumentationJosh Steadmon
Add trace2 regions to fetch-pack.c and builtins/fetch.c to better track time spent in the various phases of a fetch: * listing refs * negotiation for protocol versions v0-v2 * fetching refs * consuming refs Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-02commit-graph: emit trace2 cmd_mode for each sub-commandGarima Singh
Emit trace2_cmd_mode() messages for each commit-graph sub-command. The commit graph commands were in flux when trace2 was making it's way to git. Now that we have enough sub-commands in commit-graph, we can label the various modes within them. Distinguishing between read, write and verify is a great start. Signed-off-by: Garima Singh <garima.singh@microsoft.com> Acked-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-30Merge branch 'ds/commit-graph-on-fetch'Junio C Hamano
A configuration variable tells "git fetch" to write the commit graph after finishing. * ds/commit-graph-on-fetch: fetch: add fetch.writeCommitGraph config setting
2019-09-30Merge branch 'bw/rebase-autostash-keep-current-branch'Junio C Hamano
"git rebase --autostash <upstream> <branch>", when <branch> is different from the current branch, incorrectly moved the tip of the current branch, which has been corrected. * bw/rebase-autostash-keep-current-branch: builtin/rebase.c: Remove pointless message builtin/rebase.c: make sure the active branch isn't moved when autostashing
2019-09-30Merge branch 'ds/include-exclude'Junio C Hamano
The internal code originally invented for ".gitignore" processing got reshuffled and renamed to make it less tied to "excluding" and stress more that it is about "matching", as it has been reused for things like sparse checkout specification that want to check if a path is "included". * ds/include-exclude: unpack-trees: rename 'is_excluded_from_list()' treewide: rename 'exclude' methods to 'pattern' treewide: rename 'EXCL_FLAG_' to 'PATTERN_FLAG_' treewide: rename 'struct exclude_list' to 'struct pattern_list' treewide: rename 'struct exclude' to 'struct path_pattern'
2019-09-30Merge branch 'dl/rebase-i-keep-base'Junio C Hamano
"git rebase --keep-base <upstream>" tries to find the original base of the topic being rebased and rebase on top of that same base, which is useful when running the "git rebase -i" (and its limited variant "git rebase -x"). The command also has learned to fast-forward in more cases where it can instead of replaying to recreate identical commits. * dl/rebase-i-keep-base: rebase: teach rebase --keep-base rebase tests: test linear branch topology rebase: fast-forward --fork-point in more cases rebase: fast-forward --onto in more cases rebase: refactor can_fast_forward into goto tower t3432: test for --no-ff's interaction with fast-forward t3432: distinguish "noop-same" v.s. "work-same" in "same head" tests t3432: test rebase fast-forward behavior t3431: add rebase --fork-point tests
2019-09-30Merge branch 'jk/misc-uninitialized-fixes'Junio C Hamano
Various fixes to codepaths gcc 9 had trouble following dataflow. * jk/misc-uninitialized-fixes: pack-objects: drop packlist index_pos optimization test-read-cache: drop namelen variable diff-delta: set size out-parameter to 0 for NULL delta bulk-checkin: zero-initialize hashfile_checkpoint pack-objects: use object_id in packlist_alloc() git-am: handle missing "author" when parsing commit
2019-09-30Merge branch 'rs/get-tagged-oid'Junio C Hamano
Code cleanup. * rs/get-tagged-oid: use get_tagged_oid() tag: factor out get_tagged_oid()
2019-09-30Merge branch 'tg/push-all-in-mirror-forbidden'Junio C Hamano
Fix an earlier regression to "git push --all" which should have been forbidden when the target remote repository is set to be a mirror. * tg/push-all-in-mirror-forbidden: push: disallow --all and refspecs when remote.<name>.mirror is set
2019-09-30Merge branch 'nd/switch-and-restore'Junio C Hamano
Resurrect a performance hack. * nd/switch-and-restore: checkout: add simple check for 'git checkout -b'
2019-09-30Merge branch 'rs/strbuf-detach'Junio C Hamano
Straighten out the use of strbuf_detach() API function. * rs/strbuf-detach: grep: use return value of strbuf_detach() log-tree: always use return value of strbuf_detach()
2019-09-28builtin/submodule--helper: fix usage string for 'update-clone'Bert Wesarg
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-28fast-export: fix exporting a tag and nothing elseElijah Newren
fast-export allows specifying revision ranges, which can be used to export a tag without exporting the commit it tags. fast-export handled this rather poorly: it would emit a "from :0" directive. Since marks start at 1 and increase, this means it refers to an unknown commit and fast-import will choke on the input. When we are unable to look up a mark for the object being tagged, use a "from $HASH" directive instead to fix this problem. Note that this is quite similar to the behavior fast-export exhibits with commits and parents when --reference-excluded-parents is passed along with an excluded commit range. For tags of excluded commits we do not require the --reference-excluded-parents flag because we always have to tag something. By contrast, when dealing with commits, pruning a parent is always a viable option, so we need the flag to specify that parent pruning is not wanted. (It is slightly weird that --reference-excluded-parents isn't the default with a separate --prune-excluded-parents flag, but backward compatibility concerns resulted in the current defaults.) Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-28name-rev: avoid cutoff timestamp underflowSZEDER Gábor
When 'git name-rev' is invoked with commit-ish parameters, it tries to save some work, and doesn't visit commits older than the committer date of the oldest given commit minus a one day worth of slop. Since our 'timestamp_t' is an unsigned type, this leads to a timestamp underflow when the committer date of the oldest given commit is within a day of the UNIX epoch. As a result the cutoff timestamp ends up far-far in the future, and 'git name-rev' doesn't visit any commits, and names each given commit as 'undefined'. Check whether subtracting the slop from the oldest committer date would lead to an underflow, and use no cutoff in that case. We don't have a TIME_MIN constant, dddbad728c (timestamp_t: a new data type for timestamps, 2017-04-26) didn't add one, so do it now. Note that the type of the cutoff timestamp variable used to be signed before 5589e87fd8 (name-rev: change a "long" variable to timestamp_t, 2017-05-20). The behavior was still the same even back then, but the underflow didn't happen when substracting the slop from the oldest committer date, but when comparing the signed cutoff timestamp with unsigned committer dates in name_rev(). IOW, this underflow bug is as old as 'git name-rev' itself. Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-20stash: make sure to write refreshed cacheThomas Gummerer
When converting stash into C, calls to 'git update-index --refresh' were replaced with the 'refresh_cache()' function. That is fine as long as the index is only needed in-core, and not re-read from disk. However in many cases we do actually need the refreshed index to be written to disk, for example 'merge_recursive_generic()' discards the in-core index before re-reading it from disk, and in the case of 'apply --quiet', the 'refresh_cache()' we currently have is pointless without writing the index to disk. Always write the index after refreshing it to ensure there are no regressions in this compared to the scripted stash. In the future we can consider avoiding the write where possible after making sure none of the subsequent calls actually need the refreshed cache, and it is not expected to be refreshed after stash exits or it is written somewhere else already. Reported-by: Jeff King <peff@peff.net> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-20merge: use refresh_and_write_cacheThomas Gummerer
Use the 'refresh_and_write_cache()' convenience function introduced in the last commit, instead of refreshing and writing the index manually in merge.c Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-20factor out refresh_and_write_cache functionThomas Gummerer
Getting the lock for the index, refreshing it and then writing it is a pattern that happens more than once throughout the codebase, and isn't trivial to get right. Factor out the refresh_and_write_cache function from builtin/am.c to read-cache.c, so it can be re-used in other places in a subsequent commit. Note that we return different error codes for failing to refresh the cache, and failing to write the index. The current caller only cares about failing to write the index. However for other callers we're going to convert in subsequent patches we will need this distinction. Helped-by: Martin Ågren <martin.agren@gmail.com> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-18commit-graph: add --[no-]progress to write and verifyGarima Singh
Add --[no-]progress to git commit-graph write and verify. The progress feature was introduced in 7b0f229 ("commit-graph write: add progress output", 2018-09-17) but the ability to opt-out was overlooked. Signed-off-by: Garima Singh <garima.singh@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-18Merge branch 'md/list-objects-filter-combo'Junio C Hamano
The list-objects-filter API (used to create a sparse/lazy clone) learned to take a combined filter specification. * md/list-objects-filter-combo: list-objects-filter-options: make parser void list-objects-filter-options: clean up use of ALLOC_GROW list-objects-filter-options: allow mult. --filter strbuf: give URL-encoding API a char predicate fn list-objects-filter-options: make filter_spec a string_list list-objects-filter-options: move error check up list-objects-filter: implement composite filters list-objects-filter-options: always supply *errbuf list-objects-filter: put omits set in filter struct list-objects-filter: encapsulate filter components
2019-09-18Merge branch 'cc/multi-promisor'Junio C Hamano
Teach the lazy clone machinery that there can be more than one promisor remote and consult them in order when downloading missing objects on demand. * cc/multi-promisor: Move core_partial_clone_filter_default to promisor-remote.c Move repository_format_partial_clone to promisor-remote.c Remove fetch-object.{c,h} in favor of promisor-remote.{c,h} remote: add promisor and partial clone config to the doc partial-clone: add multiple remotes in the doc t0410: test fetching from many promisor remotes builtin/fetch: remove unique promisor remote limitation promisor-remote: parse remote.*.partialclonefilter Use promisor_remote_get_direct() and has_promisor_remote() promisor-remote: use repository_format_partial_clone promisor-remote: add promisor_remote_reinit() promisor-remote: implement promisor_remote_get_direct() Add initial support for many promisor remotes fetch-object: make functions return an error code t0410: remove pipes after git commands
2019-09-18Merge branch 'js/pre-merge-commit-hook'Junio C Hamano
A new "pre-merge-commit" hook has been introduced. * js/pre-merge-commit-hook: merge: --no-verify to bypass pre-merge-commit hook git-merge: honor pre-merge-commit hook merge: do no-verify like commit t7503: verify proper hook execution
2019-09-18Merge branch 'jk/drop-release-pack-memory'Junio C Hamano
xmalloc() used to have a mechanism to ditch memory and address space resources as the last resort upon seeing an allocation failure from the underlying malloc(), which made the code complex and thread-unsafe with dubious benefit, as major memory resource users already do limit their uses with various other mechanisms. It has been simplified away. * jk/drop-release-pack-memory: packfile: drop release_pack_memory()
2019-09-18Merge branch 'js/rebase-r-strategy'Junio C Hamano
"git rebase --rebase-merges" learned to drive different merge strategies and pass strategy specific options to them. * js/rebase-r-strategy: t3427: accelerate this test by using fast-export and fast-import rebase -r: do not (re-)generate root commits with `--root` *and* `--onto` t3418: test `rebase -r` with merge strategies t/lib-rebase: prepare for testing `git rebase --rebase-merges` rebase -r: support merge strategies other than `recursive` t3427: fix another incorrect assumption t3427: accommodate for the `rebase --merge` backend having been replaced t3427: fix erroneous assumption t3427: condense the unnecessarily repetitive test cases into three t3427: move the `filter-branch` invocation into the `setup` case t3427: simplify the `setup` test case significantly t3427: add a clarifying comment rebase: fold git-rebase--common into the -p backend sequencer: the `am` and `rebase--interactive` scripts are gone .gitignore: there is no longer a built-in `git-rebase--interactive` t3400: stop referring to the scripted rebase Drop unused git-rebase--am.sh
2019-09-17clean: fix theoretical path corruptionElijah Newren
cmd_clean() had the following code structure: struct strbuf abs_path = STRBUF_INIT; for_each_string_list_item(item, &del_list) { strbuf_addstr(&abs_path, prefix); strbuf_addstr(&abs_path, item->string); PROCESS(&abs_path); strbuf_reset(&abs_path); } where I've elided a bunch of unnecessary details and PROCESS(&abs_path) represents a big chunk of code rather than an actual function call. One piece of PROCESS was: if (lstat(abs_path.buf, &st)) continue; which would cause the strbuf_reset() to be missed -- meaning that the next path to be handled would have two paths concatenated. This path used to use die_errno() instead of continue prior to commit 396049e5fb62 ("git-clean: refactor git-clean into two phases", 2013-06-25), but my understanding of how correct_untracked_entries() works is that it will prevent both dir/ and dir/file from being in the list to clean so this should be dead code and the die_errno() should be safe. But I hesitate to remove it since I am not certain. However, we can fix both this bug and possible similar future bugs by simply moving the strbuf_reset(&abs_path) to the beginning of the loop. It'll result in N calls to strbuf_reset() instead of N-1, but that's a small price to pay to avoid sneaky bugs like this. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-17clean: rewrap overly long lineElijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-17clean: avoid removing untracked files in a nested git repositoryElijah Newren
Users expect files in a nested git repository to be left alone unless sufficiently forced (with two -f's). Unfortunately, in certain circumstances, git would delete both tracked (and possibly dirty) files and untracked files within a nested repository. To explain how this happens, let's contrast a couple cases. First, take the following example setup (which assumes we are already within a git repo): git init nested cd nested >tracked git add tracked git commit -m init >untracked cd .. In this setup, everything works as expected; running 'git clean -fd' will result in fill_directory() returning the following paths: nested/ nested/tracked nested/untracked and then correct_untracked_entries() would notice this can be compressed to nested/ and then since "nested/" is a directory, we would call remove_dirs("nested/", ...), which would check is_nonbare_repository_dir() and then decide to skip it. However, if someone also creates an ignored file: >nested/ignored then running 'git clean -fd' would result in fill_directory() returning the same paths: nested/ nested/tracked nested/untracked but correct_untracked_entries() will notice that we had ignored entries under nested/ and thus simplify this list to nested/tracked nested/untracked Since these are not directories, we do not call remove_dirs() which was the only place that had the is_nonbare_repository_dir() safety check -- resulting in us deleting both the untracked file and the tracked (and possibly dirty) file. One possible fix for this issue would be walking the parent directories of each path and checking if they represent nonbare repositories, but that would be wasteful. Even if we added caching of some sort, it's still a waste because we should have been able to check that "nested/" represented a nonbare repository before even descending into it in the first place. Add a DIR_SKIP_NESTED_GIT flag to dir_struct.flags and use it to prevent fill_directory() and friends from descending into nested git repos. With this change, we also modify two regression tests added in commit 91479b9c72f1 ("t7300: add tests to document behavior of clean and nested git", 2015-06-15). That commit, nor its series, nor the six previous iterations of that series on the mailing list discussed why those tests coded the expectation they did. In fact, it appears their purpose was simply to test _existing_ behavior to make sure that the performance changes didn't change the behavior. However, these two tests directly contradicted the manpage's claims that two -f's were required to delete files/directories under a nested git repository. While one could argue that the user gave an explicit path which matched files/directories that were within a nested repository, there's a slippery slope that becomes very difficult for users to understand once you go down that route (e.g. what if they specified "git clean -f -d '*.c'"?) It would also be hard to explain what the exact behavior was; avoid such problems by making it really simple. Also, clean up some grammar errors describing this functionality in the git-clean manpage. Finally, there are still a couple bugs with -ffd not cleaning out enough (e.g. missing the nested .git) and with -ffdX possibly cleaning out the wrong files (paying attention to outer .gitignore instead of inner). This patch does not address these cases at all (and does not change the behavior relative to those flags), it only fixes the handling when given a single -f. See https://public-inbox.org/git/20190905212043.GC32087@szeder.dev/ for more discussion of the -ffd[X?] bugs. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-17clean: disambiguate the definition of -dElijah Newren
The -d flag pre-dated git-clean's ability to have paths specified. As such, the default for git-clean was to only remove untracked files in the current directory, and -d existed to allow it to recurse into subdirectories. The interaction of paths and the -d option appears to not have been carefully considered, as evidenced by numerous bugs and a dearth of tests covering such pairings in the testsuite. The definition turns out to be important, so let's look at some of the various ways one could interpret the -d option: A) Without -d, only look in subdirectories which contain tracked files under them; with -d, also look in subdirectories which are untracked for files to clean. B) Without specified paths from the user for us to delete, we need to have some kind of default, so...without -d, only look in subdirectories which contain tracked files under them; with -d, also look in subdirectories which are untracked for files to clean. The important distinction here is that choice B says that the presence or absence of '-d' is irrelevant if paths are specified. The logic behind option B is that if a user explicitly asked us to clean a specified pathspec, then we should clean anything that matches that pathspec. Some examples may clarify. Should git clean -f untracked_dir/file remove untracked_dir/file or not? It seems crazy not to, but a strict reading of option A says it shouldn't be removed. How about git clean -f untracked_dir/file1 tracked_dir/file2 or git clean -f untracked_dir_1/file1 untracked_dir_2/file2 ? Should it remove either or both of these files? Should it require multiple runs to remove both the files listed? (If this sounds like a crazy question to even ask, see the commit message of "t7300: Add some testcases showing failure to clean specified pathspecs" added earlier in this patch series.) What if -ffd were used instead of -f -- should that allow these to be removed? Should it take multiple invocations with -ffd? What if a glob (such as '*tracked*') were used instead of spelling out the directory names? What if the filenames involved globs, such as git clean -f '*.o' or git clean -f '*/*.o' ? The current documentation actually suggests a definition that is slightly different than choice A, and the implementation prior to this series provided something radically different than either choices A or B. (The implementation, though, was clearly just buggy). There may be other choices as well. However, for almost any given choice of definition for -d that I can think of, some of the examples above will appear buggy to the user. The only case that doesn't have negative surprises is choice B: treat a user-specified path as a request to clean all untracked files which match that path specification, including recursing into any untracked directories. Change the documentation and basic implementation to use this definition. There were two regression tests that indirectly depended on the current implementation, but neither was about subdirectory handling. These two tests were introduced in commit 5b7570cfb41c ("git-clean: add tests for relative path", 2008-03-07) which was solely created to add coverage for the changes in commit fb328947c8e ("git-clean: correct printing relative path", 2008-03-07). Both tests specified a directory that happened to have an untracked subdirectory, but both were only checking that the resulting printout of a file that was removed was shown with a relative path. Update these tests appropriately. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-16fetch: use oidset to keep the want OIDs for faster lookupMasaya Suzuki
During git-fetch, the client checks if the advertised tags' OIDs are already in the fetch request's want OID set. This check is done in a linear scan. For a repository that has a lot of refs, repeating this scan takes 15+ minutes. In order to speed this up, create a oid_set for other refs' OIDs. Signed-off-by: Masaya Suzuki <masayasuzuki@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-16list-objects-filter: delay parsing of sparse oidJeff King
The list-objects-filter code has two steps to its initialization: 1. parse_list_objects_filter() makes sure the spec is a filter we know about and is syntactically correct. This step is done by "rev-list" or "upload-pack" that is going to apply a filter, but also by "git clone" or "git fetch" before they send the spec across the wire. 2. list_objects_filter__init() runs the type-specific initialization (using function pointers established in step 1). This happens at the start of traverse_commit_list_filtered(), when we're about to actually use the filter. It's a good idea to parse as much as we can in step 1, in order to catch problems early (e.g., a blob size limit that isn't a number). But one thing we _shouldn't_ do is resolve any oids at that step (e.g., for sparse-file contents specified by oid). In the case of a fetch, the oid has to be resolved on the remote side. The current code does resolve the oid during the parse phase, but ignores any error (which we must do, because we might just be sending the spec across the wire). This leads to two bugs: - if we're not in a repository (e.g., because it's git-clone parsing the spec), then we trigger a BUG() trying to resolve the name - if we did hit the error case, we still have to notice that later and bail. The code path in rev-list handles this, but the one in upload-pack does not, leading to a segfault. We can fix both by moving the oid resolution into the sparse-oid init function. At that point we know we have a repository (because we're about to traverse), and handling the error there fixes the segfault. As a bonus, we can drop the NULL sparse_oid_value check in rev-list, since this is now handled in the sparse-oid-filter init function. Signed-off-by: Jeff King <peff@peff.net> Acked-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-09Merge branch 'cb/fetch-set-upstream'Junio C Hamano
"git fetch" learned "--set-upstream" option to help those who first clone from their private fork they intend to push to, add the true upstream via "git remote add" and then "git fetch" from it. * cb/fetch-set-upstream: pull, fetch: add --set-upstream option