Skip to content

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
wants to merge 127 commits into
base: seen
Choose a base branch
from

Conversation

Ballalhossaintalukder
Copy link

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!

gitster and others added 30 commits December 14, 2024 18:19
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]>
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]>
@gitster gitster force-pushed the seen branch 11 times, most recently from 678989c to a01b8f1 Compare June 11, 2025 22:21
@gitster gitster force-pushed the seen branch 11 times, most recently from 256f341 to c75e31e Compare June 19, 2025 14:41
@gitster gitster force-pushed the seen branch 6 times, most recently from 8fdef1e to bc3287e Compare June 25, 2025 22:44
@gitster gitster force-pushed the seen branch 2 times, most recently from 50a97e0 to a842a77 Compare July 1, 2025 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.