Re: [PATCH] range-diff: allow to diff files regardless submodule

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Oct 12, 2018, 23:17 UTC
Message-ID: <20181012231742.GZ432229@genre.crustytoothpaste.net>
In-Reply-To: <nycvar.QRO.7.76.6.1810121122260.45@tvgsbejvaqbjf.bet>

On Fri, Oct 12, 2018 at 11:24:43AM +0200, Johannes Schindelin wrote:

Show 42 quoted lines

> 
> 
> On Thu, 11 Oct 2018, Lucas De Marchi wrote:
> 
> > On Wed, Oct 10, 2018 at 5:02 PM brian m. carlson
> > <sandals@crustytoothpaste.net> wrote:
> > >
> > > On Wed, Oct 10, 2018 at 08:09:16AM -0700, Lucas De Marchi wrote:
> > > > Do like it's done in grep so mode doesn't end up as
> > > > 0160000, which means range-diff doesn't work if one has
> > > > "submodule.diff = log" in the configuration. Without this
> > > > while using range-diff I only get a
> > > >
> > > >     Submodule a 0000000...0000000 (new submodule)
> > > >
> > > > instead of the diff between the revisions.
> > > >
> > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > ---
> > > >  range-diff.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/range-diff.c b/range-diff.c
> > > > index 60edb2f518..bd8083f2d1 100644
> > > > --- a/range-diff.c
> > > > +++ b/range-diff.c
> > > > @@ -354,7 +354,7 @@ static struct diff_filespec *get_filespec(const char *name, const char *p)
> > > >  {
> > > >       struct diff_filespec *spec = alloc_filespec(name);
> > > >
> > > > -     fill_filespec(spec, &null_oid, 0, 0644);
> > > > +     fill_filespec(spec, &null_oid, 0, 0100644);
> > >
> > > If we have a system that has different mode values from the common Unix
> > > ones, is this still correct or does it need to change?
> > 
> > From what I can see this would still be correct, or at least git-grep
> > implementation would be broken.
> 
> As you can see from the Windows port: we are stuck with the simplistic
> POSIX permissions in Git, and platforms that have a different permission
> system have to emulate it.

I think I may not have explained myself well. There are a small number of POSIXy systems which have mode bits that differ from the common ones (e.g., a plain file is something other than 0100000). I think one person mentioned on the list that they have a homebrew Unix that works this way, and I think I may have heard of some minor commercial Unices that work this way as well.

My question was intended to ask whether we should be using an OS-provided constant (e.g., S_IFREG) that represented that value differently because it was a system value or whether it was the internal Git representation.

I hadn't intended to inquire about Windows, as I was fairly confident that this syntax does indeed work there through our compatibility layers (because it has in the past even when we've had these kinds of issues on other Unices). But I'm glad that you chimed in and confirmed that it does.

-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204