The commit log is inconsistent with the actual patch. It says "lack of a Qt version that supports this", but the patch doesn't use QFile at all, it uses statx. Either way, the patch can be tested, no?

This patch doesn't actually directly use statx(); it uses the existing approach, which goes through QFile, which is used to populate the statbuf object that we query. QT has gained support for populating the statbuf object using statx() in Linux where available in current git master, but that change hasn't landed in an actual released QT version yet.

If you can point me to a way to test this, I'll be happy to.

I can't comment on the content of the patch, but about the commit message, in order to close a bug, use the syntax:

BUG: nnnnnn

while in order to add the commit as comment of a bug without closing it, use

CCBUG: nnnnnn

Both keyword should be in a separate line.

Also, it's Qt, not QT.

Noted! Thanks, Luigi. I'd like to add BUG: 286689 to the commit message on a separate line. Does the title of this revision become the commit message? If so, how do I add another line? The text field doesn't seem to support multi-line strings.

The title of the revision becomes the first line; all the rest (summary, test, reviewers) become the rest of the content, a blank separator line is added automatically. Please add the BUG: line in the summary, not in the title.
Also, as the title becomes the first line, it would be nice to follow the usual git rules of length.

Ah, OK, I see.
Well, still, you could test the patch by compiling qtbase dev (or by applying whichever Qt patch is necessary).

Great, thanks guys. Thanks for bearing with me here. I'll see if I can test with the unreleased qtbase.

I haven't forgotten about this, but life has intervened and I'm not going to have time to work on it for a few days, maybe even a couple of weeks. If anyone else wants to run with it and take over, I won't be in the least bit offended. If not, I'll resume work at some point in the near to medium term future.

I don't get it. Which parts of this commit depend upon Qt 5.10? I only see buff.birthTime_ being used.

Unless I'm misunderstanding everything, birthTime_ is only set to a nonzero value as of https://github.com/qt/qtbase/commit/d3393ce25833c0afd7f0fa6b85fd6f3bd7ad520a#diff-e061e747264146cf47a86b371e512348R363, which only made it into Qt 5.10 and was not backported to 5.9.

In D7423#137952, @ngraham wrote:

This patch doesn't actually directly use statx(); it uses the existing approach, which goes through QFile, which is used to populate the statbuf object that we query.

Can you show me where this code uses QFile to populate the statbuf object? One of us is missing something here.

In D7423#174599, @dfaure wrote:

One of us is missing something here.

I'm sure it's me. :-)
If this approach doesn't work, would you be able to offer some pointers regarding a better method of implementing it?

Well, this hasn't been tested, has it?

AFAIU this requires calling statx(..., AT_SYMLINK_NOFOLLOW, ...) instead of lstat, on systems where statx is available.

I took a crack at accessing statx directly, but I'm waaaaaay out of my depth here and will need some help making this actually work. There doesn't seem to be a glibc wrapper for statx yet, so we have to access it directly, which is messy and I haven't figured out how to make it work yet, though I think I might be on the right track.

As such, this patch is presented--currently in non-working form, sadly--for discussion and hopefully instructional purposes. If someone else with more domain knowledge wants to grab this and do it right themselves, please feel free!

Problems that need to be overcome:

• On my Neon test system with kernel 5.13, __NR_statx isn't actually available in /usr/include/unistd.h; it's only available in `/usr/src/linux-headers-4.13.0-38-generic/arch/x86/include/generated/uapi/asm/unistd_64.h
• The check_function_exists(statx HAVE_STATX) CMake command returns 0 even though statx is available.
• The file KIOSlave crashes in ../sysdeps/unix/syscall-template.S:84 when I try to run Dolphin with this patch for KIO.

In D7423#310509, @fvogt wrote:

On neon it won't work as the kernel everything is built against (so the minimum API/ABI) is too old. You'll either have to hack around that by messing with include paths or use something more recent.

statx does not have a glibc wrapper, which means there is no statx(...) function. So HAVE_STATX is rightfully false.

filesystemengine_unix.cpp`.

This is no longer true with glibc 2.28, released two weeks ago. The interface is defined here:

https://sourceware.org/git/?p=glibc.git;a=blob;f=io/bits/statx.h;h=e31254e3617bb17b1d4ba1dc5365529e376e257d;hb=HEAD

So, first question which arises is, should we make it depend on glibc 2.28? There is no kernel dependency,
as glibc fills out the statx struct using other syscalls if statx is not available (of course, no birthtime then).

birthTime is supported by QTs QFileInfo::birthTime().
For the implementation, see here:
https://code.woboq.org/qt5/qtbase/src/corelib/io/qfilesystemengine_unix.cpp.html#329

glibc 2.28 was released in August 2018, it is at least in (k)ubuntu 18.10, and I would expect in most other distros as well.
I would be happy to help test this.

@ngraham I guess we can mark this as abandoned in favor of D20096.