# [PATCH] portability: allow building in systems without d_type 6 messages from 2025-06-18 to 2025-06-18. Participants: Carlo Marcelo Arenas Belón, Collin Funk, Marc Branchaud, Kristoffer Haugsbakk, Junio C Hamano. Thread: https://gitlist.dev/t/63663 ## Carlo Marcelo Arenas Belón, 2025-06-18 06:23 Subject: [PATCH] portability: allow building in systems without d_type Message-ID: <20250618062331.78059-1-carenas@gmail.com> URL: https://gitlist.dev/e/20250618062331.78059-1-carenas%40gmail.com ``` Since 09fb155f11 (diff --no-index: support limiting by pathspec, 2025-05-21) will fail to build in platforms that don't have a d_type member on their struct dirent (ex: AIX, NonStop). Use the DTYPE() macro instead of a nake reference to d_type. Signed-off-by: Carlo Marcelo Arenas Belón --- diff-no-index.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diff-no-index.c b/diff-no-index.c index 4aeeb98cfa..7c95222ba6 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -46,7 +46,7 @@ static int read_directory_contents(const char *path, struct string_list *list, if (!match_leading_pathspec(NULL, pathspec, match.buf, match.len, - 0, NULL, e->d_type == DT_DIR ? 1 : 0)) + 0, NULL, DTYPE(e) == DT_DIR ? 1 : 0)) continue; } -- 2.50.0.53.g63c9ac04f7 ``` ## Collin Funk, 2025-06-18 06:39 Subject: Re: [PATCH] portability: allow building in systems without d_type Message-ID: <87zfe5h7ub.fsf@gmail.com> URL: https://gitlist.dev/e/87zfe5h7ub.fsf%40gmail.com In-Reply-To: <20250618062331.78059-1-carenas@gmail.com> ``` Hi Carlo, Carlo Marcelo Arenas Belón writes: > Since 09fb155f11 (diff --no-index: support limiting by pathspec, > 2025-05-21) will fail to build in platforms that don't have a > d_type member on their struct dirent (ex: AIX, NonStop). > > Use the DTYPE() macro instead of a nake reference to d_type. > > Signed-off-by: Carlo Marcelo Arenas Belón > --- > diff-no-index.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/diff-no-index.c b/diff-no-index.c > index 4aeeb98cfa..7c95222ba6 100644 > --- a/diff-no-index.c > +++ b/diff-no-index.c > @@ -46,7 +46,7 @@ static int read_directory_contents(const char *path, struct string_list *list, > > if (!match_leading_pathspec(NULL, pathspec, > match.buf, match.len, > - 0, NULL, e->d_type == DT_DIR ? 1 : 0)) > + 0, NULL, DTYPE(e) == DT_DIR ? 1 : 0)) > continue; > } I confirm that before this patch the build will fail with the following on AIX 7.3: CC diff-no-index.o diff-no-index.c: In function 'read_directory_contents': diff-no-index.c:49:21: error: 'struct dirent' has no member named 'd_type' 49 | 0, NULL, e->d_type == DT_DIR ? 1 : 0)) | ^~ gmake: *** [Makefile:2821: diff-no-index.o] Error 1 This patch fixes it. Thanks. Reviewed-by: Collin Funk Collin ``` ## Marc Branchaud, 2025-06-18 14:12 Subject: Re: [PATCH] portability: allow building in systems without d_type Message-ID: URL: https://gitlist.dev/e/e6fc37e6-7259-4561-888f-c3e892694421%40xiplink.com In-Reply-To: <20250618062331.78059-1-carenas@gmail.com> ``` On 2025-06-18 02:23, Carlo Marcelo Arenas Belón wrote: > Since 09fb155f11 (diff --no-index: support limiting by pathspec, > 2025-05-21) will fail to build in platforms that don't have a s/will fail/git fails/ > d_type member on their struct dirent (ex: AIX, NonStop). > > Use the DTYPE() macro instead of a nake reference to d_type. s/nake/naked/ M. > Signed-off-by: Carlo Marcelo Arenas Belón > --- > diff-no-index.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/diff-no-index.c b/diff-no-index.c > index 4aeeb98cfa..7c95222ba6 100644 > --- a/diff-no-index.c > +++ b/diff-no-index.c > @@ -46,7 +46,7 @@ static int read_directory_contents(const char *path, struct string_list *list, > > if (!match_leading_pathspec(NULL, pathspec, > match.buf, match.len, > - 0, NULL, e->d_type == DT_DIR ? 1 : 0)) > + 0, NULL, DTYPE(e) == DT_DIR ? 1 : 0)) > continue; > } > ``` ## Kristoffer Haugsbakk, 2025-06-18 14:32 Subject: Re: [PATCH] portability: allow building in systems without d_type Message-ID: URL: https://gitlist.dev/e/ffd2cf3e-95b7-4c4f-bf99-3dd624481c5c%40app.fastmail.com In-Reply-To: <20250618062331.78059-1-carenas@gmail.com> ``` On Wed, Jun 18, 2025, at 08:23, Carlo Marcelo Arenas Belón wrote: > Since 09fb155f11 (diff --no-index: support limiting by pathspec, > 2025-05-21) will fail to build in platforms that don't have a > d_type member on their struct dirent (ex: AIX, NonStop). s/build in/build on/ -- Kristoffer Haugsbakk ``` ## Junio C Hamano, 2025-06-18 18:20 Subject: Re: [PATCH] portability: allow building in systems without d_type Message-ID: URL: https://gitlist.dev/e/xmqqwm98ewsd.fsf%40gitster.g In-Reply-To: <20250618062331.78059-1-carenas@gmail.com> ``` Carlo Marcelo Arenas Belón writes: > Since 09fb155f11 (diff --no-index: support limiting by pathspec, > 2025-05-21) will fail to build in platforms that don't have a > d_type member on their struct dirent (ex: AIX, NonStop). > > Use the DTYPE() macro instead of a nake reference to d_type. This may allow you to compile and build, but does the resulting binary do what you want it to? > if (!match_leading_pathspec(NULL, pathspec, > match.buf, match.len, > - 0, NULL, e->d_type == DT_DIR ? 1 : 0)) > + 0, NULL, DTYPE(e) == DT_DIR ? 1 : 0)) On a platform without d_type member, DTYPE() macro gives DT_UNKNOWN that is not DT_DIR, so essentially you are always passing 0 even when you are looking at a directory (in which case you must pass 1) to match_leading_pathspec(). So I somehow doubt this is a correct fix. I do not know if get_dtype() helper function is easily applicable to this codepath, so I wrote this in a longhand... diff-no-index.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git c/diff-no-index.c w/diff-no-index.c index 7c95222ba6..677df91fc5 100644 --- c/diff-no-index.c +++ w/diff-no-index.c @@ -41,12 +41,28 @@ static int read_directory_contents(const char *path, struct string_list *list, while ((e = readdir_skip_dot_and_dotdot(dir))) { if (pathspec) { + int is_dir = 0; + strbuf_setlen(&match, len); strbuf_addstr(&match, e->d_name); + if (dtype != DT_UNKNOWN) { + is_dir = dtype == DT_DIR; + } else { + struct stat st; + struct strbuf pathbuf = STRBUF_INIT; + strbuf_addstr(&pathbuf, path); + strbuf_complete(&pathbuf, '/'); + strbuf_addstr(&pathbuf, e->d_name); + if (!lstat(&st, pathbuf.buf)) + is_dir = S_ISDIR(st.st_mode); + else + ; /* punt */ + strbuf_release(&pathbuf); + } if (!match_leading_pathspec(NULL, pathspec, match.buf, match.len, - 0, NULL, DTYPE(e) == DT_DIR ? 1 : 0)) + 0, NULL, is_dir)) continue; } ``` ## Collin Funk, 2025-06-18 19:32 Subject: Re: [PATCH] portability: allow building in systems without d_type Message-ID: <87frfwsv4r.fsf@gmail.com> URL: https://gitlist.dev/e/87frfwsv4r.fsf%40gmail.com In-Reply-To: ``` Hi Junio, Junio C Hamano writes: > This may allow you to compile and build, but does the resulting > binary do what you want it to? > [...] > On a platform without d_type member, DTYPE() macro gives DT_UNKNOWN > that is not DT_DIR, so essentially you are always passing 0 even > when you are looking at a directory (in which case you must pass 1) > to match_leading_pathspec(). > > So I somehow doubt this is a correct fix. Good points. I guess I had reviewed the original patch too late to realize myself... > I do not know if get_dtype() helper function is easily applicable to > this codepath, so I wrote this in a longhand... > > > diff-no-index.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git c/diff-no-index.c w/diff-no-index.c > index 7c95222ba6..677df91fc5 100644 > --- c/diff-no-index.c > +++ w/diff-no-index.c > @@ -41,12 +41,28 @@ static int read_directory_contents(const char *path, struct string_list *list, > > while ((e = readdir_skip_dot_and_dotdot(dir))) { > if (pathspec) { > + int is_dir = 0; > + > strbuf_setlen(&match, len); > strbuf_addstr(&match, e->d_name); > + if (dtype != DT_UNKNOWN) { > + is_dir = dtype == DT_DIR; > + } else { > + struct stat st; > + struct strbuf pathbuf = STRBUF_INIT; > + strbuf_addstr(&pathbuf, path); > + strbuf_complete(&pathbuf, '/'); > + strbuf_addstr(&pathbuf, e->d_name); > + if (!lstat(&st, pathbuf.buf)) > + is_dir = S_ISDIR(st.st_mode); > + else > + ; /* punt */ > + strbuf_release(&pathbuf); > + } > > if (!match_leading_pathspec(NULL, pathspec, > match.buf, match.len, > - 0, NULL, DTYPE(e) == DT_DIR ? 1 : 0)) > + 0, NULL, is_dir)) > continue; > } > Two very minor issues with with this patch. The arguments to 'lstat' are reversed and the 'dtype' variable is not declared. Here is a diff that I applied after your diff: diff --git a/diff-no-index.c b/diff-no-index.c index 677df91fc5..a768b46dcd 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -42,6 +42,7 @@ static int read_directory_contents(const char *path, struct string_list *list, while ((e = readdir_skip_dot_and_dotdot(dir))) { if (pathspec) { int is_dir = 0; + int dtype = DTYPE(e); strbuf_setlen(&match, len); strbuf_addstr(&match, e->d_name); @@ -53,7 +54,7 @@ static int read_directory_contents(const char *path, struct string_list *list, strbuf_addstr(&pathbuf, path); strbuf_complete(&pathbuf, '/'); strbuf_addstr(&pathbuf, e->d_name); - if (!lstat(&st, pathbuf.buf)) + if (!lstat(pathbuf.buf, &st)) is_dir = S_ISDIR(st.st_mode); else ; /* punt */ With Carlo's patch the following tests fail: $ sh t4053-diff-no-index.sh [...] not ok 35 - diff --no-index with pathspec nested pathspec # # test_expect_code 1 git diff --name-status --no-index c d 1/2 >actual && # cat >expect <<-EOF && # D c/1/2/a # D c/1/2/b # EOF # test_cmp expect actual # not ok 36 - diff --no-index with pathspec glob # # test_expect_code 1 git diff --name-status --no-index c d ":(glob)**/a" >actual && # cat >expect <<-EOF && # D c/1/2/a # EOF # test_cmp expect actual # ok 37 - diff --no-index with pathspec glob and exclude # failed 2 among 37 test(s) But with your patch (+ the minor corrections) the tests pass as expected: $ sh t4053-diff-no-index.sh [...] ok 35 - diff --no-index with pathspec nested pathspec ok 36 - diff --no-index with pathspec glob ok 37 - diff --no-index with pathspec glob and exclude # passed all 37 test(s) Therefore, I think your fix is good to go with a commit message and my changes. Feel free to add me to Reviewed-by/Tested-By. Collin ```