{"thread":{"id":"63663","subject":"[PATCH] portability: allow building in systems without d_type","startedAt":"2025-06-18T06:23:51Z","lastAt":"2025-06-18T19:32:54Z","messageCount":6,"participants":["Carlo Marcelo Arenas Belón","Collin Funk","Marc Branchaud","Kristoffer Haugsbakk","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"520348","messageId":"20250618062331.78059-1-carenas@gmail.com","threadId":"63663","inReplyTo":null,"subject":"[PATCH] portability: allow building in systems without d_type","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2025-06-18T06:23:31Z","receivedAt":"2025-06-18T06:23:51Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Since 09fb155f11 (diff --no-index: support limiting by pathspec,\n2025-05-21) will fail to build in platforms that don't have a\nd_type member on their struct dirent (ex: AIX, NonStop).\n\nUse the DTYPE() macro instead of a nake reference to d_type.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n diff-no-index.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 4aeeb98cfa..7c95222ba6 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -46,7 +46,7 @@ static int read_directory_contents(const char *path, struct string_list *list,\n \n \t\t\tif (!match_leading_pathspec(NULL, pathspec,\n \t\t\t\t\t\t    match.buf, match.len,\n-\t\t\t\t\t\t    0, NULL, e->d_type == DT_DIR ? 1 : 0))\n+\t\t\t\t\t\t    0, NULL, DTYPE(e) == DT_DIR ? 1 : 0))\n \t\t\t\tcontinue;\n \t\t}\n \n-- \n2.50.0.53.g63c9ac04f7\n\n"},{"id":"520349","messageId":"87zfe5h7ub.fsf@gmail.com","threadId":"63663","inReplyTo":"20250618062331.78059-1-carenas@gmail.com","subject":"Re: [PATCH] portability: allow building in systems without d_type","fromName":"Collin Funk","fromEmail":"collin.funk1@gmail.com","sentAt":"2025-06-18T06:39:08Z","receivedAt":"2025-06-18T06:39:10Z","isPatch":true,"sender":{"key":"collin.funk1@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65689063?v=4"},"body":"Hi Carlo,\n\nCarlo Marcelo Arenas Belón <carenas@gmail.com> writes:\n\n> Since 09fb155f11 (diff --no-index: support limiting by pathspec,\n> 2025-05-21) will fail to build in platforms that don't have a\n> d_type member on their struct dirent (ex: AIX, NonStop).\n>\n> Use the DTYPE() macro instead of a nake reference to d_type.\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n>  diff-no-index.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 4aeeb98cfa..7c95222ba6 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -46,7 +46,7 @@ static int read_directory_contents(const char *path, struct string_list *list,\n>  \n>  \t\t\tif (!match_leading_pathspec(NULL, pathspec,\n>  \t\t\t\t\t\t    match.buf, match.len,\n> -\t\t\t\t\t\t    0, NULL, e->d_type == DT_DIR ? 1 : 0))\n> +\t\t\t\t\t\t    0, NULL, DTYPE(e) == DT_DIR ? 1 : 0))\n>  \t\t\t\tcontinue;\n>  \t\t}\n\nI confirm that before this patch the build will fail with the following\non AIX 7.3:\n\n        CC diff-no-index.o\n    diff-no-index.c: In function 'read_directory_contents':\n    diff-no-index.c:49:21: error: 'struct dirent' has no member named 'd_type'\n       49 |           0, NULL, e->d_type == DT_DIR ? 1 : 0))\n          |                     ^~\n    gmake: *** [Makefile:2821: diff-no-index.o] Error 1\n\nThis patch fixes it. Thanks.\n\nReviewed-by: Collin Funk <collin.funk1@gmail.com>\n\nCollin\n"},{"id":"520352","messageId":"e6fc37e6-7259-4561-888f-c3e892694421@xiplink.com","threadId":"63663","inReplyTo":"20250618062331.78059-1-carenas@gmail.com","subject":"Re: [PATCH] portability: allow building in systems without d_type","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2025-06-18T14:12:33Z","receivedAt":"2025-06-18T14:12:39Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"\nOn 2025-06-18 02:23, Carlo Marcelo Arenas Belón wrote:\n> Since 09fb155f11 (diff --no-index: support limiting by pathspec,\n> 2025-05-21) will fail to build in platforms that don't have a\n\ns/will fail/git fails/\n\n> d_type member on their struct dirent (ex: AIX, NonStop).\n> \n> Use the DTYPE() macro instead of a nake reference to d_type.\n\ns/nake/naked/\n\n\t\tM.\n\n\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n>   diff-no-index.c | 2 +-\n>   1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 4aeeb98cfa..7c95222ba6 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -46,7 +46,7 @@ static int read_directory_contents(const char *path, struct string_list *list,\n>   \n>   \t\t\tif (!match_leading_pathspec(NULL, pathspec,\n>   \t\t\t\t\t\t    match.buf, match.len,\n> -\t\t\t\t\t\t    0, NULL, e->d_type == DT_DIR ? 1 : 0))\n> +\t\t\t\t\t\t    0, NULL, DTYPE(e) == DT_DIR ? 1 : 0))\n>   \t\t\t\tcontinue;\n>   \t\t}\n>   \n\n"},{"id":"520353","messageId":"ffd2cf3e-95b7-4c4f-bf99-3dd624481c5c@app.fastmail.com","threadId":"63663","inReplyTo":"20250618062331.78059-1-carenas@gmail.com","subject":"Re: [PATCH] portability: allow building in systems without d_type","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-06-18T14:32:45Z","receivedAt":"2025-06-18T14:33:07Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Wed, Jun 18, 2025, at 08:23, Carlo Marcelo Arenas Belón wrote:\n> Since 09fb155f11 (diff --no-index: support limiting by pathspec,\n> 2025-05-21) will fail to build in platforms that don't have a\n> d_type member on their struct dirent (ex: AIX, NonStop).\n\ns/build in/build on/\n\n-- \nKristoffer Haugsbakk\n\n\n"},{"id":"520358","messageId":"xmqqwm98ewsd.fsf@gitster.g","threadId":"63663","inReplyTo":"20250618062331.78059-1-carenas@gmail.com","subject":"Re: [PATCH] portability: allow building in systems without d_type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-18T18:20:50Z","receivedAt":"2025-06-18T18:20:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:\n\n> Since 09fb155f11 (diff --no-index: support limiting by pathspec,\n> 2025-05-21) will fail to build in platforms that don't have a\n> d_type member on their struct dirent (ex: AIX, NonStop).\n>\n> Use the DTYPE() macro instead of a nake reference to d_type.\n\nThis may allow you to compile and build, but does the resulting\nbinary do what you want it to?\n\n>  \t\t\tif (!match_leading_pathspec(NULL, pathspec,\n>  \t\t\t\t\t\t    match.buf, match.len,\n> -\t\t\t\t\t\t    0, NULL, e->d_type == DT_DIR ? 1 : 0))\n> +\t\t\t\t\t\t    0, NULL, DTYPE(e) == DT_DIR ? 1 : 0))\n\nOn a platform without d_type member, DTYPE() macro gives DT_UNKNOWN\nthat is not DT_DIR, so essentially you are always passing 0 even\nwhen you are looking at a directory (in which case you must pass 1)\nto match_leading_pathspec().\n\nSo I somehow doubt this is a correct fix.\n\nI do not know if get_dtype() helper function is easily applicable to\nthis codepath, so I wrote this in a longhand...\n\n\n diff-no-index.c | 18 +++++++++++++++++-\n 1 file changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git c/diff-no-index.c w/diff-no-index.c\nindex 7c95222ba6..677df91fc5 100644\n--- c/diff-no-index.c\n+++ w/diff-no-index.c\n@@ -41,12 +41,28 @@ static int read_directory_contents(const char *path, struct string_list *list,\n \n \twhile ((e = readdir_skip_dot_and_dotdot(dir))) {\n \t\tif (pathspec) {\n+\t\t\tint is_dir = 0;\n+\n \t\t\tstrbuf_setlen(&match, len);\n \t\t\tstrbuf_addstr(&match, e->d_name);\n+\t\t\tif (dtype != DT_UNKNOWN) {\n+\t\t\t\tis_dir = dtype == DT_DIR;\n+\t\t\t} else {\n+\t\t\t\tstruct stat st;\n+\t\t\t\tstruct strbuf pathbuf = STRBUF_INIT;\n+\t\t\t\tstrbuf_addstr(&pathbuf, path);\n+\t\t\t\tstrbuf_complete(&pathbuf, '/');\n+\t\t\t\tstrbuf_addstr(&pathbuf, e->d_name);\n+\t\t\t\tif (!lstat(&st, pathbuf.buf))\n+\t\t\t\t\tis_dir = S_ISDIR(st.st_mode);\n+\t\t\t\telse\n+\t\t\t\t\t; /* punt */\n+\t\t\t\tstrbuf_release(&pathbuf);\n+\t\t\t}\n \n \t\t\tif (!match_leading_pathspec(NULL, pathspec,\n \t\t\t\t\t\t    match.buf, match.len,\n-\t\t\t\t\t\t    0, NULL, DTYPE(e) == DT_DIR ? 1 : 0))\n+\t\t\t\t\t\t    0, NULL, is_dir))\n \t\t\t\tcontinue;\n \t\t}\n \n"},{"id":"520363","messageId":"87frfwsv4r.fsf@gmail.com","threadId":"63663","inReplyTo":"xmqqwm98ewsd.fsf@gitster.g","subject":"Re: [PATCH] portability: allow building in systems without d_type","fromName":"Collin Funk","fromEmail":"collin.funk1@gmail.com","sentAt":"2025-06-18T19:32:52Z","receivedAt":"2025-06-18T19:32:54Z","isPatch":true,"sender":{"key":"collin.funk1@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65689063?v=4"},"body":"Hi Junio,\n\nJunio C Hamano <gitster@pobox.com> writes:\n\n> This may allow you to compile and build, but does the resulting\n> binary do what you want it to?\n> [...]\n> On a platform without d_type member, DTYPE() macro gives DT_UNKNOWN\n> that is not DT_DIR, so essentially you are always passing 0 even\n> when you are looking at a directory (in which case you must pass 1)\n> to match_leading_pathspec().\n>\n> So I somehow doubt this is a correct fix.\n\nGood points. I guess I had reviewed the original patch too late to\nrealize myself...\n\n> I do not know if get_dtype() helper function is easily applicable to\n> this codepath, so I wrote this in a longhand...\n>\n>\n>  diff-no-index.c | 18 +++++++++++++++++-\n>  1 file changed, 17 insertions(+), 1 deletion(-)\n>\n> diff --git c/diff-no-index.c w/diff-no-index.c\n> index 7c95222ba6..677df91fc5 100644\n> --- c/diff-no-index.c\n> +++ w/diff-no-index.c\n> @@ -41,12 +41,28 @@ static int read_directory_contents(const char *path, struct string_list *list,\n>  \n>  \twhile ((e = readdir_skip_dot_and_dotdot(dir))) {\n>  \t\tif (pathspec) {\n> +\t\t\tint is_dir = 0;\n> +\n>  \t\t\tstrbuf_setlen(&match, len);\n>  \t\t\tstrbuf_addstr(&match, e->d_name);\n> +\t\t\tif (dtype != DT_UNKNOWN) {\n> +\t\t\t\tis_dir = dtype == DT_DIR;\n> +\t\t\t} else {\n> +\t\t\t\tstruct stat st;\n> +\t\t\t\tstruct strbuf pathbuf = STRBUF_INIT;\n> +\t\t\t\tstrbuf_addstr(&pathbuf, path);\n> +\t\t\t\tstrbuf_complete(&pathbuf, '/');\n> +\t\t\t\tstrbuf_addstr(&pathbuf, e->d_name);\n> +\t\t\t\tif (!lstat(&st, pathbuf.buf))\n> +\t\t\t\t\tis_dir = S_ISDIR(st.st_mode);\n> +\t\t\t\telse\n> +\t\t\t\t\t; /* punt */\n> +\t\t\t\tstrbuf_release(&pathbuf);\n> +\t\t\t}\n>  \n>  \t\t\tif (!match_leading_pathspec(NULL, pathspec,\n>  \t\t\t\t\t\t    match.buf, match.len,\n> -\t\t\t\t\t\t    0, NULL, DTYPE(e) == DT_DIR ? 1 : 0))\n> +\t\t\t\t\t\t    0, NULL, is_dir))\n>  \t\t\t\tcontinue;\n>  \t\t}\n>  \n\nTwo very minor issues with with this patch. The arguments to 'lstat' are\nreversed and the 'dtype' variable is not declared. Here is a diff that I\napplied after your diff:\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 677df91fc5..a768b46dcd 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -42,6 +42,7 @@ static int read_directory_contents(const char *path, struct string_list *list,\n \twhile ((e = readdir_skip_dot_and_dotdot(dir))) {\n \t\tif (pathspec) {\n \t\t\tint is_dir = 0;\n+\t\t\tint dtype = DTYPE(e);\n \n \t\t\tstrbuf_setlen(&match, len);\n \t\t\tstrbuf_addstr(&match, e->d_name);\n@@ -53,7 +54,7 @@ static int read_directory_contents(const char *path, struct string_list *list,\n \t\t\t\tstrbuf_addstr(&pathbuf, path);\n \t\t\t\tstrbuf_complete(&pathbuf, '/');\n \t\t\t\tstrbuf_addstr(&pathbuf, e->d_name);\n-\t\t\t\tif (!lstat(&st, pathbuf.buf))\n+\t\t\t\tif (!lstat(pathbuf.buf, &st))\n \t\t\t\t\tis_dir = S_ISDIR(st.st_mode);\n \t\t\t\telse\n \t\t\t\t\t; /* punt */\n\nWith Carlo's patch the following tests fail:\n\n    $ sh t4053-diff-no-index.sh\n    [...]\n    not ok 35 - diff --no-index with pathspec nested pathspec\n    #       \n    #               test_expect_code 1 git diff --name-status --no-index c d 1/2 >actual &&\n    #               cat >expect <<-EOF &&\n    #               D       c/1/2/a\n    #               D       c/1/2/b\n    #               EOF\n    #               test_cmp expect actual\n    #       \n    not ok 36 - diff --no-index with pathspec glob\n    #       \n    #               test_expect_code 1 git diff --name-status --no-index c d \":(glob)**/a\" >actual &&\n    #               cat >expect <<-EOF &&\n    #               D       c/1/2/a\n    #               EOF\n    #               test_cmp expect actual\n    #       \n    ok 37 - diff --no-index with pathspec glob and exclude\n    # failed 2 among 37 test(s)\n\nBut with your patch (+ the minor corrections) the tests pass as\nexpected:\n\n    $ sh t4053-diff-no-index.sh\n    [...]\n    ok 35 - diff --no-index with pathspec nested pathspec\n    ok 36 - diff --no-index with pathspec glob\n    ok 37 - diff --no-index with pathspec glob and exclude\n    # passed all 37 test(s)\n\nTherefore, I think your fix is good to go with a commit message and my\nchanges. Feel free to add me to Reviewed-by/Tested-By.\n\nCollin\n"}]}