Re: [PATCH 0/2] read-tree: improve untracked file support
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- May 7, 2019, 10:01 UTC
- Message-ID
- <994d4687-39ed-91b9-cdf9-d4be39cbb2ba@gmail.com>
- In-Reply-To
- <CACsJy8AAj8cRAc+PnS4OJy8M=K7FjCHUsL6Cx-6+RTAbTA_1fQ@mail.gmail.com>
On 02/05/2019 11:53, Duy Nguyen wrote:
Show 55 quoted lines
> On Wed, May 1, 2019 at 9:58 PM Phillip Wood <phillip.wood123@gmail.com> wrote: >> On 01/05/2019 11:31, Duy Nguyen wrote: >>> On Wed, May 1, 2019 at 5:14 PM Phillip Wood <phillip.wood123@gmail.com> wrote: >>>> >>>> From: Phillip Wood <phillip.wood@dunelm.org.uk> >>>> >>>> These two patches teach read-tree how to avoid overwriting untracked >>>> files when doing '--reset -u' and also how to respect all of git's >>>> standard excludes files. I'd like to see the porcelain commands stop >>>> overwriting untracked files, this is a first step on the way. I'm not >>>> sure if we want to add options to the porcelain commands to protect >>>> untracked files or just change their behavior and add an option to >>>> override that. I'm leaning towards the latter but I'd be interested to >>>> hear what others think. >>> >>> For new commands like git-restore, it's definitely a good thing to not >>> overwrite untracked files. >> >> I agree, unfortunately this series does not help with git-restore, only >> git-switch. For restore on an index without conflicts I think it could >> just use the pathspec in struct unpack_trees_options and set opts.rest = >> UNPACK_RESET_PROTECT_UNTRACKED but that does not help if we want to >> handle conflicted paths differently to non-conflicted paths. > > Right. I got confused. You did mention "git checkout <rev> :/" in 1/2, > which is the same as "git restore --source <rev> --staged --worktree > :/" and can also potentially overwrite untracked files, even though > it does not use unpack-trees and cannot be fixed with this. Never > mind. Let's leave git-restore out of the discussion for now. > >>> For existing commands I guess we have to go >>> over them one by one. For "git reset --hard", it should really just >>> overwrite whatever needed to get back to the known good state. "git >>> checkout -f" , not so sure (seems weird that we need force-level-two >>> option to override the protection provided by -f, if we change default >>> behavior) >> >> I think it's fine for "checkout -f" to overwrite untracked files (and if >> "switch --discard-changes" does not then there is no pressing need to >> add such a mode to checkout), --force is a good name for an option that >> nukes everything that gets in it's way. For "reset --hard" I'm not so >> sure, if I have changes to an untracked file I don't wont them >> overwritten by default. There is no porcelain equivalent to "read-tree >> --reset --protect-untracked -u" and I was hoping "reset --hard" may >> become that porcelain equivalent with a new --force or >> --overwrite-untracked option. > > My (biased, obviously) view is that "git reset --hard" is very > dangerous and I'm not trying to change that, especially when its > behavior has been like this since forever and I'm sure it's used in > scripts. > > Instead "git restore" should be used when you need "git reset --hard > HEAD", the most often use case. And since it's new, changing default > behavior is not a problem. Which brings us back to git-restore :)
Does restore clean up the branch state like reset? It's tricky because you only want to do that if there is no pathspec (or the pathspec is :/ or equivalent - I can't remember if restore always requires paths or not)
> But either way, git-restore or git-reset, I still don't see why > untracked files are more valuable in this case than tracked ones to > change the default.
My issue is that is easy to see what changes you're going to lose in tracked files by running diff. For untracked files diff just says a new file will be created, it ignores the current contents as the path is in the index so it is easy to overwrite changes without realizing. There's also a philosophical point that git should not be stomping on paths that it is not tracking though that's a bit moot if a path is tracked in one revision but not another.
Show 14 quoted lines
> I can see that sometimes you may want to restore > just tracked files, or untracked files, almost like filtering with > pathspec. > >> For the various "foo --abort" some (most?) are using "reset --merge" >> which I think declines to overwrite untracked files but rebase uses >> "reset --hard" which I'd like to change to protect untracked files in >> the same way that rebase does for the initial checkout and when picking >> commits. I haven't thought about stash. > > Yeah it looks like cherry-pick and revert use "reset --merge" too > (reset_for_rollback function). That's all of them. Probably a stupid > question, why can't rebase just use "rebase --merge" like everybody > else?
I'm not sure - if --merge works for the others I can't see why it shouldn't work for rebase as well.
Best Wishes
Phillip
Show 6 quoted lines
>> Best Wishes >> >> Phillip >> > >