RE: [PATCH] t5562: do not depend on /dev/zero
On February 15, 2019 13:01, Junio C Hamano wrote: > To: Randall S. Becker > Cc: 'Max Kirillov' ; git@vger.kernel.org; 'Johannes > Schindelin' > Subject: Re: [PATCH] t5562: do not depend on /dev/zero > > "Randall S. Becker" writes: > > > FTR, this particular subtest is not the one that is hanging. This > > subtest passes on NonStop with any and all (now) 4 solutions that have > > been floating around. > > One thing I'd like to know more is if this test passes on NonStop with this > patch, i.e. /dev/zero replaced with /dev/null. Yes, this particular subtest passes replacing /dev/null. The other three subtests still hang. This subtest never did.
RE: [PATCH] t5562: do not depend on /dev/zero
On February 15, 2019 11:43, Max Kirillov wrote: > It was reported [1] that NonStop platform does not have /dev/zero. > > The test uses /dev/zero as a dummy input. Passing case (http-backed failed > because of too big input size) should not be reading anything from it. If http- > backend would erroneously try to read any data returning EOF probably > would be even safer than providing some meaningless data. > > Replace /dev/zero with /dev/null to avoid issues with platforms which do not > have /dev/zero. > > [1] https://public-inbox.org/git/20190209185930.5256-4- > randall.s.bec...@rogers.com/ > > Reported-by: Randall S. Becker > Signed-off-by: Max Kirillov > --- > By the way, I don't think this requires such sofisticated fix. In the success > case the input would not be read at all. > You could replace it with /dev/null, the in failure (not immediate fail) git > would fail due to truncated input or something. > > Also, as you experience hang issue [2] in earlier tests, this one should not > have contributed to it. > > [2] https://public- > inbox.org/git/001901d4c22b$194bfe60$4be3fb20$@nexbridge.com/ > t/t5562-http-backend-content-length.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend- > content-length.sh > index 90d890d02f..436c261c86 100755 > --- a/t/t5562-http-backend-content-length.sh > +++ b/t/t5562-http-backend-content-length.sh > @@ -150,7 +150,7 @@ test_expect_success 'CONTENT_LENGTH overflow > ssite_t' ' > GIT_HTTP_EXPORT_ALL=TRUE \ > REQUEST_METHOD=POST \ > CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \ > - git http-backend /dev/null 2>err && > + git http-backend /dev/null 2>err && > grep "fatal:.*CONTENT_LENGTH" err FTR, this particular subtest is not the one that is hanging. This subtest passes on NonStop with any and all (now) 4 solutions that have been floating around. Cheers, Randall
RE: [ANNOUNCE] Git v2.21.0-rc1 (NonStop Results)
On February 15, 2019 8:02, SZEDER Gábor wrote: > To: Johannes Schindelin > Cc: Randall S. Becker ; 'Junio C Hamano' > ; git@vger.kernel.org; 'Max Kirillov' > > Subject: Re: [ANNOUNCE] Git v2.21.0-rc1 (NonStop Results) > > On Thu, Feb 14, 2019 at 10:36:42PM +0100, Johannes Schindelin wrote: > > On Thu, 14 Feb 2019, Randall S. Becker wrote: > > > > > t5562 still hangs (blocking) - this breaks our CI pipeline since the > > > test hangs and we have no explanation of whether the hang is in git > > > or the tests. > > > > I have "good" news: it now also hangs on Ubuntu 16.04 in Azure Pipelines' > > Linux agents. > > I haven't yet seen that hang in the wild and couldn't reproduce it on purpose, > but there is definitely something fishy with t5562 even on Linux and even > without that perl generate_zero_bytes helper. > > $ git checkout cc95bc2025^ > Previous HEAD position was cc95bc2025 t5562: replace /dev/zero with a > pipe from generate_zero_bytes > HEAD is now at 24b451e77c t5318: replace use of /dev/zero with > generate_zero_bytes > $ make > > $ cd t > # take note of the shell's PID > $ echo $$ > 15522 > $ ./t5562-http-backend-content-length.sh --stress |tee LOG > OK3.0 > OK1.0 > OK6.0 > OK0.0 > > > And then in another terminal run this: > > $ pstree -a -p 15522 > > or, to make it easier noticable what changed and what stayed the same: > > $ watch -d pstree -a -p 15522 > > The output will sooner or later will look like this: > > bash,15522 > └─t5562-http-back,21082 ./t5562-http-backend-content-length.sh --stress > ├─t5562-http-back,21089 ./t5562-http-backend-content-length.sh -- > stress > │ └─sh,24906 ./t5562-http-backend-content-length.sh --stress > ├─t5562-http-back,21090 ./t5562-http-backend-content-length.sh -- > stress > │ └─sh,26660 ./t5562-http-backend-content-length.sh --stress > ├─t5562-http-back,21092 ./t5562-http-backend-content-length.sh -- > stress > │ └─sh,4202 ./t5562-http-backend-content-length.sh --stress > │ └─sh,5696 ./t5562-http-backend-content-length.sh --stress > │ └─perl,5697 > /home/szeder/src/git/t/t5562/invoke-with-content- > length.pl push_body.gz.trunc git http-backend > │ └─(git,5722) > ├─t5562-http-back,21093 ./t5562-http-backend-content-length.sh -- > stress > │ └─sh,25572 ./t5562-http-backend-content-length.sh --stress > > > It won't show most of the processes run in the tests, because they are just > too fast and short-lived. However, occasionally it does show a stuck git > process, which is shown as in regular 'ps aux' > output: > > szeder 5722 0.0 0.0 0 0 pts/16 Z+ 13:36 0:00 [git] > > > Note that this is not a "proper" hang, in the sense that this process is not > stuck forever, but only for about 1 minute, after which it disappears, and the > test continues and eventually finishes with success. I've looked into the > logs > of a couple of such stuck jobs, and it seems that it varies in which test > that git > process happened to get stuck. We see something similar. The 60 seconds is in the support script in the t/t5562 directory. If a SIGCHLD is received, the sleep is interrupted and perl terminates (no hang). If the sleep is not interrupted, NonStop hangs in the close() after coming out of sleep because perl still has output to send somewhere. We are hung in the close call - which is really perplexing considering a close on NonStop in any other product is immediate and rather harsh, but perl's semantics for close() are: "Closing a pipe also waits for the process executing on the pipe to complete" (from the Perl spec), which seems to apply on NonStop because the git (5722) is reading but not receiving any data and not terminating - based on your tree above. Or, in other words, perl closing the pipe will not cause git (5722) to terminate because perl is waiting on git (5722) to terminate before completing the close. The only time it would not hang is if git (5722) terminates on its own so that sleep is interrupted without going back for more data to read. I am making a semi-educated guess. From my experience with the NS perl team, they are going to point at that spec and say that perl is exhibiting the correct behaviour and that the hang is expected. Another weird observation is that the test generates up to three hangs (subtests 6,8,13) at worst, and one (subtest 13) at best, depending on some unknown factor that might be system load. This is hinting at a race condition. Sadly, we don't have the above cool watch or pstree utilities on platform. What we do have is something called ptrace, which can look at the stack and I/O conditions of all open files and whether there are outstanding I/Os (and how many) on each FD, memory use.
RE: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1 (stack traces inside)
On February 14, 2019 22:48, Max Kirillov wrote: > To: Randall S. Becker > Cc: 'Max Kirillov' ; 'Johannes Schindelin via GitGitGadget' > ; git@vger.kernel.org; 'Junio C Hamano' > > Subject: Re: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1 (stack > traces inside) > > On Thu, Feb 14, 2019 at 06:33:56PM -0500, Randall S. Becker wrote: > > Here is the full set of traces (from subtest 6, which just hung). > > There are no I/O errors reported on any pipe or file descriptor. There > > is one git process waiting for a read to occur but no one is doing any > > writing. Most processes are sitting in waitpid, except for the > > initiating git, which is waiting on a read that never receives data, so > everyone is asleep and hung. > > The git process sitting in read is reading from a PIPE, not a file. > > > > There are no other processes involved in the test that I can see. > > > > Perl (waiting for output to be read): > > waitpid + 0x130 (SLr) > > $n_EnterPriv + 0x280 (Milli) > > Perl_wait4pid + 0x130 (UCr) > > Perl_my_pclose + 0x4C0 (UCr) > > Perl_io_close + 0x180 (UCr) > > Perl_do_close + 0x620 (UCr) > > Perl_pp_close + 0xA70 (UCr) > > Perl_runops_standard + 0xF0 (UCr) > > S_run_body + 0x870 (UCr) > > perl_run + 0x2D0 (UCr) > > main + 0x3D0 (UCr) > > > > git-http-backend: > > waitpid + 0x320 (SLr) > > $n_EnterPriv + 0x280 (Milli) > > cleanup_children + 0x5D0 (UCr) > > cleanup_children_on_exit + 0x70 (UCr) > > git_atexit_dispatch + 0x200 (UCr) > > __process_atexit_functions + 0xA0 (DLL zcredll) > > CRE_TERMINATOR_ + 0xB50 (DLL zcredll) > > exit + 0x2A0 (DLL zcrtldll) > > die_webcgi + 0x240 (UCr) > > die_errno + 0x360 (UCr) > > write_or_die + 0x1C0 (UCr) > > end_headers + 0x1A0 (UCr) > > die_webcgi + 0x220 (UCr) > > die + 0x320 (UCr) > > inflate_request + 0x520 (UCr) > > run_service + 0xC20 (UCr) > > service_rpc + 0x530 (UCr) > > cmd_main + 0xD00 (UCr) > > main + 0x190 (UCr) > > > > git (one of them): > > read64_ + 0x140 (SLr) > > $n_EnterPriv + 0x280 (Milli) > > xread + 0x130 (UCr) > > read_in_full + 0x130 (UCr) > > get_packet_data + 0x4B0 (UCr) > > packet_read_with_status + 0x230 (UCr) > > packet_reader_read + 0x310 (UCr) > > receive_needs + 0x300 (UCr) > > upload_pack + 0x680 (UCr) > > cmd_upload_pack + 0x830 (UCr) > > run_builtin + 0x980 (UCr) > > handle_builtin + 0x570 (UCr) > > run_argv + 0x210 (UCr) > > cmd_main + 0x710 (UCr) > > main + 0x190 (UCr) > > > > bash: > > waitpid + 0x130 (SLr) > > $n_EnterPriv + 0x280 (Milli) > > waitchld + 0x1F0 (UCr) > > wait_for + 0xFD0 (UCr) > > execute_command_internal + 0x1990 (UCr) > > execute_command + 0xC0 (UCr) > > reader_loop + 0x4F0 (UCr) > > main + 0x1140 (UCr) > > > > git (the other one): > > waitpid + 0x130 (SLr) > > $n_EnterPriv + 0x280 (Milli) > > wait_or_whine + 0xE0 (UCr) > > finish_command + 0x100 (UCr) > > run_command + 0x1F0 (UCr) > > execv_dashed_external + 0x800 (UCr) > > run_argv + 0x250 (UCr) > > cmd_main + 0x710 (UCr) > > main + 0x190 (UCr) > > This list does not say which process whose child but it seems like #3 is child of > #2 and #2 waits for #3, but #3 does not exit. Which is strange because it > should have send SIGTERM to it. Could the git-upload-pack somehow be > masking SIGTERM? SIGTERM cannot be masked or easily caught on the platform, so I don't think that's it. waitpid fails if the supplied pid is invalid, so it's not that either. However, I am not certain SIGPIPE is raised if a waited read is posted on a different pipe.
RE: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1 (stack traces inside)
On February 14, 2019 17:34, Max Kirillov wrote: > To: Randall S. Becker > Cc: 'Johannes Schindelin via GitGitGadget' ; > git@vger.kernel.org; 'Junio C Hamano' ; 'Max Kirillov' > > Subject: Re: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1 > > On Thu, Feb 14, 2019 at 05:17:26PM -0500, Randall S. Becker wrote: > > Unfortunately, subtest 13 still hangs on NonStop, even with this > > patch, so our Pipeline still hangs. I'm glad it's better on Azure, but > > I don't think this actually addresses the root cause of the hang. This > > is now the fourth attempt at fixing this. Is it possible this is not > > the test that is failing, but actually the git-http-backend? The code > > is not in a loop, if that helps. It is not consuming any significant > > cycles. I don't know that part of the code at all, sadly. The code is > > here: > > > > * in the operating system from here up * > > cleanup_children + 0x5D0 (UCr) > > ... so does the process which the stack was taken from has any children > processes still? > > I could imagine if a child somehow manages to end up in uninterruptible > sleep, then probably it would never complete this way, wouldn't it? Here is the full set of traces (from subtest 6, which just hung). There are no I/O errors reported on any pipe or file descriptor. There is one git process waiting for a read to occur but no one is doing any writing. Most processes are sitting in waitpid, except for the initiating git, which is waiting on a read that never receives data, so everyone is asleep and hung. The git process sitting in read is reading from a PIPE, not a file. There are no other processes involved in the test that I can see. Perl (waiting for output to be read): waitpid + 0x130 (SLr) $n_EnterPriv + 0x280 (Milli) Perl_wait4pid + 0x130 (UCr) Perl_my_pclose + 0x4C0 (UCr) Perl_io_close + 0x180 (UCr) Perl_do_close + 0x620 (UCr) Perl_pp_close + 0xA70 (UCr) Perl_runops_standard + 0xF0 (UCr) S_run_body + 0x870 (UCr) perl_run + 0x2D0 (UCr) main + 0x3D0 (UCr) git-http-backend: waitpid + 0x320 (SLr) $n_EnterPriv + 0x280 (Milli) cleanup_children + 0x5D0 (UCr) cleanup_children_on_exit + 0x70 (UCr) git_atexit_dispatch + 0x200 (UCr) __process_atexit_functions + 0xA0 (DLL zcredll) CRE_TERMINATOR_ + 0xB50 (DLL zcredll) exit + 0x2A0 (DLL zcrtldll) die_webcgi + 0x240 (UCr) die_errno + 0x360 (UCr) write_or_die + 0x1C0 (UCr) end_headers + 0x1A0 (UCr) die_webcgi + 0x220 (UCr) die + 0x320 (UCr) inflate_request + 0x520 (UCr) run_service + 0xC20 (UCr) service_rpc + 0x530 (UCr) cmd_main + 0xD00 (UCr) main + 0x190 (UCr) git (one of them): read64_ + 0x140 (SLr) $n_EnterPriv + 0x280 (Milli) xread + 0x130 (UCr) read_in_full + 0x130 (UCr) get_packet_data + 0x4B0 (UCr) packet_read_with_status + 0x230 (UCr) packet_reader_read + 0x310 (UCr) receive_needs + 0x300 (UCr) upload_pack + 0x680 (UCr) cmd_upload_pack + 0x830 (UCr) run_builtin + 0x980 (UCr) handle_builtin + 0x570 (UCr) run_argv + 0x210 (UCr) cmd_main + 0x710 (UCr) main + 0x190 (UCr) bash: waitpid + 0x130 (SLr) $n_EnterPriv + 0x280 (Milli) waitchld + 0x1F0 (UCr) wait_for + 0xFD0 (UCr) execute_command_internal + 0x1990 (UCr) execute_command + 0xC0 (UCr) reader_loop + 0x4F0 (UCr) main + 0x1140 (UCr) git (the other one): waitpid + 0x130 (SLr) $n_EnterPriv + 0x280 (Milli) wait_or_whine + 0xE0 (UCr) finish_command + 0x100 (UCr) run_command + 0x1F0 (UCr) execv_dashed_external + 0x800 (UCr) run_argv + 0x250 (UCr) cmd_main + 0x710 (UCr) main + 0x190 (UCr)
RE: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1
On February 14, 2019 17:34, Max Kirillov wrote: > To: Randall S. Becker > Cc: 'Johannes Schindelin via GitGitGadget' ; > git@vger.kernel.org; 'Junio C Hamano' ; 'Max Kirillov' > > Subject: Re: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1 > > On Thu, Feb 14, 2019 at 05:17:26PM -0500, Randall S. Becker wrote: > > Unfortunately, subtest 13 still hangs on NonStop, even with this > > patch, so our Pipeline still hangs. I'm glad it's better on Azure, but > > I don't think this actually addresses the root cause of the hang. This > > is now the fourth attempt at fixing this. Is it possible this is not > > the test that is failing, but actually the git-http-backend? The code > > is not in a loop, if that helps. It is not consuming any significant > > cycles. I don't know that part of the code at all, sadly. The code is > > here: > > > > * in the operating system from here up * > > cleanup_children + 0x5D0 (UCr) > > ... so does the process which the stack was taken from has any children > processes still? > > I could imagine if a child somehow manages to end up in uninterruptible > sleep, then probably it would never complete this way, wouldn't it? Yes, this is typical of a hang. Two processes reading on the same pipe, or one reading on a pipe and the other waiting for something that never shows. Or one process attempting both reading and writing on the same pipe (no kernel threads here). I did not see anything actually in sleep. perl is in a close call, waiting for its output to be consumed - which never happens, making me suspect this is a pipe setup issue, but I can't demonstrate that, sorry.
RE: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1
On February 14, 2019 17:39, Junio C Hamano wrote: > To: Randall S. Becker > Cc: 'Johannes Schindelin via GitGitGadget' ; > git@vger.kernel.org; 'Max Kirillov' > Subject: Re: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1 > > "Randall S. Becker" writes: > > > Unfortunately, subtest 13 still hangs on NonStop, even with this > > patch, so our Pipeline still hangs. I'm glad it's better on Azure, but > > I don't think this actually addresses the root cause of the hang. > > Sigh. > > > possible this is not the test that is failing, but actually the > > git-http-backend? The code is not in a loop, if that helps. It is not > > consuming any significant cycles. I don't know that part of the code > > at all, sadly. The code is here: > > > > * in the operating system from here up * > > cleanup_children + 0x5D0 (UCr) > > cleanup_children_on_exit + 0x70 (UCr) > > git_atexit_dispatch + 0x200 (UCr) > > __process_atexit_functions + 0xA0 (DLL zcredll) > > CRE_TERMINATOR_ + 0xB50 (DLL zcredll) > > exit + 0x2A0 (DLL zcrtldll) > > die_webcgi + 0x240 (UCr) > > die_errno + 0x360 (UCr) > > write_or_die + 0x1C0 (UCr) > > end_headers + 0x1A0 (UCr) > > die_webcgi + 0x220 (UCr) > > die + 0x320 (UCr) > > inflate_request + 0x520 (UCr) > > run_service + 0xC20 (UCr) > > service_rpc + 0x530 (UCr) > > cmd_main + 0xD00 (UCr) > > main + 0x190 (UCr) > > > > Best guess is that a signal (SIGCHLD?) is possibly getting eaten or > > neglected somewhere between the test, perl, and git-http-backend. > > So we are trying to die(), which actually happens in die_webcgi(), and then try > to write some message _but_ notice an error inside > write_or_dir() and try to exit because we do not want to recurse forever > trying to die, giving a message to say how/why we died, and die because > failing to give that message, forever. > > But in our attempt to exit(), we try to "cleanup children" and that is what gets > stuck. > > One big difference before and after the /dev/zero change is that the process > is now on a downstream of the pipe. If we prepare a large file with a finite > size full of NULs and replace /dev/null with it, instead of feeding NULs from > the pipe, would it change the equation? Doubtful. The processes are still around, and are waiting on read but not actively reading (CPU time is not going up, so we're not reading an infinite stream). To me, this is a pipe situation where there is simply nothing waiting on the pipe (maybe a flush missing?). I'm grasping are straws without knowing the actual process architecture of the test to debug it.
RE: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1
On February 14, 2019 17:34, Max Kirillov wrote: > On Thu, Feb 14, 2019 at 05:17:26PM -0500, Randall S. Becker wrote: > > Unfortunately, subtest 13 still hangs on NonStop, even with this > > patch, so our Pipeline still hangs. I'm glad it's better on Azure, but > > I don't think this actually addresses the root cause of the hang. This > > is now the fourth attempt at fixing this. Is it possible this is not > > the test that is failing, but actually the git-http-backend? The code > > is not in a loop, if that helps. It is not consuming any significant > > cycles. I don't know that part of the code at all, sadly. The code is > > here: > > > > * in the operating system from here up * > > cleanup_children + 0x5D0 (UCr) > > ... so does the process which the stack was taken from has any children > processes still? > > I could imagine if a child somehow manages to end up in uninterruptible > sleep, then probably it would never complete this way, wouldn't it? >From what I can tell (previously reported), none of the children are dead. git-http-backend is waiting and the others are in a read state. I can try to get full stack traces once the current cycle ends.
RE: [ANNOUNCE] Git v2.21.0-rc1 (NonStop Results)
On February 14, 2019 16:37, Johannes Schindelin wrote: > On Thu, 14 Feb 2019, Randall S. Becker wrote: > > > t5562 still hangs (blocking) - this breaks our CI pipeline since the > > test hangs and we have no explanation of whether the hang is in git or > > the tests. > > I have "good" news: it now also hangs on Ubuntu 16.04 in Azure Pipelines' > Linux agents. > > There is a silver lining with those good news, though: I found a workaround, > and it might work for you, too: > > https://github.com/gitgitgadget/git/pull/126 > > (I also submitted this to the Git mailing list, as I really wanted to tag Git for > Windows' v2.21.0-rc1.windows.1 only with a passing build, and I do not want > to keep that patch to the Windows port only.) Thanks for trying. It was a good try, but did not fix the hang. See my other response for the stack trace. I tried debugging once it hung, but the code never exits from the operating system, so I can't get inside. It is hiding in waitpid on a process that exists otherwise we would get an error (EINTR, ECHILD, EFAULT are possible returns). One thing to consider is that we do not have kernel threads, so if that is assumed, that is badness. Regards, Randall
RE: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1
On February 14, 2019 16:33, Johannes Schindelin wrote: > To: git@vger.kernel.org > Cc: Randall Becker ; Junio C Hamano > > Subject: [PATCH 0/1] Fix hang in t5562, introduced in v2.21.0-rc1 > > The last-minute patch to replace /dev/zero with a Perl script snippet broke > the Linux part of the CI builds on Azure Pipelines: it timed out. The culprit > is > the rb/no-dev-zero-in-test branch (see the build for this branch here > [https://dev.azure.com/gitgitgadget/git/_build/results?buildId=1727]). > > All of master, next, jch and pu are broken that way. You might see it in the > commit status of the active branches > [https://github.com/gitgitgadget/git/branches/active]. > > Turns out that it is that particular Perl script snippet which for some reason > hangs the build. If you kill it, t5562.15 succeeds, if you don't kill it, it > will hang > indefinitely (or until killed). > > Sadly, despite my earnest attempts, I could not figure out why it hangs in > those Linux agents (I could not reproduce that hang locally), or for that > matter, why it does not hang in the Windows and macOS agents. > > Let's avoid that hang. This patch fixes things on Azure Pipelines, and my hope > is that it also fixes the hang on NonStop. > > Johannes Schindelin (1): > tests: teach the test-tool to generate NUL bytes and use it > > Makefile | 1 + > t/helper/test-genzeros.c | 22 ++ > t/helper/test-tool.c | 1 + > t/helper/test-tool.h | 1 + > t/t5562-http-backend-content-length.sh | 2 +- > t/test-lib-functions.sh| 8 +--- > 6 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 > t/helper/test-genzeros.c > > > base-commit: 8989e1950a845ceeb186d490321a4f917ca4de47 > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr- > 126%2Fdscho%2Ffix-t5562-hang-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-126/dscho/fix- > t5562-hang-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/126 Unfortunately, subtest 13 still hangs on NonStop, even with this patch, so our Pipeline still hangs. I'm glad it's better on Azure, but I don't think this actually addresses the root cause of the hang. This is now the fourth attempt at fixing this. Is it possible this is not the test that is failing, but actually the git-http-backend? The code is not in a loop, if that helps. It is not consuming any significant cycles. I don't know that part of the code at all, sadly. The code is here: * in the operating system from here up * cleanup_children + 0x5D0 (UCr) cleanup_children_on_exit + 0x70 (UCr) git_atexit_dispatch + 0x200 (UCr) __process_atexit_functions + 0xA0 (DLL zcredll) CRE_TERMINATOR_ + 0xB50 (DLL zcredll) exit + 0x2A0 (DLL zcrtldll) die_webcgi + 0x240 (UCr) die_errno + 0x360 (UCr) write_or_die + 0x1C0 (UCr) end_headers + 0x1A0 (UCr) die_webcgi + 0x220 (UCr) die + 0x320 (UCr) inflate_request + 0x520 (UCr) run_service + 0xC20 (UCr) service_rpc + 0x530 (UCr) cmd_main + 0xD00 (UCr) main + 0x190 (UCr) Best guess is that a signal (SIGCHLD?) is possibly getting eaten or neglected somewhere between the test, perl, and git-http-backend. Stuck, Randall
RE: Re* [Breakage] 2.20.0-rc0 t1404: test_i18ngrep reports 1 instead of 0 on NonStop in one case
On February 14, 2019 15:16, Junio C Hamano wrote: > Jeff King writes: > > > On Mon, Feb 11, 2019 at 01:07:15PM -0800, Junio C Hamano wrote: > > > >> >> test_i18ngrep "Unable to create $Q.*packed-refs.lock$Q: File > >> >> exists" err > >> > > >> > The message does not match, does it? Here we grep for "File exists" > >> > but the message you showed says "File already exists". > >> > >> Hmph, this is from strerror(), right? > >> > >> The question is if we should be using grep to match on strerror() > >> result in the C locale. > > > > Yeah, I agree that's questionable. And I'm mildly surprised it hasn't > > been a problem before now. > > > >> Do we really care that the reason of the failure is due to EEXIST for > >> this particular test? > > > > Hmm. We care to _some_ degree, since that's the condition we set up > > for making sure that update-ref cannot take the lock. But it would > > probably be fine to just confirm that we failed to take the lock. And > > there, checking for just "Unable to create $Q.*packed-refs.lock" would > > be sufficient. > > Yup. > > As this came from 6a2a7736 ("t1404: demonstrate two problems with > reference transactions", 2017-09-08), that is as old as Git 2.15, I'd throw it > into "not so urgent" pile. > > -- >8 -- > Subject: [PATCH] t1404: do not rely on the exact phrasing of strerror() > > Not even in C locale, it is wrong to expect that the exact phrasing "File > exists" is used to show EEXIST. > > Reported-by: Randall S. Becker > Helped-by: Duy Nguyen > Helped-by: Jeff King > Signed-off-by: Junio C Hamano > --- > > I've grepped in t/ directory for the exact phrases of all errno on a > recent Debian box, and this was the only hit it found. There > are two other hits but both in the comments. > > t/t1404-update-ref-errors.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh index > 51a4f4c0ac..f95a64c911 100755 > --- a/t/t1404-update-ref-errors.sh > +++ b/t/t1404-update-ref-errors.sh > @@ -614,7 +614,7 @@ test_expect_success 'delete fails cleanly if packed- > refs file is locked' ' > test_when_finished "rm -f .git/packed-refs.lock" && > test_must_fail git update-ref -d $prefix/foo >out 2>err && > git for-each-ref $prefix >actual && > - test_i18ngrep "Unable to create $Q.*packed-refs.lock$Q: File exists" > err && > + test_i18ngrep "Unable to create $Q.*packed-refs.lock$Q:" err && > test_cmp unchanged actual > ' This passes on NonStop. Thanks. Randall
RE: [ANNOUNCE] Git v2.21.0-rc1 (NonStop Results)
On February 13, 2019 22:33, Junio C Hamano wrote: > A release candidate Git v2.21.0-rc1 is now available for testing at the usual > places. It is comprised of 464 non-merge commits since v2.20.0, contributed > by 60 people, 14 of which are new faces. We are currently running through a full regression of v2.21.0-rc1 on NonStop. It will take about 30 hours, but preliminary results, relative to breakages found in rc0 are: t1308 is fixed. t1404 is still broken (explainable) - scraping strerror output mismatches reported error on NonStop for EEXIST t5318 is fixed. t5403 is fixed. t5562 still hangs (blocking) - this breaks our CI pipeline since the test hangs and we have no explanation of whether the hang is in git or the tests. Cheers, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much.
RE: [Patch v1 2/3] t5318: replace use of /dev/zero with generate_zero_bytes
On February 13, 2019 16:01, Junio C Hamano wrote: > "Randall S. Becker" writes: > > > My second attempt was to create the generate_zero_bytes function to > > replace exactly what the second dd was doing but not user /dev/zero. > > Yes, and I think the patch does that ;-) It was just the original > > dd if=/dev/zero of=... bs=1 seek=$there count=$this_many > > would have been impossible to rewrite with the new generate_zero_bytes > helper unless $there weren't seeking to the end of the file. > > But the other dd before the one the patch rewrites truncates the file to make > that seek=$there seeking to the end of the file, so simply appending output > from genereate_zero_bytes is sufficient and correct conversion. I wanted to > explain that for future readers who may wonder if the patch is doing the > exact conversion. Sounds like we need a documentation patch in the actual test suite rather than in the commit . Cheers, Randall
RE: [BUG] More on t5562 hangs randomly in subtests 6,8 and 13 in 2.21.0-rc0
On February 13, 2019 12:41, Max Kirillov wrote: > On Wed, Feb 13, 2019 at 10:16:26AM -0500, randall.s.bec...@rogers.com > wrote: > > On 2019-02-13, Max Kirillov, wrote: > > As far as the unintended reuse of the output file, and issues with > > pipes, yes, the NonStop is very sensitive to complex use of pipes and > > much of the compatibility issues we have had relate to those (usually > > Linux-specific pipe assumptions). That is where I have been looking > > when trying to debug this situation (not yet found anything). This > > could very well be directly related. > > You mentioned cases 6,8,13. These are all related to gipped request body. > Could it be the git-http-backend does not clean a sub-process which > pervforms the decompression? I guess that is possible. I don't know the guts of this part of the code.
RE: [Patch v1 2/3] t5318: replace use of /dev/zero with generate_zero_bytes
> -Original Message- > From: Junio C Hamano On Behalf Of Junio C Hamano > Sent: February 13, 2019 12:25 > To: Eric Sunshine > Cc: randall.s.bec...@rogers.com; Git List ; Randall S. > Becker > Subject: Re: [Patch v1 2/3] t5318: replace use of /dev/zero with > generate_zero_bytes > > Junio C Hamano writes: > > > Eric Sunshine writes: > > > >> On Sat, Feb 9, 2019 at 2:00 PM wrote: > >>> This change removes the dependency on /dev/zero with > >>> generate_zero_bytes appending NUL values to blocks generating wrong > signatures for test cases. > >> > >> This commit message says what the patch does but not _why_. At > >> minimum, it should explain that /dev/zero is not available on all > >> platforms, therefore, not portable, and (perhaps) cite NonStop as an > >> example. > > > > Does sombody want to do the honors? [PATCH 1/3] would become wasted > > effort until that happens. On the other hand, if this is not urgent > > (it is only urgent for those without /dev/zero, and to others it may > > be distraction/disruption this close to the final release to add > > increased risk of fat finger mistakes), obviously I can wait. > > So, before I lose the access to my primary screen (I was told that somehow I > need to reimage the workstation today X-<), here is what I have now. > > -- >8 -- > From: "Randall S. Becker" > Date: Sat, 9 Feb 2019 13:59:29 -0500 > Subject: [PATCH] t5318: replace use of /dev/zero with generate_zero_bytes > > There are platforms (e.g. NonStop) that lack /dev/zero; use the > generate_zero_bytes helper we just introduced to append stream of NULs at > the end of the file. > > The original, even though it uses "dd seek=... count=..." to make it look like it > is overwriting the middle part of an existing file, has truncated the file before > this step with another use of "dd", which may make it tricky to see why this > rewrite is a correct one. Here is how I interpret the test - might be wrong, but yanno... The first dd copies something looking like reasonable data from the test case. The second dd copies zeros from seek to the end of a fixed size block. My first attempt at a fix used truncate that extended the first to the correct size (filling with zeros). My worry there is that I'm not sure there is a guarantee of zeros, but that shouldn't matter for the test which just wants a signature mismatch. Others suggested using yes to fill in junk. My second attempt was to create the generate_zero_bytes function to replace exactly what the second dd was doing but not user /dev/zero. The fix was not to change the conditions of the test - not debating the correctness of that here - but to simply replicate the use of /dev/zero in context. So the resulting file contains [reasonable-stuff]{seek}[0]{orig_size-seek}, which is sufficient to satisfy the conditions of the test. > > Signed-off-by: Randall S. Becker > Signed-off-by: Junio C Hamano > --- > t/t5318-commit-graph.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index > 16d10ebce8..d4bd1522fe 100755 > --- a/t/t5318-commit-graph.sh > +++ b/t/t5318-commit-graph.sh > @@ -383,7 +383,7 @@ corrupt_graph_and_verify() { > cp $objdir/info/commit-graph commit-graph-backup && > printf "$data" | dd of="$objdir/info/commit-graph" bs=1 > seek="$pos" conv=notrunc && > dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=0 > && > - dd if=/dev/zero of="$objdir/info/commit-graph" bs=1 > seek="$zero_pos" count=$(($orig_size - $zero_pos)) && > + generate_zero_bytes $(($orig_size - $zero_pos)) > +>>"$objdir/info/commit-graph" && > test_must_fail git commit-graph verify 2>test_err && > grep -v "^+" test_err >err && > test_i18ngrep "$grepstr" err
RE: [Proposed Fix] daemon.c: not initializing revents
On February 11, 2019 21:59, Junio C Hamano wrote: > "Randall S. Becker" writes: > > >> In any case, no matter what POSIX says, if clearing .revents before > > calling > >> poll() helps on platforms in the real world, the patch is worth > >> taking as > > a fix, I > >> would think. > > > > That's what my intent was - my explanations are suffering from a > > little work-induced sleep deprivation. Would you like this as a formal > patch? > > That depends ;-) > > At this late in the cycle, I do not see much urgency for this patch to be in the > upcoming release (after all, this code survived real world for quite a long > time, so it's only minority platforms like NonStop that haven't seen serious > porting effort until recently that would see improvement---and they have > survived without reliably working daemon for so long that they can wait for > one more release). > > Now, the knowledge that we will have long enough time before the final > version of the formal patch becomes necessary makes me wonder what the > best use of that time to polish the patch would be. Ideally we'd like to see > "this definitely fixed (or 'worked around') such and such breakages on > platform X, Y and Z" instead of my "Well, we could read POSIX that way, so > there may be some platforms that would require applications to do this, and > an extra assignment here would certainly not hurt", which was the hand- > waving I just did. > > I dunno. I hear that. I'd rather (not) be working on debugging breakages from other authors that impact my platform. Honestly, I'd rather work at my $DAYJOB, although, some days... Since this topic isn't a breakage per-se (no tests seem to be impacted on way or another), I agree that this can wait and get through the normal cycle of events at some point in the future. Now, if I could only get some help on t5562 ;) that would be time well spent for rc0. Cheers, Randall
RE: [Breakage] 2.20.0-rc0 t1404: test_i18ngrep reports 1 instead of 0 on NonStop in one case
On February 11, 2019 16:07, Junio C Hamano wrote: > Duy Nguyen writes: > > > On Mon, Feb 11, 2019 at 2:09 AM Randall S. Becker > > wrote: > >> > >> Hi All, > >> > >> I tracked down a breakage in t1404 subtest 52. The line > >> > >> test_i18ngrep "Unable to create $Q.*packed-refs.lock$Q: File exists" > >> err > > > > The message does not match, does it? Here we grep for "File exists" > > but the message you showed says "File already exists". > > Hmph, this is from strerror(), right? You can reasonably expect that NonStop error messages deviate occasionally. Scaping from strerror() is not a good plan. A worse plan is to use errno values, which I can guarantee do not match, but that's just an FYI. > The question is if we should be using grep to match on strerror() result in the > C locale. Do we really care that the reason of the failure is due to EEXIST for > this particular test? > > >> The verbose output, with diagnostics, is: > >> > >> error: 'grep Unable to create '.*packed-refs.lock': File exists err' > >> didn't find a match in: > >> error: Unable to create '/home/git/git/t/trash > >> directory.t1404-update-ref-errors/.git/packed-refs.lock': File > >> already exists. > > Otherwise, perhaps we should loosen the grep pattern, not as a part of > "NonStop fix" topic, but as "tests should not depend on having a canonical > spelling of strerror() result even in C locale" topic. I'm happy not to have the fix I supplied used if there's a better way.
RE: [Proposed Fix] daemon.c: not initializing revents
On February 11, 2019 15:57, Junio C Hamano wrote: > "Randall S. Becker" writes: > > > Hi All, > > > > I found this while trying to track down a hang in t5562 - this isn't > > the fix, but here it is something that could be considered a > > code-inspection. If there have been random unexplained hangs when git > > runs as a daemon, this might be the cause. > > > > According to many systems (other than Linux), the revents field is > > supposed to be 0 on return to poll(). This was the cause of some > > heart-ache a while back in compat/poll/poll.c. > > I am having a hard time grokking "supposed to be 0 on return to", but do you > mean "the caller must clear .revents field before calling poll()"? > > http://pubs.opengroup.org/onlinepubs/9699919799/functions/poll.html > has this > > In each pollfd structure, poll() shall clear the revents member, > except that where the application requested a report on a condition > by setting one of the bits of events listed above, poll() shall set > the corresponding bit in revents if the requested condition is > true. In addition, poll() shall set the POLLHUP, POLLERR, and > POLLNVAL flag in revents if the condition is true, even if the > application did not set the corresponding bit in events. > > and I am also having a hard time interpreting the "except that". If we asked > to report (e.g. we set POLLIN in the .events field), poll() does not have to > clear .revents but just set whatever bits it needs to set to report the > condition in the field? > > If that is the case, it makes it a bug not to clear .revents before calling poll; > the sample code snippet on the same page in EXAMPLES section does not, > though, so I am puzzled. > > In any case, no matter what POSIX says, if clearing .revents before calling > poll() helps on platforms in the real world, the patch is worth taking as a fix, I > would think. That's what my intent was - my explanations are suffering from a little work-induced sleep deprivation. Would you like this as a formal patch? > > > I am not certain whether that copy of poll() is used in daemon, but I > > wanted to point out that the value is being returned to poll, outside > > of compat/poll/poll.c and may present a potential for poll returning > > an error on that FD due to random values that might be in revents. > > > > Please see 61b2a1acaae for a related change/justification. > > > > diff --git a/daemon.c b/daemon.c > > index 9d2e0d20ef..1e275fc8b3 100644 > > --- a/daemon.c > > +++ b/daemon.c > > @@ -1194,6 +1194,7 @@ static int service_loop(struct socketlist *socklist) > > } > > handle(incoming, &ss.sa, sslen); > > } > > + pfd[i].revents = 0; > > } > > } > > } Regards, Randall
[BUG] More on t5562 hangs randomly in subtests 6,8 and 13 in 2.21.0-rc0
Hi All, I have localized the hang in t5562 (previous thread) to the invoke-with-content-length.pl script. At least on NonStop, what happens is that the perl process hangs waiting for close($out) to complete whether explicitly or implicitly (if the call is removed). The trace for the perl process shows it hung at Perl_io_close (the platform's trace anyway). My interpretation is that the reading process is still around but is no longer reading on that pipe. If any of the processes hanging around are killed, the structure unwinds. However, when some of the tests are run, git-http-backend remains running after subtest 6 and/or 8 runs even if that subtest does not hang. The presence of other git-http-backend processes seems to interfere with subsequent tests, and if you run tests individually, subtests 6,8, and 13 consistently pass. Strangely, if a bunch of print statements are added to another terminal explicitly, the test works consistently, so this is sounding a bit either like a race condition or flushes are not being handled consistently although the code appears to handle the latter case. Simply killing old git-http-backend and/or perl processes does not make a difference so the race may involve test contents, but I can't make that determination. There is no correlation with system load. That's as far as I have been able to analyze the situation at this stage. I've CC'd the author to see whether there might be some perspective that can come in here to help out. This test has broken our CI process for git on NonStop, because of the hang, so it's rather important to us to get this resolved before the official 2.21.0. Still hoping for help on this issue, Randall
RE: [Breakage] 2.20.0-rc0 t1404: test_i18ngrep reports 1 instead of 0 on NonStop in one case
On February 11, 2019 4:57, Duy Nguyen wrote: > On Mon, Feb 11, 2019 at 2:09 AM Randall S. Becker > wrote: > > > > Hi All, > > > > I tracked down a breakage in t1404 subtest 52. The line > > > > test_i18ngrep "Unable to create $Q.*packed-refs.lock$Q: File exists" > > err > > The message does not match, does it? Here we grep for "File exists" > but the message you showed says "File already exists" So if I understand this correctly, it means that NonStop is reporting a different textual error that other platforms, but is still sane. Would a fix as follows be appropriate? @@ -614,7 +614,12 @@ test_expect_success 'delete fails cleanly if packed-refs file is locked' ' test_when_finished "rm -f .git/packed-refs.lock" && test_must_fail git update-ref -d $prefix/foo >out 2>err && git for-each-ref $prefix >actual && - test_i18ngrep "Unable to create $Q.*packed-refs.lock$Q: File exists" err && + # Handle a difference in error reporting text on NonStop + if [ `uname` != "NONSTOP_KERNEL" ]; then \ + test_i18ngrep "Unable to create $Q.*packed-refs.lock$Q: File exists" err ; \ + else \ + test_i18ngrep "Unable to create $Q.*packed-refs.lock$Q: File already exists" err ; \ + fi && test_cmp unchanged actual I'm not at all confident that the committers will like a hack like this but it does work. Regards, Randall
RE: [Patch v1 1/3] test-lib-functions.sh: add generate_zero_bytes function
On February 9, 2019 21:05, Eric Sunshine wrote: > On Sat, Feb 9, 2019 at 1:59 PM wrote: > > t5318 and t5562 used /dev/zero, which is not portable. This function > > provides both a fixed block of NUL bytes and an infinite stream of NULs. > > > > Signed-off-by: Randall S. Becker > > --- > > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh @@ > > -116,6 +116,19 @@ remove_cr () { > > +# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes). > > +# If $1 is 'infinity', output forever or until the receiving pipe > > +stops reading, # whichever comes first. > > This is a somewhat unusual API. A (perhaps) more intuitive behavior would be > for it to emit an infinite stream of NULs when given no argument, and a > limited number of NULs when given an argument. > Redefining the behavior like that also fixes the "problem" with the current > implementation erroring-out if no argument is provided. At this point, I've supplied three different ways to solve this incompatibility for platforms not Linux and others have also provided fixes and discussed this at length. It is not a specific fix that matters to me, but that there is a fix at all. So thanks for all your comments and I will wait on direction on what the team wants me to do about it, if anything. Regards, Randall
[Breakage] 2.20.0-rc0 t1404: test_i18ngrep reports 1 instead of 0 on NonStop in one case
Hi All, I tracked down a breakage in t1404 subtest 52. The line test_i18ngrep "Unable to create $Q.*packed-refs.lock$Q: File exists" err is correctly working, but is reporting a completion 1. The verbose output, with diagnostics, is: error: 'grep Unable to create '.*packed-refs.lock': File exists err' didn't find a match in: error: Unable to create '/home/git/git/t/trash directory.t1404-update-ref-errors/.git/packed-refs.lock': File already exists. Another git process seems to be running in this repository, e.g. an editor opened by 'git commit'. Please make sure all processes are terminated then try again. If it still fails, a git process may have crashed in this repository earlier: remove the file manually to continue. Reported 1 from test_i18ngrep err contains (without the double quotes): "error: Unable to create '/home/git/git/t/trash directory.t1404-update-ref-errors/.git/packed-refs.lock': File already exists. Another git process seems to be running in this repository, e.g. an editor opened by 'git commit'. Please make sure all processes are terminated then try again. If it still fails, a git process may have crashed in this repository earlier: remove the file manually to continue." Which means that ! test_have_prereq C_LOCALE_OUTPUT || test_cmp "$@" is completing with a 1. Is that the intent? Any clues? Thanks, Randall
[Fix v1] config.mak.uname: add FREAD_READS_DIRECTORIES for NonStop platform
From: "Randall S. Becker" The NonStop platform needs this configuration item specified as UnfortunatelyYes so that config directory files are correctly processed. Signed-off-by: Randall S. Becker --- config.mak.uname | 1 + 1 file changed, 1 insertion(+) diff --git a/config.mak.uname b/config.mak.uname index 6bd67eb86..75ff43f1f 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -490,6 +490,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL) OLD_ICONV = UnfortunatelyYes NO_REGEX = NeedsStartEnd NO_PTHREADS = UnfortunatelyYes + FREAD_READS_DIRECTORIES = UnfortunatelyYes # Not detected (nor checked for) by './configure'. # We don't have SA_RESTART on NonStop, unfortunalety. -- 2.12.3
RE: [Possible Breakage] t1308 - Bad return value from test-tool
On February 9, 2019 18:33, Jeff King wrote: > On Sat, Feb 09, 2019 at 01:08:01PM -0500, Randall S. Becker wrote: > > > > It sounds like you might need to set FREAD_READS_DIRECTORIES in your > > > config.mak. > > > > Setting FREAD_READS_DIRECTORIES = UnfortunatelyYes > > Silly question, but you did rebuild after setting that, not just re-run the > tests, > right? Yes make was run, but some doofus (me) did not run make tests first, so test-tool was out of date, post reconfigure. When just a code change is made, test-tool gets build. I was/am confused. > > still results in > > > > Value not found for "foo.bar" > > test_expect_code: command exited with 1, we wanted 2 test-tool config > > configset_get_value foo.bar a-directory not ok 23 - proper error on > directory "files" > > # > > # echo "Error (-1) reading configuration file a-directory." > > >expect > && > > # mkdir a-directory && > > # test_expect_code 2 test-tool config configset_get_value > > foo.bar > a-directory 2>output && > > # grep "^warning:" output && > > # grep "^Error" output >actual && > > # test_cmp expect actual > > # > > > > I don't think that helped. While fopen can open a directory, fread > > does not return any data in this platform. readdir or nftw/ftw are > > pretty much the only options. However, the code still goes down the > > goto exit1 path in this situation. > > Hrm. That's the exact case FREAD_READS_DIRECTORIES is supposed to help. > It does an fstat() after fopen()ing the file to see if it points to a > directory, and > if so closes it immediately and returns EISDIR. > > Can you confirm via debugger (or printf statements) that we make it into > git_fopen, and what the resulting st.st_mode we see looks like? It did not get into git_fopen at all. Subtest 1 did, but that's no surprise. t1308 is passes now, finally. Patch coming. Thanks for the help, Randall
RE: [Hang] t5562 subtest 8 on NonStop
On February 8, 2019 7:23, I wrote: > We have suddenly encountered a hung git-http-backend in t5562 in the > NonStop port. This is a new problem not seen before on the platform, > surprisingly. I am wondering whether this is a result of not actually having an > apache2 server on-board. Is that a possibility and can that sub-test be > bypassed if no apache2 is detected? > > We also had subtest 15 and I am investigating, but it may depend on 8's data > (I had to kill the git-http-backend process, so maybe that's why). I'm back to investigating subtest 8 hanging after dealing with the /dev/zero thing. What we end up with two git processes, a git-http-backend, a perl, and a few shell processes. The act.err file had: fatal: request ended in the middle of the gzip stream Which went through die() and should have unwound the whole thing, but didn't. The git-http-backend is waiting to clean up children and each git is polling for input. It seems like SIGCHLD did not get back through the process that started it, or its parent - but I don't quite follow the flow in this test. I have seen the perl port not like SIGCHLD sometimes, so that might be what is going on. Is it possible to skip the hanging subtest during a whole test run? We don't really have anyone using the http backend on platform (SSH is the generally preferred method to get to git on box), so I am tempted to ignore the ones that hang as conditional known breakages, if that is possible, so that we can get our CI builds back in operation. Thoughts? Advice? Thanks, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much.
[Proposed Fix] daemon.c: not initializing revents
Hi All, I found this while trying to track down a hang in t5562 - this isn't the fix, but here it is something that could be considered a code-inspection. If there have been random unexplained hangs when git runs as a daemon, this might be the cause. According to many systems (other than Linux), the revents field is supposed to be 0 on return to poll(). This was the cause of some heart-ache a while back in compat/poll/poll.c. I am not certain whether that copy of poll() is used in daemon, but I wanted to point out that the value is being returned to poll, outside of compat/poll/poll.c and may present a potential for poll returning an error on that FD due to random values that might be in revents. Please see 61b2a1acaae for a related change/justification. diff --git a/daemon.c b/daemon.c index 9d2e0d20ef..1e275fc8b3 100644 --- a/daemon.c +++ b/daemon.c @@ -1194,6 +1194,7 @@ static int service_loop(struct socketlist *socklist) } handle(incoming, &ss.sa, sslen); } + pfd[i].revents = 0; } } } Regards, Randall
RE: [Fix v2] t5562: remove dependency on /dev/zero
On February 9, 2019 13:25, Junio C Hamano wrote: > Johannes Sixt writes: > > > How many bytes are needed here? yes() in test-lib.sh generates only 99 > > 'y', if I am not mistaken. > > I think we will not use "yes" in the end for this topic, which makes this > comment totally irrelevant to the thread, but I wonder why we have the limit > of 99 there? It cannot be "we do not want to worry about sigpipe" affecting > the end result of the test (after all the reader may stop reading from after > reading just one, and the status of the upstream process that would die with > sigpipe is lost anyway). > > It turns out it is about sigpipe ;-) but in somewhat a different way. To > prevent others from wasting their time wondering about this, probably we > want to have something like the attached? > > t/README | 9 + > t/test-lib.sh | 6 +- > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/t/README b/t/README > index 1326fd7505..f4e1a82657 100644 > --- a/t/README > +++ b/t/README > @@ -927,6 +927,15 @@ library for your script to use. > test_oid_init or test_oid_cache. Providing an unknown key is an > error. > > + - yes [] > + > + This is often seen in modern UNIX but some platforms lack it, so > + the test harness overrides the platform implementation with a > + more limited one. Use this only when feeding a handful lines of > + output to the downstream---unlike the real version, it generates > + only up to 99 lines. > + > + > Prerequisites > - > > diff --git a/t/test-lib.sh b/t/test-lib.sh index 42b1a0aa7f..541a37f4c0 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -1313,7 +1313,11 @@ then > fi > fi > > -# Provide an implementation of the 'yes' utility > +# Provide an implementation of the 'yes' utility; the upper bound # > +limit is there to help Windows that cannot stop this loop from # > +wasting cycles when the downstream stops reading, so do not be # > +tempted to turn it into an infinite loop. cf. 6129c930 ("test-lib: > +# limit the output of the yes utility", 2016-02-02) > yes () { > if test $# = 0 > then Sadly, I already the other path ready, but did not have a chance to send it. I'm ok either way as long as I can get the tests running. Regards, Randall
[Patch v1 2/3] t5318: replace use of /dev/zero with generate_zero_bytes
From: "Randall S. Becker" This change removes the dependency on /dev/zero with generate_zero_bytes appending NUL values to blocks generating wrong signatures for test cases. Signed-off-by: Randall S. Becker --- t/t5318-commit-graph.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index 16d10ebce..65c1f45b0 100755 --- a/t/t5318-commit-graph.sh +++ b/t/t5318-commit-graph.sh @@ -383,7 +383,7 @@ corrupt_graph_and_verify() { cp $objdir/info/commit-graph commit-graph-backup && printf "$data" | dd of="$objdir/info/commit-graph" bs=1 seek="$pos" conv=notrunc && dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=0 && - dd if=/dev/zero of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=$(($orig_size - $zero_pos)) && + generate_zero_bytes $(($orig_size - $zero_pos)) >> "$objdir/info/commit-graph" && test_must_fail git commit-graph verify 2>test_err && grep -v "^+" test_err >err && test_i18ngrep "$grepstr" err -- 2.12.3
[Patch v1 1/3] test-lib-functions.sh: add generate_zero_bytes function
From: "Randall S. Becker" t5318 and t5562 used /dev/zero, which is not portable. This function provides both a fixed block of NUL bytes and an infinite stream of NULs. Signed-off-by: Randall S. Becker --- t/test-lib-functions.sh | 13 + 1 file changed, 13 insertions(+) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 92cf8f812..bbf68712c 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -116,6 +116,19 @@ remove_cr () { tr '\015' Q | sed -e 's/Q$//' } +# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes). +# If $1 is 'infinity', output forever or until the receiving pipe stops reading, +# whichever comes first. +generate_zero_bytes () { + perl -e 'if ($ARGV[0] == "infinity") { + while (-1) { + print "\0" + } + } else { + print "\0" x $ARGV[0] + }' "$@" +} + # In some bourne shell implementations, the "unset" builtin returns # nonzero status when a variable to be unset was not set in the first # place. -- 2.12.3
[Patch v1 3/3] t5562: replace /dev/zero with a pipe from generate_zero_bytes
From: "Randall S. Becker" This change removes the dependency on /dev/zero with an equivalent pipe of deliberately NUL bytes. This allows tests to proceed where /dev/zero does not exist. Signed-off-by: Randall S. Becker --- t/t5562-http-backend-content-length.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh index 90d890d02..bbadde2c6 100755 --- a/t/t5562-http-backend-content-length.sh +++ b/t/t5562-http-backend-content-length.sh @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' ' test_expect_success 'CONTENT_LENGTH overflow ssite_t' ' NOT_FIT_IN_SSIZE=$(ssize_b100dots) && - env \ + generate_zero_bytes infinity | env \ CONTENT_TYPE=application/x-git-upload-pack-request \ QUERY_STRING=/repo.git/git-upload-pack \ PATH_TRANSLATED="$PWD"/.git/git-upload-pack \ GIT_HTTP_EXPORT_ALL=TRUE \ REQUEST_METHOD=POST \ CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \ - git http-backend /dev/null 2>err && + git http-backend >/dev/null 2>err && grep "fatal:.*CONTENT_LENGTH" err ' -- 2.12.3
[Patch v1 0/3] 2.21.0-rc0 test fixes resulting from use of /dev/zero
From: "Randall S. Becker" This is a candidate packages of fixes to remove dependence on /dev/zero and replaces it with the generate_zero_bytes function in test-lib-functions.sh Randall S. Becker (3): test-lib-functions.sh: add generate_zero_bytes function t5318: replace use of /dev/zero with generate_zero_bytes t5562: replace /dev/zero with a pipe from generate_zero_bytes t/t5318-commit-graph.sh| 2 +- t/t5562-http-backend-content-length.sh | 4 ++-- t/test-lib-functions.sh| 13 + 3 files changed, 16 insertions(+), 3 deletions(-) -- 2.12.3
RE: [Possible Breakage] t1308 - Bad return value from test-tool
On February 8, 2019 23:24, Jeff King wrote: > On Fri, Feb 08, 2019 at 04:42:19PM -0500, Randall S. Becker wrote: > > > t1308 has me perplexed - this is an old breakage on the NonStop > > platform, that I have just gotten around to checking with the new bash > > version we have. When running sub-test 23, the following was reported: > > > > Value not found for "foo.bar" > > test_expect_code: command exited with 1, we wanted 2 test-tool config > > configset_get_value foo.bar a-directory > > > > However, when I looked inside t/helper/test-config.c, every path > > reporting "Value not found" has a goto exit1 not exit2. It seems, from > > the code, that the test is actually incorrect and should be expecting > > 1 not 2, and that it is working properly on NonStop (but the test fails as a > result). > > We're expecting it to report an error reading the directory, not "value not > found". Which would yield code 2. > > It sounds like you might need to set FREAD_READS_DIRECTORIES in your > config.mak. Setting FREAD_READS_DIRECTORIES = UnfortunatelyYes still results in Value not found for "foo.bar" test_expect_code: command exited with 1, we wanted 2 test-tool config configset_get_value foo.bar a-directory not ok 23 - proper error on directory "files" # # echo "Error (-1) reading configuration file a-directory." >expect && # mkdir a-directory && # test_expect_code 2 test-tool config configset_get_value foo.bar a-directory 2>output && # grep "^warning:" output && # grep "^Error" output >actual && # test_cmp expect actual # I don't think that helped. While fopen can open a directory, fread does not return any data in this platform. readdir or nftw/ftw are pretty much the only options. However, the code still goes down the goto exit1 path in this situation. Perplexed, Randall
[Fix v2] config.mak.uname: move location of bash on NonStop to CoreUtils
From: "Randall S. Becker" The default bash is now officially in /usr/coreutils/bin instead of in /usr/local/bin. This version of bash is more stable and recommended for all use as of the J06.22 and L18.02 operating system revision levels. This new version provides more stability of test results. Signed-off-by: Randall S. Becker --- config.mak.uname | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/config.mak.uname b/config.mak.uname index 786bb2f91..6bd67eb86 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -507,9 +507,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL) # RFE 10-120912-4693 submitted to HP NonStop development. NO_SETITIMER = UnfortunatelyYes SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin - SHELL_PATH = /usr/local/bin/bash - # as of H06.25/J06.14, we might better use this - #SHELL_PATH = /usr/coreutils/bin/bash + SHELL_PATH = /usr/coreutils/bin/bash endif ifneq (,$(findstring MINGW,$(uname_S))) pathsep = ; -- 2.12.3
[Fix v1] config.mak.uname: move location of bash on NonStop to CoreUtils
From: "Randall S. Becker" The default bash is now officially in /usr/coreutils/bin instead of in /usr/local/bin. This version of bash is more stable and recommended for all use as of the J06.22 and L18.02 operating system revision levels. This new version provides more stability of test results. --- config.mak.uname | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/config.mak.uname b/config.mak.uname index 786bb2f91..6bd67eb86 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -507,9 +507,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL) # RFE 10-120912-4693 submitted to HP NonStop development. NO_SETITIMER = UnfortunatelyYes SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin - SHELL_PATH = /usr/local/bin/bash - # as of H06.25/J06.14, we might better use this - #SHELL_PATH = /usr/coreutils/bin/bash + SHELL_PATH = /usr/coreutils/bin/bash endif ifneq (,$(findstring MINGW,$(uname_S))) pathsep = ; -- 2.12.3
RE: [Fix v2] t5562: remove dependency on /dev/zero
On February 8, 2019 18:39, Junio C Hamano wrote: > randall.s.bec...@rogers.com writes: > > Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero > > with yes and a translation of its result to a stream of NULL. This is > > a more portable solution. > > > > Signed-off-by: Randall S. Becker > > --- > > t/t5562-http-backend-content-length.sh | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/t/t5562-http-backend-content-length.sh > > b/t/t5562-http-backend-content-length.sh > > index 90d890d02..b8d1913e5 100755 > > --- a/t/t5562-http-backend-content-length.sh > > +++ b/t/t5562-http-backend-content-length.sh > > @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' ' > > > > test_expect_success 'CONTENT_LENGTH overflow ssite_t' ' > > NOT_FIT_IN_SSIZE=$(ssize_b100dots) && > > - env \ > > + yes | tr "y" "\\0" | env \ > > I do not quite get this use of tr. The original feeds a stream of NULs out of > /dev/zero to the command; the yes-to-tr pipe instead feeds a stream of > alternating NUL and LF. That's why we're going to go with a generate_zero_bytes function per Peff. I'm working on a more comprehensive patch covering t5562, t5318, and test-lib-functions.sh that will (hopefully) be satisfactory and remove the dependency on /dev/zero and fixes the related new breakages in 2.21.0-rc0. The test case in t5318 is specific about wanting zero bytes although the test is just intending to generate a corrupt block that generates a different hash, so yes 'yes' might be sufficient, but I don't like randomness myself if we're taking two different tests being involved. My current intent is to add to test-lib-functions.sh, a method of generalizing blocks of zeros to a pipe: +# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes). +# If $1 is < 0, output forever or until the receiving pipe stops reading, whichever comes first. + generate_zero_bytes () { + perl -e ' if ($ARGV[0] < 0) { while (-1) { print "\0" } } else { print "\0" x $ARGV[0] }' "$@" + } And then fit that into the two tests, then submit as a patch. > Does the actual bytes fed to the consumer make any difference? If not, > perhaps we can use 'yes' as-is? > > > CONTENT_TYPE=application/x-git-upload-pack-request \ > > QUERY_STRING=/repo.git/git-upload-pack \ > > PATH_TRANSLATED="$PWD"/.git/git-upload-pack \ > > GIT_HTTP_EXPORT_ALL=TRUE \ > > REQUEST_METHOD=POST \ > > CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \ > > - git http-backend /dev/null 2>err && > > + git http-backend >/dev/null 2>err && > > grep "fatal:.*CONTENT_LENGTH" err > > ' Regards, Randall
RE: [Breakage] Git v2.21.0-rc0 - t5318 (NonStop)
On February 9, 2019 3:40, Johannes Sixt wrote: > Am 09.02.19 um 05:24 schrieb Jeff King: > > On Fri, Feb 08, 2019 at 05:53:53PM -0500, Randall S. Becker wrote: > > > >>> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index > >>> 92cf8f812c..4afab14431 100644 > >>> --- a/t/test-lib-functions.sh > >>> +++ b/t/test-lib-functions.sh > >>> @@ -1302,3 +1302,8 @@ test_set_port () { > >>> port=$(($port + ${GIT_TEST_STRESS_JOB_NR:-0})) > >>> eval $var=$port > >>> } > >>> + > >>> +# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes). > >>> +gen_zero_bytes () { > >>> + perl -e 'print "\0" x $ARGV[0]' "$@" > >>> +} > >> > >> This function does work on platform, so it's good. > > > > Great. Since it sounds like you're preparing some patches to deal with > > /dev/zero elsewhere, do you want to wrap it up in a patch as part of > > that? > > Please do not use yes to generate an infinite amount of bytes. Our > implementation of yes() in test-lib.sh generates only 99 lines. > > Perhaps do this. > > - 8< - > Subject: [PATCH] t5318: avoid /dev/zero > > Some platforms do not offer /dev/zero. Use printf and tr to generate a > certain amount of NUL bytes. > > Reported-by: Randall S. Becker > Signed-off-by: Johannes Sixt > --- > t/t5318-commit-graph.sh | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index > 16d10ebce8..04d394274f 100755 > --- a/t/t5318-commit-graph.sh > +++ b/t/t5318-commit-graph.sh > @@ -383,7 +383,8 @@ corrupt_graph_and_verify() { > cp $objdir/info/commit-graph commit-graph-backup && > printf "$data" | dd of="$objdir/info/commit-graph" bs=1 > seek="$pos" conv=notrunc && > dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=0 > && > - dd if=/dev/zero of="$objdir/info/commit-graph" bs=1 > seek="$zero_pos" count=$(($orig_size - $zero_pos)) && > + printf "%0*d" $(($orig_size - $zero_pos)) 0 | tr 0 '\0' | > + dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" > && > test_must_fail git commit-graph verify 2>test_err && > grep -v "^+" test_err >err && > test_i18ngrep "$grepstr" err > -- > 2.20.1.86.gb0de946387 This would be fine with me. I'm going to prepare an alternative and let the committers decide.
RE: [Breakage] Git v2.21.0-rc0 - t5318 (NonStop)
On February 8, 2019 23:25, Jeff King wrote: > On Fri, Feb 08, 2019 at 05:53:53PM -0500, Randall S. Becker wrote: > > > > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index > > > 92cf8f812c..4afab14431 100644 > > > --- a/t/test-lib-functions.sh > > > +++ b/t/test-lib-functions.sh > > > @@ -1302,3 +1302,8 @@ test_set_port () { > > > port=$(($port + ${GIT_TEST_STRESS_JOB_NR:-0})) > > > eval $var=$port > > > } > > > + > > > +# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes). > > > +gen_zero_bytes () { > > > + perl -e 'print "\0" x $ARGV[0]' "$@" > > > +} > > > > This function does work on platform, so it's good. > > Great. Since it sounds like you're preparing some patches to deal with > /dev/zero elsewhere, do you want to wrap it up in a patch as part of that? That's the plan. 😊 We're currently running a full test, so that takes time (32 hours on this box). Cheers, Randall
RE: [Breakage] Git v2.21.0-rc0 - t5318 (NonStop)
On February 8, 2019 17:35, Jeff King wrote: > On Fri, Feb 08, 2019 at 05:12:43PM -0500, Randall S. Becker wrote: > > On February 8, 2019 17:07, brian m. carlson wrote: > > > On Fri, Feb 08, 2019 at 02:31:57PM -0500, Jeff King wrote: > > > > > It is available AFAIK on Linux, POSIX, and Windows under Cygwin. > > > > > That's more than /dev/zero has anyway. I have the patch ready if > > > > > you want it. > > > > > > > > Is it POSIX? Certainly truncate() is, but I didn't think the > > > > command-line tool was. If it really is available everywhere, then > > > > yeah, I'd be fine with it. > > > > > > It's not. POSIX doesn't specify the command, and macOS lacks it, I > believe. > > > > I'm happy to modify the test (it is in one spot), to make a decision based > on: > > a) whether /dev/zero exists > > b) whether the system is a NonStop > > c) something else > > > > What would you all prefer? It doesn't matter to me one way or another, > > as long as I can get the dependency to /dev/zero removed so tests will > > run here. > > For the case in t5318, I think we can just put the NULs in a file. Does this > work on your platform? Yes, should work just fine. > > --- > diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index > 16d10ebce8..6d0ccc7eba 100755 > --- a/t/t5318-commit-graph.sh > +++ b/t/t5318-commit-graph.sh > @@ -383,7 +383,8 @@ corrupt_graph_and_verify() { > cp $objdir/info/commit-graph commit-graph-backup && > printf "$data" | dd of="$objdir/info/commit-graph" bs=1 > seek="$pos" conv=notrunc && > dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=0 > && > - dd if=/dev/zero of="$objdir/info/commit-graph" bs=1 > seek="$zero_pos" count=$(($orig_size - $zero_pos)) && > + gen_zero_bytes $(($orig_size - $zero_pos)) >zeroes && > + dd if=zeroes of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" > && > test_must_fail git commit-graph verify 2>test_err && > grep -v "^+" test_err >err && > test_i18ngrep "$grepstr" err > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index > 92cf8f812c..4afab14431 100644 > --- a/t/test-lib-functions.sh > +++ b/t/test-lib-functions.sh > @@ -1302,3 +1302,8 @@ test_set_port () { > port=$(($port + ${GIT_TEST_STRESS_JOB_NR:-0})) > eval $var=$port > } > + > +# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes). > +gen_zero_bytes () { > + perl -e 'print "\0" x $ARGV[0]' "$@" > +} This function does work on platform, so it's good. > For the others that need infinite zeroes, I think using "yes" makes more > sense, though we could also teach this function to accept an "infinity" > parameter. You could be sneaky about it, I suppose +# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes). + gen_zero_bytes () { + if [ $1 -eq -1 ]; then + yes | tr 'y' '\0' + else + perl -e 'print "\0" x $ARGV[0]' "$@" + } Or something alone those lines. It's not even slightly elegant, though. It would be better inside perl, so just +# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes). If $1 is < 0, output forever. + gen_zero_bytes () { + perl -e ' if ($ARGV[0] < 0) { while (-1) { print "\0" } } else { print "\0" x $ARGV[0] }' "$@" + } Cheers, Randall
RE: [Breakage] Git v2.21.0-rc0 - t5318 (NonStop)
On February 8, 2019 17:19, brian m. carlson wrote: > On Fri, Feb 08, 2019 at 05:12:43PM -0500, Randall S. Becker wrote: > > I'm happy to modify the test (it is in one spot), to make a decision based > on: > > a) whether /dev/zero exists > > b) whether the system is a NonStop > > c) something else > > > > What would you all prefer? It doesn't matter to me one way or another, as > long as I can get the dependency to /dev/zero removed so tests will run here. > > My preference is that we wrap the yes/tr invocation into a function (maybe > "infinite_nul") and use that where we currently require /dev/zero. That's simple enough to do in test-lib-functions.sh for the situation where yes|tr is being piped, but that's t5562. In t5318 we have dd if=/dev/zero, and that's where truncate would need to work. The requirements of that test seem very specific to me and not that generalizable. I'm just dealing with new breakages on the platform.
RE: [Breakage] Git v2.21.0-rc0 - t5318 (NonStop)
On February 8, 2019 17:07, brian m. carlson wrote: > On Fri, Feb 08, 2019 at 02:31:57PM -0500, Jeff King wrote: > > > It is available AFAIK on Linux, POSIX, and Windows under Cygwin. > > > That's more than /dev/zero has anyway. I have the patch ready if you > > > want it. > > > > Is it POSIX? Certainly truncate() is, but I didn't think the > > command-line tool was. If it really is available everywhere, then > > yeah, I'd be fine with it. > > It's not. POSIX doesn't specify the command, and macOS lacks it, I believe. I'm happy to modify the test (it is in one spot), to make a decision based on: a) whether /dev/zero exists b) whether the system is a NonStop c) something else What would you all prefer? It doesn't matter to me one way or another, as long as I can get the dependency to /dev/zero removed so tests will run here. Thanks, Randall
[Fix v2] t5562: remove dependency on /dev/zero
From: "Randall S. Becker" Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero with yes and a translation of its result to a stream of NULL. This is a more portable solution. Signed-off-by: Randall S. Becker --- t/t5562-http-backend-content-length.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh index 90d890d02..b8d1913e5 100755 --- a/t/t5562-http-backend-content-length.sh +++ b/t/t5562-http-backend-content-length.sh @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' ' test_expect_success 'CONTENT_LENGTH overflow ssite_t' ' NOT_FIT_IN_SSIZE=$(ssize_b100dots) && - env \ + yes | tr "y" "\\0" | env \ CONTENT_TYPE=application/x-git-upload-pack-request \ QUERY_STRING=/repo.git/git-upload-pack \ PATH_TRANSLATED="$PWD"/.git/git-upload-pack \ GIT_HTTP_EXPORT_ALL=TRUE \ REQUEST_METHOD=POST \ CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \ - git http-backend /dev/null 2>err && + git http-backend >/dev/null 2>err && grep "fatal:.*CONTENT_LENGTH" err ' -- 2.12.3
RE: [Fix v1] t5562: remove dependency on /dev/zero
Please disregard this.. I left garbage inside. > -Original Message- > From: git-ow...@vger.kernel.org On Behalf > Of randall.s.bec...@rogers.com > Sent: February 8, 2019 16:59 > To: git@vger.kernel.org > Cc: Randall S. Becker > Subject: [Fix v1] t5562: remove dependency on /dev/zero > > From: "Randall S. Becker" > > Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero > with yes and a translation of its result to a stream of NULL. This is a more > portable solution. > > Signed-off-by: Randall S. Becker > --- > t/t5562-http-backend-content-length.sh | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend- > content-length.sh > index 90d890d02..63f82cca2 100755 > --- a/t/t5562-http-backend-content-length.sh > +++ b/t/t5562-http-backend-content-length.sh > @@ -143,14 +143,16 @@ test_expect_success GZIP 'push gzipped empty' ' > > test_expect_success 'CONTENT_LENGTH overflow ssite_t' ' > NOT_FIT_IN_SSIZE=$(ssize_b100dots) && > - env \ > + yes | tr "y" "\\0" | env \ > CONTENT_TYPE=application/x-git-upload-pack-request \ > QUERY_STRING=/repo.git/git-upload-pack \ > PATH_TRANSLATED="$PWD"/.git/git-upload-pack \ > GIT_HTTP_EXPORT_ALL=TRUE \ > REQUEST_METHOD=POST \ > CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \ > - git http-backend /dev/null 2>err && > + git http-backend >/dev/null 2>err && > + echo "Err is" && > + cat err && > grep "fatal:.*CONTENT_LENGTH" err > ' > > -- > 2.12.3
[Fix v1] t5562: remove dependency on /dev/zero
From: "Randall S. Becker" Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero with yes and a translation of its result to a stream of NULL. This is a more portable solution. Signed-off-by: Randall S. Becker --- t/t5562-http-backend-content-length.sh | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh index 90d890d02..63f82cca2 100755 --- a/t/t5562-http-backend-content-length.sh +++ b/t/t5562-http-backend-content-length.sh @@ -143,14 +143,16 @@ test_expect_success GZIP 'push gzipped empty' ' test_expect_success 'CONTENT_LENGTH overflow ssite_t' ' NOT_FIT_IN_SSIZE=$(ssize_b100dots) && - env \ + yes | tr "y" "\\0" | env \ CONTENT_TYPE=application/x-git-upload-pack-request \ QUERY_STRING=/repo.git/git-upload-pack \ PATH_TRANSLATED="$PWD"/.git/git-upload-pack \ GIT_HTTP_EXPORT_ALL=TRUE \ REQUEST_METHOD=POST \ CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \ - git http-backend /dev/null 2>err && + git http-backend >/dev/null 2>err && + echo "Err is" && + cat err && grep "fatal:.*CONTENT_LENGTH" err ' -- 2.12.3
RE: [Breakage] Git v2.21.0-rc0 - t5318 (NonStop)
On February 8, 2019 16:01, Jeff King wrote: > On Fri, Feb 08, 2019 at 03:38:05PM -0500, Randall S. Becker wrote: > > > > Exactly (if we even care about them being NULs; otherwise, we can > > > omit the "tr" invocation). > > > > I'm a bit perplexed about this... Obviously added some debugging info, > > but why we're getting No REQUEST_METHOD is perplexing. Is this a lack > > of an apache2 instance? > > No, this shouldn't be using apache at all. But... > > > expecting success: > > NOT_FIT_IN_SSIZE=$(ssize_b100dots) && > > env \ > > CONTENT_TYPE=application/x-git-upload-pack-request \ > > QUERY_STRING=/repo.git/git-upload-pack \ > > PATH_TRANSLATED="$PWD"/.git/git-upload-pack \ > > GIT_HTTP_EXPORT_ALL=TRUE \ > > REQUEST_METHOD=POST \ > > CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \ > > yes | tr "y" "\\0" | git http-backend 2>err && > > echo "Err is" && > > cat err && > > grep "fatal:.*CONTENT_LENGTH" err > > > > Status: 500 Internal Server Error > > Expires: Fri, 01 Jan 1980 00:00:00 GMT > > Pragma: no-cache > > Cache-Control: no-cache, max-age=0, must-revalidate > > > > Err is > > fatal: No REQUEST_METHOD from server > > not ok 15 - CONTENT_LENGTH overflow ssite_t > > The problem is that you're setting the environment now for "yes". You'd > need: > > yes | tr "y" "\\0" | env \ > CONTENT_TYPE=... \ > REQUEST_METHOD=POST \ > ...etc... > git http-backend Aw crap. I feel really silly now. Thanks. Will advise.
[Possible Breakage] t1308 - Bad return value from test-tool
t1308 has me perplexed - this is an old breakage on the NonStop platform, that I have just gotten around to checking with the new bash version we have. When running sub-test 23, the following was reported: Value not found for "foo.bar" test_expect_code: command exited with 1, we wanted 2 test-tool config configset_get_value foo.bar a-directory However, when I looked inside t/helper/test-config.c, every path reporting "Value not found" has a goto exit1 not exit2. It seems, from the code, that the test is actually incorrect and should be expecting 1 not 2, and that it is working properly on NonStop (but the test fails as a result). What is perplexing is that someone else should be seeing this. The test-config.c code is about 5 years old, but the test was modified a year or two ago. What am I missing here? Cheers, Randall
RE: [Breakage] Git v2.21.0-rc0 - t5318 (NonStop)
> -Original Message- > From: Jeff King > Sent: February 8, 2019 14:32 > To: Randall S. Becker > Cc: 'Junio C Hamano' ; git@vger.kernel.org; 'Linux > Kernel' ; git-packag...@googlegroups.com > Subject: Re: [Breakage] Git v2.21.0-rc0 - t5318 (NonStop) > > On Fri, Feb 08, 2019 at 02:26:17PM -0500, Randall S. Becker wrote: > > > > > For this, we could use truncate -s count file instead of dd to get > > > > a fixed size file of nulls. This would remove the need for > > > > /dev/zero in > > > > t5318 (the patch below probably will wrap badly in my mailer so I > > > > can submit a real patch separately. > > > > > > I don't think "truncate" is portable, though. > > > > It is available AFAIK on Linux, POSIX, and Windows under Cygwin. > > That's more than /dev/zero has anyway. I have the patch ready if you > > want it. > > Is it POSIX? Certainly truncate() is, but I didn't think the command-line tool > was. If it really is available everywhere, then yeah, I'd be fine with it. > > > > > > Other cases don't seem to actually care that they're getting > > > > > NULs, and are just redirecting stdin from /dev/zero to get an > > > > > infinite amount of input. They could probably use "yes" for that. > > > > > > > > What about reading from /dev/null? > > > > > > That would yield zero bytes, not an infinite number of them. > > > > So something like: yes | tr 'y' '\0' | stuff? > > Exactly (if we even care about them being NULs; otherwise, we can omit the > "tr" invocation). I'm a bit perplexed about this... Obviously added some debugging info, but why we're getting No REQUEST_METHOD is perplexing. Is this a lack of an apache2 instance? expecting success: NOT_FIT_IN_SSIZE=$(ssize_b100dots) && env \ CONTENT_TYPE=application/x-git-upload-pack-request \ QUERY_STRING=/repo.git/git-upload-pack \ PATH_TRANSLATED="$PWD"/.git/git-upload-pack \ GIT_HTTP_EXPORT_ALL=TRUE \ REQUEST_METHOD=POST \ CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \ yes | tr "y" "\\0" | git http-backend 2>err && echo "Err is" && cat err && grep "fatal:.*CONTENT_LENGTH" err Status: 500 Internal Server Error Expires: Fri, 01 Jan 1980 00:00:00 GMT Pragma: no-cache Cache-Control: no-cache, max-age=0, must-revalidate Err is fatal: No REQUEST_METHOD from server not ok 15 - CONTENT_LENGTH overflow ssite_t Cheers, Randall
[Proposed Fix v1] t5318: replace dd if=/dev/zero with truncate
From: "Randall S. Becker" The corrupt_graph_and_verify method has been modified to use truncate instead of the /dev/zero pseudo-device, which breaks on platforms where the pseudo-device does not exist. truncate extends files to a specified length filling with nulls, providing a similar function to the use of dd in this case. Signed-off-by: Randall S. Becker --- t/t5318-commit-graph.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index 16d10ebce..d627ffe09 100755 --- a/t/t5318-commit-graph.sh +++ b/t/t5318-commit-graph.sh @@ -383,7 +383,7 @@ corrupt_graph_and_verify() { cp $objdir/info/commit-graph commit-graph-backup && printf "$data" | dd of="$objdir/info/commit-graph" bs=1 seek="$pos" conv=notrunc && dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=0 && - dd if=/dev/zero of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=$(($orig_size - $zero_pos)) && + truncate -s $orig_size "$objdir/info/commit-graph" && test_must_fail git commit-graph verify 2>test_err && grep -v "^+" test_err >err && test_i18ngrep "$grepstr" err -- 2.12.3
RE: [Breakage] Git v2.21.0-rc0 - t5318 (NonStop)
On February 8, 2019 14:15, Jeff King wrote: > On Fri, Feb 08, 2019 at 01:47:04PM -0500, Randall S. Becker wrote: > > > > Though I suspect we may be able to just find a solution that works > > > everywhere, without having two different implementations. If we know > > > we need $count bytes for dd, we could probably just generate a file > > > with that many NULs in it. > > > > For this, we could use truncate -s count file instead of dd to get a > > fixed size file of nulls. This would remove the need for /dev/zero in > > t5318 (the patch below probably will wrap badly in my mailer so I can > > submit a real patch separately. > > I don't think "truncate" is portable, though. It is available AFAIK on Linux, POSIX, and Windows under Cygwin. That's more than /dev/zero has anyway. I have the patch ready if you want it. > > > Other cases don't seem to actually care that they're getting NULs, > > > and are just redirecting stdin from /dev/zero to get an infinite > > > amount of input. They could probably use "yes" for that. > > > > What about reading from /dev/null? > > That would yield zero bytes, not an infinite number of them. So something like: yes | tr 'y' '\0' | stuff?
RE: [Breakage] Git v2.21.0-rc0 - t5318 (NonStop)
On February 8, 2019 13:03, Jeff King wrote: > To: Randall S. Becker > Cc: 'Junio C Hamano' ; git@vger.kernel.org; 'Linux > Kernel' ; git-packag...@googlegroups.com > Subject: Re: [Breakage] Git v2.21.0-rc0 - t5318 (NonStop) > > On Fri, Feb 08, 2019 at 12:49:59PM -0500, Randall S. Becker wrote: > > > > We did discuss this at the time of the patch, but it seems we > > > already use /dev/zero in a bunch of places: > > > > > > > > > https://public-inbox.org/git/xmqqbm57rkg5.fsf@gitster-ct.c.googlers. > > > com/ > > > > > > Were you just skipping the other tests before? > > > > I did not catch the implications of the review at the time - my bad. We > were not intentionally skipping the tests. It looks like some are > automatically > skipped. t4153 automatically skips (missing TTY), and t5562 fails also but for > a different reason (hang - we don't have apache2 to serve up http content). > > > > Would you object to something like this: > > > > if [ ! -e /dev/zero ]; then > > # use shred or some other mechanism (still trying to figure out a > > solution) else > > # existing dd > > fi > > That's fine, as long as it's wrapped up in a function in order to keep the > tests > readable. > > Though I suspect we may be able to just find a solution that works > everywhere, without having two different implementations. If we know we > need $count bytes for dd, we could probably just generate a file with that > many NULs in it. For this, we could use truncate -s count file instead of dd to get a fixed size file of nulls. This would remove the need for /dev/zero in t5318 (the patch below probably will wrap badly in my mailer so I can submit a real patch separately. @@ -383,7 +383,7 @@ corrupt_graph_and_verify() { cp $objdir/info/commit-graph commit-graph-backup && printf "$data" | dd of="$objdir/info/commit-graph" bs=1 seek="$pos" conv=notrunc && dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=0 && - dd if=/dev/zero of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=$(($orig_size - $zero_pos)) && + truncate -s $orig_size "$objdir/info/commit-graph" && test_must_fail git commit-graph verify 2>test_err && grep -v "^+" test_err >err && test_i18ngrep "$grepstr" err This passes on my platform. > Other cases don't seem to actually care that they're getting NULs, and are > just redirecting stdin from /dev/zero to get an infinite amount of input. They > could probably use "yes" for that. What about reading from /dev/null? Regards, Randall
RE: [Breakage] Git v2.21.0-rc0 - t5318 (NonStop)
On February 8, 2019 11:51, Jeff King wrote: > On Fri, Feb 08, 2019 at 06:08:33AM -0500, Randall S. Becker wrote: > > > t5318 is rather problematic and I have no good way to fix this. There > > is no /dev/zero on the platform, and the corrupt_graph_and_verify > > hard-codes if=/dev/zero, which is a linux-specific pseudo device. > > Please provide a more platform independent way of testing this > > feature. Pretty much all subtests from 46 onward fail as a result. > > We did discuss this at the time of the patch, but it seems we already use > /dev/zero in a bunch of places: > > https://public-inbox.org/git/xmqqbm57rkg5@gitster-ct.c.googlers.com/ > > Were you just skipping the other tests before? I did not catch the implications of the review at the time - my bad. We were not intentionally skipping the tests. It looks like some are automatically skipped. t4153 automatically skips (missing TTY), and t5562 fails also but for a different reason (hang - we don't have apache2 to serve up http content). Would you object to something like this: if [ ! -e /dev/zero ]; then # use shred or some other mechanism (still trying to figure out a solution) else # existing dd fi
[Hang] t5562 subtest 8 on NonStop
Hi All, We have suddenly encountered a hung git-http-backend in t5562 in the NonStop port. This is a new problem not seen before on the platform, surprisingly. I am wondering whether this is a result of not actually having an apache2 server on-board. Is that a possibility and can that sub-test be bypassed if no apache2 is detected? We also had subtest 15 and I am investigating, but it may depend on 8's data (I had to kill the git-http-backend process, so maybe that's why). Thanks, Randall
RE: [Breakage] Git v2.21.0-rc0 - t5403 (NonStop)
On February 8, 2019 6:10, SZEDER Gábor > On Fri, Feb 08, 2019 at 05:48:27AM -0500, Randall S. Becker wrote: > > We have a few new breakages on the NonStop port in 2.21.0-rc0. The first > is in t5403, as below: > > > > /home/git/git/t/trash > > directory.t5403-post-checkout-hook/clone3/.git/hooks/post-checkout: > > line 2: $GIT_DIR/post-checkout.args: ambiguous redirect not ok 8 - post- > checkout hook is triggered by clone # > > # mkdir -p templates/hooks && > > # write_script templates/hooks/post-checkout <<-\EOF && > > # echo "$@" >$GIT_DIR/post-checkout.args > > # EOF > > # git clone --template=templates . clone3 && > > # test -f clone3/.git/post-checkout.args > > # > > > > The post-checkout hook is: > > #!/usr/local/bin/bash > > echo "$@" >$GIT_DIR/post-checkout.args > > > > This looks like it is a "bash thing" and $GIT_DIR might have to be in > > quotes, and is not be specific to the platform. If I replace > > > > echo "$@" >$GIT_DIR/post-checkout.args > > > > with > > > > echo "$@" >"$GIT_DIR/post-checkout.args" > > > > The test passes. > > Wow, this is the second time this "redirection to a filename with spaces > under Bash" issue pops up today, see the other one here: > > https://public-inbox.org/git/20190208031746.22683-2- > t...@pobox.com/T/#u > > In short, Bash (when invoked as bash) doesn't conform to POSIX in this > respect; for a (too detailed) explanation see: > > https://public-inbox.org/git/20180926121107.GH27036@localhost/ > > Even our CodingGuidelines suggest the use of quotes around the > redirection's target. > > > I wonder I should provide this patch or whether the author would like to do > so. > > Well, since you didn't Cc the author, he might very well overlook this issue, > so I think you should ;) + Nguyen and Orgad Thanks. Fix has been submitted as a patch. This is definitely a common one. 😊
[Fix v1] t5403: correct bash ambiguous redirect error in subtest 8 by quoting $GIT_DIR
From: "Randall S. Becker" The embedded blanks in the full path of the test git repository cased bash to generate an ambugious redirect error. Signed-off-by: Randall S. Becker --- t/t5403-post-checkout-hook.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh index a539ffc08..a39b3b5c7 100755 --- a/t/t5403-post-checkout-hook.sh +++ b/t/t5403-post-checkout-hook.sh @@ -67,7 +67,7 @@ test_expect_success 'post-checkout is triggered on rebase with fast-forward' ' test_expect_success 'post-checkout hook is triggered by clone' ' mkdir -p templates/hooks && write_script templates/hooks/post-checkout <<-\EOF && - echo "$@" >$GIT_DIR/post-checkout.args + echo "$@" >"$GIT_DIR/post-checkout.args" EOF git clone --template=templates . clone3 && test -f clone3/.git/post-checkout.args -- 2.12.3
[Breakage] Git v2.21.0-rc0 - t5318 (NonStop)
Hi All, t5318 is rather problematic and I have no good way to fix this. There is no /dev/zero on the platform, and the corrupt_graph_and_verify hard-codes if=/dev/zero, which is a linux-specific pseudo device. Please provide a more platform independent way of testing this feature. Pretty much all subtests from 46 onward fail as a result. expecting success: corrupt_graph_and_verify 0 "\0" \ "graph signature" 1+0 records in 1+0 records out 1 byte copied, 0.002248 s, 0.4 kB/s 0+0 records in 0+0 records out 0 bytes copied, 0.001815 s, 0.0 kB/s dd: failed to open '/dev/zero': No such file or directory not ok 46 - detect bad signature # # corrupt_graph_and_verify 0 "\0" \ # "graph signature" # Regards, Randall
RE: [Breakage] Git v2.21.0-rc0 - t5403 (NonStop)
On February 8, 2019 5:56, Johannes Schindelin wrote: > To: Randall S. Becker > Cc: 'Junio C Hamano' ; git@vger.kernel.org; 'Linux > Kernel' ; git-packag...@googlegroups.com > Subject: Re: [Breakage] Git v2.21.0-rc0 - t5403 (NonStop) > > Hi Randall, > > On Fri, 8 Feb 2019, Randall S. Becker wrote: > > > This looks like it is a "bash thing" and $GIT_DIR might have to be in > > quotes, and is not be specific to the platform. If I replace > > > > echo "$@" >$GIT_DIR/post-checkout.args > > > > with > > > > echo "$@" >"$GIT_DIR/post-checkout.args" > > > > The test passes. I wonder I should provide this patch or whether the > > author would like to do so. > > It is the correct fix, you came up with it, why not simply provide a patch > yourself? Will do (later today)
[Breakage] Git v2.21.0-rc0 - t5403 (NonStop)
Hi All, We have a few new breakages on the NonStop port in 2.21.0-rc0. The first is in t5403, as below: /home/git/git/t/trash directory.t5403-post-checkout-hook/clone3/.git/hooks/post-checkout: line 2: $GIT_DIR/post-checkout.args: ambiguous redirect not ok 8 - post-checkout hook is triggered by clone # # mkdir -p templates/hooks && # write_script templates/hooks/post-checkout <<-\EOF && # echo "$@" >$GIT_DIR/post-checkout.args # EOF # git clone --template=templates . clone3 && # test -f clone3/.git/post-checkout.args # The post-checkout hook is: #!/usr/local/bin/bash echo "$@" >$GIT_DIR/post-checkout.args This looks like it is a "bash thing" and $GIT_DIR might have to be in quotes, and is not be specific to the platform. If I replace echo "$@" >$GIT_DIR/post-checkout.args with echo "$@" >"$GIT_DIR/post-checkout.args" The test passes. I wonder I should provide this patch or whether the author would like to do so. Thanks, Randall
RE: t0025 flakey?
On February 7, 2019 18:57, SZEDER Gábor wrote: > On Thu, Feb 07, 2019 at 11:58:08AM -0500, Randall S. Becker wrote: > > > The NonStop port has traditionally had issues with t0025, which we > > > tended to ignore because things did work. We wrote those off as bash > > > issues in > > > t0025 since they seemed to be corrected when we picked up a new bash > > > version about a year ago. I will keep monitoring this, particularly > > > when > > 2.21 > > > comes out. > > > > FYI: t0020-t0027 all passed on the NonStop port for 2.21.0-rc0 - so no > > issues for us on this one. > > Note that t0021 is very likely flaky on NonStop, too: > > https://public-inbox.org/git/2019040408.gc...@szeder.dev/T/#u We will keep a watch on it, thanks. t0021 has been stable on this platform for at least a year and passes for 2.21.0-rc0 as well as 2.20.0. Cheers, Randall
RE: t0025 flakey?
On February 6, 2019 13:01, I wrote: > On February 6, 2019 12:15, Torsten Bögershausen wrote: > > To: Johannes Schindelin > > Cc: SZEDER Gábor ; Jeff King ; > > git@vger.kernel.org > > Subject: Re: t0025 flakey? > > > > On Wed, Feb 06, 2019 at 02:52:53PM +0100, Johannes Schindelin wrote: > > > Hi Gábor, > > > > > > On Wed, 6 Feb 2019, SZEDER Gábor wrote: > > > > > > > On Wed, Feb 06, 2019 at 11:25:38AM +0100, Johannes Schindelin > wrote: > > > > > > > > > at first I thought that those intermittent test failures were > > > > > limited to Windows, but they are not: I can see it now in a > > > > > build on 32-bit Linux. > > > > > Full logs here: > > > > > > > > > > https://dev.azure.com/gitgitgadget/git/_build/results?buildId=10 > > > > > 32 &_a=summary&view=ms.vss-test-web.build-test-results-tab > > > > > > > > > > Excerpt from the failing test case: > > > > > > > > > > -- snip -- > > > > > not ok 2 - renormalize CRLF in repo expecting success: > > > > > echo "*.txt text=auto" >.gitattributes && > > > > > git add --renormalize "*.txt" && > > > > > cat >expect <<-\EOF && > > > > > i/lf w/crlf attr/text=auto CRLF.txt > > > > > i/lf w/lf attr/text=auto LF.txt > > > > > i/lf w/mixed attr/text=auto CRLF_mix_LF.txt > > > > > EOF > > > > > git ls-files --eol | > > > > > sed -e "s/ / /g" -e "s/ */ /g" | > > > > > sort >actual && > > > > > test_cmp expect actual > > > > > > > > > > + echo *.txt text=auto > > > > > + git add --renormalize *.txt > > > > > + cat > > > > > + sort > > > > > + sed -e s/ / /g -e s/ */ /g > > > > > + git ls-files --eol > > > > > + test_cmp expect actual > > > > > + diff -u expect actual > > > > > --- expect2019-02-06 09:39:42.080733629 + > > > > > +++ actual2019-02-06 09:39:42.088733629 + > > > > > @@ -1,3 +1,3 @@ > > > > > -i/lf w/crlf attr/text=auto CRLF.txt > > > > > +i/crlf w/crlf attr/text=auto CRLF.txt > > > > > i/lf w/lf attr/text=auto LF.txt -i/lf w/mixed attr/text=auto > > > > > CRLF_mix_LF.txt > > > > > +i/mixed w/mixed attr/text=auto CRLF_mix_LF.txt > > > > > error: last command exited with $?=1 > > > > > -- snap -- > > > > > > > > > > Any ideas? > > > > > > > > I reported this and Peff looked into it on the way to Git Merge, > > > > but not working solution yet. > > > > > > > > https://public-inbox.org/git/20190129225121.gd1...@sigill.intra.pe > > > > ff > > > > .net/T/#u > > > > > > Thank you! > > > Dscho > > > > I shortly looked into the pointers here - Is t0025 flaky after the fix from > Peff: > > > > [PATCH] add: use separate ADD_CACHE_RENORMALIZE flag > > > > Or has it always been shaky ? > > Does anybody know ? > > The NonStop port has traditionally had issues with t0025, which we tended > to ignore because things did work. We wrote those off as bash issues in > t0025 since they seemed to be corrected when we picked up a new bash > version about a year ago. I will keep monitoring this, particularly when 2.21 > comes out. FYI: t0020-t0027 all passed on the NonStop port for 2.21.0-rc0 - so no issues for us on this one. Cheers, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much.
RE: t0025 flakey?
On February 6, 2019 12:15, Torsten Bögershausen wrote: > To: Johannes Schindelin > Cc: SZEDER Gábor ; Jeff King ; > git@vger.kernel.org > Subject: Re: t0025 flakey? > > On Wed, Feb 06, 2019 at 02:52:53PM +0100, Johannes Schindelin wrote: > > Hi Gábor, > > > > On Wed, 6 Feb 2019, SZEDER Gábor wrote: > > > > > On Wed, Feb 06, 2019 at 11:25:38AM +0100, Johannes Schindelin wrote: > > > > > > > at first I thought that those intermittent test failures were > > > > limited to Windows, but they are not: I can see it now in a build > > > > on 32-bit Linux. > > > > Full logs here: > > > > > > > > https://dev.azure.com/gitgitgadget/git/_build/results?buildId=1032 > > > > &_a=summary&view=ms.vss-test-web.build-test-results-tab > > > > > > > > Excerpt from the failing test case: > > > > > > > > -- snip -- > > > > not ok 2 - renormalize CRLF in repo expecting success: > > > > echo "*.txt text=auto" >.gitattributes && > > > > git add --renormalize "*.txt" && > > > > cat >expect <<-\EOF && > > > > i/lf w/crlf attr/text=auto CRLF.txt > > > > i/lf w/lf attr/text=auto LF.txt > > > > i/lf w/mixed attr/text=auto CRLF_mix_LF.txt > > > > EOF > > > > git ls-files --eol | > > > > sed -e "s/ / /g" -e "s/ */ /g" | > > > > sort >actual && > > > > test_cmp expect actual > > > > > > > > + echo *.txt text=auto > > > > + git add --renormalize *.txt > > > > + cat > > > > + sort > > > > + sed -e s/ / /g -e s/ */ /g > > > > + git ls-files --eol > > > > + test_cmp expect actual > > > > + diff -u expect actual > > > > --- expect 2019-02-06 09:39:42.080733629 + > > > > +++ actual 2019-02-06 09:39:42.088733629 + > > > > @@ -1,3 +1,3 @@ > > > > -i/lf w/crlf attr/text=auto CRLF.txt > > > > +i/crlf w/crlf attr/text=auto CRLF.txt > > > > i/lf w/lf attr/text=auto LF.txt > > > > -i/lf w/mixed attr/text=auto CRLF_mix_LF.txt > > > > +i/mixed w/mixed attr/text=auto CRLF_mix_LF.txt > > > > error: last command exited with $?=1 > > > > -- snap -- > > > > > > > > Any ideas? > > > > > > I reported this and Peff looked into it on the way to Git Merge, but > > > not working solution yet. > > > > > > https://public-inbox.org/git/20190129225121.gd1...@sigill.intra.peff > > > .net/T/#u > > > > Thank you! > > Dscho > > I shortly looked into the pointers here - Is t0025 flaky after the fix from Peff: > > [PATCH] add: use separate ADD_CACHE_RENORMALIZE flag > > Or has it always been shaky ? > Does anybody know ? The NonStop port has traditionally had issues with t0025, which we tended to ignore because things did work. We wrote those off as bash issues in t0025 since they seemed to be corrected when we picked up a new bash version about a year ago. I will keep monitoring this, particularly when 2.21 comes out. Cheers, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much.
RE: [BUG]: git checkout on a new tag with current HEAD shows "head detached" at previous tag
On February 5, 2019 4:20, Dani Koretsky wrote: > I have 2 tags such as release-17.6.230 and release-17.6.220: > If I'm changing commits, all works as expected. > > If however both are pointing to the same commit, the output is as follows: > > git checkout release-17.6.220 > git status > HEAD detached at release-17.6.220 > > now if I run: > git checkout release-17.6.230 > git status > HEAD detached at release-17.6.220 > > Which is theoretically correct, but I'd expect after checking out a certain > tag > I'd be see that specific tag... > > Sorry if this is intended behavior, I couldn't find clear mention of this > behavior on the git checkout documentation online.. Please use git checkout -b release-17.6.220 What your commands above have done is resolved release-17.6.220 to a commit, then checked out that commit instead of creating a branch. Alternatively, use git checkout -b new-branch release-17.6.220 to name it something else. Regards, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much.
RE: Git checkout multiple options issue
On January 28, 2019 11:03, COLLOMB Joris wrote: >> -Message d'origine----- >> De : Randall S. Becker Envoyé : lundi 28 janvier >> 2019 16:20 À : COLLOMB Joris -EXT ; >> git@vger.kernel.org Objet : RE: Git checkout multiple options issue >> >> On January 28, 2019 9:25, COLLOMB Joris wrote: >> > -Message d'origine- >> >> De : Randall S. Becker Envoyé : lundi 28 >> >> janvier >> >> 2019 15:12 À : COLLOMB Joris -EXT >> >> ; >> >> git@vger.kernel.org Objet : RE: Git checkout multiple options issue >> >> >> >> On January 28, 2019 8:25, COLLOMB Joris wrote: >> >> > git checkout -fb "branch_name" >> >> > (force branch creation and checkout it) >> >> > >> >> > doesn't work (even if option a separated). >> >> > >> >> > I don't know if this is consider as an issue, but here it is. >> >>. >> >> I think you might mean (which works on every platform I have): >> >> >> >> git checkout -f -b "branch_name" >> >> >> >> There is no provision for aggregating options into one. -fb (invalid) >> >> is >> not the >> >> same as -f -b (valid). >> >> > git checkout -f -b "branch_name" >> > gives me " Fatal: A branch named 'branch_name' already exists." >> >> Once the branch is created, you can't force its creation, because it is >> already >> created. Just >> >> git checkout "branch_name" >> >> is sufficient at this point. git is correct to complain that you are trying >> to >> create a branch that already exists. >> >> git log --decorate --oneline --graph --all >> >> will show you where your branch points in history at any given moment in >> time in a convenient form. > >> > I understand that here the checkout is force, but not the branch creation. >> > The opposite option order doesn't work: >> > >> > git checkout -b -f "branch_name" >> > gives me "Fatal: '-f' is not a valid branch name." >> >> In this case, you are asking git to create a branch named -f (the -b branch >> option). Then "branch_name" becomes the reference that would be used to >> find the commit that -f would have pointed to. However, -f is not a valid >> name because it is an option and git is correct to reject it. >> >> git checkout options are described here: >> https://git-scm.com/docs/git-checkout >> > Once the branch is created, you can't force its creation, because it is >> already created. > Sorry to not be agree, in the man page of git branch: > >-f, --force >Reset to if exists already. > Without -f git branch refuses to change an existing branch. In combination > with -d (or --delete), allow >deleting the branch irrespective of its merged status. In > combination > with -m (or --move), allow renaming the branch even if the new branch name > already exists. > > The behavior I was expecting with > git checkout -b -f "branch_name" > is a checkout on a forced branch creation at . > > So the only solution for me is : > git branch -f "branch_name" && git checkout "branch_name" > > So to resume: > - This is not an issue, just a divergence between my logic and git > implementation. > - The message "Fatal: '-f' is not a valid branch name." is maybe not optimal, > and it may better be " Fatal: you trying to force the creation of a branch. > Please do "git branch -f" if you know what you're doing" A few things: 1. Please put your responses at the end, by convention, for this distribution list so that I don't have to reformat the message each time 😉 2. You cannot assume that a flag in one command is going to do the same thing in another command. -f in checkout means one thing. -f in branch means something different. It is dangerous to conflate the meaning of flags between commands. 3. git branch -f is used to force a branch move on a branch that you do not have checked out, so mixing these is semantically incorrect. git branch -f will deliberately fail if you try to use it on the branch that is checked out. This is correct behaviour. 4. git checkout -f means to do the checkout even if the HEAD and working index do not match and has nothing to do with branch creation. These are entirely different meanings and must not be "blended" into one concept. 5.
RE: Git checkout multiple options issue
On January 28, 2019 9:25, COLLOMB Joris wrote: > -Message d'origine- >> De : Randall S. Becker Envoyé : lundi 28 janvier >> 2019 15:12 À : COLLOMB Joris -EXT ; >> git@vger.kernel.org Objet : RE: Git checkout multiple options issue >> >> On January 28, 2019 8:25, COLLOMB Joris wrote: >> > git checkout -fb "branch_name" >> > (force branch creation and checkout it) >> > >> > doesn't work (even if option a separated). >> > >> > I don't know if this is consider as an issue, but here it is. >> >> I think you might mean (which works on every platform I have): >> >> git checkout -f -b "branch_name" >> >> There is no provision for aggregating options into one. -fb (invalid) is not the >> same as -f -b (valid). > git checkout -f -b "branch_name" > gives me " Fatal: A branch named 'branch_name' already exists." Once the branch is created, you can't force its creation, because it is already created. Just git checkout "branch_name" is sufficient at this point. git is correct to complain that you are trying to create a branch that already exists. git log --decorate --oneline --graph --all will show you where your branch points in history at any given moment in time in a convenient form. > I understand that here the checkout is force, but not the branch creation. > The opposite option order doesn't work: > > git checkout -b -f "branch_name" > gives me "Fatal: '-f' is not a valid branch name." In this case, you are asking git to create a branch named -f (the -b branch option). Then "branch_name" becomes the reference that would be used to find the commit that -f would have pointed to. However, -f is not a valid name because it is an option and git is correct to reject it. git checkout options are described here: https://git-scm.com/docs/git-checkout Regards, Randall
RE: Git checkout multiple options issue
On January 28, 2019 8:25, COLLOMB Joris wrote: > git checkout -fb "branch_name" > (force branch creation and checkout it) > > doesn't work (even if option a separated). > > I don't know if this is consider as an issue, but here it is. I think you might mean (which works on every platform I have): git checkout -f -b "branch_name" There is no provision for aggregating options into one. -fb (invalid) is not the same as -f -b (valid). Regards, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much.
RE:
On January 23, 2019 11:00, Christopher Hagler wrote: > Send the email to this address > majord...@vger.kernel.org and it will work > > On Jan 23, 2019, at 8:16 AM, Cody Kratzer > > I've sent this same email 3 times. I don't think it works. I'm > > researching this morning how to unsubscribe from this git group. > > > > CODY KRATZER WEB DEVELOPMENT MANAGER > > 866-344-3875 x145 > > c...@lightingnewyork.com > > M - F 9 - 5:30 > > On Wed, Jan 23, 2019 at 5:51 AM Christopher Hagler > > wrote: > >> > >> Unsubscribe git Reference information to the mailing lists are available here: http://vger.kernel.org/vger-lists.html#git
RE: [Not Git Bug] Commit generates GC repack failure
On January 14, 2019 10:07, Duy Nguyen wrote: > To: Randall S. Becker > Cc: Git Mailing List > Subject: Re: [Possible Bug] Commit generates GC repack failure > > On Mon, Jan 14, 2019 at 10:03 PM Duy Nguyen > wrote: > > > > On Mon, Jan 14, 2019 at 9:51 PM Randall S. Becker > > wrote: > > > > > > Hi All, > > > > > > I'm trying to track down what happened this morning. We had a commit > > > that caused a background gc to occur. What happened was: > > > > > > $ git commit -m "history commit ... " > > > Auto packing the repository in background for optimum performance. > > > See "git help gc" for manual housekeeping. > > > warning: The last gc run reported the following. Please correct the > > > root cause and remove .git/gc.log. > > > Automatic cleanup will not be performed until the file is removed. > > > > > > fatal: open /dev/null failed: Invalid function argument > > > > sanitize_stdfds() in setup.c can attempt to open /dev/null then > > redirect stdout and stderr to it. I think this is part of the > > daemonization that is done in background gc. > > However the message does not match. I think you will find this "open > /dev/null failed" line in run-command.c Mystery solved. The problem seems to occur with garbage collection on older versions of the operating system than the one we built 2.20.1 on. We sent out the advice to revert to 2.16.2, which was built on a prior revision that did not introduce incompatibilities. This was a specific situation with header files on a new version of the OS that introduced incompatibilities. The impact was not limited to the line in run-command.c, but was way more extensive. Thanks for the help, Randall
RE: [PATCH] blame: add the ability to ignore commits
On January 14, 2019 12:46, Junio C Hamano wrote: > Barret Rhoden writes: > > > On 2019-01-10 at 14:29 Junio C Hamano wrote: > >> > For instance, commit X does this: > >> > > >> > -foo(x,y); > >> > +foo(x,y,z); > >> > > >> > Then commit Y comes along to reformat it: > >> > > >> > -foo(x,y,z); > >> > +foo(x, y, z); > >> > > >> > And the history / rev-list for the file looks like: > >> > > >> > ---O---A---X---B---C---D---Y---E---F > >> > > >> > I want to ignore/skip Y and see X in the blame output. > >> > >> If you skip Y, the altered history would have "foo(x, y, z)" in E, > >> "foo(x,y,z)" in X, and "foo(x,y)" in A. If you start blaming from F, > >> you'd get E as the commit that explains the latest state. If you do > >> not skip Y, you'd get Y. I am not sure how you'd get X in either > >> case. > > > > The way to do it is ... > > Sorry, I made a too-fuzzy statement. What I meant was, that unless you are > ignoring E, I do not know why you "would want to" attribute a line "foo(x, y, > z)" that appears in F to X. Starting from X up to D (and to Y in real history, but > you are ignoring Y), the line was "foo(x,y,z)", after E, it is "foo(x, y, z)". I > didn't mean to ask how you "would show" such a result---as I do not yet > understand why you would want such a result to begin with. >From my own community, this came up also. The intent was to show everyone who touched a particular line, throughout history, not just the current one. Perhaps that is what Barret is going for. Regards, Randall
RE: [Possible Bug] Commit generates GC repack failure
On January 14, 2019 10:12, Ævar Arnfjörð Bjarmason wrote: > On Mon, Jan 14 2019, Randall S. Becker wrote: > > > Hi All, > > > > I'm trying to track down what happened this morning. We had a commit > > that caused a background gc to occur. What happened was: > > > > $ git commit -m "history commit ... " > > Auto packing the repository in background for optimum performance. > > See "git help gc" for manual housekeeping. > > warning: The last gc run reported the following. Please correct the > > root cause and remove .git/gc.log. > > Automatic cleanup will not be performed until the file is removed. > > > > fatal: open /dev/null failed: Invalid function argument > > fatal: failed to run repack > > > > Obviously the /dev/null open was not right, but I don't know where > > this is in the git code to investigate further. Any pointers on where > > to look would be appreciated. This started happening at 2.20.1 on the > NonStop platform. > > We use start_command() when gc does run_command_v_opt() of e.g. git- > repack. See in that file: > > null_fd = open("/dev/null", O_RDWR | O_CLOEXEC) > > Maybe that sort of code just fails on NonStop? That line of code works fine on the platform. I tested it in isolation at various OS levels. Let me go back and say that I'm not 100% sure this is new, but may have been there since just before 2.16. Still perplexed recognizing that that is the only point in git code where that can happen. Cheers, Randall
[Possible Bug] Commit generates GC repack failure
Hi All, I'm trying to track down what happened this morning. We had a commit that caused a background gc to occur. What happened was: $ git commit -m "history commit ... " Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. warning: The last gc run reported the following. Please correct the root cause and remove .git/gc.log. Automatic cleanup will not be performed until the file is removed. fatal: open /dev/null failed: Invalid function argument fatal: failed to run repack Obviously the /dev/null open was not right, but I don't know where this is in the git code to investigate further. Any pointers on where to look would be appreciated. This started happening at 2.20.1 on the NonStop platform. Cheers, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much.
RE: [Patch v5 3/4] git-compat-util.h: add FLOSS headers for HPE NonStop
On January 3, 2019 16:41, Eric Sunshine wrote: > On Thu, Jan 3, 2019 at 4:04 PM wrote: > > The HPE NonStop (a.k.a. __TANDEM) platform cannot build git without > > using the FLOSS package supplied by HPE. The convenient location for > > including the relevant headers is in this file. > > > > The NSIG define is also not defined on __TANDEM, so we define it here > > as 100 if it is not defined only for __TANDEM builds. > > > > Signed-off-by: Randall S. Becker > > --- > > diff --git a/git-compat-util.h b/git-compat-util.h @@ -397,6 +397,17 > > @@ static inline char *git_find_last_dir_sep(const char *path) > > +#ifdef __TANDEM > > +#include > > +#include > > +#ifndef NSIG > > +/* NonStop NSE and NSX do not provide NSIG. SIGGUARDIAN(99) is the > highest > > + known, by detective work using kill -l as a list is all signals > > + instead of signal.h where it should be. */ > > Style nit: With two minor exceptions, all other multi-line comments in this > file > are formatted as: > > /* > * Multi-line > * comment. > */ Doh! I just missed that one. I'll fix this on the next round, if that's ok.
RE: [Patch v5 2/4] config.mak.uname: support for modern HPE NonStop config.
On January 3, 2019 16:38, Eric Sunshine wrote: > On Thu, Jan 3, 2019 at 4:04 PM wrote: > > A number of configuration options are not automatically detected by > > configure mechanisms, including the location of Perl and Python. > > > > There was a problem at a specific set of operating system versions > > that caused getopt to have compile errors. Account for this by > > providing emulation defines for those versions. > > > > Signed-off-by: Randall S. Becker > > --- > > diff --git a/config.mak.uname b/config.mak.uname @@ -470,7 +487,7 > @@ > > ifeq ($(uname_S),NONSTOP_KERNEL) > > NO_MKDTEMP = YesPlease > > OLD_ICONV = UnfortunatelyYes > > - NO_REGEX = YesPlease > > + NO_REGEX=NeedsStartEnd > > NO_PTHREADS = UnfortunatelyYes > > Style nit (probably not worth a re-roll): you lost the whitespace surrounding > '=' I can fix this one in the next round of changes, or after this gets in, if that works, or signoff on this being fixed in place.
[Patch v5 4/4] compat/regex/regcomp.c: define intptr_t and uintptr_t on NonStop
From: "Randall S. Becker" The system definition header files on HPE NonStop do not define intptr_t and uintptr_t as do other platforms. These typedefs are added specifically wrapped in a __TANDEM ifdef. Signed-off-by: Randall S. Becker --- compat/regex/regcomp.c | 8 1 file changed, 8 insertions(+) diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c index 51cd60baa..c0d838834 100644 --- a/compat/regex/regcomp.c +++ b/compat/regex/regcomp.c @@ -17,6 +17,14 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ +#if defined __TANDEM + /* This is currently duplicated from git-compat-utils.h */ +# ifdef NO_INTPTR_T + typedef long intptr_t; + typedef unsigned long uintptr_t; +# endif +#endif + static reg_errcode_t re_compile_internal (regex_t *preg, const char * pattern, size_t length, reg_syntax_t syntax); static void re_compile_fastmap_iter (regex_t *bufp, -- 2.12.3
[Patch v5 0/4] HPE NonStop Port Commits
From: "Randall S. Becker" This set of patches is a distilled version of the minimal set of changes to git that will allow it to run as client and server on HPE NonStop NSE and NSX systems. NSR systems are no longer under support so references to them have been removed. Each patch in this set is independent but required for correctness. Randall S. Becker (4): transport-helper: use xread instead of read config.mak.uname: support for modern HPE NonStop config. git-compat-util.h: add FLOSS headers for HPE NonStop compat/regex/regcomp.c: define intptr_t and uintptr_t on NonStop compat/regex/regcomp.c | 8 config.mak.uname | 29 +++-- git-compat-util.h | 11 +++ transport-helper.c | 5 ++--- 4 files changed, 44 insertions(+), 9 deletions(-) -- 2.12.3
[Patch v5 2/4] config.mak.uname: support for modern HPE NonStop config.
From: "Randall S. Becker" A number of configuration options are not automatically detected by configure mechanisms, including the location of Perl and Python. There was a problem at a specific set of operating system versions that caused getopt to have compile errors. Account for this by providing emulation defines for those versions. Signed-off-by: Randall S. Becker --- config.mak.uname | 29 +++-- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/config.mak.uname b/config.mak.uname index 3ee7da0e2..686156d53 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -441,26 +441,43 @@ ifeq ($(uname_S),NONSTOP_KERNEL) # INLINE='' would just replace one set of warnings with another and # still not compile in c89 mode, due to non-const array initializations. CC = cc -c99 + # Build down-rev compatible objects that don't use our new getopt_long. + ifeq ($(uname_R).$(uname_V),J06.21) + CC += -WRVU=J06.20 + endif + ifeq ($(uname_R).$(uname_V),L17.02) + CC += -WRVU=L16.05 + endif # Disable all optimization, seems to result in bad code, with -O or -O2 # or even -O1 (default), /usr/local/libexec/git-core/git-pack-objects # abends on "git push". Needs more investigation. - CFLAGS = -g -O0 + CFLAGS = -g -O0 -Winline # We'd want it to be here. prefix = /usr/local - # Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl). - PERL_PATH = ${prefix}/bin/perl - PYTHON_PATH = ${prefix}/bin/python - + # perl and python must be in /usr/bin on NonStop - supplied by HPE + # with operating system in that managed directory. + PERL_PATH = /usr/bin/perl + PYTHON_PATH = /usr/bin/python + # The current /usr/coreutils/rm at lowest support level does not work + # with the git test structure. Long paths as in + # 'trash directory...' cause rm to terminate prematurely without fully + # removing the directory at OS releases J06.21 and L17.02. + # Default to the older rm until those two releases are deprecated. + RM = /bin/rm -f # As detected by './configure'. # Missdetected, hence commented out, see below. #NO_CURL = YesPlease # Added manually, see above. + NEEDS_SSL_WITH_CURL = YesPlease + NEEDS_CRYPTO_WITH_SSL = YesPlease + HAVE_DEV_TTY = YesPlease HAVE_LIBCHARSET_H = YesPlease HAVE_STRINGS_H = YesPlease NEEDS_LIBICONV = YesPlease NEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease NO_SYS_SELECT_H = UnfortunatelyYes NO_D_TYPE_IN_DIRENT = YesPlease + NO_GETTEXT = YesPlease NO_HSTRERROR = YesPlease NO_STRCASESTR = YesPlease NO_MEMMEM = YesPlease @@ -470,7 +487,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL) NO_MKDTEMP = YesPlease # Currently libiconv-1.9.1. OLD_ICONV = UnfortunatelyYes - NO_REGEX = YesPlease + NO_REGEX=NeedsStartEnd NO_PTHREADS = UnfortunatelyYes # Not detected (nor checked for) by './configure'. -- 2.12.3
[Patch v5 3/4] git-compat-util.h: add FLOSS headers for HPE NonStop
From: "Randall S. Becker" The HPE NonStop (a.k.a. __TANDEM) platform cannot build git without using the FLOSS package supplied by HPE. The convenient location for including the relevant headers is in this file. The NSIG define is also not defined on __TANDEM, so we define it here as 100 if it is not defined only for __TANDEM builds. Signed-off-by: Randall S. Becker --- git-compat-util.h | 11 +++ 1 file changed, 11 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 09b0102ca..3da6f0673 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -397,6 +397,17 @@ static inline char *git_find_last_dir_sep(const char *path) #define query_user_email() NULL #endif +#ifdef __TANDEM +#include +#include +#ifndef NSIG +/* NonStop NSE and NSX do not provide NSIG. SIGGUARDIAN(99) is the highest + known, by detective work using kill -l as a list is all signals + instead of signal.h where it should be. */ +# define NSIG 100 +#endif +#endif + #if defined(__HP_cc) && (__HP_cc >= 61000) #define NORETURN __attribute__((noreturn)) #define NORETURN_PTR -- 2.12.3
[Patch v5 1/4] transport-helper: use xread instead of read
From: "Randall S. Becker" This fix was needed on HPE NonStop NSE and NSX where SSIZE_MAX is less than BUFFERSIZE resulting in EINVAL. The call to read in transport-helper.c was the only place outside of wrapper.c where it is used instead of xread. Signed-off-by: Randall S. Becker --- transport-helper.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/transport-helper.c b/transport-helper.c index bf225c698..5afead9f8 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -1225,9 +1225,8 @@ static int udt_do_read(struct unidirectional_transfer *t) return 0; /* No space for more. */ transfer_debug("%s is readable", t->src_name); - bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); - if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN && - errno != EINTR) { + bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); + if (bytes < 0 && errno != EINTR) { error_errno(_("read(%s) failed"), t->src_name); return -1; } else if (bytes == 0) { -- 2.12.3
[BUG] t0410 2.20.1 fails on NonStop NSX platform
This is a bit of a head-scratcher. There are two NonStop platforms, which behave essentially the same way. One, NSE, uses ia64, while NSX uses x64. There are subtle differences in ksh and bash for the platforms based on what is released when. The NSE platform has no problem running t0410 in Jenkins. However, NSX fails in a few spots: This is at 2.20.1. Our last port on this platform variant was 2.16.1, where there were no unexplained issues. I am suspecting that the problem is shell related, but I'm just not sure what the test is supposed to do here. There are 9 other failures that appear related, with the No such file or directory condition. If I add the -f flag to rm in delete_object(), all t0410 tests pass except the last (22), but I don't think that meets the intent of test itself. I wonder whether one of the authors of the test might chime in on this. The first, subtest 12 results in: Initialized empty Git repository in /mypath/git/t/trash directory.t0410-partial-clone/repo/.git/ [master (root-commit) 7745948] foo Author: A U Thor 1 file changed, 1 insertion(+) create mode 100644 foo.t [master e514b54] bar Author: A U Thor 1 file changed, 1 insertion(+) create mode 100644 bar.t Enumerating objects: 1, done. Counting objects: 100% (1/1), done. Writing objects: 100% (1/1), done. Total 1 (delta 0), reused 0 (delta 0) 2b7df34bc09be010087307b898e994ce709c0db1 rm: cannot remove 'repo/.git/objects/2b/7df34bc09be010087307b898e994ce709c0db1': No such file or directory not ok 12 - rev-list stops traversal at missing and promised commit # # rm -rf repo && # test_create_repo repo && # test_commit -C repo foo && # test_commit -C repo bar && # # FOO=$(git -C repo rev-parse foo) && # promise_and_delete "$FOO" && # # git -C repo config core.repositoryformatversion 1 && # git -C repo config extensions.partialclone "arbitrary string" && # GIT_TEST_COMMIT_GRAPH=0 git -C repo rev-list --exclude-promisor-objects --objects bar >out && # grep $(git -C repo rev-parse bar) out && # ! grep $FOO out # Cheers, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much.
[PATCH v4 1/4] transport-helper: use xread instead of read
From: "Randall S. Becker" This fix was needed on HPE NonStop NSE and NSX where SSIZE_MAX is less than BUFFERSIZE resulting in EINVAL. The call to read in transport-helper.c was the only place outside of wrapper.c where it is used instead of xread. Signed-off-by: Randall S. Becker --- transport-helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transport-helper.c b/transport-helper.c index bf225c698f..a290695a12 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -1225,7 +1225,7 @@ static int udt_do_read(struct unidirectional_transfer *t) return 0; /* No space for more. */ transfer_debug("%s is readable", t->src_name); - bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); + bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); - if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN && - errno != EINTR) { + if (bytes < 0 && errno != EINTR) { error_errno(_("read(%s) failed"), t->src_name); -- 2.17.0.10.gb132f7033
RE: [PATCH v1 1/4] transport-helper: use xread instead of read
On December 28, 2018 15:11, Junio C Hamano wrote: > randall.s.bec...@rogers.com writes: > > > From: "Randall S. Becker" > > > > This fix was needed on HPE NonStop NSE and NSX where SSIZE_MAX is less > > than BUFFERSIZE resulting in EINVAL. The call to read in > > transport-helper.c was the only place outside of wrapper.c where it is used > instead of xread. > > > > Signed-off-by: Randall S. Becker > > --- > > transport-helper.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/transport-helper.c b/transport-helper.c index > > bf225c698f..a290695a12 100644 > > --- a/transport-helper.c > > +++ b/transport-helper.c > > @@ -1225,7 +1225,7 @@ static int udt_do_read(struct > unidirectional_transfer *t) > > return 0; /* No space for more. */ > > > > transfer_debug("%s is readable", t->src_name); > > - bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); > > + bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); > > if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN && > > errno != EINTR) { > > error_errno(_("read(%s) failed"), t->src_name); > > As Peff pointed out in the earlier round of the same patch, replacing read() > with xread() here will affect what errno's can be possible after the function > returns. The checks affected by this change must also be updated, either in > the same patch, or a follow-up patch in the same series. Otherwise we > _will_ forget to clean them up. If I read the xread code correctly, the change would be to leave EINTR and remove the other two conditions. Correct?
RE: [PATCH v1 2/4] config.mak.uname: support for modern HPE NonStop config.
On December 28, 2018 15:07, Junio C Hamano > "Randall S. Becker" writes: > > > On December 27, 2018 12:03, Eric Sunshine wrote: > >> On Wed, Dec 26, 2018 at 6:05 PM wrote: > >> > A number of configuration options are not automatically detected by > >> > configure mechanisms, including the location of Perl and Python. > >> > [...] > >> > Signed-off-by: Randall S. Becker > >> > --- > >> > diff --git a/config.mak.uname b/config.mak.uname @@ -441,26 > +441,45 > >> @@ > >> > ifeq ($(uname_S),NONSTOP_KERNEL) > >> > # Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl). > >> > - PERL_PATH = ${prefix}/bin/perl > >> > - PYTHON_PATH = ${prefix}/bin/python > >> > + PERL_PATH = /usr/bin/perl > >> > + PYTHON_PATH = /usr/bin/python > >> > >> Is the in-code comment about ${prefix} still applicable after this change? > > > > The ${prefix} is not applicable on this platform for perl and python. > > Those locations must be in /usr/bin and are managed by the Operating > > System vendor not by customers. The change is wrapped in an IF so is > > only applicable to NonStop. > > So the answer is "Our's are in ${prefix}/bin..." is no longer correct? If so, this > patch must remove that stale comment at the same time, no? Yessir. Fixed at v3 (now at v4).
[PATCH v4 2/4] config.mak.uname: support for modern HPE NonStop config.
From: "Randall S. Becker" A number of configuration options are not automatically detected by configure mechanisms, including the location of Perl and Python. There was a problem at a specific set of operating system versions that caused getopt to have compile errors. Account for this by providing emulation defines for those versions. Signed-off-by: Randall S. Becker --- config.mak.uname | 34 +- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/config.mak.uname b/config.mak.uname index 3ee7da0e23..aa4432ac2f 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -441,26 +441,45 @@ ifeq ($(uname_S),NONSTOP_KERNEL) # INLINE='' would just replace one set of warnings with another and # still not compile in c89 mode, due to non-const array initializations. CC = cc -c99 + # Build down-rev compatible objects that don't use our new getopt_long. + ifeq ($(uname_R).$(uname_V),J06.21) + CC += -WRVU=J06.20 + endif + ifeq ($(uname_R).$(uname_V),L17.02) + CC += -WRVU=L16.05 + endif + # Disable all optimization, seems to result in bad code, with -O or -O2 # or even -O1 (default), /usr/local/libexec/git-core/git-pack-objects # abends on "git push". Needs more investigation. - CFLAGS = -g -O0 + CFLAGS = -g -O0 -Winline # We'd want it to be here. prefix = /usr/local - # Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl). + # perl and python must be in /usr/bin on NonStop - supplied by HPE + # with operating system in that managed directory. - PERL_PATH = ${prefix}/bin/perl - PYTHON_PATH = ${prefix}/bin/python - + PERL_PATH = /usr/bin/perl + PYTHON_PATH = /usr/bin/python + # The current /usr/coreutils/rm at lowest support level does not work + # with the git test structure. Long paths as in + # 'trash directory...' cause rm to terminate prematurely without fully + # removing the directory at OS releases J06.21 and L17.02. + # Default to the older rm until those two releases are deprecated. + RM = /bin/rm -f # As detected by './configure'. # Missdetected, hence commented out, see below. #NO_CURL = YesPlease # Added manually, see above. + NEEDS_SSL_WITH_CURL = YesPlease + NEEDS_CRYPTO_WITH_SSL = YesPlease + HAVE_DEV_TTY = YesPlease HAVE_LIBCHARSET_H = YesPlease HAVE_STRINGS_H = YesPlease NEEDS_LIBICONV = YesPlease NEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease NO_SYS_SELECT_H = UnfortunatelyYes NO_D_TYPE_IN_DIRENT = YesPlease + NO_GETTEXT = YesPlease NO_HSTRERROR = YesPlease NO_STRCASESTR = YesPlease NO_MEMMEM = YesPlease @@ -470,8 +489,13 @@ ifeq ($(uname_S),NONSTOP_KERNEL) NO_MKDTEMP = YesPlease # Currently libiconv-1.9.1. OLD_ICONV = UnfortunatelyYes - NO_REGEX = YesPlease + NO_REGEX=NeedsStartEnd NO_PTHREADS = UnfortunatelyYes # Not detected (nor checked for) by './configure'. # We don't have SA_RESTART on NonStop, unfortunalety. -- 2.17.0.10.gb132f7033
RE: [PATCH v3 2/4] config.mak.uname: support for modern HPE NonStop config.
On December 28, 2018 14:43, Eric Sunshine wrote: > On Thu, Dec 27, 2018 at 5:39 PM wrote: > > A number of configuration options are not automatically detected by > > configure mechanisms, including the location of Perl and Python. > > > > There was a problem at a specific set of operating system versions > > that caused getopt to have compile errors. Account for this by > > providing emulation defines for those versions. > > This version of the patch looks much better and addresses my comments on > previous attempts. One note below... > > > Signed-off-by: Randall S. Becker > > --- > > diff --git a/config.mak.uname b/config.mak.uname @@ -441,26 +441,45 > @@ > > ifeq ($(uname_S),NONSTOP_KERNEL) > > + # The current /usr/coreutils/rm at lowest support level does not > > work > > + # with the git test structure. Long paths cause nftw as in > > + # 'trash directory...' cause rm to terminate prematurely without > > fully > > + # removing the directory at OS releases J06.21 and L17.02. > > + # Default to the older rm until those two releases are deprecated. > > + RM = /bin/rm -f > > Thanks, this comment does a much better job of explaining the actual > problem and selling the solution. > > There is a slight grammatical problem: > > Long paths _cause_ ... _cause_ rm to terminate... > > which might deserve fixing. Rerolled. Thanks.
RE: Missed Commit in 2.20.1
On December 27, 2018 17:40, Ævar Arnfjörð Bjarmason wrote: > On Wed, Dec 26 2018, Randall S. Becker wrote: > > > Please stay tuned for patches. We are very much looking forward to > > having the two (or three) different NonStop hardware personalities > > supported without mods in the very near future. Our goal, assuming > > those patches are acceptable, is to move our build/test/distro into a > > Jenkins config that runs with minimal human involvement (a.k.a. me). > > Portability patches like that are definitely wanted. > > In case you haven't seen my recent work on getting GitLab CI up & running > check out https://public- > inbox.org/git/875zwm15k2@evledraar.gmail.com/ > > It differs from existing CI implementations for git.git in being focused on > doing the actual run on remote hosts that can be ssh'd to. > > So perhaps you'd be interested in some of: > > a) Contributing a NonStop box to the GCC Compile Farm >(https://cfarm.tetaneutral.net/machines/list/). Then I can add it to >my tests, but also other people porting free software will fix bugs >pro-actively before you spot them. If I win the lottery, sure. Right now, a contribution like that is a bit beyond my budget. I'm not sure that anything "GCC" will fly with management since GCC does not port to the platform at all at this point in time. Many have tried. Many have failed. We're limited to c89 and c99. > b) I now have a gitlab-runner I maintain powering this git-ci.git stuff >& presenting it on gitlab.com, if you give me SSH access I can add it >to my own runs... Sorry, no can do on this one. > c) ...or you can just run your own gitlab-runner on >https://gitlab.com/git-vcs/git-ci/ (although this amounts to giving >me ssh access, since you'll be running my code) This may be more possible. I've been considering putting up a GitLab instance but it's a matter of not having enough time. I have more than enough LXC ubuntu instances still available for something like that. > d) ... or reuse the CI code I wrote to setup your own runner/pusher >against NonStop, only you'd have access to this More likely. Private chat worth it perhaps. > e) Or do whatever you're planning with Jenkins. We are currently using Jenkins to build/test git. I was thinking about contributing a Jenkinsfile that would build on a Controller (what happens today for our git port), or setting up a parameterized form for SSH for an Agent that might be better in a farm setting. I am close to the point where human interaction is limited to 'git branch -f production vn.mm.l' and git is tested and built for distribution without further touching. At least once my platform patches are applied it will be. > If you want to just go with e) that's fine, just saying that you could re-use > some existing stuff with a-d) if you wanted. I am interested. Let's see how my $DAYJOB goes in the next few months. I really do like the idea of setting up a community instance of GitLab to do this and include a CI runner. Hmmm. Cheers, Randall
[PATCH v3 2/4] config.mak.uname: support for modern HPE NonStop config.
From: "Randall S. Becker" A number of configuration options are not automatically detected by configure mechanisms, including the location of Perl and Python. There was a problem at a specific set of operating system versions that caused getopt to have compile errors. Account for this by providing emulation defines for those versions. Signed-off-by: Randall S. Becker --- config.mak.uname | 34 +- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/config.mak.uname b/config.mak.uname index 3ee7da0e23..aa4432ac2f 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -441,26 +441,45 @@ ifeq ($(uname_S),NONSTOP_KERNEL) # INLINE='' would just replace one set of warnings with another and # still not compile in c89 mode, due to non-const array initializations. CC = cc -c99 + # Build down-rev compatible objects that don't use our new getopt_long. + ifeq ($(uname_R).$(uname_V),J06.21) + CC += -WRVU=J06.20 + endif + ifeq ($(uname_R).$(uname_V),L17.02) + CC += -WRVU=L16.05 + endif + # Disable all optimization, seems to result in bad code, with -O or -O2 # or even -O1 (default), /usr/local/libexec/git-core/git-pack-objects # abends on "git push". Needs more investigation. - CFLAGS = -g -O0 + CFLAGS = -g -O0 -Winline # We'd want it to be here. prefix = /usr/local - # Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl). + # perl and python must be in /usr/bin on NonStop - supplied by HPE + # with operating system in that managed directory. - PERL_PATH = ${prefix}/bin/perl - PYTHON_PATH = ${prefix}/bin/python - + PERL_PATH = /usr/bin/perl + PYTHON_PATH = /usr/bin/python + # The current /usr/coreutils/rm at lowest support level does not work + # with the git test structure. Long paths cause nftw as in + # 'trash directory...' cause rm to terminate prematurely without fully + # removing the directory at OS releases J06.21 and L17.02. + # Default to the older rm until those two releases are deprecated. + RM = /bin/rm -f # As detected by './configure'. # Missdetected, hence commented out, see below. #NO_CURL = YesPlease # Added manually, see above. + NEEDS_SSL_WITH_CURL = YesPlease + NEEDS_CRYPTO_WITH_SSL = YesPlease + HAVE_DEV_TTY = YesPlease HAVE_LIBCHARSET_H = YesPlease HAVE_STRINGS_H = YesPlease NEEDS_LIBICONV = YesPlease NEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease NO_SYS_SELECT_H = UnfortunatelyYes NO_D_TYPE_IN_DIRENT = YesPlease + NO_GETTEXT = YesPlease NO_HSTRERROR = YesPlease NO_STRCASESTR = YesPlease NO_MEMMEM = YesPlease @@ -470,8 +489,13 @@ ifeq ($(uname_S),NONSTOP_KERNEL) NO_MKDTEMP = YesPlease # Currently libiconv-1.9.1. OLD_ICONV = UnfortunatelyYes - NO_REGEX = YesPlease + NO_REGEX=NeedsStartEnd NO_PTHREADS = UnfortunatelyYes # Not detected (nor checked for) by './configure'. # We don't have SA_RESTART on NonStop, unfortunalety. -- 2.17.0.10.gb132f7033
[PATCH v2 2/4] config.mak.uname: support for modern HPE NonStop config.
From: "Randall S. Becker" A number of configuration options are not automatically detected by configure mechanisms, including the location of Perl and Python. There was a problem at a specific set of operating system versions that caused getopt to have compile errors. Accounted for this by providing emulation defines for those versions. Signed-off-by: Randall S. Becker --- config.mak.uname | 34 +- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/config.mak.uname b/config.mak.uname index 3ee7da0e23..aa4432ac2f 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -441,26 +441,45 @@ ifeq ($(uname_S),NONSTOP_KERNEL) # INLINE='' would just replace one set of warnings with another and # still not compile in c89 mode, due to non-const array initializations. CC = cc -c99 + # Build down-rev compatible objects that don't use our new getopt_long. + ifeq ($(uname_R).$(uname_V),J06.21) + CC += -WRVU=J06.20 + endif + ifeq ($(uname_R).$(uname_V),L17.02) + CC += -WRVU=L16.05 + endif + # Disable all optimization, seems to result in bad code, with -O or -O2 # or even -O1 (default), /usr/local/libexec/git-core/git-pack-objects # abends on "git push". Needs more investigation. - CFLAGS = -g -O0 + CFLAGS = -g -O0 -Winline # We'd want it to be here. prefix = /usr/local # Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl). - PERL_PATH = ${prefix}/bin/perl - PYTHON_PATH = ${prefix}/bin/python - + PERL_PATH = /usr/bin/perl + PYTHON_PATH = /usr/bin/python + # The current /usr/coreutils/rm at lowest support level does not work + # with the git test structure. Long paths cause nftw as in + # 'trash directory...' break at OS releases J06.21 and L17.02. + # Default to the older rm until those two releases are deprecated. + RM = /bin/rm -f # As detected by './configure'. # Missdetected, hence commented out, see below. #NO_CURL = YesPlease # Added manually, see above. + NEEDS_SSL_WITH_CURL = YesPlease + NEEDS_CRYPTO_WITH_SSL = YesPlease + HAVE_DEV_TTY = YesPlease HAVE_LIBCHARSET_H = YesPlease HAVE_STRINGS_H = YesPlease NEEDS_LIBICONV = YesPlease NEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease NO_SYS_SELECT_H = UnfortunatelyYes NO_D_TYPE_IN_DIRENT = YesPlease + NO_GETTEXT = YesPlease NO_HSTRERROR = YesPlease NO_STRCASESTR = YesPlease NO_MEMMEM = YesPlease @@ -470,8 +489,13 @@ ifeq ($(uname_S),NONSTOP_KERNEL) NO_MKDTEMP = YesPlease # Currently libiconv-1.9.1. OLD_ICONV = UnfortunatelyYes - NO_REGEX = YesPlease + NO_REGEX=NeedsStartEnd NO_PTHREADS = UnfortunatelyYes # Not detected (nor checked for) by './configure'. # We don't have SA_RESTART on NonStop, unfortunalety. -- 2.17.0.10.gb132f7033
RE: [PATCH v1 2/4] config.mak.uname: support for modern HPE NonStop config.
On December 27, 2018 12:03, Eric Sunshine wrote: > On Wed, Dec 26, 2018 at 6:05 PM wrote: > > A number of configuration options are not automatically detected by > > configure mechanisms, including the location of Perl and Python. > > [...] > > Signed-off-by: Randall S. Becker > > --- > > diff --git a/config.mak.uname b/config.mak.uname @@ -441,26 +441,45 > @@ > > ifeq ($(uname_S),NONSTOP_KERNEL) > > # Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl). > > - PERL_PATH = ${prefix}/bin/perl > > - PYTHON_PATH = ${prefix}/bin/python > > + PERL_PATH = /usr/bin/perl > > + PYTHON_PATH = /usr/bin/python > > Is the in-code comment about ${prefix} still applicable after this change? The ${prefix} is not applicable on this platform for perl and python. Those locations must be in /usr/bin and are managed by the Operating System vendor not by customers. The change is wrapped in an IF so is only applicable to NonStop. > > > + # The current /usr/coreutils/rm at lowest support level does not > > work > > + # with the git test structure. Default to the older rm. > > + RM = /bin/rm -f > > This comment would be far more helpful if it explained in what way 'rm' > actually fails with the test suite. Without that information, the comment is > effectively useless. There is a temporary failure in the vendor supplied rm. The cause we never disclosed to my team. Honestly, it created a large amount of angst that should be gone but there are still old OS versions that have this issue. This is as much as we know. > > > # As detected by './configure'. > > # Missdetected, hence commented out, see below. > > #NO_CURL = YesPlease > > # Added manually, see above. > > + # Missdetected, hence commented out, see below. > > + #NO_CURL = YesPlease > > + # Added manually, see above. > > These added lines are just duplicating the existing line immediately above. > That's weird. I'll fix it. Thanks for the catch. > > > + # Not detected by ./configure. Add manually. > > + NEEDS_SSL_WITH_CURL = YesPlease > > + NEEDS_CRYPTO_WITH_SSL = YesPlease > > + HAVE_DEV_TTY = YesPlease > > I find these comments about 'configure' "misdetecting" or "not detecting" > features confusing. The point of config.mak.uname is to provide sane > defaults for people building without using the 'configure' script, so it feels > weird to be talking about 'configure' > here. Also, what does it mean to say that 'configure' "misdetects"? > Does that mean that it fails to write assignments such as "NO_CURL = > YesPlease" to config.autogen or does it mean that it writes incorrect > assignments to that file? This came from another team. We can't currently use config.autogen anyway on the platform - this came from the prior attempt at porting. By misdetect I mean just that, however. We do not get good values. > > > @@ -470,8 +489,13 @@ ifeq ($(uname_S),NONSTOP_KERNEL) > > + ifdef NO_PTHREADS > > + else # WIP, use of Posix User Threads is planned but not working yet > > + PTHREAD_CFLAGS = -D_PUT_MODEL_ -I/usr/include > > + PTHREAD_LIBS = -lput > > + endif > > Why not a simpler 'ifndef'? Another team is current working on the PTHREAD implementation and wanted this. I think what happened was that we have non-pthread requirements under development. I can remove this.
RE: [PATCH v1 0/4] HPE NonStop Port Commits
On December 27, 2018 7:13, Derrick Stolee: > On 12/26/2018 6:05 PM, randall.s.bec...@rogers.com wrote: > > From: "Randall S. Becker" > > > > This set of patches is a distilled version of the minimal set of > > changes to git that will allow it to run as client and server on HPE > > NonStop NSE and NSX systems. NSR systems are no longer under support > > so references to them have been removed. Each patch in this set is > > independent but required for correctness. > > > > Randall S. Becker (4): > >transport-helper: use xread instead of read > >config.mak.uname: support for modern HPE NonStop config. > >git-compat-util.h: add FLOSS headers for HPE NonStop > >compat/regex/regcomp.c: define intptr_t and uintptr_t on NonStop > > These patches look correct to me. Just one question on patch 3 > (git-compat-util.h: add FLOSS headers for HPE NonStop). > > I'm not able to comment on patch 2 (config.mak.uname: support for modern > HPE NonStop config.), but it looks to be wrapped in a platform-specific 'if', > so should be fine if you are happy with it. Thanks! Please go for it, with v2 3/4 that should be available now. This will radically simplify our life on maintaining the port. Kind Regards, Randall et. Cohorts who know who they are.
[PATCH v2 3/4] git-compat-util.h: add FLOSS headers for HPE NonStop
From: "Randall S. Becker" The HPE NonStop (a.k.a. __TANDEM) platform cannot build git without using the FLOSS package supplied by HPE. The convenient location for including the relevant headers is in this file. The NSIG define is also not defined on __TANDEM, so we define it here as 100 if it is not defined only for __TANDEM builds. Signed-off-by: Randall S. Becker --- git-compat-util.h | 15 +++ 1 file changed, 15 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 09b0102cae..3f615f7ed8 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -397,6 +397,21 @@ static inline char *git_find_last_dir_sep(const char *path) #define query_user_email() NULL #endif +#ifdef __TANDEM +#include +#include +#ifndef NSIG +/* NonStop NSE and NSX do not provide NSIG. SIGGUARDIAN(99) is the highest + known, by detective work using kill -l as a list is all signals + instead of signal.h where it should be. */ +# define NSIG 100 +#endif +#endif + #if defined(__HP_cc) && (__HP_cc >= 61000) #define NORETURN __attribute__((noreturn)) #define NORETURN_PTR -- 2.17.0.10.gb132f7033
RE: [PATCH v1 3/4] git-compat-util.h: add FLOSS headers for HPE NonStop
On December 27, 2018 7:10, Derrick Stolee wrote: > On 12/26/2018 6:05 PM, randall.s.bec...@rogers.com wrote: > > The NSIG define is also not defined on __TANDEM, so we define it here > > as 100 if it is not defined only for __TANDEM builds. > [snip] > > +#if ! defined NSIG > > Why didn't you use "#ifndef" here? > > Taking a look at the file, I see both "#ifdef" and "#if defined" but no "#if ! > defined". I'm good with revising as follows and removing the irrelevant lines (stay tuned): +#ifdef __TANDEM +#include +#include +#ifndef NSIG +/* NonStop NSE and NSX do not provide NSIG. SIGGUARDIAN(99) is the highest + known, by detective work using kill -l as a list is all signals + instead of signal.h where it should be. */ +# define NSIG 100 +#endif +#endif + Cheers, Randall
[PATCH v1 3/4] git-compat-util.h: add FLOSS headers for HPE NonStop
From: "Randall S. Becker" The HPE NonStop (a.k.a. __TANDEM) platform cannot build git without using the FLOSS package supplied by HPE. The convenient location for including the relevant headers is in this file. The NSIG define is also not defined on __TANDEM, so we define it here as 100 if it is not defined only for __TANDEM builds. Signed-off-by: Randall S. Becker --- git-compat-util.h | 15 +++ 1 file changed, 15 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 09b0102cae..3f615f7ed8 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -397,6 +397,21 @@ static inline char *git_find_last_dir_sep(const char *path) #define query_user_email() NULL #endif +#ifdef __TANDEM +#if !defined(_THREAD_SUPPORT_FUNCTIONS) && !defined(_PUT_MODEL_) +/* #include */ +/* #include */ +#endif +#include +#include +#if ! defined NSIG +/* NonStop NSE and NSX do not provide NSIG. SIGGUARDIAN(99) is the highest + known, by detective work using kill -l as a list is all signals + instead of signal.h where it should be. */ +# define NSIG 100 +#endif +#endif + #if defined(__HP_cc) && (__HP_cc >= 61000) #define NORETURN __attribute__((noreturn)) #define NORETURN_PTR -- 2.17.0.10.gb132f7033
[PATCH v1 0/4] HPE NonStop Port Commits
From: "Randall S. Becker" This set of patches is a distilled version of the minimal set of changes to git that will allow it to run as client and server on HPE NonStop NSE and NSX systems. NSR systems are no longer under support so references to them have been removed. Each patch in this set is independent but required for correctness. Randall S. Becker (4): transport-helper: use xread instead of read config.mak.uname: support for modern HPE NonStop config. git-compat-util.h: add FLOSS headers for HPE NonStop compat/regex/regcomp.c: define intptr_t and uintptr_t on NonStop compat/regex/regcomp.c | 8 config.mak.uname | 34 +- git-compat-util.h | 15 +++ transport-helper.c | 2 +- 4 files changed, 53 insertions(+), 6 deletions(-) -- 2.17.0.10.gb132f7033
[PATCH v1 4/4] compat/regex/regcomp.c: define intptr_t and uintptr_t on NonStop
From: "Randall S. Becker" The system definition header files on HPE NonStop do not define intptr_t and uintptr_t as do other platforms. These typedefs are added specifically wrapped in a __TANDEM ifdef. Signed-off-by: Randall S. Becker --- compat/regex/regcomp.c | 8 1 file changed, 8 insertions(+) diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c index 51cd60baa3..c0d838834a 100644 --- a/compat/regex/regcomp.c +++ b/compat/regex/regcomp.c @@ -17,6 +17,14 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ +#if defined __TANDEM + /* This is currently duplicated from git-compat-utils.h */ +# ifdef NO_INTPTR_T + typedef long intptr_t; + typedef unsigned long uintptr_t; +# endif +#endif + static reg_errcode_t re_compile_internal (regex_t *preg, const char * pattern, size_t length, reg_syntax_t syntax); static void re_compile_fastmap_iter (regex_t *bufp, -- 2.17.0.10.gb132f7033
[PATCH v1 1/4] transport-helper: use xread instead of read
From: "Randall S. Becker" This fix was needed on HPE NonStop NSE and NSX where SSIZE_MAX is less than BUFFERSIZE resulting in EINVAL. The call to read in transport-helper.c was the only place outside of wrapper.c where it is used instead of xread. Signed-off-by: Randall S. Becker --- transport-helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transport-helper.c b/transport-helper.c index bf225c698f..a290695a12 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -1225,7 +1225,7 @@ static int udt_do_read(struct unidirectional_transfer *t) return 0; /* No space for more. */ transfer_debug("%s is readable", t->src_name); - bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); + bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN && errno != EINTR) { error_errno(_("read(%s) failed"), t->src_name); -- 2.17.0.10.gb132f7033
[PATCH v1 2/4] config.mak.uname: support for modern HPE NonStop config.
From: "Randall S. Becker" A number of configuration options are not automatically detected by configure mechanisms, including the location of Perl and Python. There was a problem at a specific set of operating system versions that caused getopt to have compile errors. Accounted for this by providing emulation defines for those versions. Signed-off-by: Randall S. Becker --- config.mak.uname | 34 +- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/config.mak.uname b/config.mak.uname index 3ee7da0e23..aa4432ac2f 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -441,26 +441,45 @@ ifeq ($(uname_S),NONSTOP_KERNEL) # INLINE='' would just replace one set of warnings with another and # still not compile in c89 mode, due to non-const array initializations. CC = cc -c99 + # Build down-rev compatible objects that don't use our new getopt_long. + ifeq ($(uname_R).$(uname_V),J06.21) + CC += -WRVU=J06.20 + endif + ifeq ($(uname_R).$(uname_V),L17.02) + CC += -WRVU=L16.05 + endif + # Disable all optimization, seems to result in bad code, with -O or -O2 # or even -O1 (default), /usr/local/libexec/git-core/git-pack-objects # abends on "git push". Needs more investigation. - CFLAGS = -g -O0 + CFLAGS = -g -O0 -Winline # We'd want it to be here. prefix = /usr/local # Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl). - PERL_PATH = ${prefix}/bin/perl - PYTHON_PATH = ${prefix}/bin/python - + PERL_PATH = /usr/bin/perl + PYTHON_PATH = /usr/bin/python + # The current /usr/coreutils/rm at lowest support level does not work + # with the git test structure. Default to the older rm. + RM = /bin/rm -f # As detected by './configure'. # Missdetected, hence commented out, see below. #NO_CURL = YesPlease # Added manually, see above. + # Missdetected, hence commented out, see below. + #NO_CURL = YesPlease + # Added manually, see above. + # Not detected by ./configure. Add manually. + NEEDS_SSL_WITH_CURL = YesPlease + NEEDS_CRYPTO_WITH_SSL = YesPlease + HAVE_DEV_TTY = YesPlease + HAVE_LIBCHARSET_H = YesPlease HAVE_STRINGS_H = YesPlease NEEDS_LIBICONV = YesPlease NEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease NO_SYS_SELECT_H = UnfortunatelyYes NO_D_TYPE_IN_DIRENT = YesPlease + NO_GETTEXT = YesPlease NO_HSTRERROR = YesPlease NO_STRCASESTR = YesPlease NO_MEMMEM = YesPlease @@ -470,8 +489,13 @@ ifeq ($(uname_S),NONSTOP_KERNEL) NO_MKDTEMP = YesPlease # Currently libiconv-1.9.1. OLD_ICONV = UnfortunatelyYes - NO_REGEX = YesPlease + NO_REGEX=NeedsStartEnd NO_PTHREADS = UnfortunatelyYes + ifdef NO_PTHREADS + else # WIP, use of Posix User Threads is planned but not working yet + PTHREAD_CFLAGS = -D_PUT_MODEL_ -I/usr/include + PTHREAD_LIBS = -lput + endif # Not detected (nor checked for) by './configure'. # We don't have SA_RESTART on NonStop, unfortunalety. -- 2.17.0.10.gb132f7033
[Bug] t0410 breakages at 2.20.1 on NonStop platform
Hi All, Were getting some new breakages in t0410 that I cant explain easily or either it is the test itself at commit 0d0ac3826a. [Filtered] *** t0410-partial-clone.sh *** not ok 5 - missing ref object, but promised, passes fsck not ok 6 - missing object, but promised, passes fsck not ok 7 - missing CLI object, but promised, passes fsck not ok 12 - rev-list stops traversal at missing and promised commit not ok 13 - missing tree objects with --missing=allow-promisor and --exclude-promisor-objects not ok 14 - missing non-root tree object and rev-list not ok 15 - rev-list stops traversal at missing and promised tree not ok 16 - rev-list stops traversal at missing and promised blob not ok 22 - gc stops traversal when a missing but promised object is reached The first one to fail which probably would give me a hint on others is: Initialized empty Git repository in /home/git/git/t/trash directory.t0410-partial-clone/repo/.git/ [master (root-commit) 6591d03] 1 Author: A U Thor 1 file changed, 1 insertion(+) create mode 100644 1.t [master 5355e57] 2 Author: A U Thor 1 file changed, 1 insertion(+) create mode 100644 2.t [master a3007a6] 3 Author: A U Thor 1 file changed, 1 insertion(+) create mode 100644 3.t Enumerating objects: 1, done. Counting objects: 100% (1/1), done. Writing objects: 100% (1/1), done. Total 1 (delta 0), reused 0 (delta 0) fa10eb4e855a356f0abe9c352b62b71d481918b1 rm: cannot remove 'repo/.git/objects/fa/10eb4e855a356f0abe9c352b62b71d481918b1': No such file or directory not ok 6 - missing object, but promised, passes fsck # # rm -rf repo && # test_create_repo repo && # test_commit -C repo 1 && # test_commit -C repo 2 && # test_commit -C repo 3 && # git -C repo tag -a annotated_tag -m "annotated tag" && # # C=$(git -C repo rev-parse 1) && # T=$(git -C repo rev-parse 2^{tree}) && # B=$(git hash-object repo/3.t) && # AT=$(git -C repo rev-parse annotated_tag) && # # promise_and_delete "$C" && # promise_and_delete "$T" && # promise_and_delete "$B" && # promise_and_delete "$AT" && # # git -C repo config core.repositoryformatversion 1 && # git -C repo config extensions.partialclone "arbitrary string" && # git -C repo fsck # The repo/.git contains the following at the time of failure: .git/objects .git/objects/info .git/objects/pack .git/objects/pack/pack-fa10eb4e855a356f0abe9c352b62b71d481918b1.idx .git/objects/pack/pack-fa10eb4e855a356f0abe9c352b62b71d481918b1.pack .git/objects/pack/pack-fa10eb4e855a356f0abe9c352b62b71d481918b1.promisor .git/refs .git/refs/heads .git/refs/heads/master .git/refs/tags .git/refs/tags/1 .git/refs/tags/2 .git/refs/tags/3 .git/refs/tags/annotated_tag Given the contents, I'm not surprised the 'rm' failed. Any help would be appreciated in tracking down what happened. Thanks, Randall -- Brief whoami: NonStop developer since approximately NonStop(2112884442) UNIX developer since approximately 421664400 -- In my real life, I talk too much.
Missed Commit in 2.20.1
On April 2, 2018 4:02 PM, Stefan Beller found my change: > On Mon, Apr 2, 2018 at 12:57 PM, Randall S. Becker > wrote: > > On April 2, 2018 3:34 PM, Junio C Hamano wrote: > >> The latest feature release Git v2.17.0 is now available at the usual > >> places. It is comprised of 516 non-merge commits since v2.16.0, > >> contributed by 71 people, 20 of which are new faces. > > > > Just a heads up. I think this one might have gotten missed at some point a > few months back. I think it was submitted back in January. Not sure where it > fell off or whether it was just dropped. > > > > diff --git a/transport-helper.c b/transport-helper.c index > > 3f380d87d..5ee7007f6 100644 > > --- a/transport-helper.c > > +++ b/transport-helper.c > > @@ -1212,7 +1212,7 @@ static int udt_do_read(struct > unidirectional_transfer *t) > > return 0; /* No space for more. */ > > > > transfer_debug("%s is readable", t->src_name); > > - bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); > > + bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse); > > if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN && > > errno != EINTR) { > > error_errno("read(%s) failed", t->src_name); > > Patch at https://public- > inbox.org/git/010f01d38a9e$a5c4f290$f14ed7b0$@nexbridge.com/ Just a subtle reminder that this particular change does not seem to have made it into 2.20.1. I've retrofitted xread into the NonStop port and will be re-submitting it with the remaining platform changes assuming the test suite runs at least as well as our last port at 2.17.0 (in progress as we speak). We are down to four variances from standard code: *git-compat-util.h referencing FLOSS headers needed on platform. *this one (xread instead of read in transport-helper.c) *config.mak.uname specific settings for NonStop not currently present. *t/Makefile - have to comment out all test-lint dependencies because of segfault at 2.20.1. (to be investigated but we don't submit this unless we find a good fix) Please stay tuned for patches. We are very much looking forward to having the two (or three) different NonStop hardware personalities supported without mods in the very near future. Our goal, assuming those patches are acceptable, is to move our build/test/distro into a Jenkins config that runs with minimal human involvement (a.k.a. me). Cheers, Randall -- Brief whoami: NonStop developer since approximately NonStop(2112884442) UNIX developer since approximately 421664400 -- In my real life, I talk too much.
RE: [Question] Complex textconv text
On December 13, 2018 10:08, I wrote: > I have a strange situation and need help with resolving funky characters in > .git/config. My situation is this: > > [diff "*.dat"] > textconv = enscribe-conv > --format=-a1\(A=-a1,-a16,-a32\|P=-a1,-a32,-a16\|=-a1,-d,a14\),-a224 > > Basically this is a formatter for diff so that I can show structured binary data. > The unquoted syntax of the format string is: > --format=-a1(A=-a1,-a16,-a32|P=-a1,-a32,-a16|=-a1,-d,a14),-a224 > > Content is not really important. The issue is that git is reporting fatal: > bad config line 2 in file .git/config when I escape the (, ), and | characters. I > get syntax errors otherwise from the shell running the textconv. I have tried - > -format="-a1(A=-a1,-a16,-a32|P=-a1,-a32,-a16|=-a1,-d,a14),-a224", to no > avail. How should I safely escape the characters in here? Easiest bypass was to wrap the mess above into a shell script and execute it that way, which works just fine. I'm not sure it's possible to escape all of the special characters. Seems like the safest approach anyway. Cheers, Randall
[Question] Complex textconv text
Hi all, I have a strange situation and need help with resolving funky characters in .git/config. My situation is this: [diff "*.dat"] textconv = enscribe-conv --format=-a1\(A=-a1,-a16,-a32\|P=-a1,-a32,-a16\|=-a1,-d,a14\),-a224 Basically this is a formatter for diff so that I can show structured binary data. The unquoted syntax of the format string is: --format=-a1(A=-a1,-a16,-a32|P=-a1,-a32,-a16|=-a1,-d,a14),-a224 Content is not really important. The issue is that git is reporting fatal: bad config line 2 in file .git/config when I escape the (, ), and | characters. I get syntax errors otherwise from the shell running the textconv. I have tried --format="-a1(A=-a1,-a16,-a32|P=-a1,-a32,-a16|=-a1,-d,a14),-a224", to no avail. How should I safely escape the characters in here? Thanks, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much.
RE: [RFC] git clean --local
On December 2, 2018 8:26, Ævar Arnfjörð Bjarmason wrote: > > On Sat, Dec 01 2018, Cameron Boehmer wrote: > > > 1) add a new flag > > -l, --local > > Do not consult git config --global core.excludesFile in > > determining what files git ignores. This is useful in conjunction with > > -x/-X to preserve user files while removing build artifacts. > > Or perhaps a general flag to ignore configuration would be useful for such > cases, see https://public- > inbox.org/git/87zhtqvm66@evledraar.gmail.com/ Would something like git clean --exclude=file-pattern work as a compromise notion? Files matching the pattern would not be cleaned regardless of .gitignore or their potential preciousness status long-term. Multiple repetitions of the --exclude option might be supportable. I could see that being somewhat useful in scripting. Cheers, Randall
RE: Git Tags
On November 29, 2018 6:56, Mateusz Loskot wrote: > On Thu, 29 Nov 2018 at 12:50, Stefanie Leisestreichler > wrote: > > > > git tag -a 0.9.0 > > git push origin master > > > > In my local repository, when I run "git tag" it is showing me "0.9.0". > > > > Then I did (on box B) > > git clone ssh://user@host:/path/project.git cd project git tag > > > > Now git tag is showing nothing. > > > > Why is the tag only available in my local repository? > > >From https://git-scm.com/book/en/v2/Git-Basics-Tagging > "By default, the git push command doesn’t transfer tags to remote servers. > You will have to explicitly push tags to a shared server after you have > created > them." git push --tags and git fetch --tags to be symmetrical Cheers, Randall -- Brief whoami: NonStop developer since approximately 2112884442 UNIX developer since approximately 421664400 -- In my real life, I talk too much.
RE: Cygwin Git with Windows paths
> -Original Message- > From: git-ow...@vger.kernel.org On Behalf Of > Junio C Hamano > Sent: November 18, 2018 19:07 > To: Torsten Bögershausen > Cc: Steven Penny ; git@vger.kernel.org > Subject: Re: Cygwin Git with Windows paths > > Torsten Bögershausen writes: > > > And it may even be that we need a special handling for the "\" to be > > treated as "/". > > I do not do Windows, but is_dir_sep() needs to be tweaked if you want to do > that. Heavy Cygwin user here. It is used in my environment for cross-compilation. Everything should be done using / separators in Cygwin, not \. So /cygdrive/c, /cygdrive/d always prefaces the path rather than C:\ or D:\, which won't parse. It is, essentially, a bash environment, including that git completions work properly. Backslash ends up doing what it would in bash.