# Regression introduced in 03267e8656truct object_id oid;

3 messages from 2023-04-16 to 2023-04-17. Participants: Lars Kellogg-Stedman, Junio C Hamano.
Thread: https://gitlist.dev/t/59604

## Lars Kellogg-Stedman, 2023-04-16 17:20

Subject: Regression introduced in 03267e8656truct object_id oid;
Message-ID: <bt4342bdip3nzlikjsv6wozszmcbsc2av6cyo3z2lra4jhx3ii@ut2sl5h4f5xn>
URL: https://gitlist.dev/e/bt4342bdip3nzlikjsv6wozszmcbsc2av6cyo3z2lra4jhx3ii%40ut2sl5h4f5xn

```
[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

```

## Lars Kellogg-Stedman, 2023-04-16 17:30

Subject: Re: Regression introduced in 03267e8656
Message-ID: <5vey46onc6xwy5ifqieuut6m4ni3gpy5mweucmc2lroemtau3n@d5x4rgauq5zt>
URL: https://gitlist.dev/e/5vey46onc6xwy5ifqieuut6m4ni3gpy5mweucmc2lroemtau3n%40d5x4rgauq5zt
In-Reply-To: <bt4342bdip3nzlikjsv6wozszmcbsc2av6cyo3z2lra4jhx3ii@ut2sl5h4f5xn>

```
...with apologies for that munged subject: line.

-- 
Lars Kellogg-Stedman <lars@oddbit.com> | larsks @ {irc,twitter,github}
http://blog.oddbit.com/                | N1LKS

```

## Junio C Hamano, 2023-04-17 16:54

Subject: Re: Regression introduced in 03267e86
Message-ID: <xmqqttxemotc.fsf@gitster.g>
URL: https://gitlist.dev/e/xmqqttxemotc.fsf%40gitster.g
In-Reply-To: <bt4342bdip3nzlikjsv6wozszmcbsc2av6cyo3z2lra4jhx3ii@ut2sl5h4f5xn>

```
Lars Kellogg-Stedman <lars@oddbit.com> writes:

> [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.

```