Re: [PATCH 1/1] attr: add native file mode values support

[Junio C Hamano <gitster@pobox.com>]

Torsten Bögershausen <tboegi@web.de> writes:

> On Sat, Nov 11, 2023 at 04:03:08AM +0000, Joanna Wang wrote:
>
> Some thoughts and comments inline...
>
>> Gives all paths inherent 'mode' attribute values based on the paths'
>> modes (one of 100644, 100755, 120000, 040000, 160000). Users may use
>> this feature to filter by file types. For example a pathspec such as
>> ':(attr:mode=160000)' could filter for submodules without needing
>
> My spontanous feeling is that filetype may be another choice:
>> ':(attr:filetype=160000)' could filter for submodules without needing

I do agree that "mode" invites "mode of what???" reaction, and that
a term that narrows the scope would be preferrable.  "Filemode" is a
bit questionable, though, as we give this permbits to non-files like
submodules.  "ls-tree" documentation seems to call it %(objectmode).

> And having written this, we can think using something borrowed from
> `find . -type f`
>
> :(attr:filetype=f)' or :(attr:filetype=x)' (for executable)

This would not work for submodules, though.  Naively one might want
to abuse 'd' but I suspect we would eventually want to be able to
give the mode bits to an out-of-cone directory storeed in the index
as a tree in a cone-mode sparse checkout, which would be 040000,
which deserves 'd' more than submodules.

> But then I missed the point why we need an attribute here?
> The mode is already defined by the the file system (and Git),
> is there a special reason that the user can define or re-define the
> value here ?

I think the idea is that "mode" being a too generic word can be used
for totally different purposes in existing projects and the addition
did not want to disturb their own use.  But stepping back a bit,
such an application is likely marking selected few paths with the
attribute, and paths for which "mode" was "unset" are now given
these natural "mode"; it is inevitable to crash with such uses.  If
we want to introduce "native" attributes of this kind, we would
probably need to carve out namespaces a bit more clearaly.

> May be there is, when working with pathspec.
> But then "pathspec=" could be a better construction.
> Since "mode" could make a reader think that Git does somewhat with the file
> when checking out.
> My personal hope reading "mode=100755" in .gitattributes would
> be that Git makes it executable when checking out, if if it is
> recorded in Git as 100644, probably coming from a file-system that
> doesn't support the executable bit in a Unix way.

That is not the intended way this attribute is to be used.  Perhaps
we should make it an error (or ignored) when certain built-in/native
attributes are seen in the attribute file, but again that takes some
namespace carved out to avoid crashing with end-user names.

>> If there is any existing mode attribute for a path (e.g. there is
>> !mode, -mode, mode, mode=<value> in .gitattributes) that setting will
>> take precedence over the native mode value.

Again, this has one hole, I think.  Paths that are not mentioned
(not even with "!mode") would come to the function as ATTR__UNKNOWN
and trigger the fallback behaviour, even when other paths are given
end-user specified "mode" attribute values.