aboutsummaryrefslogtreecommitdiffstats
path: root/builtin/fetch.c (follow)
AgeCommit message (Collapse)AuthorFilesLines
2024-12-06global: mark code units that generate warnings with `-Wsign-compare`Patrick Steinhardt1-0/+3
Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-06fetch set_head: add warn-if-not-$branch optionBence Ferdinandy1-5/+11
Currently if we want to have a remote/HEAD locally that is different from the one on the remote, but we still want to get a warning if remote changes HEAD, our only option is to have an indiscriminate warning with "follow_remote_head" set to "warn". Add a new option "warn-if-not-$branch", where $branch is a branch name we do not wish to get a warning about. If the remote HEAD is $branch do not warn, otherwise, behave as "warn". E.g. let's assume, that our remote origin has HEAD set to "master", but locally we have "git remote set-head origin seen". Setting 'remote.origin.followRemoteHEAD = "warn"' will always print a warning, even though the remote has not changed HEAD from "master". Setting 'remote.origin.followRemoteHEAD = "warn-if-not-master" will squelch the warning message, unless the remote changes HEAD from "master". Note, that should the remote change HEAD to "seen" (which we have locally), there will still be no warning. Improve the advice message in report_set_head to also include silencing the warning message with "warn-if-not-$branch". Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-06fetch set_head: move warn advice into advise_if_enabledBence Ferdinandy1-4/+13
Advice about what to do when getting a warning is typed out explicitly twice and is printed as regular output. The output is also tested for. Extract the advice message into a single place and use a wrapper function, so if later the advice is made more chatty the signature only needs to be changed in once place. Remove the testing for the advice output in the tests. Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04Merge branch 'ps/ref-backend-migration-optim'Junio C Hamano1-2/+2
The migration procedure between two ref backends has been optimized. * ps/ref-backend-migration-optim: reftable: rename scratch buffer refs: adapt `initial_transaction` flag to be unsigned reftable/block: optimize allocations by using scratch buffer reftable/block: rename `block_writer::buf` variable reftable/writer: optimize allocations by using a scratch buffer refs: don't normalize log messages with `REF_SKIP_CREATE_REFLOG` refs: skip collision checks in initial transactions refs: use "initial" transaction semantics to migrate refs refs/files: support symbolic and root refs in initial transaction refs: introduce "initial" transaction flag refs/files: move logic to commit initial transaction refs: allow passing flags when setting up a transaction
2024-12-02fetch: add configuration for set_head behaviourBence Ferdinandy1-6/+40
In the current implementation, if refs/remotes/$remote/HEAD does not exist, running fetch will create it, but if it does exist it will not do anything, which is a somewhat safe and minimal approach. Unfortunately, for users who wish to NOT have refs/remotes/$remote/HEAD set for any reason (e.g. so that `git rev-parse origin` doesn't accidentally point them somewhere they do not want to), there is no way to remove this behaviour. On the other side of the spectrum, users may want fetch to automatically update HEAD or at least give them a warning if something changed on the remote. Introduce a new setting, remote.$remote.followRemoteHEAD with four options: - "never": do not ever do anything, not even create - "create": the current behaviour, now the default behaviour - "warn": print a message if remote and local HEAD is different - "always": silently update HEAD on every change Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-27Merge branch 'bf/set-head-symref' into bf/fetch-set-head-configJunio C Hamano1-0/+74
* bf/set-head-symref: fetch set_head: handle mirrored bare repositories fetch: set remote/HEAD if it does not exist refs: add create_only option to refs_update_symref_extended refs: add TRANSACTION_CREATE_EXISTS error remote set-head: better output for --auto remote set-head: refactor for readability refs: atomically record overwritten ref in update_symref refs: standardize output of refs_read_symbolic_ref t/t5505-remote: test failure of set-head t/t5505-remote: set default branch to main
2024-11-25fetch set_head: handle mirrored bare repositoriesBence Ferdinandy1-5/+11
When adding a remote to bare repository with "git remote add --mirror", running fetch will fail to update HEAD to the remote's HEAD, since it does not know how to handle bare repositories. On the other hand HEAD already has content, since "git init --bare" has already set HEAD to whatever is the default branch set for the user. Unless this - by chance - is the same as the remote's HEAD, HEAD will be pointing to a bad symref. Teach set_head to handle bare repositories, by overwriting HEAD so it mirrors the remote's HEAD. Note, that in this case overriding the local HEAD reference is necessary, since HEAD will exist before fetch can be run, but this should not be an issue, since the whole purpose of --mirror is to be an exact mirror of the remote, so following any changes to HEAD makes sense. Also note, that although "git remote set-head" also fails when trying to update the remote's locally tracked HEAD in a mirrored bare repository, the usage of the command does not make much sense after this patch: fetch will update the remote HEAD correctly, and setting it manually to something else is antithetical to the concept of mirroring. Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-25fetch: set remote/HEAD if it does not existBence Ferdinandy1-0/+68
When cloning a repository remote/HEAD is created, but when the user creates a repository with git init, and later adds a remote, remote/HEAD is only created if the user explicitly runs a variant of "remote set-head". Attempt to set remote/HEAD during fetch, if the user does not have it already set. Silently ignore any errors. Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-22Merge branch 'jk/fetch-prefetch-double-free-fix'Junio C Hamano1-6/+2
Double-free fix. * jk/fetch-prefetch-double-free-fix: refspec: store raw refspecs inside refspec_item refspec: drop separate raw_nr count fetch: adjust refspec->raw_nr when filtering prefetch refspecs
2024-11-21refs: allow passing flags when setting up a transactionPatrick Steinhardt1-2/+2
Allow passing flags when setting up a transaction such that the behaviour of the transaction itself can be altered. This functionality will be used in a subsequent patch. Adapt callers accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-12refspec: store raw refspecs inside refspec_itemJeff King1-6/+2
The refspec struct keeps two matched arrays: one for the refspec_item structs and one for the original raw refspec strings. The main reason for this is that there are other users of refspec_item that do not care about the raw strings. But it does make managing the refspec struct awkward, as we must keep the two arrays in sync. This has led to bugs in the past (both leaks and double-frees). Let's just store a copy of the raw refspec string directly in each refspec_item struct. This simplifies the handling at a small cost: 1. Direct callers of refspec_item_init() will now get an extra copy of the refspec string, even if they don't need it. This should be negligible, as the struct is already allocating two strings for the parsed src/dst values (and we tend to only do it sparingly anyway for things like the TAG_REFSPEC literal). 2. Users of refspec_appendf() will now generate a temporary string, copy it, and then free the result (versus handing off ownership of the temporary string). We could get around this by having a "nodup" variant of refspec_item_init(), but it doesn't seem worth the extra complexity for something that is not remotely a hot code path. Code which accesses refspec->raw now needs to look at refspec->item.raw. Other callers which just use refspec_item directly can remain the same. We'll free the allocated string in refspec_item_clear(), which they should be calling anyway to free src/dst. One subtle note: refspec_item_init() can return an error, in which case we'll still have set its "raw" field. But that is also true of the "src" and "dst" fields, so any caller which does not _clear() the failed item is already potentially leaking. In practice most code just calls die() on an error anyway, but you can see the exception in valid_fetch_refspec(), which does correctly call _clear() even on error. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-12refspec: drop separate raw_nr countJeff King1-1/+0
A refspec struct contains zero or more refspec_item structs, along with matching "raw" strings. The items and raw strings are kept in separate arrays, but those arrays will always have the same length (because we write them only via refspec_append_nodup(), which grows both). This can lead to bugs when manipulating the array, since the arrays and lengths must be modified in lockstep. For example, the bug fixed in the previous commit, which forgot to decrement raw_nr. So let's get rid of "raw_nr" and have only "nr", making this kind of bug impossible (and also making it clear that the two are always matched, something that existing code already assumed but was not guaranteed by the interface). Even though we'd expect "alloc" and "raw_alloc" to likewise move in lockstep, we still need to keep separate counts there if we want to continue to use ALLOC_GROW() for both. Conceptually this would all be simpler if refspec_item just held onto its own raw string, and we had a single array. But there are callers which use refspec_item outside of "struct refspec" (and so don't hold on to a matching "raw" string at all), which we'd possibly need to adjust. So let's not worry about refactoring that for now, and just get rid of the redundant count variable. That is the first step on the road to combining them anyway. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-12fetch: adjust refspec->raw_nr when filtering prefetch refspecsJeff King1-0/+1
In filter_prefetch_refspecs(), we may remove one or more refspecs if they point into refs/tags/. When we do, we remove the item from the refspec->items array, shifting subsequent items down, and then decrement the refspec->nr count. We also remove the item from the refspec->raw array, but fail to decrement refspec->raw_nr. This leaves us with a count that is too high, and anybody looking at the "raw" array will erroneously see either: 1. The removed entry, if there were no subsequent items to shift down. 2. A duplicate of the final entry, as everything is shifted down but there was nothing to overwrite the final item. The obvious culprit to run into this is calling refspec_clear(), which will try to free the removed entry (case 1) or double-free the final entry (case 2). But even though the bug has existed since the function was added in 2e03115d0c (fetch: add --prefetch option, 2021-04-16), we did not trigger it in the test suite. The --prefetch option is normally only used with configured refspecs, and we never bother to call refspec_clear() on those (they are stored as part of a struct remote, which is held in a global variable). But you could trigger case 2 manually like: git fetch --prefetch . refs/tags/foo refs/tags/bar Ironically you couldn't trigger case 1, because the code accidentally leaked the string in the raw array, and the two bugs (the leak and the double-free) cancelled out. But when we fixed the leak in ea4780307c (fetch: free "raw" string when shrinking refspec, 2024-09-24), it became possible to trigger that, too, with a single item: git fetch --prefetch . refs/tags/foo We can fix both cases by just correctly decrementing "raw_nr" when we shrink the array. Even though we don't expect people to use --prefetch with command-line refspecs, we'll add a test to make sure it behaves well (like the test just before it, we're just confirming that the filtered prefetch succeeds at all). Reported-by: Eric Mills <ermills@epic.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-04doc: correct misleading descriptions for --shallow-excludeElijah Newren1-2/+2
The documentation for the --shallow-exclude option to clone/fetch/etc. claims that the option takes a revision, but it does not. As per upload-pack.c's process_deepen_not(), it passes the option to expand_ref() and dies if it does not find exactly one ref matching the name passed. Further, this has always been the case ever since these options were introduced by the commits merged in a460ea4a3cb1 (Merge branch 'nd/shallow-deepen', 2016-10-10). Fix the documentation to match the implementation. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-08fetch: respect --server-option when fetching multiple remotesXing Xin1-0/+2
Fix an issue where server options specified via the command line (`--server-option` or `-o`) were not sent when fetching from multiple remotes using Git protocol v2. To reproduce the issue with a repository containing multiple remotes: GIT_TRACE_PACKET=1 git -c protocol.version=2 fetch --server-option=demo --all Observe that no server options are sent to any remote. The root cause was identified in `builtin/fetch.c:fetch_multiple`, which is invoked when fetching from more than one remote. This function forks a `git-fetch` subprocess for each remote but did not include the specified server options in the subprocess arguments. This commit ensures that command-line specified server options are properly passed to each subprocess. Relevant tests have been added. Signed-off-by: Xing Xin <xingxin.xx@bytedance.com> Reviewed-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-25fetch: free "raw" string when shrinking refspecJeff King1-0/+1
The "--prefetch" option to git-fetch modifies the default refspec, including eliminating some entries entirely. When we drop an entry we free the strings in the refspec_item, but we forgot to free the matching string in the "raw" array of the refspec struct. There's no behavioral bug here (since we correctly shrink the raw array, too), but we're leaking the allocated string. Let's add in the leak-fix, and while we're at it drop "const" from the type of the raw string array. These strings are always allocated by refspec_append(), etc, and this makes the memory ownership more clear. This is all a bit more intimate with the refspec code than I'd like, and I suspect it would be better if each refspec_item held on to its own raw string, we had a single array, and we could use refspec_item_clear() to clean up everything. But that's a non-trivial refactoring, since refspec_item structs can be held outside of a "struct refspec", without having a matching raw string at all. So let's leave that for now and just fix the leak in the most immediate way. This lets us mark t5582 as leak-free. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-23Merge branch 'jc/pass-repo-to-builtins'Junio C Hamano1-2/+5
The convention to calling into built-in command implementation has been updated to pass the repository, if known, together with the prefix value. * jc/pass-repo-to-builtins: add: pass in repo variable instead of global the_repository builtin: remove USE_THE_REPOSITORY for those without the_repository builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.h builtin: add a repository parameter for builtin functions
2024-09-13builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.hJohn Cai1-1/+1
Instead of including USE_THE_REPOSITORY_VARIABLE by default on every builtin, remove it from builtin.h and add it to all the builtins that include builtin.h (by definition, that means all builtins/*.c). Also, remove the include statement for repository.h since it gets brought in through builtin.h. The next step will be to migrate each builtin from having to use the_repository. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-13builtin: add a repository parameter for builtin functionsJohn Cai1-1/+4
In order to reduce the usage of the global the_repository, add a parameter to builtin functions that will get passed a repository variable. This commit uses UNUSED on most of the builtin functions, as subsequent commits will modify the actual builtins to pass the repository parameter down. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05drop trailing newline from warning/error/die messagesJeff King1-2/+2
Our error reporting routines append a trailing newline, and the strings we pass to them should not include them (otherwise we get an extra blank line after the message). These cases were all found by looking at the results of: git grep -P '[^_](error|error_errno|warning|die|die_errno)\(.*\\n"[,)]' '*.c' Note that we _do_ sometimes include a newline in the middle of such messages, to create multiline output (hence our grep matching "," or ")" after we see the newline, so we know we're at the end of the string). It's possible that one or more of these cases could intentionally be including a blank line at the end, but having looked at them all manually, I think these are all just mistakes. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-03Merge branch 'js/fetch-push-trace2-annotation'Junio C Hamano1-1/+15
More trace2 events at key points on push and fetch code paths have been added. * js/fetch-push-trace2-annotation: send-pack: add new tracing regions for push fetch: add top-level trace2 regions trace2: implement trace2_printf() for event target
2024-08-22fetch: add top-level trace2 regionsJosh Steadmon1-1/+15
At $DAYJOB we experienced some slow fetch operations and needed some additional data to help diagnose the issue. Add top-level trace2 regions for the various modes of operation of `git-fetch`. None of these regions are in recursive code, so any enclosed trace messages should only see their nesting level increase by one. Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-22builtin/fetch: fix leaking transaction with `--atomic`Patrick Steinhardt1-4/+4
With the `--atomic` flag, we use a single ref transaction to commit all ref updates in git-fetch(1). The lifetime of transactions is somewhat weird: while `ref_transaction_abort()` will free the transaction, a call to `ref_transaction_commit()` won't. We thus have to manually free the transaction in the successful case. Adapt the code to free the transaction in the exit path to plug the resulting memory leak. As `ref_transaction_abort()` already freed the transaction for us, we have to unset the transaction when we hit that code path to not cause a double free. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-09refs: add referent to each_ref_fnJohn Cai1-1/+2
Add a parameter to each_ref_fn so that callers to the ref APIs that use this function as a callback can have acess to the unresolved value of a symbolic ref. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-20Merge branch 'kn/update-ref-symref'Junio C Hamano1-2/+2
"git update-ref --stdin" learned to handle transactional updates of symbolic-refs. * kn/update-ref-symref: update-ref: add support for 'symref-update' command reftable: pick either 'oid' or 'target' for new updates update-ref: add support for 'symref-create' command update-ref: add support for 'symref-delete' command update-ref: add support for 'symref-verify' command refs: specify error for regular refs with `old_target` refs: create and use `ref_update_expects_existing_old_ref()`
2024-06-17Merge branch 'ps/no-writable-strings'Junio C Hamano1-3/+8
Building with "-Werror -Wwrite-strings" is now supported. * ps/no-writable-strings: (27 commits) config.mak.dev: enable `-Wwrite-strings` warning builtin/merge: always store allocated strings in `pull_twohead` builtin/rebase: always store allocated string in `options.strategy` builtin/rebase: do not assign default backend to non-constant field imap-send: fix leaking memory in `imap_server_conf` imap-send: drop global `imap_server_conf` variable mailmap: always store allocated strings in mailmap blob revision: always store allocated strings in output encoding remote-curl: avoid assigning string constant to non-const variable send-pack: always allocate receive status parse-options: cast long name for OPTION_ALIAS http: do not assign string constant to non-const field compat/win32: fix const-correctness with string constants pretty: add casts for decoration option pointers object-file: make `buf` parameter of `index_mem()` a constant object-file: mark cached object buffers as const ident: add casts for fallback name and GECOS entry: refactor how we remove items for delayed checkouts line-log: always allocate the output prefix line-log: stop assigning string constant to file parent buffer ...
2024-06-07refspec: remove global tag refspec structurePatrick Steinhardt1-3/+8
We have a global tag refspec structure that is used by both git-clone(1) and git-fetch(1). Initialization of the structure will break once we enable `-Wwrite-strings`, even though the breakage is harmless. While we could just add casts, the structure isn't really required in the first place as we can simply initialize the structures at the respective callsites. Refactor the code accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07update-ref: add support for 'symref-delete' commandKarthik Nayak1-2/+2
Add a new command 'symref-delete' to allow deletions of symbolic refs in a transaction via the '--stdin' mode of the 'git-update-ref' command. The 'symref-delete' command can, when given an <old-target>, delete the provided <ref> only when it points to <old-target>. This command is only compatible with the 'no-deref' mode because we optionally want to check the 'old_target' of the ref being deleted. De-referencing a symbolic ref would provide a regular ref and we already have the 'delete' command for regular refs. While users can also use 'git symbolic-ref -d' to delete symbolic refs, the 'symref-delete' command in 'git-update-ref' allows users to do so within a transaction, which promises atomicity of the operation and can be batched with other commands. When no 'old_target' is provided it can also delete regular refs, similar to how the 'delete' command can delete symrefs when no 'old_oid' is provided. Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-30Merge branch 'ps/refs-without-the-repository-updates'Junio C Hamano1-1/+2
Further clean-up the refs subsystem to stop relying on the_repository, and instead use the repository associated to the ref_store object. * ps/refs-without-the-repository-updates: refs/packed: remove references to `the_hash_algo` refs/files: remove references to `the_hash_algo` refs/files: use correct repository refs: remove `dwim_log()` refs: drop `git_default_branch_name()` refs: pass repo when peeling objects refs: move object peeling into "object.c" refs: pass ref store when detecting dangling symrefs refs: convert iteration over replace refs to accept ref store refs: retrieve worktree ref stores via associated repository refs: refactor `resolve_gitlink_ref()` to accept a repository refs: pass repo when retrieving submodule ref store refs: track ref stores via strmap refs: implement releasing ref storages refs: rename `init_db` callback to avoid confusion refs: adjust names for `init` and `init_db` callbacks
2024-05-23Merge branch 'kn/ref-transaction-symref' into kn/update-ref-symrefJunio C Hamano1-1/+1
* kn/ref-transaction-symref: refs: remove `create_symref` and associated dead code refs: rename `refs_create_symref()` to `refs_update_symref()` refs: use transaction in `refs_create_symref()` refs: add support for transactional symref updates refs: move `original_update_refname` to 'refs.c' refs: support symrefs in 'reference-transaction' hook files-backend: extract out `create_symref_lock()` refs: accept symref values in `ref_transaction_update()`
2024-05-20Merge branch 'kn/ref-transaction-symref'Junio C Hamano1-1/+1
Updates to symbolic refs can now be made as a part of ref transaction. * kn/ref-transaction-symref: refs: remove `create_symref` and associated dead code refs: rename `refs_create_symref()` to `refs_update_symref()` refs: use transaction in `refs_create_symref()` refs: add support for transactional symref updates refs: move `original_update_refname` to 'refs.c' refs: support symrefs in 'reference-transaction' hook files-backend: extract out `create_symref_lock()` refs: accept symref values in `ref_transaction_update()`
2024-05-17refs: pass ref store when detecting dangling symrefsPatrick Steinhardt1-1/+2
Both `warn_dangling_symref()` and `warn_dangling_symrefs()` derive the ref store via `the_repository`. Adapt them to instead take in the ref store as a parameter. While at it, rename the functions to have a `ref_` prefix to align them with other functions that take a ref store. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-07cocci: apply rules to rewrite callers of "refs" interfacesPatrick Steinhardt1-6/+14
Apply the rules that rewrite callers of "refs" interfaces to explicitly pass `struct ref_store`. The resulting patch has been applied with the `--whitespace=fix` option. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-07refs: accept symref values in `ref_transaction_update()`Karthik Nayak1-1/+1
The function `ref_transaction_update()` obtains ref information and flags to create a `ref_update` and add them to the transaction at hand. To extend symref support in transactions, we need to also accept the old and new ref targets and process it. This commit adds the required parameters to the function and modifies all call sites. The two parameters added are `new_target` and `old_target`. The `new_target` is used to denote what the reference should point to when the transaction is applied. Some functions allow this parameter to be NULL, meaning that the reference is not changed. The `old_target` denotes the value the reference must have before the update. Some functions allow this parameter to be NULL, meaning that the old value of the reference is not checked. We also update the internal function `ref_transaction_add_update()` similarly to take the two new parameters. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-15Merge branch 'ds/fetch-config-parse-microfix'Junio C Hamano1-0/+1
A config parser callback function fell through instead of returning after recognising and processing a variable, wasting cycles, which has been corrected. * ds/fetch-config-parse-microfix: fetch: return when parsing submodule.recurse
2024-04-05fetch: return when parsing submodule.recurseDerrick Stolee1-0/+1
When parsing config keys, the normal pattern is to return 0 after completing the logic for a specific config key, since no other key will match. One instance, for "submodule.recurse", was missing this case in builtin/fetch.c. This is a very minor change, and will have minimal impact to performance. This particular block was edited recently in 56e8bb4fb4 (fetch: use `fetch_config` to store "fetch.recurseSubmodules" value, 2023-05-17), which led to some hesitation that perhaps this omission was on purpose. However, no later cases within git_fetch_config() will match the key if equal to "submodule.recurse" and neither will any key matches within the catch-all git_default_config(). Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-11Merge branch 'js/merge-base-with-missing-commit'Junio C Hamano1-0/+2
Make sure failure return from merge_bases_many() is properly caught. * js/merge-base-with-missing-commit: merge-ort/merge-recursive: do report errors in `merge_submodule()` merge-recursive: prepare for `merge_submodule()` to report errors commit-reach(repo_get_merge_bases_many_dirty): pass on errors commit-reach(repo_get_merge_bases_many): pass on "missing commits" errors commit-reach(get_octopus_merge_bases): pass on "missing commits" errors commit-reach(repo_get_merge_bases): pass on "missing commits" errors commit-reach(get_merge_bases_many_0): pass on "missing commits" errors commit-reach(merge_bases_many): pass on "missing commits" errors commit-reach(paint_down_to_common): start reporting errors commit-reach(paint_down_to_common): prepare for handling shallow commits commit-reach(repo_in_merge_bases_many): report missing commits commit-reach(repo_in_merge_bases_many): optionally expect missing commits commit-reach(paint_down_to_common): plug two memory leaks
2024-02-28commit-reach(repo_in_merge_bases_many): report missing commitsJohannes Schindelin1-0/+2
Some functions in Git's source code follow the convention that returning a negative value indicates a fatal error, e.g. repository corruption. Let's use this convention in `repo_in_merge_bases()` to report when one of the specified commits is missing (i.e. when `repo_parse_commit()` reports an error). Also adjust the callers of `repo_in_merge_bases()` to handle such negative return values. Note: As of this patch, errors are returned only if any of the specified merge heads is missing. Over the course of the next patches, missing commits will also be reported by the `paint_down_to_common()` function, which is called by `repo_in_merge_bases_many()`, and those errors will be properly propagated back to the caller at that stage. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-26fetch: convert strncmp() with strlen() to starts_with()René Scharfe1-3/+2
Using strncmp() and strlen() to check whether a string starts with another one requires repeating the prefix candidate. Use starts_with() instead, which reduces repetition and is more readable. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-01-19Merge branch 'tb/fetch-all-configuration'Junio C Hamano1-1/+16
"git fetch" learned to pay attention to "fetch.all" configuration variable, which pretends as if "--all" was passed from the command line when no remote parameter was given. * tb/fetch-all-configuration: fetch: add new config option fetch.all
2024-01-08Merge branch 'en/header-cleanup'Junio C Hamano1-2/+0
Remove unused header "#include". * en/header-cleanup: treewide: remove unnecessary includes in source files treewide: add direct includes currently only pulled in transitively trace2/tr2_tls.h: remove unnecessary include submodule-config.h: remove unnecessary include pkt-line.h: remove unnecessary include line-log.h: remove unnecessary include http.h: remove unnecessary include fsmonitor--daemon.h: remove unnecessary includes blame.h: remove unnecessary includes archive.h: remove unnecessary include treewide: remove unnecessary includes in source files treewide: remove unnecessary includes from header files
2024-01-08fetch: add new config option fetch.allTamino Bauknecht1-1/+16
Introduce a boolean configuration option fetch.all which allows to fetch all available remotes by default. The config option can be overridden by explicitly specifying a remote or by using --no-all. The behavior for --all is unchanged and calling git-fetch with --all and a remote will still result in an error. Additionally, describe the configuration variable in the config documentation and implement new tests to cover the expected behavior. Also add --no-all to the command-line documentation of git-fetch. Signed-off-by: Tamino Bauknecht <dev@tb6.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-12-26treewide: remove unnecessary includes in source filesElijah Newren1-2/+0
Each of these were checked with gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE} to ensure that removing the direct inclusion of the header actually resulted in that header no longer being included at all (i.e. that no other header pulled it in transitively). ...except for a few cases where we verified that although the header was brought in transitively, nothing from it was directly used in that source file. These cases were: * builtin/credential-cache.c * builtin/pull.c * builtin/send-pack.c Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-12-18fetch: no redundant error message for atomic fetchJiang Xin1-5/+9
If an error occurs during an atomic fetch, a redundant error message will appear at the end of do_fetch(). It was introduced in b3a804663c (fetch: make `--atomic` flag cover backfilling of tags, 2022-02-17). Because a failure message is displayed before setting retcode in the function do_fetch(), calling error() on the err message at the end of this function may result in redundant or empty error message to be displayed. We can remove the redundant error() function, because we know that the function ref_transaction_abort() never fails. While we can find a common pattern for calling ref_transaction_abort() by running command "git grep -A1 ref_transaction_abort", e.g.: if (ref_transaction_abort(transaction, &error)) error("abort: %s", error.buf); Following this pattern, we can tolerate the return value of the function ref_transaction_abort() being changed in the future. We also delay the output of the err message to the end of do_fetch() to reduce redundant code. With these changes, the test case "fetch porcelain output (atomic)" in t5574 will also be fixed. Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-13Merge branch 'jk/unused-post-2.42-part2'Junio C Hamano1-2/+2
Unused parameters to functions are marked as such, and/or removed, in order to bring us closer to -Wunused-parameter clean. * jk/unused-post-2.42-part2: parse-options: mark unused parameters in noop callback interpret-trailers: mark unused "unset" parameters in option callbacks parse-options: add more BUG_ON() annotations merge: do not pass unused opt->value parameter parse-options: mark unused "opt" parameter in callbacks parse-options: prefer opt->value to globals in callbacks checkout-index: delay automatic setting of to_tempfile format-patch: use OPT_STRING_LIST for to/cc options merge: simplify parsing of "-n" option merge: make xopts a strvec
2023-09-05parse-options: prefer opt->value to globals in callbacksJeff King1-2/+2
We have several parse-options callbacks that ignore their "opt" parameters entirely. This is a little unusual, as we'd normally put the result of the parsing into opt->value. In the case of these callbacks, though, they directly manipulate global variables instead (and in most cases the caller sets opt->value to NULL in the OPT_CALLBACK declaration). The immediate symptom we'd like to deal with is that the unused "opt" variables trigger -Wunused-parameter. But how to fix that is debatable. One option is to annotate them with UNUSED. But another is to have the caller pass in the appropriate variable via opt->value, and use it. That has the benefit of making the callbacks reusable (in theory at least), and makes it clear from the OPT_CALLBACK declaration which variables will be affected (doubly so for the cases in builtin/fast-export.c, where we do set opt->value, but it is completely ignored!). The slight downside is that we lose type safety, since they're now passing through void pointers. I went with the "just use them" approach here. The loss of type safety is unfortunate, but that is already an issue with most of the other callbacks. If we want to try to address that, we should do so more consistently (and this patch would prepare these callbacks for whatever we choose to do there). Note that in the cases in builtin/fast-export.c, we are passing anonymous enums. We'll have to give them names so that we can declare the appropriate pointer type within the callbacks. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-29fetch: mark unused parameter in ref_transaction callbackJeff King1-1/+1
Since this callback is just trying to collect the set of queued tag updates, there is no need for it to look at old_oid at all. Mark it as unused to appease -Wunused-parameter. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-27Merge branch 'jc/transport-parseopt-fix'Junio C Hamano1-4/+1
Command line parser fixes. * jc/transport-parseopt-fix: fetch: reject --no-ipv[46] parse-options: introduce OPT_IPVERSION()
2023-07-18parse-options: introduce OPT_IPVERSION()Junio C Hamano1-4/+1
The command line option parsing for "git clone", "git fetch", and "git push" have duplicated implementations of parsing "--ipv4" and "--ipv6" options, by having two OPT_SET_INT() for "ipv4" and "ipv6". Introduce a new OPT_IPVERSION() macro and use it in these three commands. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-06Merge branch 'gc/config-context'Junio C Hamano1-5/+8
Reduce reliance on a global state in the config reading API. * gc/config-context: config: pass source to config_parser_event_fn_t config: add kvi.path, use it to evaluate includes config.c: remove config_reader from configsets config: pass kvi to die_bad_number() trace2: plumb config kvi config.c: pass ctx with CLI config config: pass ctx with config files config.c: pass ctx in configsets config: add ctx arg to config_fn_t urlmatch.h: use config_fn_t type config: inline git_color_default_config