git amend treats intended-to-add files as staged changes

[arxanas]

Description of the bug

git add --intent-to-add/git add -N can be used to add untracked files to diff/commit later, without staging them. git-branchless treats the intended-to-add file as if it were a staged change, not an unstaged change. This is correct from a technical perspective, but it leads to bizarre behavior when running git branchless amend where Git thinks immediately afterwards that the file in question has been staged as deleted.

It also means that git branchless amend won't have amended any unstaged changes which the user may have intended to include (because it only ever amends using either staged changes or unstaged changes).

This happens because git add -N adds the entry to the index with empty contents, even though, logically, an intended-to-add file is an unstaged change. Fortunately, the index should include a bit indicating that the file in question was intended-to-add https://github.com/git/git/blob/867b1c1bf68363bcfd17667d6d4b9031fa6a1300/Documentation/technical/index-format.txt#L103:

  (Version 3 or later) A 16-bit field, only applicable if the
  "extended flag" above is 1, split into (high to low bits).

    1-bit reserved for future

    1-bit skip-worktree flag (used by sparse checkout)

    1-bit intent-to-add flag (used by "git add -N")

    13-bit unused, must be zero

We need to check the intent-to-add flag and behave differently in that case. To fix this, we need to update the get_status function

git-branchless/git-branchless-lib/src/git/repo.rs

Lines 703 to 707 in f6b4df2

	pub fn get_status(
	&self,
	git_run_info: &GitRunInfo,
	event_tx_id: Option<EventTransactionId>,
	) -> eyre::Result<Vec<StatusEntry>> {

to detect intended-to-add files and not treat them as if they were staged changes.

I don't know if a file in practice a file can have stage = 1 and intent-to-add = 1. (Perhaps that can happen if you immediately git add the file afterwards?) We should check all the combinations where stage = {0,1} and intent-to-add = {0,1} and see what would be the most sensible behavior.

Expected behavior

$ git add -N foo
$ git branchless amend
Amended with 1 uncommitted change.
$ git status
On branch main
nothing to commit, working tree clean

Actual behavior

$ git add -N foo
$ git branchless amend
Amended with 1 staged change. (Some uncommitted changes were not amended.)
$ git status
On branch main
Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        deleted:    foo

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        new file:   foo

Version of git-branchless

git-branchless 0.3.12

Version of git

git version 2.30.2

Version of rustc

No response

Automated bug report

Software version

git-branchless 0.3.12 (git-branchless-lib-v0.3.11-5-ge5e77c9)

Operating system

macOS 11.6.5 (Darwin 20.6.0)

Command-line

/Users/Waleed/.cargo/bin/git-branchless bug-report 

Environment variables

SHELL=/bin/zsh
EDITOR=/usr/bin/vim

Git version

> git version 
git version 2.30.2

Events

Show 5 events

Event ID: 7963, transaction ID: 36213

1. RewriteEvent { timestamp: 1649886197.651536, event_tx_id: EventTransactionId(36213), old_commit_oid: 49e37aaebbe149edf91ca62fed10332c141d9db7, new_commit_oid: d33ae4374335f42eb49143ea5f5741805e62c32f }

:
O 1e15da3 4h (remote origin/master) xxxxxxxxxxxxx xxxxxxx xxxxxxxx xx xxxxxxxx xxxxxxx xxxxxxxxx
|\
| o 99142ba3 4h xxxxxxx xxxxx xxxxxxxx xxxxx xx xxxx xx xx
|\
| o d9e2e2c 4h xxxxx xxxxxxxxxxxxxx
| |
| o 88518421 4h xxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
| o 3839a97 4h xxxxxxxxxxxxxxxxxxxxxx xxxxxx xxxxxxxxxxxxxx xxxxxxxxxxxxxxxxx
| |
| o 33da3a49 4h xxxxxxxxxxxxxxxxxxxxxx xxx xxxxxxxxxxxxxxxxxxxxxxxxx xxxxxx
| |
| o 08348da 4h xxxxx xxxxx xxxxx xx xxxx
| |
| o 1ce32a0 4h xxxxx xxx xxxxxxxxxxxxxxxxx xxx xxxxxxx
| |
| o 93c2d94 4h xxxxxxxxxxxxxxxxxxxxxx xxx xxxxxxxxxxxxxxxxxxxxxxx xxxxxx
| |
| o c2943bb 4h xxxxxxxxxxxxxxxx xxxxxx xxxxxxxxx xxx xxxxxxx
| |
| o f75d95b 4h xxxxx xxxxxxxxx xxxxxxx xxxxxxxxxx xxxxx
| |
| o a914204 4h xxxxx xxxx xxxxx xxxxxxx
|\
| o 7e33776 4h xxxxx xxxxxxxxxx xxxxxx
|\
| o b89735e 4h xxxxx xxxx xxxxxxxxx xxxxxx xx xxxxxxxxxxxxxxxx
|\
| o f21c104 4h xxxxxx xxxxxx xxxx xxxxxx xxxx xxxxxxx
| |
| o 92c18cb 4h xxxxxxx xxxxxxxx xxxx xxx xxxxxxxxxx
| |
| o 66324f9 4h xxxxx xxxx xxxxxx xxxx xxxxxx
|
o 462add4 2h xxxxx xxxxxx xx xxxxxxxxxx xxxxxxx xxxxxxxxx
|\
| x 49e37aa 25m (rewritten as d33ae437) xxxxx xxx xxxxxxxxxxxxxxxxxx xx xxxxxxx xxxxx
| |
| o da43056b 25m xxxxx xxxxxxxxxx xxxxxxxx
|
@ d33ae43 24m xxxxx xxx xxxxxxxxxxxxxxxxxx xx xxxxxxx xxxxx

Event ID: 7961, transaction ID: 36204

1. RewriteEvent { timestamp: 1649886183.306387, event_tx_id: EventTransactionId(36204), old_commit_oid: 16b9df0074e734b179f13b3941b7692c63e92ed6, new_commit_oid: 49e37aaebbe149edf91ca62fed10332c141d9db7 }
2. RewriteEvent { timestamp: 1649886187.012609, event_tx_id: EventTransactionId(36204), old_commit_oid: 9a1296e85b91fc4ab4f41d274eeb30b1a2257102, new_commit_oid: da43056b92a7968978e09f2e73113faca845ea96 }

:
O 1e15da3 4h (remote origin/master) xxxxxxxxxxxxx xxxxxxx xxxxxxxx xx xxxxxxxx xxxxxxx xxxxxxxxx
|\
| o 99142ba3 4h xxxxxxx xxxxx xxxxxxxx xxxxx xx xxxx xx xx
|\
| o d9e2e2c 4h xxxxx xxxxxxxxxxxxxx
| |
| o 88518421 4h xxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
| o 3839a97 4h xxxxxxxxxxxxxxxxxxxxxx xxxxxx xxxxxxxxxxxxxx xxxxxxxxxxxxxxxxx
| |
| o 33da3a49 4h xxxxxxxxxxxxxxxxxxxxxx xxx xxxxxxxxxxxxxxxxxxxxxxxxx xxxxxx
| |
| o 08348da 4h xxxxx xxxxx xxxxx xx xxxx
| |
| o 1ce32a0 4h xxxxx xxx xxxxxxxxxxxxxxxxx xxx xxxxxxx
| |
| o 93c2d94 4h xxxxxxxxxxxxxxxxxxxxxx xxx xxxxxxxxxxxxxxxxxxxxxxx xxxxxx
| |
| o c2943bb 4h xxxxxxxxxxxxxxxx xxxxxx xxxxxxxxx xxx xxxxxxx
| |
| o f75d95b 4h xxxxx xxxxxxxxx xxxxxxx xxxxxxxxxx xxxxx
| |
| o a914204 4h xxxxx xxxx xxxxx xxxxxxx
|\
| o 7e33776 4h xxxxx xxxxxxxxxx xxxxxx
|\
| o b89735e 4h xxxxx xxxx xxxxxxxxx xxxxxx xx xxxxxxxxxxxxxxxx
|\
| o f21c104 4h xxxxxx xxxxxx xxxx xxxxxx xxxx xxxxxxx
| |
| o 92c18cb 4h xxxxxxx xxxxxxxx xxxx xxx xxxxxxxxxx
| |
| o 66324f9 4h xxxxx xxxx xxxxxx xxxx xxxxxx
|
o 462add4 2h xxxxx xxxxxx xx xxxxxxxxxx xxxxxxx xxxxxxxxx
|\
| x 49e37aa 25m xxxxx xxx xxxxxxxxxxxxxxxxxx xx xxxxxxx xxxxx
| |
| o da43056b 25m xxxxx xxxxxxxxxx xxxxxxxx
|
@ d33ae43 24m xxxxx xxx xxxxxxxxxxxxxxxxxx xx xxxxxxx xxxxx

Event ID: 7960, transaction ID: 36122

1. RefUpdateEvent { timestamp: 1649885241.916306, event_tx_id: EventTransactionId(36122), ref_name: "HEAD", old_oid: 9a1296e85b91fc4ab4f41d274eeb30b1a2257102, new_oid: 16b9df0074e734b179f13b3941b7692c63e92ed6, message: None }

:
O 1e15da3 4h (remote origin/master) xxxxxxxxxxxxx xxxxxxx xxxxxxxx xx xxxxxxxx xxxxxxx xxxxxxxxx
|\
| o 99142ba3 4h xxxxxxx xxxxx xxxxxxxx xxxxx xx xxxx xx xx
|\
| o d9e2e2c 4h xxxxx xxxxxxxxxxxxxx
| |
| o 88518421 4h xxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
| o 3839a97 4h xxxxxxxxxxxxxxxxxxxxxx xxxxxx xxxxxxxxxxxxxx xxxxxxxxxxxxxxxxx
| |
| o 33da3a49 4h xxxxxxxxxxxxxxxxxxxxxx xxx xxxxxxxxxxxxxxxxxxxxxxxxx xxxxxx
| |
| o 08348da 4h xxxxx xxxxx xxxxx xx xxxx
| |
| o 1ce32a0 4h xxxxx xxx xxxxxxxxxxxxxxxxx xxx xxxxxxx
| |
| o 93c2d94 4h xxxxxxxxxxxxxxxxxxxxxx xxx xxxxxxxxxxxxxxxxxxxxxxx xxxxxx
| |
| o c2943bb 4h xxxxxxxxxxxxxxxx xxxxxx xxxxxxxxx xxx xxxxxxx
| |
| o f75d95b 4h xxxxx xxxxxxxxx xxxxxxx xxxxxxxxxx xxxxx
| |
| o a914204 4h xxxxx xxxx xxxxx xxxxxxx
|\
| o 7e33776 4h xxxxx xxxxxxxxxx xxxxxx
|\
| o b89735e 4h xxxxx xxxx xxxxxxxxx xxxxxx xx xxxxxxxxxxxxxxxx
|\
| o f21c104 4h xxxxxx xxxxxx xxxx xxxxxx xxxx xxxxxxx
| |
| o 92c18cb 4h xxxxxxx xxxxxxxx xxxx xxx xxxxxxxxxx
| |
| o 66324f9 4h xxxxx xxxx xxxxxx xxxx xxxxxx
|
o 462add4 2h xxxxx xxxxxx xx xxxxxxxxxx xxxxxxx xxxxxxxxx
|\
| x 49e37aa 25m xxxxx xxx xxxxxxxxxxxxxxxxxx xx xxxxxxx xxxxx
| |
| o da43056b 25m xxxxx xxxxxxxxxx xxxxxxxx
|
@ d33ae43 24m xxxxx xxx xxxxxxxxxxxxxxxxxx xx xxxxxxx xxxxx

Event ID: 7959, transaction ID: 36107

1. RewriteEvent { timestamp: 1649885178.308212, event_tx_id: EventTransactionId(36107), old_commit_oid: 74ef43562b1375482e55a78fed7f921025659a2f, new_commit_oid: 9a1296e85b91fc4ab4f41d274eeb30b1a2257102 }

:
O 1e15da3 4h (remote origin/master) xxxxxxxxxxxxx xxxxxxx xxxxxxxx xx xxxxxxxx xxxxxxx xxxxxxxxx
|\
| o 99142ba3 4h xxxxxxx xxxxx xxxxxxxx xxxxx xx xxxx xx xx
|\
| o d9e2e2c 4h xxxxx xxxxxxxxxxxxxx
| |
| o 88518421 4h xxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
| o 3839a97 4h xxxxxxxxxxxxxxxxxxxxxx xxxxxx xxxxxxxxxxxxxx xxxxxxxxxxxxxxxxx
| |
| o 33da3a49 4h xxxxxxxxxxxxxxxxxxxxxx xxx xxxxxxxxxxxxxxxxxxxxxxxxx xxxxxx
| |
| o 08348da 4h xxxxx xxxxx xxxxx xx xxxx
| |
| o 1ce32a0 4h xxxxx xxx xxxxxxxxxxxxxxxxx xxx xxxxxxx
| |
| o 93c2d94 4h xxxxxxxxxxxxxxxxxxxxxx xxx xxxxxxxxxxxxxxxxxxxxxxx xxxxxx
| |
| o c2943bb 4h xxxxxxxxxxxxxxxx xxxxxx xxxxxxxxx xxx xxxxxxx
| |
| o f75d95b 4h xxxxx xxxxxxxxx xxxxxxx xxxxxxxxxx xxxxx
| |
| o a914204 4h xxxxx xxxx xxxxx xxxxxxx
|\
| o 7e33776 4h xxxxx xxxxxxxxxx xxxxxx
|\
| o b89735e 4h xxxxx xxxx xxxxxxxxx xxxxxx xx xxxxxxxxxxxxxxxx
|\
| o f21c104 4h xxxxxx xxxxxx xxxx xxxxxx xxxx xxxxxxx
| |
| o 92c18cb 4h xxxxxxx xxxxxxxx xxxx xxx xxxxxxxxxx
| |
| o 66324f9 4h xxxxx xxxx xxxxxx xxxx xxxxxx
|
o 462add4 2h xxxxx xxxxxx xx xxxxxxxxxx xxxxxxx xxxxxxxxx
|\
| x 49e37aa 25m xxxxx xxx xxxxxxxxxxxxxxxxxx xx xxxxxxx xxxxx
| |
| o da43056b 25m xxxxx xxxxxxxxxx xxxxxxxx
|
@ d33ae43 24m xxxxx xxx xxxxxxxxxxxxxxxxxx xx xxxxxxx xxxxx

Event ID: 7958, transaction ID: 36092

1. CommitEvent { timestamp: 1649883794.0, event_tx_id: EventTransactionId(36092), commit_oid: NonZeroOid(74ef43562b1375482e55a78fed7f921025659a2f) }

:
O 1e15da3 4h (remote origin/master) xxxxxxxxxxxxx xxxxxxx xxxxxxxx xx xxxxxxxx xxxxxxx xxxxxxxxx
|\
| o 99142ba3 4h xxxxxxx xxxxx xxxxxxxx xxxxx xx xxxx xx xx
|\
| o d9e2e2c 4h xxxxx xxxxxxxxxxxxxx
| |
| o 88518421 4h xxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
|\
| o 3839a97 4h xxxxxxxxxxxxxxxxxxxxxx xxxxxx xxxxxxxxxxxxxx xxxxxxxxxxxxxxxxx
| |
| o 33da3a49 4h xxxxxxxxxxxxxxxxxxxxxx xxx xxxxxxxxxxxxxxxxxxxxxxxxx xxxxxx
| |
| o 08348da 4h xxxxx xxxxx xxxxx xx xxxx
| |
| o 1ce32a0 4h xxxxx xxx xxxxxxxxxxxxxxxxx xxx xxxxxxx
| |
| o 93c2d94 4h xxxxxxxxxxxxxxxxxxxxxx xxx xxxxxxxxxxxxxxxxxxxxxxx xxxxxx
| |
| o c2943bb 4h xxxxxxxxxxxxxxxx xxxxxx xxxxxxxxx xxx xxxxxxx
| |
| o f75d95b 4h xxxxx xxxxxxxxx xxxxxxx xxxxxxxxxx xxxxx
| |
| o a914204 4h xxxxx xxxx xxxxx xxxxxxx
|\
| o 7e33776 4h xxxxx xxxxxxxxxx xxxxxx
|\
| o b89735e 4h xxxxx xxxx xxxxxxxxx xxxxxx xx xxxxxxxxxxxxxxxx
|\
| o f21c104 4h xxxxxx xxxxxx xxxx xxxxxx xxxx xxxxxxx
| |
| o 92c18cb 4h xxxxxxx xxxxxxxx xxxx xxx xxxxxxxxxx
| |
| o 66324f9 4h xxxxx xxxx xxxxxx xxxx xxxxxx
|
o 462add4 2h xxxxx xxxxxx xx xxxxxxxxxx xxxxxxx xxxxxxxxx
|\
| x 49e37aa 25m xxxxx xxx xxxxxxxxxxxxxxxxxx xx xxxxxxx xxxxx
| |
| o da43056b 25m xxxxx xxxxxxxxxx xxxxxxxx
|
@ d33ae43 24m xxxxx xxx xxxxxxxxxxxxxxxxxx xx xxxxxxx xxxxx

[JaniM]

I spent some time analyzying this, but I hit a bit of a wall. I found that intent-to-add files can be recognized from index_status == Unmodified && working_copy_status == Added in git-branchless-lib/src/git/status.rs. I also presume the index bitfield you mentioned is read in create_commit_for_stage in git-branchless-lib/src/git/snapshot.rs, when we read the index.

However, I'm not at all sure how to combine either of these facts to change the result of WorkingCopySnapshot::get_working_copy_changes_type.

[arxanas]

@JaniM My hope was that we could simply change the returned statuses when we detected this condition in such a way that it would match a normal uncommitted change, but not sure if it fails to work out for some reason. Or maybe we could skip files that appear to be intended-to-add in WorkingCopySnapshot::create_commit_for_stage by checking the status in the for loop (but we'd have to make sure that it correctly includes files that are intended-to-add and have actually been staged).