aboutsummaryrefslogtreecommitdiff
path: root/sequencer.c
AgeCommit message (Collapse)Author
2022-09-26sequencer: avoid dropping fixup commit that targets self via commit-ishJohannes Altmanninger
Commit 68d5d03bc4 (rebase: teach --autosquash to match on sha1 in addition to message, 2010-11-04) taught autosquash to recognize subjects like "fixup! 7a235b" where 7a235b is an OID-prefix. It actually did more than advertised: 7a235b can be an arbitrary commit-ish (as long as it's not trailed by spaces). Accidental(?) use of this secret feature revealed a bug where we would silently drop a fixup commit. The bug can also be triggered when using an OID-prefix but that's unlikely in practice. Let the commit with subject "fixup! main" be the tip of the "main" branch. When computing the fixup target for this commit, we find the commit itself. This is wrong because, by definition, a fixup target must be an earlier commit in the todo list. We wrongly find the current commit because we added it to the todo list prematurely. Avoid these fixup-cycles by only adding the current commit to the todo list after we have finished looking for the fixup target. Reported-by: Erik Cervin Edin <erik@cervined.in> Signed-off-by: Johannes Altmanninger <aclopte@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-14Merge branch 'ab/unused-annotation'Junio C Hamano
Undoes 'jk/unused-annotation' topic and redoes it to work around Coccinelle rules misfiring false positives in unrelated codepaths. * ab/unused-annotation: git-compat-util.h: use "deprecated" for UNUSED variables git-compat-util.h: use "UNUSED", not "UNUSED(var)"
2022-09-14Merge branch 'jk/unused-annotation'Junio C Hamano
Annotate function parameters that are not used (but cannot be removed for structural reasons), to prepare us to later compile with -Wunused warning turned on. * jk/unused-annotation: is_path_owned_by_current_uid(): mark "report" parameter as unused run-command: mark unused async callback parameters mark unused read_tree_recursive() callback parameters hashmap: mark unused callback parameters config: mark unused callback parameters streaming: mark unused virtual method parameters transport: mark bundle transport_options as unused refs: mark unused virtual method parameters refs: mark unused reflog callback parameters refs: mark unused each_ref_fn parameters git-compat-util: add UNUSED macro
2022-09-01git-compat-util.h: use "UNUSED", not "UNUSED(var)"Ævar Arnfjörð Bjarmason
As reported in [1] the "UNUSED(var)" macro introduced in 2174b8c75de (Merge branch 'jk/unused-annotation' into next, 2022-08-24) breaks coccinelle's parsing of our sources in files where it occurs. Let's instead partially go with the approach suggested in [2] of making this not take an argument. As noted in [1] "coccinelle" will ignore such tokens in argument lists that it doesn't know about, and it's less of a surprise to syntax highlighters. This undoes the "help us notice when a parameter marked as unused is actually use" part of 9b240347543 (git-compat-util: add UNUSED macro, 2022-08-19), a subsequent commit will further tweak the macro to implement a replacement for that functionality. 1. https://lore.kernel.org/git/220825.86ilmg4mil.gmgdl@evledraar.gmail.com/ 2. https://lore.kernel.org/git/220819.868rnk54ju.gmgdl@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-19sequencer: do not translate command namesMichael J Gruber
When action_name is used to denote a command `git %s` do not translate since command names are never translated. Suggested-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Michael J Gruber <git@grubix.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-19sequencer: do not translate parameters to error_resolve_conflict()Michael J Gruber
`error_resolve_conflict()` checks the untranslated action_name parameter, so pass it as is. Suggested-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Michael J Gruber <git@grubix.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-19sequencer: do not translate reflog messagesMichael J Gruber
Traditionally, reflog messages were never translated, in particular not on storage. Due to the switch of more parts of git to the sequencer, old changes in the sequencer code may lead to recent changes in git's behaviour. E.g.: c28cbc5ea6 ("sequencer: mark action_name() for translation", 2016-10-21) marked several uses of `action_name()` for translation. Recently, this lead to a partially translated reflog: `rebase: fast-forward` is translated (e.g. in de to `Rebase: Vorspulen`) whereas other reflog entries such as `rebase (pick):` remain untranslated as they should be. Change the relevant line in the sequencer so that this reflog entry remains untranslated, as well. Signed-off-by: Michael J Gruber <git@grubix.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-19hashmap: mark unused callback parametersJeff King
Hashmap comparison functions must conform to a particular callback interface, but many don't use all of their parameters. Especially the void cmp_data pointer, but some do not use keydata either (because they can easily form a full struct to pass when doing lookups). Let's mark these to make -Wunused-parameter happy. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19sequencer: notify user of --update-refs activityDerrick Stolee
When the user runs 'git rebase -i --update-refs', the end message still says only Successfully rebased and updated <HEAD-ref>. Update the sequencer to collect the successful (and unsuccessful) ref updates due to the --update-refs option, so the end message now says Successfully rebased and updated <HEAD-ref>. Updated the following refs with --update-refs: refs/heads/first refs/heads/third Failed to update the following refs with --update-refs: refs/heads/second To test this output, we need to be very careful to format the expected error to drop the leading tab characters. Also, we need to be aware that the verbose output from 'git rebase' is writing progress lines which don't use traditional newlines but clear the line after every progress item is complete. When opening the error file in an editor, these lines are visible, but when looking at the diff in a terminal those lines disappear because of the characters that delete the previous characters. Use 'sed' to clear those progress lines and clear the tabs so we can get an exact match on our expected output. Reported-by: Elijah Newren <newren@gmail.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19sequencer: ignore HEAD ref under --update-refsDerrick Stolee
When using the 'git rebase -i --update-refs' option, the todo list is populated with 'update-ref' commands for all tip refs in the history that is being rebased. Refs that are checked out by some worktree are instead added as a comment to warn the user that they will not be updated. Until now, this included the HEAD ref, which is being updated by the rebase process itself, regardless of the --update-refs option. Remove the comment in this case by ignoring any decorations that match the HEAD ref. Reported-by: Elijah Newren <newren@gmail.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19sequencer: rewrite update-refs as user edits todo listDerrick Stolee
An interactive rebase provides opportunities for the user to edit the todo list. The --update-refs option initializes the list with some 'update-ref <ref>' steps, but the user could add these manually. Further, the user could add or remove these steps during pauses in the interactive rebase. Add a new method, todo_list_filter_update_refs(), that scans a todo_list and compares it to the stored update-refs file. There are two actions that can happen at this point: 1. If a '<ref>/<before>/<after>' triple in the update-refs file does not have a matching 'update-ref <ref>' command in the todo-list _and_ the <after> value is the null OID, then remove that triple. Here, the user removed the 'update-ref <ref>' command before it was executed, since if it was executed then the <after> value would store the commit at that position. 2. If a 'update-ref <ref>' command in the todo-list does not have a matching '<ref>/<before>/<after>' triple in the update-refs file, then insert a new one. Store the <before> value to be the current OID pointed at by <ref>. This is handled inside of the init_update_ref_record() helper method. We can test that this works by rewriting the todo-list several times in the course of a rebase. Check that each ref is locked or unlocked for updates after each todo-list update. We can also verify that the ref update fails if a concurrent process updates one of the refs after the rebase process records the "locked" ref location. To help these tests, add a new 'set_replace_editor' helper that will replace the todo-list with an exact file. Reported-by: Phillip Wood <phillip.wood123@gmail.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19rebase: update refs from 'update-ref' commandsDerrick Stolee
The previous change introduced the 'git rebase --update-refs' option which added 'update-ref <ref>' commands to the todo list of an interactive rebase. Teach Git to record the HEAD position when reaching these 'update-ref' commands. The ref/before/after triple is stored in the $GIT_DIR/rebase-merge/update-refs file. A previous change parsed this file to avoid having other processes updating the refs in that file while the rebase is in progress. Not only do we update the file when the sequencer reaches these 'update-ref' commands, we then update the refs themselves at the end of the rebase sequence. If the rebase is aborted before this final step, then the refs are not updated. The 'before' value is used to ensure that we do not accidentally obliterate a ref that was updated concurrently (say, by an older version of Git or a third-party tool). Now that the 'git rebase --update-refs' command is implemented to write to the update-refs file, we can remove the fake construction of the update-refs file from a test in t2407-worktree-heads.sh. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19rebase: add --update-refs optionDerrick Stolee
When working on a large feature, it can be helpful to break that feature into multiple smaller parts that become reviewed in sequence. During development or during review, a change to one part of the feature could affect multiple of these parts. An interactive rebase can help adjust the multi-part "story" of the branch. However, if there are branches tracking the different parts of the feature, then rebasing the entire list of commits can create commits not reachable from those "sub branches". It can take a manual step to update those branches. Add a new --update-refs option to 'git rebase -i' that adds 'update-ref <ref>' steps to the todo file whenever a commit that is being rebased is decorated with that <ref>. At the very end, the rebase process updates all of the listed refs to the values stored during the rebase operation. Be sure to iterate after any squashing or fixups are placed. Update the branch only after those squashes and fixups are complete. This allows a --fixup commit at the tip of the feature to apply correctly to the sub branch, even if it is fixing up the most-recent commit in that part. This change update the documentation and builtin to accept the --update-refs option as well as updating the todo file with the 'update-ref' commands. Tests are added to ensure that these todo commands are added in the correct locations. This change does _not_ include the actual behavior of tracking the updated refs and writing the new ref values at the end of the rebase process. That is deferred to a later change. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19sequencer: add update-ref commandDerrick Stolee
Add the boilerplate for an "update-ref" command in the sequencer. This connects to the current no-op do_update_ref() which will be filled in after more connections are created. The syntax in the todo list will be "update-ref <ref-name>" to signal that we should store the current commit as the value for updating <ref-name> at the end of the rebase. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19sequencer: define array with enum valuesDerrick Stolee
The todo_command_info array defines which strings match with which todo_command enum values. The array is defined in the same order as the enum values, but if one changed without the other, then we would have unexpected results. Make it easier to see changes to the enum and this array by using the enum values as the indices of the array. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19branch: consider refs under 'update-refs'Derrick Stolee
The branch_checked_out() helper helps commands like 'git branch' and 'git fetch' from overwriting refs that are currently checked out in other worktrees. A future update to 'git rebase' will introduce a new '--update-refs' option which will update the local refs that point to commits that are being rebased. To avoid collisions as the rebase completes, we want to make the future data store for these refs to be considered by branch_checked_out(). The data store is a plaintext file inside the 'rebase-merge' directory for that worktree. The file lists refnames followed by two OIDs, each on separate lines. The OIDs will be used to store the original values of the refs and the to-be-written values as the rebase progresses, but can be ignored at the moment. Create a new sequencer_get_update_refs_state() method that parses this file and populates a struct string_list with the ref-OID pairs. We can then use this list to add to the current_checked_out_branches strmap used by branch_checked_out(). To properly navigate to the rebase directory for a given worktree, extract the static strbuf_worktree_gitdir() method to a public API method. We can test that this works without having Git write this file by artificially creating one in our test script, at least until 'git rebase --update-refs' is implemented and we can use it directly. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-06-15Merge branch 'jc/revert-show-parent-info'Junio C Hamano
"git revert" learns "--reference" option to use more human-readable reference to the commit it reverts in the message template it prepares for the user. * jc/revert-show-parent-info: revert: --reference should apply only to 'revert', not 'cherry-pick' revert: optionally refer to commit in the "reference" format
2022-06-10Merge branch 'ab/env-array'Junio C Hamano
Rename .env_array member to .env in the child_process structure. * ab/env-array: run-command API users: use "env" not "env_array" in comments & names run-command API: rename "env_array" to "env"
2022-06-08Merge branch 'rs/commit-summary-wo-break-rewrite' into maintJunio C Hamano
The commit summary shown after making a commit is matched to what is given in "git status" not to use the break-rewrite heuristics. source: <c35bd0aa-2e46-e710-2b39-89f18bad0097@web.de> * rs/commit-summary-wo-break-rewrite: commit, sequencer: turn off break_opt for commit summary
2022-06-07Merge branch 'ab/plug-leak-in-revisions'Junio C Hamano
Plug the memory leaks from the trickiest API of all, the revision walker. * ab/plug-leak-in-revisions: (27 commits) revisions API: add a TODO for diff_free(&revs->diffopt) revisions API: have release_revisions() release "topo_walk_info" revisions API: have release_revisions() release "date_mode" revisions API: call diff_free(&revs->pruning) in revisions_release() revisions API: release "reflog_info" in release revisions() revisions API: clear "boundary_commits" in release_revisions() revisions API: have release_revisions() release "prune_data" revisions API: have release_revisions() release "grep_filter" revisions API: have release_revisions() release "filter" revisions API: have release_revisions() release "cmdline" revisions API: have release_revisions() release "mailmap" revisions API: have release_revisions() release "commits" revisions API users: use release_revisions() for "prune_data" users revisions API users: use release_revisions() with UNLEAK() revisions API users: use release_revisions() in builtin/log.c revisions API users: use release_revisions() in http-push.c revisions API users: add "goto cleanup" for release_revisions() stash: always have the owner of "stash_info" free it revisions API users: use release_revisions() needing REV_INFO_INIT revision.[ch]: document and move code declared around "init" ...
2022-06-02run-command API users: use "env" not "env_array" in comments & namesÆvar Arnfjörð Bjarmason
Follow-up on a preceding commit which changed all references to the "env_array" when referring to the "struct child_process" member. These changes are all unnecessary for the compiler, but help the code's human readers. All the comments that referred to "env_array" have now been updated, as well as function names and variables that had "env_array" in their name, they now refer to "env". In addition the "out" name for the submodule.h prototype was inconsistent with the function definition's use of "env_array" in submodule.c. Both of them use "env" now. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-06-02run-command API: rename "env_array" to "env"Ævar Arnfjörð Bjarmason
Start following-up on the rename mentioned in c7c4bdeccf3 (run-command API: remove "env" member, always use "env_array", 2021-11-25) of "env_array" to "env". The "env_array" name was picked in 19a583dc39e (run-command: add env_array, an optional argv_array for env, 2014-10-19) because "env" was taken. Let's not forever keep the oddity of "*_array" for this "struct strvec", but not for its "args" sibling. This commit is almost entirely made with a coccinelle rule[1]. The only manual change here is in run-command.h to rename the struct member itself and to change "env_array" to "env" in the CHILD_PROCESS_INIT initializer. The rest of this is all a result of applying [1]: * make contrib/coccinelle/run_command.cocci.patch * patch -p1 <contrib/coccinelle/run_command.cocci.patch * git add -u 1. cat contrib/coccinelle/run_command.pending.cocci @@ struct child_process E; @@ - E.env_array + E.env @@ struct child_process *E; @@ - E->env_array + E->env I've avoided changing any comments and derived variable names here, that will all be done in the next commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-05-31revert: --reference should apply only to 'revert', not 'cherry-pick'Junio C Hamano
As 'revert' and 'cherry-pick' share a lot of code, it is easy to modify the behaviour of one command and inadvertently affect the other. An earlier change to teach the '--reference' option and the 'revert.reference' configuration variable to the former was not careful enough and 'cherry-pick --reference' wasn't rejected as an error. It is possible to think 'cherry-pick -x' might benefit from the '--reference' option, but it is fundamentally different from 'revert' in at least two ways to make it questionable: - 'revert' names a commit that is ancestor of the resulting commit, so an abbreviated object name with human readable title is sufficient to identify the named commit uniquely without using the full object name. On the other hand, 'cherry-pick' usually [*] picks a commit that is not an ancestor. It might be even picking a private commit that never becomes part of the public history. - The whole commit message of 'cherry-pick' is a copy of the original commit, and there is nothing gained to repeat only the title part on 'cherry-picked from' message. [*] well, you could revert and then you can pick the original that was reverted to get back to where you were, but then you can revert the revert to do the same thing. Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-05-26revert: optionally refer to commit in the "reference" formatJunio C Hamano
A typical "git revert" commit uses the full title of the original commit in its title, and starts its body of the message with: This reverts commit 8fa7f667cf61386257c00d6e954855cc3215ae91. This does not encourage the best practice of describing not just "what" (i.e. "Revert X" on the title says what we did) but "why" (i.e. and it does not say why X was undesirable). We can instead phrase this first line of the body to be more like This reverts commit 8fa7f667 (do this and that, 2022-04-25) so that the title does not have to be Revert "do this and that" We can instead use the title to describe "why" we are reverting the original commit. Introduce the "--reference" option to "git revert", and also the revert.reference configuration variable, which defaults to false, to tweak the title and the first line of the draft commit message for when creating a "revert" commit. When this option is in use, the first line of the pre-filled editor buffer becomes a comment line that tells the user to say _why_. If the user exits the editor without touching this line by mistake, what we prepare to become the first line of the body, i.e. "This reverts commit 8fa7f667 (do this and that, 2022-04-25)", ends up to be the title of the resulting commit. This behaviour is designed to help such a user to identify such a revert in "git log --oneline" easily so that it can be further reworded with "git rebase -i" later. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-05-11Merge branch 'rs/commit-summary-wo-break-rewrite'Junio C Hamano
The commit summary shown after making a commit is matched to what is given in "git status" not to use the break-rewrite heuristics. * rs/commit-summary-wo-break-rewrite: commit, sequencer: turn off break_opt for commit summary
2022-04-13revisions API users: add "goto cleanup" for release_revisions()Ævar Arnfjörð Bjarmason
Add a release_revisions() to various users of "struct rev_info" which requires a minor refactoring to a "goto cleanup" pattern to use that function. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-04-13revisions API users: add straightforward release_revisions()Ævar Arnfjörð Bjarmason
Add a release_revisions() to various users of "struct rev_list" in those straightforward cases where we only need to add the release_revisions() call to the end of a block, and don't need to e.g. refactor anything to use a "goto cleanup" pattern. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-04-13Revert "Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'"Junio C Hamano
This reverts commit 991b4d47f0accd3955d05927d5ce434e03ffbdb6, reversing changes made to bcd020f88e1e22f38422ac3f73ab06b34ec4bef1.
2022-04-06commit, sequencer: turn off break_opt for commit summaryRené Scharfe
dc6b1d92ca (wt-status: use settings from git_diff_ui_config, 2018-05-04) disabled diffopt.break_opt for diffstats shown by git status and in commit templates. For git status there isn't even a way to enable it. Make the commit summary (shown after the commit) consistent by disabling it there as well. Reported-by: Laurent Lyaudet <laurent.lyaudet@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-23Merge branch 'js/in-place-reverse-in-sequencer'Junio C Hamano
Code clean-up. * js/in-place-reverse-in-sequencer: sequencer: use reverse_commit_list() helper
2022-03-16sequencer: use reverse_commit_list() helperJayati Shrivastava
Instead of creating a new allocation, reverse the original list in-place by calling the reverse_commit_list() helper. The original code discards the list "bases" after storing its reverse copy in a newly created list "reversed". If the code that followed from here used both "bases" and "reversed", the modification would not have worked, but since the original list "bases" gets discarded, we can simply reverse "bases" in-place with the reverse_commit_list() helper and reuse the same variable in the code that follows. builtin/merge.c has been left unmodified, since in its case, the original list is needed separately from its reverse copy by the code. Signed-off-by: Jayati Shrivastava <gaurijove@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07hooks: fix an obscure TOCTOU "did we just run a hook?" raceÆvar Arnfjörð Bjarmason
Fix a Time-of-check to time-of-use (TOCTOU) race in code added in 680ee550d72 (commit: skip discarding the index if there is no pre-commit hook, 2017-08-14). This obscure race condition can occur if we e.g. ran the "pre-commit" hook and it modified the index, but hook_exists() returns false later on (e.g., because the hook itself went away, the directory became unreadable, etc.). Then we won't call discard_cache() when we should have. The race condition itself probably doesn't matter, and users would have been unlikely to run into it in practice. This problem has been noted on-list when 680ee550d72 was discussed[1], but had not been fixed. This change is mainly intended to improve the readability of the code involved, and to make reasoning about it more straightforward. It wasn't as obvious what we were trying to do here, but by having an "invoked_hook" it's clearer that e.g. our discard_cache() is happening because of the earlier hook execution. Let's also change this for the push-to-checkout hook. Now instead of checking if the hook exists and either doing a push to checkout or a push to deploy we'll always attempt a push to checkout. If the hook doesn't exist we'll fall back on push to deploy. The same behavior as before, without the TOCTOU race. See 0855331941b (receive-pack: support push-to-checkout hook, 2014-12-01) for the introduction of the previous behavior. This leaves uses of hook_exists() in two places that matter. The "reference-transaction" check in refs.c, see 67541597670 (refs: implement reference transaction hook, 2020-06-19), and the "prepare-commit-msg" hook, see 66618a50f9c (sequencer: run 'prepare-commit-msg' hook, 2018-01-24). In both of those cases we're saving ourselves CPU time by not preparing data for the hook that we'll then do nothing with if we don't have the hook. So using this "invoked_hook" pattern doesn't make sense in those cases. The "reference-transaction" and "prepare-commit-msg" hook also aren't racy. In those cases we'll skip the hook runs if we race with a new hook being added, whereas in the TOCTOU races being fixed here we were incorrectly skipping the required post-hook logic. 1. https://lore.kernel.org/git/20170810191613.kpmhzg4seyxy3cpq@sigill.intra.peff.net/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-25Merge branch 'ja/i18n-common-messages'Junio C Hamano
Unify more messages to help l10n. * ja/i18n-common-messages: i18n: fix some misformated placeholders in command synopsis i18n: remove from i18n strings that do not hold translatable parts i18n: factorize "invalid value" messages i18n: factorize more 'incompatible options' messages
2022-02-18Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'Junio C Hamano
Because a deletion of ref would need to remove it from both the loose ref store and the packed ref store, a delete-ref operation that logically removes one ref may end up invoking ref-transaction hook twice, which has been corrected. * ps/avoid-unnecessary-hook-invocation-with-packed-refs: refs: skip hooks when deleting uncovered packed refs refs: do not execute reference-transaction hook on packing refs refs: demonstrate excessive execution of the reference-transaction hook refs: allow skipping the reference-transaction hook refs: allow passing flags when beginning transactions refs: extract packed_refs_delete_refs() to allow control of transaction
2022-02-18Merge branch 'pw/use-in-process-checkout-in-rebase'Junio C Hamano
Use an internal call to reset_head() helper function instead of spawning "git checkout" in "rebase", and update code paths that are involved in the change. * pw/use-in-process-checkout-in-rebase: rebase -m: don't fork git checkout rebase --apply: set ORIG_HEAD correctly rebase --apply: fix reflog reset_head(): take struct rebase_head_opts rebase: cleanup reset_head() calls create_autostash(): remove unneeded parameter reset_head(): make default_reflog_action optional reset_head(): factor out ref updates reset_head(): remove action parameter rebase --apply: don't run post-checkout hook if there is an error rebase: do not remove untracked files on checkout rebase: pass correct arguments to post-checkout hook t5403: refactor rebase post-checkout hook tests rebase: factor out checkout for up to date branch
2022-02-11Merge branch 'ab/no-errno-from-resolve-ref-unsafe'Junio C Hamano
Remaining code-clean-up. * ab/no-errno-from-resolve-ref-unsafe: refs API: remove "failure_errno" from refs_resolve_ref_unsafe() sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure
2022-02-04i18n: factorize "invalid value" messagesJean-Noël Avila
Use the same message when an invalid value is passed to a command line option or a configuration variable. Signed-off-by: Jean-Noël Avila <jn.avila@free.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-28Merge branch 'en/keep-cwd' into maintJunio C Hamano
Fix a regression in 2.35 that roke the use of "rebase" and "stash" in a secondary worktree. * en/keep-cwd: sequencer, stash: fix running from worktree subdir
2022-01-26Merge branch 'en/keep-cwd'Junio C Hamano
Fix a regression in 2.35 that roke the use of "rebase" and "stash" in a secondary worktree. * en/keep-cwd: sequencer, stash: fix running from worktree subdir
2022-01-26refs API: remove "failure_errno" from refs_resolve_ref_unsafe()Ævar Arnfjörð Bjarmason
Remove the now-unused "failure_errno" parameter from the refs_resolve_ref_unsafe() signature. In my recent 96f6623ada0 (Merge branch 'ab/refs-errno-cleanup', 2021-11-29) series we made all of its callers explicitly request the errno via an output parameter. As that series shows all but one caller ended up passing in a boilerplate "ignore_errno", since they only cared about whether the return value was NULL or not, i.e. if the ref could be resolved. There was one small issue with that series fixed with a follow-up in 31e39123695 (Merge branch 'ab/refs-errno-cleanup', 2022-01-14) a small bug in that series was fixed. After those two there was one caller left in sequencer.c that used the "failure_errno', but as of the preceding commit it uses a boilerplate "ignore_errno" instead. This leaves the public refs API without any use of "failure_errno" at all. We could still do with a bit of cleanup and generalization between refs.c and refs/files-backend.c before the "reftable" integration lands, but that's all internal to the reference code itself. So let's remove this output parameter. Not only isn't it used now, but it's unlikely that we'll want it again in the future. We'd like to slowly move the refs API to a more file-backend independent way of communicating error codes, having it use a "failure_errno" was only the first step in that direction. If this or any other function needs to communicate what specifically is wrong with the requested "refname" it'll be better to have the function set some output enum of well-defined error states than piggy-backend on "errno". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-26sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failureÆvar Arnfjörð Bjarmason
Change code that was faithfully migrated to the new "resolve_errno" API in ed90f04155d (refs API: make resolve_ref_unsafe() not set errno, 2021-10-16) to stop caring about the errno at all. When we fail to resolve "HEAD" after the sequencer runs it doesn't really help to say what the "errno" value is, since the fake backend errno may or may not reflect anything real about the state of the ".git/HEAD". With the upcoming reftable backend this fakery will become even more pronounced. So let's just die() instead of die_errno() here. This will also help simplify the refs_resolve_ref_unsafe() API. This was the only user of it that wasn't ignoring the "failure_errno" output parameter. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-26rebase -m: don't fork git checkoutPhillip Wood
Now that reset_head() can handle the initial checkout of onto correctly use it in the "merge" backend instead of forking "git checkout". This opens the way for us to stop calling the post-checkout hook in the future. Not running "git checkout" means that "rebase -i/m" no longer recurse submodules when checking out "onto" (thanks to Philippe Blain for pointing this out). As the rest of rebase does not know what to do with submodules this is probably a good thing. When using merge-ort rebase ought be able to handle submodules correctly if it parsed the submodule config, such a change is left for a future patch series. The "apply" based rebase has avoided forking git checkout since ac7f467fef ("builtin/rebase: support running "git rebase <upstream>"", 2018-08-07). The code that handles the checkout was moved into libgit by b309a97108 ("reset: extract reset_head() from rebase", 2020-04-07). Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-26reset_head(): take struct rebase_head_optsPhillip Wood
This function takes a confusingly large number of parameters which makes it difficult to remember which order to pass them in. The following commits will add a couple more parameters which makes the problem worse. To address this change the function to take a struct of options. Using a struct means that it is no longer necessary to remember which order to pass the parameters in and anyone reading the code can easily see which value is passed to each parameter. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-26create_autostash(): remove unneeded parameterPhillip Wood
The default_reflog parameter of create_autostash() is passed to reset_head(). However as creating a stash does not involve updating any refs the parameter is not used by reset_head(). Removing the parameter from create_autostash() simplifies the callers. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-26reset_head(): remove action parameterPhillip Wood
The only use of the action parameter is to setup the error messages for unpack_trees(). All but two cases pass either "checkout" or "reset". The case that passes "reset --hard" would be better passing "reset" so that the error messages match the builtin reset command like all the other callers that are doing a reset. The case that passes "Fast-forwarded" is only updating HEAD and so the parameter is unused in that case as it does not call unpack_trees(). The value to pass to setup_unpack_trees_porcelain() can be determined by checking whether flags contains RESET_HEAD_HARD without the caller having to specify it. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-26sequencer, stash: fix running from worktree subdirElijah Newren
In commits bc3ae46b42 ("rebase: do not attempt to remove startup_info->original_cwd", 2021-12-09) and 0fce211ccc ("stash: do not attempt to remove startup_info->original_cwd", 2021-12-09), we wanted to allow the subprocess to know which directory the parent process was running from, so that the subprocess could protect it. However... When run from a non-main worktree, setup_git_directory() will note that the discovered git directory (/PATH/TO/.git/worktree/non-main-worktree) does not match DEFAULT_GIT_DIR_ENVIRONMENT (see setup_discovered_git_dir()), and decide to set GIT_DIR in the environment. This matters because... Whenever git is run with the GIT_DIR environment variable set, and GIT_WORK_TREE not set, it presumes that '.' is the working tree. So... This combination results in the subcommand being very confused about the working tree. Fix it by also setting the GIT_WORK_TREE environment variable along with setting cmd.dir. A possibly more involved fix we could consider for later would be to make setup.c set GIT_WORK_TREE whenever (a) it discovers both the git directory and the working tree and (b) it decides to set GIT_DIR in the environment. I did not attempt that here as such would be too big of a change for a 2.35.1 release. Test-case-by: Glen Choo <chooglen@google.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-17refs: allow passing flags when beginning transactionsPatrick Steinhardt
We do not currently have any flags when creating reference transactions, but we'll add one to disable execution of the reference transaction hook in some cases. Allow passing flags to `ref_store_transaction_begin()` to prepare for this change. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-05Merge branch 'en/keep-cwd'Junio C Hamano
Many git commands that deal with working tree files try to remove a directory that becomes empty (i.e. "git switch" from a branch that has the directory to another branch that does not would attempt remove all files in the directory and the directory itself). This drops users into an unfamiliar situation if the command was run in a subdirectory that becomes subject to removal due to the command. The commands have been taught to keep an empty directory if it is the directory they were started in to avoid surprising users. * en/keep-cwd: t2501: simplify the tests since we can now assume desired behavior dir: new flag to remove_dir_recurse() to spare the original_cwd dir: avoid incidentally removing the original_cwd in remove_path() stash: do not attempt to remove startup_info->original_cwd rebase: do not attempt to remove startup_info->original_cwd clean: do not attempt to remove startup_info->original_cwd symlinks: do not include startup_info->original_cwd in dir removal unpack-trees: add special cwd handling unpack-trees: refuse to remove startup_info->original_cwd setup: introduce startup_info->original_cwd t2501: add various tests for removing the current working directory
2021-12-21Merge branch 'en/rebase-x-wo-git-dir-env'Junio C Hamano
"git rebase -x" by mistake started exporting the GIT_DIR and GIT_WORK_TREE environment variables when the command was rewritten in C, which has been corrected. * en/rebase-x-wo-git-dir-env: sequencer: do not export GIT_DIR and GIT_WORK_TREE for 'exec'
2021-12-15Merge branch 'ab/run-command'Junio C Hamano
API clean-up. * ab/run-command: run-command API: remove "env" member, always use "env_array" difftool: use "env_array" to simplify memory management run-command API: remove "argv" member, always use "args" run-command API users: use strvec_push(), not argv construction run-command API users: use strvec_pushl(), not argv construction run-command tests: use strvec_pushv(), not argv assignment run-command API users: use strvec_pushv(), not argv assignment upload-archive: use regular "struct child_process" pattern worktree: stop being overly intimate with run_command() internals