[BUG] git add -A --dry-run adds files to .git directory

Thank you for filling out a Git bug report! Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)

mat@charly:~$ git init repo mat@charly:~$ cat /dev/random | head -c 100000000 > repo/randomfile mat@charly:~$ git -C repo add -A -n mat@charly:~$ du -hs repo/.git

What did you expect to happen? (Expected behavior)

  76K    repo/.git

What happened instead? (Actual behavior)

  97M    repo/.git

What's different between what you expected and what actually happened?

Even though I specified -n on the command line to make a dry run, it still added the large file as an object.

Anything else you want to add:

This is counter-intuitive behaviour.  When the documentation says "Don't actually add the file(s)", it's reasonable to expect that this command will leave the repository state as it found it.  If there's a good reason why these files should be added to the .git directory, that behaviour should be made clear in the documentation, probably including the reason why that counter-intuitive behaviour is necessary.

This bit me due to a script that was using git add -A -n to detect if a repository had been locally modified.  While I do have alternative (actually better) ways to get this information, it wasn't an unreasonable command to use in the first place, and it caused a real issue in a production system.

Please review the rest of the bug report below. You can delete any lines you don't wish to share.

[System Info] git version: git version 2.30.1 (Apple Git-130) cpu: x86_64 no commit associated with this build sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh uname: Darwin 20.6.0 Darwin Kernel Version 20.6.0: Mon Aug 30 06:12:21 PDT 2021; root:xnu-7195.141.6~3/RELEASE_X86_64 x86_64 compiler info: clang: 13.0.0 (clang-1300.0.29.3) libc info: no libc information available $SHELL (typically, interactive shell): /bin/bash

[Enabled Hooks]

When the option --dry-run/-n is given, "git add" doesn't change the index, but still writes out new object files. Only hash the latter without writing instead to make the run as dry as possible.

Use this opportunity to also make the hash_flags variable unsigned, to match the index_path() parameter it is used as.

Reported-by: git.mexon@spamgourmet.com
Signed-off-by: René Scharfe <l.s.r@web.de>
---
Am I missing something?  Do we sometimes rely on the written objects
within the "git add --dry-run" command?

 read-cache.c          | 2 +-
 t/t2200-add-update.sh | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

René Scharfe <l.s.r@web.de> writes:

Good question. I do not think of anything offhand, but this obvious "omission" makes me suspect that we may be forgetting something.

Thanks.

On Tue, Oct 12 2021, René Scharfe wrote:

Probably not, here's a semi-related patch of mine that never got integrated. E.g. you'll probably find that even if you're not writing objects we're still doing things like zlib compression here too (or not, I haven't looked): https://lore.kernel.org/git/20190520222932.22843-1-avarab@gmail.com/

I think the "git fetch --dry-run" command behaves like this too, i.e. doesn't update refs, but fetches and writes objects.

For the patch I hacked up I think it's easy to argue that it shouldn't do compression etc.

For this sort of thing and "fetch" I'm not so sure. Do we really know that there aren't people who rely on this for say the performance of seeing what an operation would do, and then not pay as much for the "real one" that updates the index/refs/etc. later? Is that subsequent "fetch" cheaper because of the --dry-run?

Maybe not, but it seems like something to look into.

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

The answer to the last one is an easy "yes". Trying to gauge the time it would take for a real fetch with "--dry-run" is a losing battle, I would think, as the pre-fetching would make the "real" one cheaper, so from that point of view, I think we can ignore those who time "--dry-run" and try to figure out anything meaningful.

This in any case is an interesting area, as the definition of correctness of what "dry-run" does can be quite fuzzy. As long as it does not change the index, "git add --dry-run", even if it writes objects or detects filesystem corruption by noticing I/O error while compressing the data taken from the working tree files, is still correct and the patch in question is not technically a bugfix (it is a performance thing). "git fetch --dry-run" would fall into the same category, so would "git hash-object" without "-w".

All can use performance enhancement without breaking existing users, I would think.

Thanks.