-
Notifications
You must be signed in to change notification settings - Fork 26.4k
Update coverity.yml #1932
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Ballalhossaintalukder
wants to merge
127
commits into
git:seen
Choose a base branch
from
Ballalhossaintalukder:patch-1
base: seen
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Update coverity.yml #1932
+10,898
−5,079
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In "git help config" output, attr.tree mentions both --attr-source and GIT_ATTR_SOURCE, but the description of --attr-source and GIT_ATTR_SOURCE that appear in "git help git", attr.tree is missing. Add it so that these three are described together in both places. Signed-off-by: Junio C Hamano <[email protected]>
Not effect on the test logic, but as "-G" argument is a regex it is more accurate to use "regex" as a dummy argument value rather than "string". In all the other case when "-G" is passed a dummy value it is spelled as "regex" rather than as "string". Signed-off-by: Junio C Hamano <[email protected]>
Current description for -G is incorrect, seems like it was copied from the description for -S. Signed-off-by: Junio C Hamano <[email protected]>
`-S` shows changes that modify the number of occurrences of the specified string, rather than only those that either completely remove it or add it for the first time. Signed-off-by: Junio C Hamano <[email protected]>
-G and --pickaxe-grep seems to be on par with -S and --pickaxe-all that are already mentioned. Signed-off-by: Junio C Hamano <[email protected]>
In the rest of the documentation (and in the code) we use `regex` and `string` as `-G` and `-S` argument placeholders. While `regular-expression` and `block-of-text` are a bit easier to read, it is a bit consistent. And we could assume that everyone who uses git should be able to understand that a "string" and a "block-of-text", as well as a "regex" and "regular-expression" are the same thing. So, using a shorter version is also more consistent. Signed-off-by: Junio C Hamano <[email protected]>
Most arguments have both short and long versions. Long versions are easier to read, especially in scripts and command history. This change mostly keeps existing uses of -G and -S as is in the tests, documentation and help output. Tests that check just the option parsing are duplicated to check both short and long argument options. Signed-off-by: Illia Bobyr <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
Long argument names are easier to read, compared to short ones. So while short arguments are great when you want to type a command quickly, tests are more readable if we use long argument names. There are still test that verify that both short and long arguments work interchangeably when parsing the arguments. Tests where the focus is not on the argument names are updated to use long argument names. Signed-off-by: Junio C Hamano <[email protected]>
For less experienced users --patch-{grep,modifies} should be easier to understand than just -S or -G. By mentioning the long argument names in the help messages we save those users from having to search the list of options for an explanation of what -S or -G stand for. Signed-off-by: Junio C Hamano <[email protected]>
Long argument names are easier to read, compared to short ones. So while short arguments are great when you want to type a command quickly, the documentation readability is improved if we use long argument names. Note for reviewers: All changes are just a replacement of `-G` with `--patch-grep` and `-S` with `--patch-modifies`. But as the text was reformatted to fit the same width in a few places it might look like there are more changes, if the diff is only line-wise and not word-wise. The only an exception are changes in `gitdiffcore.adoc`, where I did rephrase a sentence. I've moved introduction of the short versions of the `--patch-{grep,modifies}` into a subsequent paragraph. The reason is that I wanted to keep a note on the `-G` mnemonic, and it was awkward if I would repeat the short definition twice over a span of two paragraphs. Signed-off-by: Junio C Hamano <[email protected]>
We already have strtoul_ui() and similar functions that provide proper error handling using strtoul from the standard library. However, there isn't currently a variant that returns an unsigned long. This commit introduces strtoul_ul() to address this gap, enabling the return of an unsigned long with proper error handling. Signed-off-by: Junio C Hamano <[email protected]>
Some code used in this series declares variable i and only uses it in a for loop, not in any other logic outside the loop. Change the declaration of i to be inside the for loop for readability. While at it, we also change its type from "int" to "size_t" where the latter makes more sense. Helped-by: Christian Couder <[email protected]> Signed-off-by: Eric Ju <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
This refactor extracts utility functions from the cat-file's test script "t1006-cat-file.sh" into a new "lib-cat-file.sh" dedicated library file. The goal is to improve code reuse and readability, enabling future tests to leverage these utilities without duplicating code. Signed-off-by: Junio C Hamano <[email protected]>
Refactor write_fetch_command_and_capabilities() to a more general-purpose function, write_command_and_capabilities(), enabling it to serve both fetch and additional commands. In this context, "command" refers to the "operations" supported by Git's wire protocol https://git-scm.com/docs/protocol-v2, such as a Git subcommand (e.g., git-fetch(1)) or a server-side operation like "object-info" as implemented in commit a2ba162 (object-info: support for retrieving object info, 2021-04-20). Furthermore, write_command_and_capabilities() is moved to connect.c, making it accessible to additional commands in the future. To move write_command_and_capabilities() to connect.c, we need to adjust how `advertise_sid` is managed. Previously, in fetch_pack.c, `advertise_sid` was a static variable, modified using git_config_get_bool(). In connect.c, we now initialize `advertise_sid` at the beginning by directly using git_config_get_bool(). This change is safe because: In the original fetch-pack.c code, there are only two places that write `advertise_sid` : 1. In function do_fetch_pack: if (!server_supports("session-id")) advertise_sid = 0; 2. In function fetch_pack_config(): git_config_get_bool("transfer.advertisesid", &advertise_sid); About 1, since do_fetch_pack() is only relevant for protocol v1, this assignment can be ignored in our refactor, as write_command_and_capabilities() is only used in protocol v2. About 2, git_config_get_bool() is from config.h and it is an out-of-box dependency of connect.c, so we can reuse it directly. Helped-by: Jonathan Tan <[email protected]> Helped-by: Christian Couder <[email protected]> Signed-off-by: Calvin Wan <[email protected]> Signed-off-by: Eric Ju <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
There are some variables initialized at the start of the do_fetch_pack_v2() state machine. Currently, they are initialized in FETCH_CHECK_LOCAL, which is the initial state set at the beginning of the function. However, a subsequent patch will allow for another initial state, while still requiring these initialized variables. Move the initialization to be before the state machine, so that they are set regardless of the initial state. Note that there is no change in behavior, because we're moving code from the beginning of the first state to just before the execution of the state machine. Helped-by: Jonathan Tan <[email protected]> Helped-by: Christian Couder <[email protected]> Signed-off-by: Calvin Wan <[email protected]> Signed-off-by: Eric Ju <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
In order for a client to know what object-info components a server can provide, advertise supported object-info features. This will allow a client to decide whether to query the server for object-info or fetch as a fallback. Helped-by: Jonathan Tan <[email protected]> Helped-by: Christian Couder <[email protected]> Signed-off-by: Calvin Wan <[email protected]> Signed-off-by: Eric Ju <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
Sometimes, it is beneficial to retrieve information about an object without downloading it entirely. The server-side logic for this functionality was implemented in commit "a2ba162cda (object-info: support for retrieving object info, 2021-04-20)." And the wire format is documented at https://git-scm.com/docs/protocol-v2#_object_info. This commit introduces client functions to interact with the server. Currently, the client supports requesting a list of object IDs with the 'size' feature from a v2 server. If the server does not advertise this feature (i.e., transfer.advertiseobjectinfo is set to false), the client will return an error and exit. Notice that the entire request is written into req_buf before being sent to the remote. This approach follows the pattern used in the `send_fetch_request()` logic within fetch-pack.c. Streaming the request is not addressed in this patch. Helped-by: Jonathan Tan <[email protected]> Helped-by: Christian Couder <[email protected]> Signed-off-by: Calvin Wan <[email protected]> Signed-off-by: Eric Ju <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
Since the `info` command in `cat-file --batch-command` prints object info for a given object, it is natural to add another command in `cat-file --batch-command` to print object info for a given object from a remote. Add `remote-object-info` to `cat-file --batch-command`. While `info` takes object ids one at a time, this creates overhead when making requests to a server. So `remote-object-info` instead can take multiple object ids at once. The `cat-file --batch-command` command is generally implemented in the following manner: - Receive and parse input from user - Call respective function attached to command - Get object info, print object info In --buffer mode, this changes to: - Receive and parse input from user - Store respective function attached to command in a queue - After flush, loop through commands in queue - Call respective function attached to command - Get object info, print object info Notice how the getting and printing of object info is accomplished one at a time. As described above, this creates a problem for making requests to a server. Therefore, `remote-object-info` is implemented in the following manner: - Receive and parse input from user If command is `remote-object-info`: - Get object info from remote - Loop through and print each object info Else: - Call respective function attached to command - Parse input, get object info, print object info And finally for --buffer mode `remote-object-info`: - Receive and parse input from user - Store respective function attached to command in a queue - After flush, loop through commands in queue: If command is `remote-object-info`: - Get object info from remote - Loop through and print each object info Else: - Call respective function attached to command - Get object info, print object info To summarize, `remote-object-info` gets object info from the remote and then loops through the object info passed in, printing the info. In order for `remote-object-info` to avoid remote communication overhead in the non-buffer mode, the objects are passed in as such: remote-object-info <remote> <oid> <oid> ... <oid> rather than remote-object-info <remote> <oid> remote-object-info <remote> <oid> ... remote-object-info <remote> <oid> Helped-by: Jonathan Tan <[email protected]> Helped-by: Christian Couder <[email protected]> Signed-off-by: Calvin Wan <[email protected]> Signed-off-by: Eric Ju <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
We're about to expose `struct cmd_reflog_expire_cb` via "reflog.h" so that we can also use this structure in "builtin/gc.c". Once we make it accessible to a wider scope though it becomes awkwardly named, as it isn't only useful in the context of a callback. Instead, the function is containing all kinds of options relevant to whether or not a reflog entry should be expired. Rename the structure to `reflog_expire_options` to prepare for this. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
When expiring reflog entries, it is possible to configure expiry dates that depend on the name of the reflog. This requires us to store a couple of different expiry dates: - The default expiry date for reflog entries that aren't otherwise specified. - The per-reflog expiry date. - The currently active set of expiry dates for a given reference. While the last item is stored in `struct reflog_expiry_options`, the other items aren't, which makes it hard to reuse the structure in other places. Refactor the code so that the default expiry date is stored as part of the structure. The per-reflog expiry dates will be adapted accordingly in the subsequent commit. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
As described in the preceding commit, the per-reflog expiry dates are stored in a global pair of variables. Refactor the code so that they are contained in `sturct reflog_expire_options` to make the structure useful in other contexts. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
Make functions that are required to manage `reflog_expire_options` available elsewhere by moving them into "reflog.c" and exposing them in the corresponding header. The functions will be used in a subsequent commit. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
We're about to introduce a new task for git-maintenance(1) that knows to expire reflog entries. The logic will be shared with git-gc(1), which already knows how to do this. Pull out the common logic into a separate function so that we can share the implementation between both builtins. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
By default, git-maintenance(1) uses the "gc" task to ensure that the repository is well-maintained. This can be changed, for example by either explicitly configuring which tasks should be enabled or by using the "incremental" maintenance strategy. If so, git-maintenance(1) does not know to expire reflog entries, which is a subtask that git-gc(1) knows to perform for the user. Consequently, the reflog will grow indefinitely unless the user manually trims it. Introduce a new "reflog-expire" task that plugs this gap: - When running the task directly, then we simply execute `git reflog expire --all`, which is the same as git-gc(1). - When running git-maintenance(1) with the `--auto` flag, then we only run the task in case the "HEAD" reflog has at least N reflog entries that would be discarded. By default, N is set to 100, but this can be configured via "maintenance.reflog-expire.auto". When a negative integer has been provided we always expire entries, zero causes us to never expire entries, and a positive value specifies how many entries need to exist before we consider pruning the entries. Note that the condition for the `--auto` flags is merely a heuristic and optimized for being fast. This is because `git maintenance run --auto` will be executed quite regularly, so scanning through all reflogs would likely be too expensive in many repositories. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
test_environment is only defined with -Dtests, so use it conditionally and define a stub environment() instead, to avoid erroring out: ``` $ meson setup -Dtests=false -Dcontrib=subtree build [...] contrib/subtree/meson.build:15:27: ERROR: Unknown variable "test_environment". ``` Do the same for 'netrc' in contrib/ as it uses the same pattern. Signed-off-by: Junio C Hamano <[email protected]>
Why should we even do this? Doesn't it run on ubuntu-latest which has its own apt--get install for meson in the $distro specific set-up before this part? Signed-off-by: Junio C Hamano <[email protected]>
When downloading bundles via the bundle-uri functionality, we only copy the references from refs/heads into the refs/bundle space. I'm not sure why this refspec is hardcoded to be so limited, but it makes the ref negotiation on the subsequent fetch suboptimal, since it won't use objects that are referenced outside of the current heads of the bundled repository. This change to copy everything in refs/ in the bundle to refs/bundles/ significantly helps the subsequent fetch, since nearly all the references are now included in the negotiation. The update to the bundle-uri unbundling refspec puts all the heads from a bundle file into refs/bundle/heads instead of directly into refs/bundle/ so the tests also need to be updated to look in the new heirarchy. Signed-off-by: Scott Chacon <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
The change to the bundle-uri unbundling refspec now includes tags, so this adds a simple test to make sure that tags in a bundle are properly added to the cloned repository and will be included in ref negotiation with the subsequent fetch. Signed-off-by: Scott Chacon <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
In `split_symref_update()`, there were two checks for duplicate refnames: - At the start, `string_list_has_string()` ensures the refname is not already in `affected_refnames`, preventing duplicates from being added. - After adding the refname, another check verifies whether the newly inserted item has a `util` value. The second check is unnecessary because the first one guarantees that `string_list_insert()` will never encounter a preexisting entry. The `item->util` field is assigned to validate that a rename doesn't already exist in the list. The validation is done after the first check. As this check is removed, clean up the validation and the assignment of this field in `split_head_update()` and `files_transaction_prepare()`. Signed-off-by: Karthik Nayak <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
678989c
to
a01b8f1
Compare
256f341
to
c75e31e
Compare
8fdef1e
to
bc3287e
Compare
50a97e0
to
a842a77
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Go
Thanks for taking the time to contribute to Git! Please be advised that the
Git community does not use github.com for their contributions. Instead, we use
a mailing list ([email protected]) for code submissions, code reviews, and
bug reports. Nevertheless, you can use GitGitGadget (https://gitgitgadget.github.io/)
to conveniently send your Pull Requests commits to our mailing list.
For a single-commit pull request, please leave the pull request description
empty: your commit message itself should describe your changes.
Please read the "guidelines for contributing" linked above!