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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 13, 2023, 22:22 UTC
Message-ID: <xmqqo7fx5m4r.fsf@gitster.g>
In-Reply-To: <20231113165031.GA28778@tb-raspi4>

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

Show 11 quoted lines

> 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.

Show 8 quoted lines

> 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.