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@