{"thread":{"id":"60997","subject":"[BUG] 2.44.0 t7704.9 Fails on NonStop ia64","startedAt":"2024-02-25T18:44:59Z","lastAt":"2024-02-27T14:28:25Z","messageCount":15,"participants":["rsbecker@nexbridge.com","Torsten Bögershausen","Phillip Wood","phillip.wood123@gmail.com","Patrick Steinhardt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"489325","messageId":"01bd01da681a$b8d70a70$2a851f50$@nexbridge.com","threadId":"60997","inReplyTo":null,"subject":"[BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2024-02-25T18:44:48Z","receivedAt":"2024-02-25T18:44:59Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"This appears to be a new issue introduced at 2.44.0. It only occurs on\nNonStop ia64 but not on x86. I am not sure why this is happening although\n1Mb exceeds the single I/O size on this machine.\n\nexpecting success of 7704.9 '--max-cruft-size with pruning':\n        git init max-cruft-size-prune &&\n        (\n                cd max-cruft-size-prune &&\n\n                test_commit base &&\n                foo=\"$(generate_random_blob foo $((1024*1024)))\" &&\n                bar=\"$(generate_random_blob bar $((1024*1024)))\" &&\n                baz=\"$(generate_random_blob baz $((1024*1024)))\" &&\n\n                test-tool chmtime -10000 \"$objdir/$(test_oid_to_path\n\"$foo\")\" &&\n\n                git repack -d --cruft --max-cruft-size=1M &&\n\n                # backdate the mtimes of all cruft packs to validate\n                # that they were rewritten as a result of pruning\n                ls $packdir/pack-*.mtimes | sort >cruft.before &&\n                for cruft in $(cat cruft.before)\n                do\n                        mtime=\"$(test-tool chmtime --get -10000 \"$cruft\")\"\n&&\n                        echo $cruft $mtime >>mtimes || return 1\n                done &&\n\n                # repack (and prune) with a --max-cruft-size to ensure\n                # that we appropriately split the resulting set of packs\n                git repack -d --cruft --max-cruft-size=1M \\\n                        --cruft-expiration=10.seconds.ago &&\n                ls $packdir/pack-*.mtimes | sort >cruft.after &&\n\n                for cruft in $(cat cruft.after)\n                do\n                        old_mtime=\"$(grep $cruft mtimes | cut -d\" \" -f2)\" &&\n                        new_mtime=\"$(test-tool chmtime --get $cruft)\" &&\n                        test $old_mtime -lt $new_mtime || return 1\n                done &&\n\n                test_line_count = 3 cruft.before &&\n                test_line_count = 2 cruft.after &&\n                test_must_fail git cat-file -e $foo &&\n                git cat-file -e $bar &&\n                git cat-file -e $baz\n        )\n\nInitialized empty Git repository in\n/home/ituglib/randall/jenkins/.jenkins/workspace/Git_Pipeline/t/trash\ndirectory.t7704-repack-cruft/max-cruft-size-prune/.git/\n[master (root-commit) d1ff1c9] base\n Author: A U Thor mailto:author@example.com\n 1 file changed, 1 insertion(+)\n create mode 100644 base.t\nEnumerating objects: 3, done.\nCounting objects: 100% (3/3), done.\nWriting objects: 100% (3/3), done.\nTotal 3 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)\nEnumerating cruft objects: 6, done.\nCounting objects: 100% (3/3), done.\nCompressing objects: 100% (3/3), done.\nWriting objects: 100% (3/3), done.\nTotal 3 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)\nEnumerating objects: 3, done.\nCounting objects: 100% (3/3), done.\nWriting objects: 100% (3/3), done.\nTotal 3 (delta 0), reused 3 (delta 0), pack-reused 0 (from 0)\nls: cannot access '.git/objects/pack/pack-*.mtimes': No such file or\ndirectory\ntest_line_count: line count for cruft.after != 2\nnot ok 9 - --max-cruft-size with pruning\n#\n#               git init max-cruft-size-prune &&\n#               (\n#                       cd max-cruft-size-prune &&\n#\n#                       test_commit base &&\n#                       foo=\"$(generate_random_blob foo $((1024*1024)))\" &&\n#                       bar=\"$(generate_random_blob bar $((1024*1024)))\" &&\n#                       baz=\"$(generate_random_blob baz $((1024*1024)))\" &&\n#\n#                       test-tool chmtime -10000 \"$objdir/$(test_oid_to_path\n\"$foo\")\" &&\n#\n#                       git repack -d --cruft --max-cruft-size=1M &&\n#\n#                       # backdate the mtimes of all cruft packs to validate\n#                       # that they were rewritten as a result of pruning\n#                       ls $packdir/pack-*.mtimes | sort >cruft.before &&\n#                       for cruft in $(cat cruft.before)\n#                       do\n#                               mtime=\"$(test-tool chmtime --get -10000\n\"$cruft\")\" &&\n#                               echo $cruft $mtime >>mtimes || return 1\n#                       done &&\n#\n#                       # repack (and prune) with a --max-cruft-size to\nensure\n#                       # that we appropriately split the resulting set of\npacks\n#                       git repack -d --cruft --max-cruft-size=1M \\\n#                               --cruft-expiration=10.seconds.ago &&\n#                       ls $packdir/pack-*.mtimes | sort >cruft.after &&\n#\n#                       for cruft in $(cat cruft.after)\n#                       do\n#                               old_mtime=\"$(grep $cruft mtimes | cut -d\" \"\n-f2)\" &&\n#                               new_mtime=\"$(test-tool chmtime --get\n$cruft)\" &&\n#                               test $old_mtime -lt $new_mtime || return 1\n#                       done &&\n#\n#                       test_line_count = 3 cruft.before &&\n#                       test_line_count = 2 cruft.after &&\n#                       test_must_fail git cat-file -e $foo &&\n#                       git cat-file -e $bar &&\n#                       git cat-file -e $baz\n#               )\n#\n1..9\n\n--\nBrief whoami: NonStop&UNIX developer since approximately\nUNIX(421664400)\nNonStop(211288444200000000)\n-- In real life, I talk too much.\n\n\n"},{"id":"489326","messageId":"01be01da681e$0c349090$249db1b0$@nexbridge.com","threadId":"60997","inReplyTo":"01bd01da681a$b8d70a70$2a851f50$@nexbridge.com","subject":"RE: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2024-02-25T19:08:35Z","receivedAt":"2024-02-25T19:08:43Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Sunday, February 25, 2024 1:45 PM, I wrote:\n>To: git@vger.kernel.org\n>Subject: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64\n>\n>This appears to be a new issue introduced at 2.44.0. It only occurs on\nNonStop ia64\n>but not on x86. I am not sure why this is happening although 1Mb exceeds\nthe\n>single I/O size on this machine.\n>\n>expecting success of 7704.9 '--max-cruft-size with pruning':\n>        git init max-cruft-size-prune &&\n>        (\n>                cd max-cruft-size-prune &&\n>\n>                test_commit base &&\n>                foo=\"$(generate_random_blob foo $((1024*1024)))\" &&\n>                bar=\"$(generate_random_blob bar $((1024*1024)))\" &&\n>                baz=\"$(generate_random_blob baz $((1024*1024)))\" &&\n>\n>                test-tool chmtime -10000 \"$objdir/$(test_oid_to_path\n\"$foo\")\" &&\n>\n>                git repack -d --cruft --max-cruft-size=1M &&\n>\n>                # backdate the mtimes of all cruft packs to validate\n>                # that they were rewritten as a result of pruning\n>                ls $packdir/pack-*.mtimes | sort >cruft.before &&\n>                for cruft in $(cat cruft.before)\n>                do\n>                        mtime=\"$(test-tool chmtime --get -10000 \"$cruft\")\"\n>&&\n>                        echo $cruft $mtime >>mtimes || return 1\n>                done &&\n>\n>                # repack (and prune) with a --max-cruft-size to ensure\n>                # that we appropriately split the resulting set of packs\n>                git repack -d --cruft --max-cruft-size=1M \\\n>                        --cruft-expiration=10.seconds.ago &&\n>                ls $packdir/pack-*.mtimes | sort >cruft.after &&\n>\n>                for cruft in $(cat cruft.after)\n>                do\n>                        old_mtime=\"$(grep $cruft mtimes | cut -d\" \" -f2)\"\n&&\n>                        new_mtime=\"$(test-tool chmtime --get $cruft)\" &&\n>                        test $old_mtime -lt $new_mtime || return 1\n>                done &&\n>\n>                test_line_count = 3 cruft.before &&\n>                test_line_count = 2 cruft.after &&\n>                test_must_fail git cat-file -e $foo &&\n>                git cat-file -e $bar &&\n>                git cat-file -e $baz\n>        )\n>\n>Initialized empty Git repository in\n>/home/ituglib/randall/jenkins/.jenkins/workspace/Git_Pipeline/t/trash\n>directory.t7704-repack-cruft/max-cruft-size-prune/.git/\n>[master (root-commit) d1ff1c9] base\n> Author: A U Thor mailto:author@example.com\n> 1 file changed, 1 insertion(+)\n> create mode 100644 base.t\n>Enumerating objects: 3, done.\n>Counting objects: 100% (3/3), done.\n>Writing objects: 100% (3/3), done.\n>Total 3 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0) Enumerating\ncruft\n>objects: 6, done.\n>Counting objects: 100% (3/3), done.\n>Compressing objects: 100% (3/3), done.\n>Writing objects: 100% (3/3), done.\n>Total 3 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0) Enumerating\nobjects: 3,\n>done.\n>Counting objects: 100% (3/3), done.\n>Writing objects: 100% (3/3), done.\n>Total 3 (delta 0), reused 3 (delta 0), pack-reused 0 (from 0)\n>ls: cannot access '.git/objects/pack/pack-*.mtimes': No such file or\ndirectory\n>test_line_count: line count for cruft.after != 2 not ok 9 -\n--max-cruft-size with\n>pruning #\n>#               git init max-cruft-size-prune &&\n>#               (\n>#                       cd max-cruft-size-prune &&\n>#\n>#                       test_commit base &&\n>#                       foo=\"$(generate_random_blob foo $((1024*1024)))\" &&\n>#                       bar=\"$(generate_random_blob bar $((1024*1024)))\" &&\n>#                       baz=\"$(generate_random_blob baz $((1024*1024)))\" &&\n>#\n>#                       test-tool chmtime -10000\n\"$objdir/$(test_oid_to_path\n>\"$foo\")\" &&\n>#\n>#                       git repack -d --cruft --max-cruft-size=1M &&\n>#\n>#                       # backdate the mtimes of all cruft packs to\nvalidate\n>#                       # that they were rewritten as a result of pruning\n>#                       ls $packdir/pack-*.mtimes | sort >cruft.before &&\n>#                       for cruft in $(cat cruft.before)\n>#                       do\n>#                               mtime=\"$(test-tool chmtime --get -10000\n>\"$cruft\")\" &&\n>#                               echo $cruft $mtime >>mtimes || return 1\n>#                       done &&\n>#\n>#                       # repack (and prune) with a --max-cruft-size to\n>ensure\n>#                       # that we appropriately split the resulting set of\n>packs\n>#                       git repack -d --cruft --max-cruft-size=1M \\\n>#                               --cruft-expiration=10.seconds.ago &&\n>#                       ls $packdir/pack-*.mtimes | sort >cruft.after &&\n>#\n>#                       for cruft in $(cat cruft.after)\n>#                       do\n>#                               old_mtime=\"$(grep $cruft mtimes | cut -d\" \"\n>-f2)\" &&\n>#                               new_mtime=\"$(test-tool chmtime --get\n>$cruft)\" &&\n>#                               test $old_mtime -lt $new_mtime || return 1\n>#                       done &&\n>#\n>#                       test_line_count = 3 cruft.before &&\n>#                       test_line_count = 2 cruft.after &&\n>#                       test_must_fail git cat-file -e $foo &&\n>#                       git cat-file -e $bar &&\n>#                       git cat-file -e $baz\n>#               )\n>#\n>1..9\n\nI did find the following calls to write(), one of which might be involved.\nwrite() should not be used directly unless the count is clearly very small.\nXwrite() should be used instead. There are other calls but those are either\nsmall or not on platform.\n\nreftable/writer.c:              int n = w->write(w->write_arg, zeroed,\nw->pending_padding);\nreftable/writer.c:      n = w->write(w->write_arg, data, len);\nrun-command.c:                  len = write(io->fd, io->u.out.buf,\nt/helper/test-path-utils.c:                     if (write(1, buffer, count)\n< 0)\nt/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\nt/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\ntrace2/tr2_dst.c:       bytes = write(fd, buf_line->buf, buf_line->len);\n\n\n\n"},{"id":"489327","messageId":"20240225191954.GA28646@tb-raspi4","threadId":"60997","inReplyTo":"01be01da681e$0c349090$249db1b0$@nexbridge.com","subject":"Re: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2024-02-25T19:19:54Z","receivedAt":"2024-02-25T19:20:05Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Sun, Feb 25, 2024 at 02:08:35PM -0500, rsbecker@nexbridge.com wrote:\n> On Sunday, February 25, 2024 1:45 PM, I wrote:\n> >To: git@vger.kernel.org\n> >Subject: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64\n> >\n> >This appears to be a new issue introduced at 2.44.0. It only occurs on\n> NonStop ia64\n> >1..9\n[snip]\n>\n> I did find the following calls to write(), one of which might be involved.\n> write() should not be used directly unless the count is clearly very small.\n> Xwrite() should be used instead. There are other calls but those are either\n> small or not on platform.\n\n(Probably a typ0: Xwrite() -> xwrite()\n\nBut I think that this should be used:\nwrite_in_full()\n\n\n\n>\n> reftable/writer.c:              int n = w->write(w->write_arg, zeroed,\n> w->pending_padding);\n> reftable/writer.c:      n = w->write(w->write_arg, data, len);\n> run-command.c:                  len = write(io->fd, io->u.out.buf,\n> t/helper/test-path-utils.c:                     if (write(1, buffer, count)\n> < 0)\n> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\n> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\n> trace2/tr2_dst.c:       bytes = write(fd, buf_line->buf, buf_line->len);\n>\n>\n>\n>\n"},{"id":"489328","messageId":"01ca01da682a$5f6a7b60$1e3f7220$@nexbridge.com","threadId":"60997","inReplyTo":"20240225191954.GA28646@tb-raspi4","subject":"RE: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2024-02-25T20:36:49Z","receivedAt":"2024-02-25T20:36:56Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Sunday, February 25, 2024 2:20 PM, Torsten Bögershausen wrote:\n>On Sun, Feb 25, 2024 at 02:08:35PM -0500, rsbecker@nexbridge.com wrote:\n>> On Sunday, February 25, 2024 1:45 PM, I wrote:\n>> >To: git@vger.kernel.org\n>> >Subject: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64\n>> >\n>> >This appears to be a new issue introduced at 2.44.0. It only occurs\n>> >on\n>> NonStop ia64\n>> >1..9\n>[snip]\n>>\n>> I did find the following calls to write(), one of which might be\ninvolved.\n>> write() should not be used directly unless the count is clearly very\nsmall.\n>> Xwrite() should be used instead. There are other calls but those are\n>> either small or not on platform.\n>\n>(Probably a typ0: Xwrite() -> xwrite()\n>\n>But I think that this should be used:\n>write_in_full()\n\nMy mailer autocorrected, yes, xwrite. write_in_full() would be safe,\nalthough a bit redundant since xwrite() does similar things and is used by\nwrite_in_full(). The question is which call is bad? The cruft stuff is\nrelatively new and I don't know the code.\n\n>> reftable/writer.c:              int n = w->write(w->write_arg, zeroed,\n>> w->pending_padding);\n>> reftable/writer.c:      n = w->write(w->write_arg, data, len);\n>> run-command.c:                  len = write(io->fd, io->u.out.buf,\n>> t/helper/test-path-utils.c:                     if (write(1, buffer,\ncount)\n>> < 0)\n>> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\n>> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\n>> trace2/tr2_dst.c:       bytes = write(fd, buf_line->buf, buf_line->len);\n\n"},{"id":"489378","messageId":"5e807c1c-20a0-407b-9fc2-acd38521ba45@gmail.com","threadId":"60997","inReplyTo":"01ca01da682a$5f6a7b60$1e3f7220$@nexbridge.com","subject":"Re: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-26T15:32:14Z","receivedAt":"2024-02-26T15:32:17Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Randal\n\n[cc'ing Patrick for the reftable writer]\n\nOn 25/02/2024 20:36, rsbecker@nexbridge.com wrote:\n> On Sunday, February 25, 2024 2:20 PM, Torsten Bögershausen wrote:\n>> On Sun, Feb 25, 2024 at 02:08:35PM -0500, rsbecker@nexbridge.com wrote:\n>>> On Sunday, February 25, 2024 1:45 PM, I wrote:\n>>>> To: git@vger.kernel.org\n>> But I think that this should be used:\n>> write_in_full()\n> \n> My mailer autocorrected, yes, xwrite. write_in_full() would be safe,\n> although a bit redundant since xwrite() does similar things and is used by\n> write_in_full().\n\nNote that unlike write_in_full(), xwrite() does not guarantee to write \nthe whole buffer passed to it. In general unless a caller is writing a \nsingle byte or writing less than PIPE_BUF bytes to a pipe it should use \nwrite_in_full().\n\n> The question is which call is bad? The cruft stuff is\n> relatively new and I don't know the code.\n> \n>>> reftable/writer.c:              int n = w->write(w->write_arg, zeroed,\n>>> w->pending_padding);\n>>> reftable/writer.c:      n = w->write(w->write_arg, data, len);\n\nNeither of these appear to check for short writes and \nreftable_fd_write() is a thin wrapper around write(). Maybe \nreftable_fd_write() should be using write_in_full()?\n\n>>> run-command.c:                  len = write(io->fd, io->u.out.buf,\n\nThis call to write() looks correct as it is in the io pump loop.\n\n>>> t/helper/test-path-utils.c:                     if (write(1, buffer,\n> count)\n>>> < 0) >>> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\n>>> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\n\nIn principle these all look like they are prone to short writes.\n\n>>> trace2/tr2_dst.c:       bytes = write(fd, buf_line->buf, buf_line->len);\n\nThis caller explicitly says it prefers short writes over retrying\n\nBest Wishes\n\nPhillip\n"},{"id":"489379","messageId":"022001da68cb$d29a2c60$77ce8520$@nexbridge.com","threadId":"60997","inReplyTo":"5e807c1c-20a0-407b-9fc2-acd38521ba45@gmail.com","subject":"RE: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2024-02-26T15:52:31Z","receivedAt":"2024-02-26T15:52:44Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Monday, February 26, 2024 10:32 AM, Philip Wood wrote:\n>On 25/02/2024 20:36, rsbecker@nexbridge.com wrote:\n>> On Sunday, February 25, 2024 2:20 PM, Torsten Bögershausen wrote:\n>>> On Sun, Feb 25, 2024 at 02:08:35PM -0500, rsbecker@nexbridge.com wrote:\n>>>> On Sunday, February 25, 2024 1:45 PM, I wrote:\n>>>>> To: git@vger.kernel.org\n>>> But I think that this should be used:\n>>> write_in_full()\n>>\n>> My mailer autocorrected, yes, xwrite. write_in_full() would be safe,\n>> although a bit redundant since xwrite() does similar things and is\n>> used by write_in_full().\n>\n>Note that unlike write_in_full(), xwrite() does not guarantee to write the whole\n>buffer passed to it. In general unless a caller is writing a single byte or writing less\n>than PIPE_BUF bytes to a pipe it should use write_in_full().\n>\n>> The question is which call is bad? The cruft stuff is relatively new\n>> and I don't know the code.\n>>\n>>>> reftable/writer.c:              int n = w->write(w->write_arg, zeroed,\n>>>> w->pending_padding);\n>>>> reftable/writer.c:      n = w->write(w->write_arg, data, len);\n>\n>Neither of these appear to check for short writes and\n>reftable_fd_write() is a thin wrapper around write(). Maybe\n>reftable_fd_write() should be using write_in_full()?\n>\n>>>> run-command.c:                  len = write(io->fd, io->u.out.buf,\n>\n>This call to write() looks correct as it is in the io pump loop.\n>\n>>>> t/helper/test-path-utils.c:                     if (write(1, buffer,\n>> count)\n>>>> < 0) >>> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\n>>>> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\n>\n>In principle these all look like they are prone to short writes.\n>\n>>>> trace2/tr2_dst.c:       bytes = write(fd, buf_line->buf, buf_line->len);\n>\n>This caller explicitly says it prefers short writes over retrying\n\nThe real issue is that t7704.9 fails as follows:\n\nTotal 3 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0) Enumerating objects: 3, done.\nCounting objects: 100% (3/3), done.\nWriting objects: 100% (3/3), done.\nTotal 3 (delta 0), reused 3 (delta 0), pack-reused 0 (from 0)\nls: cannot access '.git/objects/pack/pack-*.mtimes': No such file or directory\ntest_line_count: line count for cruft.after != 2 not ok 9 - --max-cruft-size with pruning #\n\nSo something is not writing the mtimes file correctly. That's what I am trying to track down. The write issue is a possible cause but not necessarily the root cause.\n\n--Randall\n\n"},{"id":"489381","messageId":"76962a0c-adfd-47a5-a017-a117ba14ae09@gmail.com","threadId":"60997","inReplyTo":"5e807c1c-20a0-407b-9fc2-acd38521ba45@gmail.com","subject":"Re: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-26T16:00:10Z","receivedAt":"2024-02-26T16:00:13Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 26/02/2024 15:32, Phillip Wood wrote:\n> Hi Randal\n> \n> [cc'ing Patrick for the reftable writer]\n> \n> On 25/02/2024 20:36, rsbecker@nexbridge.com wrote:\n>> On Sunday, February 25, 2024 2:20 PM, Torsten Bögershausen wrote:\n>>> On Sun, Feb 25, 2024 at 02:08:35PM -0500, rsbecker@nexbridge.com wrote:\n>>>> On Sunday, February 25, 2024 1:45 PM, I wrote:\n>>>>> To: git@vger.kernel.org\n>>> But I think that this should be used:\n>>> write_in_full()\n>>\n>> My mailer autocorrected, yes, xwrite. write_in_full() would be safe,\n>> although a bit redundant since xwrite() does similar things and is \n>> used by\n>> write_in_full().\n> \n> Note that unlike write_in_full(), xwrite() does not guarantee to write \n> the whole buffer passed to it. In general unless a caller is writing a \n> single byte or writing less than PIPE_BUF bytes to a pipe it should use \n> write_in_full().\n> \n>> The question is which call is bad? The cruft stuff is\n>> relatively new and I don't know the code.\n\nI should have been clearer that I do not think any of these calls are \nthe likely problem for the cruft pack code. I do think the reftable \nwriters are worth looking at though for git in general.\n\nFor the cruft pack problem you might want to look for suspect xwrite() \ncalls where the caller does not handle a short write correctly for \nexample under builtin/ we have\n\nbuiltin/index-pack.c:                   err = xwrite(1, input_buffer + \ninput_offset, input_len);\nbuiltin/receive-pack.c:         xwrite(2, msg, sz);\nbuiltin/repack.c:       xwrite(cmd->in, oid_to_hex(oid), \nthe_hash_algo->hexsz);\nbuiltin/repack.c:       xwrite(cmd->in, \"\\n\", 1);\nbuiltin/unpack-objects.c:               int ret = xwrite(1, buffer + \noffset, len);\n\nBest Wishes\n\nPhillip\n\n>>>> reftable/writer.c:              int n = w->write(w->write_arg, zeroed,\n>>>> w->pending_padding);\n>>>> reftable/writer.c:      n = w->write(w->write_arg, data, len);\n> \n> Neither of these appear to check for short writes and \n> reftable_fd_write() is a thin wrapper around write(). Maybe \n> reftable_fd_write() should be using write_in_full()?\n> \n>>>> run-command.c:                  len = write(io->fd, io->u.out.buf,\n> \n> This call to write() looks correct as it is in the io pump loop.\n> \n>>>> t/helper/test-path-utils.c:                     if (write(1, buffer,\n>> count)\n>>>> < 0) >>> t/helper/test-windows-named-pipe.c:             write(1, \n>>>> buf, nbr);\n>>>> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\n> \n> In principle these all look like they are prone to short writes.\n> \n>>>> trace2/tr2_dst.c:       bytes = write(fd, buf_line->buf, \n>>>> buf_line->len);\n> \n> This caller explicitly says it prefers short writes over retrying\n> \n> Best Wishes\n> \n> Phillip\n"},{"id":"489400","messageId":"023c01da68de$196615c0$4c324140$@nexbridge.com","threadId":"60997","inReplyTo":"76962a0c-adfd-47a5-a017-a117ba14ae09@gmail.com","subject":"RE: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2024-02-26T18:03:21Z","receivedAt":"2024-02-26T18:03:34Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":">From: Phillip Wood <phillip.wood123@gmail.com>\nOn Monday, February 26, 2024 11:00 AM, Phillip Wood wrote:\n>To: rsbecker@nexbridge.com; 'Torsten Bögershausen' <tboegi@web.de>\n>Cc: git@vger.kernel.org; Patrick Steinhardt <ps@pks.im>\n>Subject: Re: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64\n>\n>On 26/02/2024 15:32, Phillip Wood wrote:\n>> Hi Randal\n>>\n>> [cc'ing Patrick for the reftable writer]\n>>\n>> On 25/02/2024 20:36, rsbecker@nexbridge.com wrote:\n>>> On Sunday, February 25, 2024 2:20 PM, Torsten Bögershausen wrote:\n>>>> On Sun, Feb 25, 2024 at 02:08:35PM -0500, rsbecker@nexbridge.com wrote:\n>>>>> On Sunday, February 25, 2024 1:45 PM, I wrote:\n>>>>>> To: git@vger.kernel.org\n>>>> But I think that this should be used:\n>>>> write_in_full()\n>>>\n>>> My mailer autocorrected, yes, xwrite. write_in_full() would be safe,\n>>> although a bit redundant since xwrite() does similar things and is\n>>> used by write_in_full().\n>>\n>> Note that unlike write_in_full(), xwrite() does not guarantee to write\n>> the whole buffer passed to it. In general unless a caller is writing a\n>> single byte or writing less than PIPE_BUF bytes to a pipe it should\n>> use write_in_full().\n>>\n>>> The question is which call is bad? The cruft stuff is relatively new\n>>> and I don't know the code.\n>\n>I should have been clearer that I do not think any of these calls are the likely\n>problem for the cruft pack code. I do think the reftable writers are worth looking at\n>though for git in general.\n>\n>For the cruft pack problem you might want to look for suspect xwrite() calls where\n>the caller does not handle a short write correctly for example under builtin/ we have\n>\n>builtin/index-pack.c:                   err = xwrite(1, input_buffer +\n>input_offset, input_len);\n>builtin/receive-pack.c:         xwrite(2, msg, sz);\n>builtin/repack.c:       xwrite(cmd->in, oid_to_hex(oid),\n>the_hash_algo->hexsz);\n>builtin/repack.c:       xwrite(cmd->in, \"\\n\", 1);\n>builtin/unpack-objects.c:               int ret = xwrite(1, buffer +\n>offset, len);\n>\n>Best Wishes\n>\n>Phillip\n>\n>>>>> reftable/writer.c:              int n = w->write(w->write_arg,\n>>>>> zeroed,\n>>>>> w->pending_padding);\n>>>>> reftable/writer.c:      n = w->write(w->write_arg, data, len);\n>>\n>> Neither of these appear to check for short writes and\n>> reftable_fd_write() is a thin wrapper around write(). Maybe\n>> reftable_fd_write() should be using write_in_full()?\n>>\n>>>>> run-command.c:                  len = write(io->fd, io->u.out.buf,\n>>\n>> This call to write() looks correct as it is in the io pump loop.\n>>\n>>>>> t/helper/test-path-utils.c:                     if (write(1,\n>>>>> buffer,\n>>> count)\n>>>>> < 0) >>> t/helper/test-windows-named-pipe.c:             write(1,\n>>>>> buf, nbr);\n>>>>> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\n>>\n>> In principle these all look like they are prone to short writes.\n>>\n>>>>> trace2/tr2_dst.c:       bytes = write(fd, buf_line->buf,\n>>>>> buf_line->len);\n>>\n>> This caller explicitly says it prefers short writes over retrying\n\nI'm getting caught a bit behind the curve. After rebuilding from master, I'm now getting:\n\n+ test 1708960150 -lt 1708970156\n+ test_line_count = 3 cruft.before\n+ test_line_count = 2 cruft.after\ntest_line_count: line count for cruft.after != 2\n\nThis is looking more like a different problem than xwrite().\n\n"},{"id":"489410","messageId":"025301da68e6$68956940$39c03bc0$@nexbridge.com","threadId":"60997","inReplyTo":"76962a0c-adfd-47a5-a017-a117ba14ae09@gmail.com","subject":"RE: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2024-02-26T19:02:50Z","receivedAt":"2024-02-26T19:03:08Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Monday, February 26, 2024 11:00 AM, Phillip Wood wrote:\n>On 26/02/2024 15:32, Phillip Wood wrote:\n>> Hi Randal\n>>\n>> [cc'ing Patrick for the reftable writer]\n>>\n>> On 25/02/2024 20:36, rsbecker@nexbridge.com wrote:\n>>> On Sunday, February 25, 2024 2:20 PM, Torsten Bögershausen wrote:\n>>>> On Sun, Feb 25, 2024 at 02:08:35PM -0500, rsbecker@nexbridge.com wrote:\n>>>>> On Sunday, February 25, 2024 1:45 PM, I wrote:\n>>>>>> To: git@vger.kernel.org\n>>>> But I think that this should be used:\n>>>> write_in_full()\n>>>\n>>> My mailer autocorrected, yes, xwrite. write_in_full() would be safe,\n>>> although a bit redundant since xwrite() does similar things and is\n>>> used by write_in_full().\n>>\n>> Note that unlike write_in_full(), xwrite() does not guarantee to write\n>> the whole buffer passed to it. In general unless a caller is writing a\n>> single byte or writing less than PIPE_BUF bytes to a pipe it should\n>> use write_in_full().\n>>\n>>> The question is which call is bad? The cruft stuff is relatively new\n>>> and I don't know the code.\n>\n>I should have been clearer that I do not think any of these calls are the likely\n>problem for the cruft pack code. I do think the reftable writers are worth looking at\n>though for git in general.\n>\n>For the cruft pack problem you might want to look for suspect xwrite() calls where\n>the caller does not handle a short write correctly for example under builtin/ we have\n>\n>builtin/index-pack.c:                   err = xwrite(1, input_buffer +\n>input_offset, input_len);\n>builtin/receive-pack.c:         xwrite(2, msg, sz);\n>builtin/repack.c:       xwrite(cmd->in, oid_to_hex(oid),\n>the_hash_algo->hexsz);\n>builtin/repack.c:       xwrite(cmd->in, \"\\n\", 1);\n>builtin/unpack-objects.c:               int ret = xwrite(1, buffer +\n>offset, len);\n>\n>Best Wishes\n>\n>Phillip\n>\n>>>>> reftable/writer.c:              int n = w->write(w->write_arg,\n>>>>> zeroed,\n>>>>> w->pending_padding);\n>>>>> reftable/writer.c:      n = w->write(w->write_arg, data, len);\n>>\n>> Neither of these appear to check for short writes and\n>> reftable_fd_write() is a thin wrapper around write(). Maybe\n>> reftable_fd_write() should be using write_in_full()?\n>>\n>>>>> run-command.c:                  len = write(io->fd, io->u.out.buf,\n>>\n>> This call to write() looks correct as it is in the io pump loop.\n>>\n>>>>> t/helper/test-path-utils.c:                     if (write(1,\n>>>>> buffer,\n>>> count)\n>>>>> < 0) >>> t/helper/test-windows-named-pipe.c:             write(1,\n>>>>> buf, nbr);\n>>>>> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\n>>\n>> In principle these all look like they are prone to short writes.\n>>\n>>>>> trace2/tr2_dst.c:       bytes = write(fd, buf_line->buf,\n>>>>> buf_line->len);\n>>\n>> This caller explicitly says it prefers short writes over retrying\n\nReplacing xwrite with write_in_full  the above worked correctly. Do you want it or should I write this up?\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex a3a37bd215..f80b8d101a 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1571,7 +1571,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n                 * the last part of the input buffer to stdout.\n                 */\n                while (input_len) {\n-                       err = xwrite(1, input_buffer + input_offset, input_len);\n+                       err = write_in_full(1, input_buffer + input_offset, input_len);\n                        if (err <= 0)\n                                break;\n                        input_len -= err;\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex db65607485..4277c63d08 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -456,7 +456,7 @@ static void report_message(const char *prefix, const char *err, va_list params)\n        if (use_sideband)\n                send_sideband(1, 2, msg, sz, use_sideband);\n        else\n-               xwrite(2, msg, sz);\n+               write_in_full(2, msg, sz);\n }\n\n __attribute__((format (printf, 1, 2)))\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex ede36328a3..4916870992 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -314,8 +314,8 @@ static int write_oid(const struct object_id *oid,\n                        die(_(\"could not start pack-objects to repack promisor objects\"));\n        }\n\n-       xwrite(cmd->in, oid_to_hex(oid), the_hash_algo->hexsz);\n-       xwrite(cmd->in, \"\\n\", 1);\n+       write_in_full(cmd->in, oid_to_hex(oid), the_hash_algo->hexsz);\n+       write_in_full(cmd->in, \"\\n\", 1);\n        return 0;\n }\n\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex e0a701f2b3..6935c4574e 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -680,7 +680,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED)\n\n        /* Write the last part of the buffer to stdout */\n        while (len) {\n-               int ret = xwrite(1, buffer + offset, len);\n+               int ret = write_in_full(1, buffer + offset, len);\n                if (ret <= 0)\n                        break;\n                len -= ret;\n\n\n"},{"id":"489413","messageId":"1eba5cd0-67d1-461a-b525-05d2c1b8a55d@gmail.com","threadId":"60997","inReplyTo":"025301da68e6$68956940$39c03bc0$@nexbridge.com","subject":"Re: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-26T19:45:03Z","receivedAt":"2024-02-26T19:45:07Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 26/02/2024 19:02, rsbecker@nexbridge.com wrote:\n> On Monday, February 26, 2024 11:00 AM, Phillip Wood wrote:\n>> On 26/02/2024 15:32, Phillip Wood wrote:\n>>> Hi Randal\n>>>\n>>> [cc'ing Patrick for the reftable writer]\n>>>\n>>> On 25/02/2024 20:36, rsbecker@nexbridge.com wrote:\n>>>> On Sunday, February 25, 2024 2:20 PM, Torsten Bögershausen wrote:\n>>>>> On Sun, Feb 25, 2024 at 02:08:35PM -0500, rsbecker@nexbridge.com wrote:\n>>>>>> On Sunday, February 25, 2024 1:45 PM, I wrote:\n>>>>>>> To: git@vger.kernel.org\n>>>>> But I think that this should be used:\n>>>>> write_in_full()\n>>>>\n>>>> My mailer autocorrected, yes, xwrite. write_in_full() would be safe,\n>>>> although a bit redundant since xwrite() does similar things and is\n>>>> used by write_in_full().\n>>>\n>>> Note that unlike write_in_full(), xwrite() does not guarantee to write\n>>> the whole buffer passed to it. In general unless a caller is writing a\n>>> single byte or writing less than PIPE_BUF bytes to a pipe it should\n>>> use write_in_full().\n>>>\n>>>> The question is which call is bad? The cruft stuff is relatively new\n>>>> and I don't know the code.\n>>\n>> I should have been clearer that I do not think any of these calls are the likely\n>> problem for the cruft pack code. I do think the reftable writers are worth looking at\n>> though for git in general.\n>>\n>> For the cruft pack problem you might want to look for suspect xwrite() calls where\n>> the caller does not handle a short write correctly for example under builtin/ we have\n>>\n>> builtin/index-pack.c:                   err = xwrite(1, input_buffer +\n>> input_offset, input_len);\n>> builtin/receive-pack.c:         xwrite(2, msg, sz);\n>> builtin/repack.c:       xwrite(cmd->in, oid_to_hex(oid),\n>> the_hash_algo->hexsz);\n>> builtin/repack.c:       xwrite(cmd->in, \"\\n\", 1);\n>> builtin/unpack-objects.c:               int ret = xwrite(1, buffer +\n>> offset, len);\n>>\n>> Best Wishes\n>>\n>> Phillip\n>>\n>>>>>> reftable/writer.c:              int n = w->write(w->write_arg,\n>>>>>> zeroed,\n>>>>>> w->pending_padding);\n>>>>>> reftable/writer.c:      n = w->write(w->write_arg, data, len);\n>>>\n>>> Neither of these appear to check for short writes and\n>>> reftable_fd_write() is a thin wrapper around write(). Maybe\n>>> reftable_fd_write() should be using write_in_full()?\n>>>\n>>>>>> run-command.c:                  len = write(io->fd, io->u.out.buf,\n>>>\n>>> This call to write() looks correct as it is in the io pump loop.\n>>>\n>>>>>> t/helper/test-path-utils.c:                     if (write(1,\n>>>>>> buffer,\n>>>> count)\n>>>>>> < 0) >>> t/helper/test-windows-named-pipe.c:             write(1,\n>>>>>> buf, nbr);\n>>>>>> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);\n>>>\n>>> In principle these all look like they are prone to short writes.\n>>>\n>>>>>> trace2/tr2_dst.c:       bytes = write(fd, buf_line->buf,\n>>>>>> buf_line->len);\n>>>\n>>> This caller explicitly says it prefers short writes over retrying\n> \n> Replacing xwrite with write_in_full  the above worked correctly. Do you want it or should I write this up?\n\nI'm glad that worked for you. If you are able to write it up that would \nbe great. There are some dodgy looking xwrite() calls outside builtin/ \nas well. It is probably worth checking the return value of \nwrite_in_full() when you do the conversion for the sites where we ignore \nerrors at the moment.\n\nBest Wishes\n\nPhillip\n\n> diff --git a/builtin/index-pack.c b/builtin/index-pack.c\n> index a3a37bd215..f80b8d101a 100644\n> --- a/builtin/index-pack.c\n> +++ b/builtin/index-pack.c\n> @@ -1571,7 +1571,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n>                   * the last part of the input buffer to stdout.\n>                   */\n>                  while (input_len) {\n> -                       err = xwrite(1, input_buffer + input_offset, input_len);\n> +                       err = write_in_full(1, input_buffer + input_offset, input_len);\n>                          if (err <= 0)\n>                                  break;\n>                          input_len -= err;\n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index db65607485..4277c63d08 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -456,7 +456,7 @@ static void report_message(const char *prefix, const char *err, va_list params)\n>          if (use_sideband)\n>                  send_sideband(1, 2, msg, sz, use_sideband);\n>          else\n> -               xwrite(2, msg, sz);\n> +               write_in_full(2, msg, sz);\n>   }\n> \n>   __attribute__((format (printf, 1, 2)))\n> diff --git a/builtin/repack.c b/builtin/repack.c\n> index ede36328a3..4916870992 100644\n> --- a/builtin/repack.c\n> +++ b/builtin/repack.c\n> @@ -314,8 +314,8 @@ static int write_oid(const struct object_id *oid,\n>                          die(_(\"could not start pack-objects to repack promisor objects\"));\n>          }\n> \n> -       xwrite(cmd->in, oid_to_hex(oid), the_hash_algo->hexsz);\n> -       xwrite(cmd->in, \"\\n\", 1);\n> +       write_in_full(cmd->in, oid_to_hex(oid), the_hash_algo->hexsz);\n> +       write_in_full(cmd->in, \"\\n\", 1);\n>          return 0;\n>   }\n> \n> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\n> index e0a701f2b3..6935c4574e 100644\n> --- a/builtin/unpack-objects.c\n> +++ b/builtin/unpack-objects.c\n> @@ -680,7 +680,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED)\n> \n>          /* Write the last part of the buffer to stdout */\n>          while (len) {\n> -               int ret = xwrite(1, buffer + offset, len);\n> +               int ret = write_in_full(1, buffer + offset, len);\n>                  if (ret <= 0)\n>                          break;\n>                  len -= ret;\n> \n> \n"},{"id":"489475","messageId":"Zd2hMmIzHKQ7JE45@tanuki","threadId":"60997","inReplyTo":"5e807c1c-20a0-407b-9fc2-acd38521ba45@gmail.com","subject":"Re: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-02-27T08:45:38Z","receivedAt":"2024-02-27T08:45:44Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Feb 26, 2024 at 03:32:14PM +0000, Phillip Wood wrote:\n> Hi Randal\n> \n> [cc'ing Patrick for the reftable writer]\n> \n> > The question is which call is bad? The cruft stuff is\n> > relatively new and I don't know the code.\n> > \n> > > > reftable/writer.c:              int n = w->write(w->write_arg, zeroed,\n> > > > w->pending_padding);\n> > > > reftable/writer.c:      n = w->write(w->write_arg, data, len);\n> \n> Neither of these appear to check for short writes and reftable_fd_write() is\n> a thin wrapper around write(). Maybe reftable_fd_write() should be using\n> write_in_full()?\n\nIt already does starting with 85a8c899ce (reftable: handle interrupted\nwrites, 2023-12-11):\n\n```\nstatic ssize_t reftable_fd_write(void *arg, const void *data, size_t sz)\n{\n\tint *fdp = (int *)arg;\n\treturn write_in_full(*fdp, data, sz);\n}\n```\n\nPatrick\n"},{"id":"489484","messageId":"ded76144-91e4-4b24-bcb9-8eec5f589c4a@gmail.com","threadId":"60997","inReplyTo":"Zd2hMmIzHKQ7JE45@tanuki","subject":"Re: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-27T10:43:38Z","receivedAt":"2024-02-27T10:43:42Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Patrick\n\nOn 27/02/2024 08:45, Patrick Steinhardt wrote:\n> On Mon, Feb 26, 2024 at 03:32:14PM +0000, Phillip Wood wrote:\n>>>>> reftable/writer.c:              int n = w->write(w->write_arg, zeroed,\n>>>>> w->pending_padding);\n>>>>> reftable/writer.c:      n = w->write(w->write_arg, data, len);\n>>\n>> Neither of these appear to check for short writes and reftable_fd_write() is\n>> a thin wrapper around write(). Maybe reftable_fd_write() should be using\n>> write_in_full()?\n> \n> It already does starting with 85a8c899ce (reftable: handle interrupted\n> writes, 2023-12-11):\n> \n> ```\n> static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz)\n> {\n> \tint *fdp = (int *)arg;\n> \treturn write_in_full(*fdp, data, sz);\n> }\n> ```\n\nOh, the branch I had checkout out was older than I realized, sorry for \nthe confusion.\n\nBest Wishes\n\nPhillip\n"},{"id":"489499","messageId":"02c501da6986$cb7c5c30$62751490$@nexbridge.com","threadId":"60997","inReplyTo":"Zd2hMmIzHKQ7JE45@tanuki","subject":"RE: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2024-02-27T14:10:55Z","receivedAt":"2024-02-27T14:11:08Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Tuesday, February 27, 2024 3:46 AM, Patrick Steinhardt wrote:\n>On Mon, Feb 26, 2024 at 03:32:14PM +0000, Phillip Wood wrote:\n>> Hi Randal\n>>\n>> [cc'ing Patrick for the reftable writer]\n>>\n>> > The question is which call is bad? The cruft stuff is relatively new\n>> > and I don't know the code.\n>> >\n>> > > > reftable/writer.c:              int n = w->write(w->write_arg,\nzeroed,\n>> > > > w->pending_padding);\n>> > > > reftable/writer.c:      n = w->write(w->write_arg, data, len);\n>>\n>> Neither of these appear to check for short writes and\n>> reftable_fd_write() is a thin wrapper around write(). Maybe\n>> reftable_fd_write() should be using write_in_full()?\n>\n>It already does starting with 85a8c899ce (reftable: handle interrupted\nwrites, 2023-12-11):\n>\n>```\n>static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz) {\n>\tint *fdp = (int *)arg;\n>\treturn write_in_full(*fdp, data, sz);\n>}\n\nUnfortunately, this fix is included in what I am testing but does not impact\nthe issue I am seeing one way or another, but thank you. \n\n"},{"id":"489506","messageId":"Zd3wGKaDIEztGrsn@tanuki","threadId":"60997","inReplyTo":"02c501da6986$cb7c5c30$62751490$@nexbridge.com","subject":"Re: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-02-27T14:22:16Z","receivedAt":"2024-02-27T14:22:21Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Feb 27, 2024 at 09:10:55AM -0500, rsbecker@nexbridge.com wrote:\n> On Tuesday, February 27, 2024 3:46 AM, Patrick Steinhardt wrote:\n> >On Mon, Feb 26, 2024 at 03:32:14PM +0000, Phillip Wood wrote:\n> >> Hi Randal\n> >>\n> >> [cc'ing Patrick for the reftable writer]\n> >>\n> >> > The question is which call is bad? The cruft stuff is relatively new\n> >> > and I don't know the code.\n> >> >\n> >> > > > reftable/writer.c:              int n = w->write(w->write_arg,\n> zeroed,\n> >> > > > w->pending_padding);\n> >> > > > reftable/writer.c:      n = w->write(w->write_arg, data, len);\n> >>\n> >> Neither of these appear to check for short writes and\n> >> reftable_fd_write() is a thin wrapper around write(). Maybe\n> >> reftable_fd_write() should be using write_in_full()?\n> >\n> >It already does starting with 85a8c899ce (reftable: handle interrupted\n> writes, 2023-12-11):\n> >\n> >```\n> >static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz) {\n> >\tint *fdp = (int *)arg;\n> >\treturn write_in_full(*fdp, data, sz);\n> >}\n> \n> Unfortunately, this fix is included in what I am testing but does not impact\n> the issue I am seeing one way or another, but thank you. \n\nI didn't expect it to :) The mentioned commit only fixes things with the\nreftable backend, which is not tested by default. I assume that you\ndidn't run tests with GIT_TEST_DEFAULT_REF_FORMAT=reftable, and thus\nt7704 wouldn't use the reftable code in the first place.\n\nPatrick\n"},{"id":"489513","messageId":"02e401da6989$35fd7510$a1f85f30$@nexbridge.com","threadId":"60997","inReplyTo":"Zd3wGKaDIEztGrsn@tanuki","subject":"RE: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2024-02-27T14:28:13Z","receivedAt":"2024-02-27T14:28:25Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Tuesday, February 27, 2024 9:22 AM, Patrick Steinhardt wrote:\n>On Tue, Feb 27, 2024 at 09:10:55AM -0500, rsbecker@nexbridge.com wrote:\n>> On Tuesday, February 27, 2024 3:46 AM, Patrick Steinhardt wrote:\n>> >On Mon, Feb 26, 2024 at 03:32:14PM +0000, Phillip Wood wrote:\n>> >> Hi Randal\n>> >>\n>> >> [cc'ing Patrick for the reftable writer]\n>> >>\n>> >> > The question is which call is bad? The cruft stuff is relatively\n>> >> > new and I don't know the code.\n>> >> >\n>> >> > > > reftable/writer.c:              int n = w->write(w->write_arg,\n>> zeroed,\n>> >> > > > w->pending_padding);\n>> >> > > > reftable/writer.c:      n = w->write(w->write_arg, data, len);\n>> >>\n>> >> Neither of these appear to check for short writes and\n>> >> reftable_fd_write() is a thin wrapper around write(). Maybe\n>> >> reftable_fd_write() should be using write_in_full()?\n>> >\n>> >It already does starting with 85a8c899ce (reftable: handle\n>> >interrupted\n>> writes, 2023-12-11):\n>> >\n>> >```\n>> >static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz)\n{\n>> >\tint *fdp = (int *)arg;\n>> >\treturn write_in_full(*fdp, data, sz); }\n>>\n>> Unfortunately, this fix is included in what I am testing but does not\n>> impact the issue I am seeing one way or another, but thank you.\n>\n>I didn't expect it to :) The mentioned commit only fixes things with the\nreftable backend, which is not tested by default. I assume that\n>you didn't run tests with GIT_TEST_DEFAULT_REF_FORMAT=reftable, and thus\n>t7704 wouldn't use the reftable code in the first place.\n\nThat is correct, I did not. Thanks.\n\n"}]}