threads / discuss / 59604

Regression introduced in 03267e8656truct object_id oid;

Subject: Regression introduced in 03267e8656truct object_id oid;

## tl;dr

3 messages between Apr 16, 2023 and Apr 17, 2023.

replies: 2people: 2as markdown or json

Lars Kellogg-Stedman· Apr 16, 2023, 17:20 UTC · lore

[This behavior was originally reported in https://stackoverflow.com/q/76022720/147356.]

Prior to commit 03267e8656, the following sequence would result in an empty commit:

    git init testrepo
    date > testrepo/testfile
    git -C testrepo add testfile
    git -C testrepo commit -m test --allow-empty --only

After commit 03267e8656, the same sequence creates a commit that contains "testfile". I believe the original behavior was correct, based on the documentation for the `--only` option:

> If used together with --allow-empty paths are also not required, and
> an empty commit will be created.

A simple change that corrects this behavior would be to prevent git from refreshing the cache when --only is active:

    diff --git a/builtin/commit.c b/builtin/commit.c
    index 9d8e1ea91a..8c441548f8 100644
    --- a/builtin/commit.c
    +++ b/builtin/commit.c
    @@ -995,7 +995,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
        struct object_id oid;
        const char *parent = "HEAD";
     
    -		if (!the_index.cache_nr) {
    +		if (!the_index.cache_nr && !only) {
          discard_index(&the_index);
          if (repo_read_index(the_repository) < 0)
            die(_("Cannot read index"));

And while that passes all the tests, I'm not sure if its correct; the change in 03267e8656 was correcting a memory leak and I'm not sure I completely understand the details.

-- 
Lars Kellogg-Stedman <lars@oddbit.com> | larsks @ {irc,twitter,github}
http://blog.oddbit.com/                | N1LKS
Junio C Hamano· Apr 17, 2023, 16:54 UTC · re: Lars Kellogg-Stedman · lore

Re: Regression introduced in 03267e86

Lars Kellogg-Stedman <lars@oddbit.com> writes:
Show 14 quoted lines
> [This behavior was originally reported in
> https://stackoverflow.com/q/76022720/147356.]
>
> Prior to commit 03267e8656, the following sequence would result in an
> empty commit:
>
>     git init testrepo
>     date > testrepo/testfile
>     git -C testrepo add testfile
>     git -C testrepo commit -m test --allow-empty --only
>
> After commit 03267e8656, the same sequence creates a commit that
> contains "testfile". I believe the original behavior was correct,
> based on the documentation for the `--only` option:

It looks like a convoluted way (aka use of an obscure option) to create something useless (aka an empty root commit), but it should be allowed and the last step should do what you expected.

> And while that passes all the tests, I'm not sure if its correct; the
> change in 03267e8656 was correcting a memory leak and I'm not sure I
> completely understand the details.

Yeah, figuring out how 03267e86 (commit: discard partial cache before (re-)reading it, 2022-11-08) broke the behaviour is a good first step.

Thanks for reporting.

← back to recent threads