From: Stefan Sperling Subject: Re: make worktree diffs record xbit of new files To: Mark Jamsek Cc: Game of Trees Date: Thu, 22 Sep 2022 16:45:09 +0200 On Fri, Sep 23, 2022 at 12:20:26AM +1000, Mark Jamsek wrote: > On 22-09-22 05:40PM, Mark Jamsek wrote: > > On 22-09-21 10:22PM, Mark Jamsek wrote: > > > Related to the recent work: when running `got diff` in a worktree with > > > a file that has the x-bit and has been added with `got add`, we don't > > > presently record this mode like we do with `got diff obj1 obj2` or > > > `got diff -c obj`. > > > > > > The below change makes `got diff` in a worktree record this information > > > like other diff modes. > > > > Actually, there's more to consider; > > ... > > What do we actually want to do here? It seems at present we only record > > the file mode for newly added files with all `got diff` formats _except_ > > for worktree diffs. I think we should be consistent and show the same > > info in all `got diff` modes. Otherwise, the proposed diff should be > > modified to only show the file mode in worktree diffs for new > > files--like we already do for our other diffs. > > > > I think at least one of the above is needed, however, because at > > present, a worktree diff of a new added file with +x doesn't contain > > that information. So `got patch` will fail to set the x-bit. > > > > In sum: > > > > - `got diff obj obj` and `got diff -c obj [-c obj]` show the file mode > > for _new_ +x files; however, this is not shown for changed files > > - `got diff` in a work tree does not show this information for neither > > new nor changed files > > - all versions of `git diff` show the file mode for both new and > > changed files > > After thinking about this more, I think it makes sense to make `got > diff` in a worktree display the same information as `got diff obj1 obj2` > and `got diff -c obj [-c obj]` and only show the file mode for new added > files. This enables `got patch` to successfully apply patches made with > `got diff` of changes in the worktree. I think it's safe to assume that > there's enough information on disk to successfully apply patches > involving managed files, and showing the file mode all the time is > overkill--we don't currently do this so why start now. > > The following diff makes this change. I've also tentatively updated diff > regress tests in a separate diff (at the end of this mail) so you can > run the existing tests first to easily see the change this diff makes. Makes sense to me. Thanks for working this out. ok stsp@