| Message ID | 1500053986-38406-4-git-send-email-yamamoto@ovn.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On Sat, Jul 15, 2017 at 02:39:44AM +0900, YAMAMOTO Takashi wrote: > SUS says: > When jobs reports the termination status of a job, > the shell removes its process ID from the list of those > "known in the current shell execution environment"; > > With NetBSD /bin/sh, the list involves zombie processes and > ends up with "can not fork" during test runs. Can you squash the new patch into the existing one? And then we can apply it unconditionally? It isn't great to have conditional code here, especially since the testsuite is distributed. Did you report this problem to the Autoconf maintainers? It would be best to fix it in upstream Autoconf.
Hi, 2017/07/15 6:55 "Ben Pfaff" <blp@ovn.org>: On Sat, Jul 15, 2017 at 02:39:44AM +0900, YAMAMOTO Takashi wrote: > SUS says: > When jobs reports the termination status of a job, > the shell removes its process ID from the list of those > "known in the current shell execution environment"; > > With NetBSD /bin/sh, the list involves zombie processes and > ends up with "can not fork" during test runs. Can you squash the new patch into the existing one? And then we can apply it unconditionally? It isn't great to have conditional code here, especially since the testsuite is distributed. I thought the existing windows patch was conditional for some reasons. Do you remember the reason? I'll look at it closely to see if it's ok for non windows platforms. Did you report this problem to the Autoconf maintainers? It would be best to fix it in upstream Autoconf. Not yet. I will.
On Sat, Jul 15, 2017 at 11:23:04AM +0900, Takashi YAMAMOTO wrote: > Hi, > > 2017/07/15 6:55 "Ben Pfaff" <blp@ovn.org>: > > On Sat, Jul 15, 2017 at 02:39:44AM +0900, YAMAMOTO Takashi wrote: > > SUS says: > > When jobs reports the termination status of a job, > > the shell removes its process ID from the list of those > > "known in the current shell execution environment"; > > > > With NetBSD /bin/sh, the list involves zombie processes and > > ends up with "can not fork" during test runs. > > Can you squash the new patch into the existing one? And then we can > apply it unconditionally? It isn't great to have conditional code here, > especially since the testsuite is distributed. > > > I thought the existing windows patch was conditional for some reasons. Do > you remember the reason? Thanks for reminding me of that. It's because the patch only works with some Autoconf versions: commit 83e09b5dfa35b95e9995713bdfcb9a27f9b4ed7f Author: Gurucharan Shetty <guru@ovn.org> Thu Apr 23 07:13:04 2015 Committer: Gurucharan Shetty <guru@ovn.org> Thu Apr 23 10:49:13 2015 testsuite: Don't apply the testsuite.patch on non-Windows platforms. On CentOS machines which use autoconf version 2.63, the patch application would fail. Reported-by: Ian Stokes <ian.stokes@intel.com> Tested-by: Ian Stokes <ian.stokes@intel.com> Signed-off-by: Gurucharan Shetty <gshetty@nicira.com> This is probably true of the new patch that you are providing, too.
diff --git a/tests/automake.mk b/tests/automake.mk index 3118bbc..fef5db9 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -12,7 +12,8 @@ EXTRA_DIST += \ tests/atlocal.in \ $(srcdir)/package.m4 \ $(srcdir)/tests/testsuite \ - $(srcdir)/tests/testsuite.patch + $(srcdir)/tests/testsuite.patch \ + $(srcdir)/tests/testsuite.unix.patch COMMON_MACROS_AT = \ tests/ovsdb-macros.at \ @@ -125,7 +126,11 @@ SYSTEM_OFFLOADS_TESTSUITE_AT = \ check_SCRIPTS += tests/atlocal TESTSUITE = $(srcdir)/tests/testsuite +if WIN32 TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch +else +TESTSUITE_PATCH = $(srcdir)/tests/testsuite.unix.patch +endif SYSTEM_KMOD_TESTSUITE = $(srcdir)/tests/system-kmod-testsuite SYSTEM_USERSPACE_TESTSUITE = $(srcdir)/tests/system-userspace-testsuite SYSTEM_OFFLOADS_TESTSUITE = $(srcdir)/tests/system-offloads-testsuite @@ -249,16 +254,10 @@ clean-local: AUTOTEST = $(AUTOM4TE) --language=autotest -if WIN32 $(TESTSUITE): package.m4 $(TESTSUITE_AT) $(COMMON_MACROS_AT) $(TESTSUITE_PATCH) $(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o testsuite.tmp $@.at patch -p0 testsuite.tmp $(TESTSUITE_PATCH) $(AM_V_at)mv testsuite.tmp $@ -else -$(TESTSUITE): package.m4 $(TESTSUITE_AT) $(COMMON_MACROS_AT) - $(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at - $(AM_V_at)mv $@.tmp $@ -endif $(SYSTEM_KMOD_TESTSUITE): package.m4 $(SYSTEM_TESTSUITE_AT) $(SYSTEM_KMOD_TESTSUITE_AT) $(COMMON_MACROS_AT) $(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at diff --git a/tests/testsuite.unix.patch b/tests/testsuite.unix.patch new file mode 100644 index 0000000..f56daf0 --- /dev/null +++ b/tests/testsuite.unix.patch @@ -0,0 +1,10 @@ +--- testsuite 2017-07-14 13:41:25.000000000 +0000 ++++ testsuite 2017-07-14 13:44:04.000000000 +0000 +@@ -5959,6 +5959,7 @@ $as_echo "$as_me: WARNING: unable to par + echo >&7 + ) & + $at_job_control_off ++ jobs > /dev/null || : # drain the list of jobs + if $at_first; then + at_first=false + exec 6<"$at_job_fifo" 7>"$at_job_fifo"
SUS says: When jobs reports the termination status of a job, the shell removes its process ID from the list of those "known in the current shell execution environment"; With NetBSD /bin/sh, the list involves zombie processes and ends up with "can not fork" during test runs. --- tests/automake.mk | 13 ++++++------- tests/testsuite.unix.patch | 10 ++++++++++ 2 files changed, 16 insertions(+), 7 deletions(-) create mode 100644 tests/testsuite.unix.patch