summaryrefslogtreecommitdiff
path: root/t
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'jk/close-duped-fd-before-unlock-for-bundle'Junio C Hamano2018-11-181-0/+6
|\ | | | | | | | | | | | | | | | | | | | | When "git bundle" aborts due to an empty commit ranges (i.e. resulting in an empty pack), it left a file descriptor to an lockfile open, which resulted in leftover lockfile on Windows where you cannot remove a file with an open file descriptor. This has been corrected. * jk/close-duped-fd-before-unlock-for-bundle: bundle: dup() output descriptor closer to point-of-use
| * bundle: dup() output descriptor closer to point-of-useJeff King2018-11-171-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When writing a bundle to a file, the bundle code actually creates "your.bundle.lock" using our lockfile interface. We feed that output descriptor to a child git-pack-objects via run-command, which has the quirk that it closes the output descriptor in the parent. To avoid confusing the lockfile code (which still thinks the descriptor is valid), we dup() it, and operate on the duplicate. However, this has a confusing side effect: after the dup() but before we call pack-objects, we have _two_ descriptors open to the lockfile. If we call die() during that time, the lockfile code will try to clean up the partially-written file. It knows to close() the file before unlinking, since on some platforms (i.e., Windows) the open file would block the deletion. But it doesn't know about the duplicate descriptor. On Windows, triggering an error at the right part of the code will result in the cleanup failing and the lockfile being left in the filesystem. We can solve this by moving the dup() much closer to start_command(), shrinking the window in which we have the second descriptor open. It's easy to place this in such a way that no die() is possible. We could still die due to a signal in the exact wrong moment, but we already tolerate races there (e.g., a signal could come before we manage to put the file on the cleanup list in the first place). As a bonus, this shields create_bundle() itself from the duplicate-fd trick, and we can simplify its error handling (note that the lock rollback now happens unconditionally, but that's OK; it's a noop if we didn't open the lock in the first place). The included test uses an empty bundle to cause a failure at the right spot in the code, because that's easy to trigger (the other likely errors are write() problems like ENOSPC). Note that it would already pass on non-Windows systems (because they are happy to unlink an already-open file). Based-on-a-patch-by: Gaël Lhez <gael.lhez@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Tested-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/rebase-in-c-escape-hatch'Junio C Hamano2018-11-181-0/+4
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | The recently merged "rebase in C" has an escape hatch to use the scripted version when necessary, but it hasn't been documented, which has been corrected. * ab/rebase-in-c-escape-hatch: tests: add a special setup where rebase.useBuiltin is off rebase doc: document rebase.useBuiltin
| * | tests: add a special setup where rebase.useBuiltin is offÆvar Arnfjörð Bjarmason2018-11-161-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a GIT_TEST_REBASE_USE_BUILTIN=false test mode which is equivalent to running with rebase.useBuiltin=false. This is needed to spot that we're not introducing any regressions in the legacy rebase version while we're carrying both it and the new builtin version. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'js/rebase-am-options'Junio C Hamano2018-11-181-0/+7
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The way "git rebase" parses and forwards the command line options meant for underlying "git am" has been revamped, which fixed for options with parameters that were not passed correctly. * js/rebase-am-options: rebase: validate -C<n> and --whitespace=<mode> parameters early rebase: really just passthru the `git am` options
| * | | rebase: validate -C<n> and --whitespace=<mode> parameters earlyJohannes Schindelin2018-11-161-0/+7
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It is a good idea to error out early upon seeing, say, `-Cbad`, rather than starting the rebase only to have the `--am` backend complain later. Let's do this. The only options accepting parameters which we pass through to `git am` (which may, or may not, forward them to `git apply`) are `-C` and `--whitespace`. The other options we pass through do not accept parameters, so we do not have to validate them here. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'sg/ref-filter-wo-repository'Junio C Hamano2018-11-181-0/+6
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git ls-remote --sort=<thing>" can feed an object that is not yet available into the comparison machinery and segfault, which has been corrected to check such a request upfront and reject it. * sg/ref-filter-wo-repository: ref-filter: don't look for objects when outside of a repository
| * | | ref-filter: don't look for objects when outside of a repositorySZEDER Gábor2018-11-161-0/+6
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The command 'git ls-remote --sort=authordate <remote>' segfaults when run outside of a repository, ever since the introduction of its '--sort' option in 1fb20dfd8e (ls-remote: create '--sort' option, 2018-04-09). While in general the 'git ls-remote' command can be run outside of a repository just fine, its '--sort=<key>' option with certain keys does require access to the referenced objects. This sorting is implemented using the generic ref-filter sorting facility, which already handles missing objects gracefully with the appropriate 'missing object deadbeef for HEAD' message. However, being generic means that it checks replace refs while trying to retrieve an object, and while doing so it accesses the 'git_replace_ref_base' variable, which has not been initialized and is still a NULL pointer when outside of a repository, thus causing the segfault. Make ref-filter more careful upfront while parsing the format string, and make it error out when encountering a format atom requiring object access when we are not in a repository. Also add a test to ensure that 'git ls-remote --sort' fails gracefully when executed outside of a repository. Reported-by: H.Merijn Brand <h.m.brand@xs4all.nl> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'ag/p3400-force-checkout'Junio C Hamano2018-11-181-5/+5
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | Perf test tweak. * ag/p3400-force-checkout: p3400: replace calls to `git checkout -b' by `git checkout -B'
| * | | p3400: replace calls to `git checkout -b' by `git checkout -B'Alban Gruin2018-11-121-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | p3400 makes a copy of the current repository to test git-rebase performance, and creates new branches in the copy with `git checkout -b'. If the original repository has branches with the same name as the script is trying to create, this operation will fail. This replaces these calls by `git checkout -B' to force the creation and update of these branches. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'js/rebase-r-and-merge-head'Junio C Hamano2018-11-181-0/+16
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Bugfix for the recently graduated "git rebase --rebase-merges". * js/rebase-r-and-merge-head: status: rebase and merge can be in progress at the same time built-in rebase --skip/--abort: clean up stale .git/<name> files rebase -i: include MERGE_HEAD into files to clean up rebase -r: do not write MERGE_HEAD unless needed rebase -r: demonstrate bug with conflicting merges
| * | | | rebase -r: do not write MERGE_HEAD unless neededJohannes Schindelin2018-11-131-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we detect that a `merge` can be skipped because the merged commit is already an ancestor of HEAD, we do not need to commit, therefore writing the MERGE_HEAD file is useless. It is actually worse than useless: a subsequent `git commit` will pick it up and think that we want to merge that commit, still. To avoid that, move the code that writes the MERGE_HEAD file to a location where we already know that the `merge` cannot be skipped. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | rebase -r: demonstrate bug with conflicting mergesJohannes Schindelin2018-11-131-0/+16
| |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When calling `merge` on a branch that has already been merged, that `merge` is skipped quietly, but currently a MERGE_HEAD file is being left behind and will then be grabbed by the next `pick` (that did not want to create a *merge* commit). Demonstrate this. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'js/apply-recount-allow-noop'Junio C Hamano2018-11-181-0/+12
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When editing a patch in a "git add -i" session, a hunk could be made to no-op. The "git apply" program used to reject a patch with such a no-op hunk to catch user mistakes, but it is now updated to explicitly allow a no-op hunk in an edited patch. * js/apply-recount-allow-noop: apply --recount: allow "no-op hunks"
| * | | | apply --recount: allow "no-op hunks"Johannes Schindelin2018-11-131-0/+12
| | |_|/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When editing patches e.g. in `git add -e`, it is quite common that a hunk ends up having no -/+ lines, i.e. it is now supposed to do nothing. This use case was broken by ad6e8ed37bc1 (apply: reject a hunk that does not do anything, 2015-06-01) with the good intention of catching a very real, different issue in hand-edited patches. So let's use the `--recount` option as the tell-tale whether the user would actually be okay with no-op hunks. Add a test case to make sure that this use case does not regress again. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'ra/rev-parse-exclude-glob'Junio C Hamano2018-11-181-3/+57
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "rev-parse --exclude=<pattern> --branches=<pattern>" etc. did not quite work, which has been corrected. * ra/rev-parse-exclude-glob: refs: fix some exclude patterns being ignored refs: show --exclude failure with --branches/tags/remotes=glob
| * | | | refs: fix some exclude patterns being ignoredRafael Ascensão2018-11-131-12/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `--exclude` from rev-list and rev-parse fails to exclude references if the next `--branches`, `--tags` or `--remotes` use the optional inclusive glob because those options are implemented as particular cases of `--glob=`, which itself requires that exclude patterns begin with 'refs/'. But it makes sense for `--branches=glob` and friends to be aware that exclusions patterns for them shouldn't be 'refs/<type>/' prefixed, the same way exclude patterns for `--branches` and friends (without the optional glob) already are. Let's record in 'refs.c:struct ref_filter' which context the exclude pattern is tied to, so refs.c:filter_refs() can decide if it should ignore the prefix when trying to match. Signed-off-by: Rafael Ascensão <rafa.almas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | refs: show --exclude failure with --branches/tags/remotes=globRafael Ascensão2018-11-131-3/+57
| |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The documentation of `--exclude=` option from rev-list and rev-parse explicitly states that exclude patterns *should not* start with 'refs/' when used with `--branches`, `--tags` or `--remotes`. However, following this advice results in refereces not being excluded if the next `--branches`, `--tags`, `--remotes` use the optional inclusive glob. Demonstrate this failure. Signed-off-by: Rafael Ascensão <rafa.almas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'js/rebase-autostash-detach-fix'Junio C Hamano2018-11-181-0/+8
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git rebase --autostash" did not correctly re-attach the HEAD at times. * js/rebase-autostash-detach-fix: built-in rebase --autostash: leave the current branch alone if possible built-in rebase: demonstrate regression with --autostash
| * | | | built-in rebase --autostash: leave the current branch alone if possibleJohannes Schindelin2018-11-081-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we converted a `git reset --hard` call in the original Unix shell script to built-in code, we asked to reset the worktree and the index and explicitly *not* to detach the HEAD. By mistake, though, we still did. Let's fix this. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | built-in rebase: demonstrate regression with --autostashJohannes Schindelin2018-11-081-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | An unnamed colleague of Ævar Arnfjörð Bjarmason reported a breakage where a `pull --rebase` (which did not really need to do anything but stash, see that nothing was changed, and apply the stash again) also detached the HEAD. This patch adds a minimal reproducer for this regression. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'ab/range-diff-no-patch'Junio C Hamano2018-11-181-0/+30
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The "--no-patch" option, which can be used to get a high-level overview without the actual line-by-line patch difference shown, of the "range-diff" command was earlier broken, which has been corrected. * ab/range-diff-no-patch: range-diff: make diff option behavior (e.g. --stat) consistent range-diff: fix regression in passing along diff options range-diff doc: add a section about output stability
| * | | | | range-diff: make diff option behavior (e.g. --stat) consistentÆvar Arnfjörð Bjarmason2018-11-141-23/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Make the behavior when diff options (e.g. "--stat") are passed consistent with how "diff" behaves. Before 73a834e9e2 ("range-diff: relieve callers of low-level configuration burden", 2018-07-22) running range-diff with "--stat" would produce stat output and the diff output, as opposed to how "diff" behaves where once "--stat" is specified "--patch" also needs to be provided to emit the patch output. As noted in a previous change ("range-diff doc: add a section about output stability", 2018-11-07) the "--stat" output with "range-diff" is useless at the moment. But we should behave consistently with "diff" in anticipation of such output being useful in the future, because it would make for confusing UI if "diff" and "range-diff" behaved differently when it came to how they interpret diff options. The new behavior is also consistent with the existing documentation added in ba931edd28 ("range-diff: populate the man page", 2018-08-13). See "[...]also accepts the regular diff options[...]" in git-range-diff(1). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | range-diff: fix regression in passing along diff optionsÆvar Arnfjörð Bjarmason2018-11-121-0/+53
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In 73a834e9e2 ("range-diff: relieve callers of low-level configuration burden", 2018-07-22) we broke passing down options like --no-patch, --stat etc. Fix that regression, and add a test asserting the pre-73a834e9e2 behavior for some of these diff options. As noted in a change leading up to this ("range-diff doc: add a section about output stability", 2018-11-07) the output is not meant to be stable. So this regression test will likely need to be tweaked once we get a "proper" --stat option. See https://public-inbox.org/git/nycvar.QRO.7.76.6.1811071202480.39@tvgsbejvaqbjf.bet/ for a further explanation of the regression. The fix here is not the same as in Johannes's on-list patch, for reasons that'll be explained in a follow-up commit. The quoting of "EOF" here mirrors that of an earlier test. Perhaps that should be fixed, but let's leave that up to a later cleanup change. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'jk/verify-sig-merge-into-void'Junio C Hamano2018-11-182-0/+14
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git merge" and "git pull" that merges into an unborn branch used to completely ignore "--verify-signatures", which has been corrected. * jk/verify-sig-merge-into-void: pull: handle --verify-signatures for unborn branch merge: handle --verify-signatures for unborn branch merge: extract verify_merge_signature() helper
| * | | | | | pull: handle --verify-signatures for unborn branchJeff King2018-11-071-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We usually just forward the --verify-signatures option along to git-merge, and trust it to do the right thing. However, when we are on an unborn branch (i.e., there is no HEAD yet), we handle this case ourselves without even calling git-merge. And in this code path, we do not respect the verification option at all. It may be more maintainable in the long run to call git-merge for the unborn case. That would fix this bug, as well as prevent similar ones in the future. But unfortunately it's not easy to do. As t5520.3 demonstrates, there are some special cases that git-merge does not handle, like "git pull .. master:master" (by the time git-merge is invoked, we've overwritten the unborn HEAD). So for now let's just teach git-pull to handle this feature. Reported-by: Felix Eckhofer <felix@eckhofer.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | merge: handle --verify-signatures for unborn branchJeff King2018-11-071-0/+7
| | |_|/ / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When git-merge sees that we are on an unborn branch (i.e., there is no HEAD), it follows a totally separate code path than the usual merge logic. This code path does not know about verify_signatures, and so we fail to notice bad or missing signatures. This has been broken since --verify-signatures was added in efed002249 (merge/pull: verify GPG signatures of commits being merged, 2013-03-31). In an ideal world, we'd unify the flow for this case with the regular merge logic, which would fix this bug and avoid introducing similar ones. But because the unborn case is so different, it would be a burden on the rest of the function to continually handle the missing HEAD. So let's just port the verification check to this special case. Reported-by: Felix Eckhofer <felix@eckhofer.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'jk/unused-parameter-fixes'Junio C Hamano2018-11-182-23/+1
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Various functions have been audited for "-Wunused-parameter" warnings and bugs in them got fixed. * jk/unused-parameter-fixes: midx: double-check large object write loop assert NOARG/NONEG behavior of parse-options callbacks parse-options: drop OPT_DATE() apply: return -1 from option callback instead of calling exit(1) cat-file: report an error on multiple --batch options tag: mark "--message" option with NONEG show-branch: mark --reflog option as NONEG format-patch: mark "--no-numbered" option with NONEG status: mark --find-renames option with NONEG cat-file: mark batch options with NONEG pack-objects: mark index-version option as NONEG ls-files: mark exclude options as NONEG am: handle --no-patch-format option apply: mark include/exclude options as NONEG
| * | | | | | assert NOARG/NONEG behavior of parse-options callbacksJeff King2018-11-061-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we define a parse-options callback, the flags we put in the option struct must match what the callback expects. For example, a callback which does not handle the "unset" parameter should only be used with PARSE_OPT_NONEG. But since the callback and the option struct are not defined next to each other, it's easy to get this wrong (as earlier patches in this series show). Fortunately, the compiler can help us here: compiling with -Wunused-parameters can show us which callbacks ignore their "unset" parameters (and likewise, ones that ignore "arg" expect to be triggered with PARSE_OPT_NOARG). But after we've inspected a callback and determined that all of its callers use the right flags, what do we do next? We'd like to silence the compiler warning, but do so in a way that will catch any wrong calls in the future. We can do that by actually checking those variables and asserting that they match our expectations. Because this is such a common pattern, we'll introduce some helper macros. The resulting messages aren't as descriptive as we could make them, but the file/line information from BUG() is enough to identify the problem (and anyway, the point is that these should never be seen). Each of the annotated callbacks in this patch triggers -Wunused-parameters, and was manually inspected to make sure all callers use the correct options (so none of these BUGs should be triggerable). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | parse-options: drop OPT_DATE()Jeff King2018-11-062-23/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There are no users of OPT_DATE except for test-parse-options; its only caller went away in 27ec394a97 (prune: introduce OPT_EXPIRY_DATE() and use it, 2013-04-25). It also has a bug: it does not specify PARSE_OPT_NONEG, but its callback does not respect the "unset" flag, and will feed NULL to approxidate() and segfault. Probably this should be marked with NONEG, or the callback should set the timestamp to some sentinel value (e.g,. "0", or "(time_t)-1"). But since there are no callers, deleting it means we don't even have to think about what the right behavior should be. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'mg/gpg-fingerprint-test'Junio C Hamano2018-11-182-14/+70
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a few tests for a topic already in 'master'. * mg/gpg-fingerprint-test: t/t7510-signed-commit.sh: add signing subkey to Eris Discordia key t/t7510-signed-commit.sh: Add %GP to custom format checks
| * | | | | | | t/t7510-signed-commit.sh: add signing subkey to Eris Discordia keyMichał Górny2018-11-052-9/+59
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a dedicated signing subkey to the key identified as 'Eris Discordia', and update tests appropriately. GnuPG will now sign commits using the dedicated signing subkey, changing the value of %GK and %GF, and effectively creating a test case for %GF!=%GP. Signed-off-by: Michał Górny <mgorny@gentoo.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | t/t7510-signed-commit.sh: Add %GP to custom format checksMichał Górny2018-11-051-6/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Test %GP in addition to %GF in custom format checks. With current keyring, both have the same value. Signed-off-by: Michał Górny <mgorny@gentoo.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | Merge branch 'ds/reachable-topo-order'Junio C Hamano2018-11-184-21/+160
|\ \ \ \ \ \ \ \ | |_|_|_|_|_|_|/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The revision walker machinery learned to take advantage of the commit generation numbers stored in the commit-graph file. * ds/reachable-topo-order: t6012: make rev-list tests more interesting revision.c: generation-based topo-order algorithm commit/revisions: bookkeeping before refactoring revision.c: begin refactoring --topo-order logic test-reach: add rev-list tests test-reach: add run_three_modes method prio-queue: add 'peek' operation
| * | | | | | | t6012: make rev-list tests more interestingDerrick Stolee2018-11-021-9/+36
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As we are working to rewrite some of the revision-walk machinery, there could easily be some interesting interactions between the options that force topological constraints (--topo-order, --date-order, and --author-date-order) along with specifying a path. Add extra tests to t6012-rev-list-simplify.sh to add coverage of these interactions. To ensure interesting things occur, alter the repo data shape to have different orders depending on topo-, date-, or author-date-order. When testing using GIT_TEST_COMMIT_GRAPH, this assists in covering the new logic for topo-order walks using generation numbers. The extra tests can be added indepently. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | test-reach: add rev-list testsDerrick Stolee2018-11-021-0/+84
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The rev-list command is critical to Git's functionality. Ensure it works in the three commit-graph environments constructed in t6600-test-reach.sh. Here are a few important types of rev-list operations: * Basic: git rev-list --topo-order HEAD * Range: git rev-list --topo-order compare..HEAD * Ancestry: git rev-list --topo-order --ancestry-path compare..HEAD * Symmetric Difference: git rev-list --topo-order compare...HEAD Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | test-reach: add run_three_modes methodDerrick Stolee2018-11-021-4/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The 'test_three_modes' method assumes we are using the 'test-tool reach' command for our test. However, we may want to use the data shape of our commit graph and the three modes (no commit-graph, full commit-graph, partial commit-graph) for other git commands. Split test_three_modes to be a simple translation on a more general run_three_modes method that executes the given command and tests the actual output to the expected output. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | prio-queue: add 'peek' operationDerrick Stolee2018-11-022-8/+32
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When consuming a priority queue, it can be convenient to inspect the next object that will be dequeued without actually dequeueing it. Our existing library did not have such a 'peek' operation, so add it as prio_queue_peek(). Add a reference-level comparison in t/helper/test-prio-queue.c so this method is exercised by t0009-prio-queue.sh. Further, add a test that checks the behavior when the compare function is NULL (i.e. the queue becomes a stack). Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | Merge branch 'nd/tree-walk-path-exclusion'Junio C Hamano2018-11-131-0/+17
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Pathspec matching against a tree object were buggy when negative pathspec elements were involved, which has been fixed. * nd/tree-walk-path-exclusion: tree-walk.c: fix overoptimistic inclusion in :(exclude) matching
| * | | | | | | | tree-walk.c: fix overoptimistic inclusion in :(exclude) matchingNguyễn Thái Ngọc Duy2018-11-051-0/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | tree_entry_interesting() is used for matching pathspec on a tree. The interesting thing about this function is that, because the tree entries are known to be sorted, this function can return more than just "yes, matched" and "no, not matched". It can also say "yes, this entry is matched and so is the remaining entries in the tree". This is where I made a mistake when matching exclude pathspec. For exclude pathspec, we do matching twice, one with positive patterns and one with negative ones, then a rule table is applied to determine the final "include or exclude" result. Note that "matched" does not necessarily mean include. For negative patterns, "matched" means exclude. This particular rule is too eager to include everything. Rule 8 says that "if all entries are positively matched" and the current entry is not negatively matched (i.e. not excluded), then all entries are positively matched and therefore included. But this is not true. If the _current_ entry is not negatively matched, it does not mean the next one will not be and we cannot conclude right away that all remaining entries are positively matched and can be included. Rules 8 and 18 are now updated to be less eager. We conclude that the current entry is positively matched and included. But we say nothing about remaining entries. tree_entry_interesting() will be called again for those entries where we will determine entries individually. Reported-by: Christophe Bliard <christophe.bliard@trux.info> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | | Merge branch 'jk/misc-unused-fixes'Junio C Hamano2018-11-132-0/+9
|\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Assorted fixes for bugs found while auditing -Wunused-parameter warnings. * jk/misc-unused-fixes: approxidate: fix NULL dereference in date_time() pathspec: handle non-terminated strings with :(attr) approxidate: handle pending number for "specials" rev-list: handle flags for --indexed-objects
| * | | | | | | | | approxidate: fix NULL dereference in date_time()Jeff King2018-11-071-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we see a time like "noon", we pass "12" to our date_time() helper, which sets the hour to 12pm. If the current time is before noon, then we wrap around to yesterday using date_yesterday(). But unlike the normal calls to date_yesterday() from approxidate_alpha(), we pass a NULL "num" parameter. Since c27cc94fad (approxidate: handle pending number for "specials", 2018-11-02), that causes a segfault. One way to fix this is by checking for NULL. But arguably date_time() is abusing our helper by passing NULL in the first place (and this is the only case where one of these "special" parsers is used this way). So instead, let's have it just do the 1-day subtraction itself. It's still just a one-liner due to our update_tm() helper. Note that the test added here is a little funny, as we say "10am noon", which makes the "10am" seem pointless. But this bug can only be triggered when it the currently-parsed hour is before the special time. The latest special time is "tea" at 1700, but t0006 uses a hard-coded TEST_DATE_NOW of 1900. We could reset TEST_DATE_NOW, but that may lead to confusion in other tests. Just saying "10am noon" makes this test self-contained. Reported-by: Carlo Arenas <carenas@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | approxidate: handle pending number for "specials"Jeff King2018-11-021-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The approxidate parser has a table of special keywords like "yesterday", "noon", "pm", etc. Some of these, like "pm", do the right thing if we've recently seen a number: "3pm" is what you'd think. However, most of them do not look at or modify the pending-number flag at all, which means a number may "jump" across a significant keyword and be used unexpectedly. For example, when parsing: January 5th noon pm we'd connect the "5" to "pm", and ignore it as a day-of-month. This is obviously a bit silly, as "noon" already implies "pm". And other mis-parsed things are generally as silly ("January 5th noon, years ago" would connect the 5 to "years", but probably nobody would type that). However, the fix is simple: when we see a keyword like "noon", we should flush the pending number (as we would if we hit another number, or the end of the string). In a few of the specials that actually modify the day, we can simply throw away the number (saying "Jan 5 yesterday" should not respect the number at all). Note that we have to either move or forward-declare the static pending_number() to make it accessible to these functions; this patch moves it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | rev-list: handle flags for --indexed-objectsJeff King2018-11-021-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a traversal sees the --indexed-objects option, it adds all blobs and valid cache-trees from the index to the traversal using add_index_objects_to_pending(). But that function totally ignores its flags parameter! That means that doing: git rev-list --objects --indexed-objects and git rev-list --objects --not --indexed-objects produce the same output, because we ignore the UNINTERESTING flag when walking the index in the second example. Nobody noticed because this feature was added as a way for tools like repack to increase their coverage of reachable objects, meaning it would only be used like the first example above. But since it's user facing (and because the documentation describes it "as if the objects are listed on the command line"), we should make sure the negative case behaves sensibly. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | | | Merge branch 'nd/per-worktree-ref-iteration'Junio C Hamano2018-11-134-0/+131
|\ \ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The code to traverse objects for reachability, used to decide what objects are unreferenced and expendable, have been taught to also consider per-worktree refs of other worktrees as starting points to prevent data loss. * nd/per-worktree-ref-iteration: git-worktree.txt: correct linkgit command name reflog expire: cover reflog from all worktrees fsck: check HEAD and reflog from other worktrees fsck: move fsck_head_link() to get_default_heads() to avoid some globals revision.c: better error reporting on ref from different worktrees revision.c: correct a parameter name refs: new ref types to make per-worktree refs visible to all worktrees Add a place for (not) sharing stuff between worktrees refs.c: indent with tabs, not spaces
| * | | | | | | | | | reflog expire: cover reflog from all worktreesNguyễn Thái Ngọc Duy2018-10-221-0/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Reported-by: Jeff King <peff@peff.net> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | fsck: check HEAD and reflog from other worktreesNguyễn Thái Ngọc Duy2018-10-221-0/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | fsck is a repo-wide operation and should check all references no matter which worktree they are associated to. Reported-by: Jeff King <peff@peff.net> Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | refs: new ref types to make per-worktree refs visible to all worktreesNguyễn Thái Ngọc Duy2018-10-221-0/+46
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | One of the problems with multiple worktree is accessing per-worktree refs of one worktree from another worktree. This was sort of solved by multiple ref store, where the code can open the ref store of another worktree and has access to the ref space of that worktree. The problem with this is reporting. "HEAD" in another ref space is also called "HEAD" like in the current ref space. In order to differentiate them, all the code must somehow carry the ref store around and print something like "HEAD from this ref store". But that is not feasible (or possible with a _lot_ of work). With the current design, we pass a reference around as a string (so called "refname"). Extending this design to pass a string _and_ a ref store is a nightmare, especially when handling extended SHA-1 syntax. So we do it another way. Instead of entering a separate ref space, we make refs from other worktrees available in the current ref space. So "HEAD" is always HEAD of the current worktree, but then we can have "worktrees/blah/HEAD" to denote HEAD from a worktree named "blah". This syntax coincidentally matches the underlying directory structure which makes implementation a bit easier. The main worktree has to be treated specially because well... it's special from the beginning. So HEAD from the main worktree is acccessible via the name "main-worktree/HEAD" instead of "worktrees/main/HEAD" because "main" could be just another secondary worktree. This patch also makes it possible to specify refs from one worktree in another one, e.g. git log worktrees/foo/HEAD Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | Add a place for (not) sharing stuff between worktreesNguyễn Thái Ngọc Duy2018-10-072-0/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When multiple worktrees are used, we need rules to determine if something belongs to one worktree or all of them. Instead of keeping adding rules when new stuff comes (*), have a generic rule: - Inside $GIT_DIR, which is per-worktree by default, add $GIT_DIR/common which is always shared. New features that want to share stuff should put stuff under this directory. - Inside refs/, which is shared by default except refs/bisect, add refs/worktree/ which is per-worktree. We may eventually move refs/bisect to this new location and remove the exception in refs code. (*) And it may also include stuff from external commands which will have no way to modify common/per-worktree rules. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | | | | Merge branch 'al/send-email-auto-cte-fixup'Junio C Hamano2018-11-131-0/+15
|\ \ \ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git send-email --transfer-encoding=..." in recent versions of Git sometimes produced an empty "Content-Transfer-Encoding:" header, which has been corrected. * al/send-email-auto-cte-fixup: send-email: avoid empty transfer encoding header