Add --force flag to stg push?

[larsks]

Using the rust branch, I run into this a lot:

1. stg pop --spill to incorporate some changes into a previous patch
2. git reset to unstage everything
3. stg refresh to update the patch after adding some updated files
4. stg push to return to the next patch in the series

This invariably fails with:

error: Index/worktree dirty

And that's because the stg pop --spill + git reset means that any files in the subsequent patch that aren't included in the current patch are now in the way. Using git checkout, we would see something like this:

error: The following untracked working tree files would be overwritten by checkout:
        cluster-scope/base/core/namespaces/external-secrets-operator/namespace.yaml
Please move or remove them before you switch branches.
Aborting

The solution here is generally git checkout --force, which will replace the indicated files.

It would be great if stg push had the same option, because sometimes cleaning this up is a pain (it would also be nice if stg push were to provide a list of files that are causing the error).

[larsks]

As a workaround:

current=$(git branch --show-current)
git checkout -f $(stg id $(stg next))
git checkout $current
stg push

[jpgrayson]

I'm having a hard time reproducing this issue. I've tried some simple test cases that perform the stg pop --spill, git reset, stg refresh, stg push sequence with various intervening edits to files, but in all the cases I've come up with the push works with the pushed patch either being gracefully modified or resulting in an expected merge conflict outcome.

So I could use some help fleshing out a reproducing test case.

Another thing to note is that stg push does list conflicting files when it encounters merge conflicts. The error: Index/worktree dirty error being encountered is outside the expected merge conflicts path of stg push.

[NonLogicalDev]

@jpgrayson I think the @larsks refers to having some files in the modified but Unstaged state after refreshing a subset.

I presume, they want to be able to ask push command to drop all changes that are not staged, or perhaps all changes that are not committed.

That sounds a bit dangerous though.

@larsks if you are looking for a quicker—get rid of my extra local changes—workaround I would suggest:

git stash -a
stg push

I also wonder if perhaps --keep flag is what might help as well.

[larsks]

I'm having a hard time reproducing this issue.

Here's a complete reproducer:

#!/bin/sh

rm -rf stgit-example-193
git init stgit-example-193
cd stgit-example-193
cat > README.md <<EOF
Reproducer for https://github.com/stacked-git/stgit/issues/193
EOF
git add README.md
git commit -m "Add README"

git checkout -b devel
stg init
stg new -m "Add files" add-files
date > file1
date > file2
stg add file{1,2}
stg refresh
stg pop --spill
git reset

stg new -m "Add file1" add-file1
stg add file1
stg refresh -i

# This will fail with `error: Index/worktree dirty`
stg push

Despite what I said earlier, there's not really a good workaround here -- in this failure, file2 is "in the way". The only solution is to first remove the conflicting file(s)...and stgit doesn't even tell us what the conflicting files are.

Having a stg push --force that was analagous to git checkout --force would make things much easier.

Note that in this situation, git checkout also provides a much better error message:

$ git checkout $(stg id add-files)
error: The following untracked working tree files would be overwritten by checkout:
        file2
Please move or remove them before you switch branches.
Aborting

And using the --force option replaces the conflicting files as desired:

$ git checkout --force $(stg id add-files)
HEAD is now at ecbf2d5 Add files

[larsks]

@jpgrayson just wanted to follow up and see if that last comment helped clarify things?

[jpgrayson]

Yes, this helps. Thank you for providing this use case, @larsks.

I agree that the error message provided by StGit is insufficient. There is a straightforward change to make StGit show the same message as git checkout, which is also what the Python implementation did. I will make that change.

For this particular use case, there is another workflow that might help:

date > file1
date > file2
stg add file{1,2}
stg new -rm add-files
# Instead of `stg pop --spill`, use `stg spill`
stg spill --reset
stg add file1
stg refresh -i
stg add file2
stg new -rm add-file2

The idea is to use stg spill to empty the current patch and then selectively reincorporate changes into the current patch and new follow-on patches. This approach doesn't require any pushes and thus avoids merges.

Regarding adding a --force flag to stg push, I'm currently feeling against it. As far as I can tell the only use case for the --force flag would be for this particular case, to force overwriting untracked files in the working tree. With the improved error messaging, which I will add, I prefer to let the user decide what to do with this kind of untracked file conflict versus complicating stg push with a narrowly applicable option. Let me know if I'm missing other use cases of --force.

[larsks]

Let me know if I'm missing other use cases of --force.

The problem I see is that if there are a large number of conflicting files, the cleanup may still be messy even with a better error message. Absent something like --force, we would need something like stg list-files-that-conflict-with-next-patch so that we could pipe that into xargs rm or something to clean things up.

In my experience it's relatively easy to get into this state when using stg pop --spill to split up a large commit. I like the alternate workflow you've proposed, but for situations in which you want to take changes in the current patch and insert them before the current patch, the pop --spill workflow seems more intuitive.

[jpgrayson]

This use case and the problem of push-time conflicts with untracked files is not something I've run into before, or at least not enough to want StGit to do more. So I'm trying to decide how much weight to give this issue. I appreciate the detail you've provided about your experience.

If we were to add a stg push --force option, the next issue is that it's not immediately clear to me how to implement that feature in StGit. There are two distinct code paths that can be tripped-up by conflicts with untracked files during a push.

1. If the push requires a merge, then StGit attempts to realize the push using git merge-recursive. The cases detailed above exercise this path.
2. If the push does not require a merge, i.e. because tree ids of the patch, patch parent, and current commit match up nicely, StGit can bump into untracked file conflicts later in the process, when the stack transaction is being executed and the final working tree checkout is being performed with git read-tree -m -u.

Neither git merge-recursive nor git read-tree have an option that would map to the --force behavior being discussed. So that leaves us with either attempting to perform the push-time merge and/or execute-time checkout using another mechanism, or having StGit attempt a post-failure recovery that correctly identifies the untracked conflict error condition and does something to recover. Neither of these seem straightforward to me and both seem highly risky; in my experience push and transaction execute are the most sensitive/tricky code paths in StGit, which is why they are implemented with subordinate git processes instead of libgit2.

So I'm sympathetic to this use case, wary of an extra --force option to stg push, and not sure of a technical path to do much more about it.

What would be ideal, IMO, would be if the git merge-recursive could be replaced with git2 API calls. This approach would give us more control over the post-merge checkout which is where untracked file conflicts would be uncovered. I could imagine comparing the hash of the untracked file in the working tree with that file's hash in the index to determine whether it is safe to overwrite. But again, when reimplementing StGit with Rust my first attempts favored git2 versus punting to the git executable, but I had lots of trouble with the git2 API behaving subtly different than the analogous git invocations.

[NeMeCChip]

Using the rust branch, I run into this a lot:

1. stg pop --spill to incorporate some changes into a previous patch
2. git reset to unstage everything
3. stg refresh to update the patch after adding some updated files
4. stg push to return to the next patch in the series

This invariably fails with:

error: Index/worktree dirty

And that's because the stg pop --spill + git reset means that any files in the subsequent patch that aren't included in the current patch are now in the way. Using git checkout, we would see something like this:

error: The following untracked working tree files would be overwritten by checkout:
        cluster-scope/base/core/namespaces/external-secrets-operator/namespace.yaml
Please move or remove them before you switch branches.
Aborting

The solution here is generally git checkout --force, which will replace the indicated files.

It would be great if stg push had the same option, because sometimes cleaning this up is a pain (it would also be nice if stg push were to provide a list of files that are causing the error).

@jpgrayson just wanted to follow up and see if that last comment helped clarify things?