[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Hurd port for gcc go PATCH 1-4(23)



Hello,

Svante Signell, on Sun 27 Nov 2016 17:33:52 +0100, wrote:
> > > Index: gcc-6-6.2.1-4.1/src/libgo/go/syscall/wait.c
> > > ===================================================================
> > > --- gcc-6-6.2.1-4.1.orig/src/libgo/go/syscall/wait.c
> > > +++ gcc-6-6.2.1-4.1/src/libgo/go/syscall/wait.c
> > > @@ -8,6 +8,9 @@
> > >     OS-independent.  */
> > >  
> > >  #include <stdint.h>
> > > +#ifndef WCONTINUED
> > > +#define WCONTINUED 0
> > > +#endif
> > >  #include <sys/wait.h>
> > >  
> > >  #include "runtime.h"
> > 
> > That looks odd at best: WCONTINUED can't be defined at this place
> > anyway, so it'll get defined to 0. Fortunately the sys/wait.h
> > definition
> > would override this. But then wait.c will define something which does
> > not make sense since WCONTINUED is not implemented. Better use #ifdef
> > WCONTINUED inside the Continued function, to always return 0 when
> > it's
> > not defined.
> > 
> 
> I've been implementing that version too, with no visible differences.

"no visible differences" doesn't mean there is none :)

> But as you wish, an updated patch is attached.

 _Bool
 Continued (uint32_t *w)
 {
+#ifndef WCONTINUED
+  *w = 0;
+  return 0;
+#else
   return WIFCONTINUED (*w) != 0;
+#endif
 }

Err, recheck the semantic of WCONTINUED again, it doesn't modify its
parameter, it just tests its value.

Do as I said, just return 0.



> This is for upstream to decide.

I'm just afraid they'd just frown on the code submission and not take
the time to explain how they want it to look like if we don't raise the
discussion ourselves.



> > And
> > src_libgo_go_syscall_syscall_gnu_test.go: New file:
> >   Define Type and Whence as 32bit in syscall.Flock_t
> > 
> > Again, you'll probably have to discuss with upstream to see how they
> > prefer to make it configurable rather than forking the whole file.
> >
> 
> I tried to patch the syscall_unix_test.go file, but did not succeed. 
> Definitely if runtime.GOOS == "GNU" ... else ... or switch runtime.GOOS
> ... does not work. The compiler sees all code and complains, also the
> else part of the code :( Therefore I created a new file.

Then ask upstream how they think it can and should be done.



> > > @@ -4431,7 +4505,7 @@ mostlyclean-local:
> > >  	find . -name '*-testsum' -print | xargs rm -f
> > >  	find . -name '*-testlog' -print | xargs rm -f
> > >  
> > > -CLEANFILES = *.go *.gox goc2c *.c s-version libgo.sum libgo.log
> > > +CLEANFILES = *.go *.gox goc2c *.c s-* libgo.sum libgo.log
> > 
> > This seems unrelated?
> > 
> No, this is not unrelated: With this patch you can
> make -C build/i686-gnu/libgo clean
> make -C build/i686-gnu/libgo
> to rebuild libgo. Otherwise libcalls.go is not regenerated,
> mksysinfo.sh is not run etc. 

That's still unrelated to the matter here: porting go to GNU/Hurd.  It
looks like a bug fix which is completely independant from GNU/Hurd.

> > Svante Signell, on Fri 25 Nov 2016 21:04:58 +0100, wrote:
> > > * src_libgo_runtime_netpoll.goc.diff: Fix restricted word bug.
> > >   Rename variable errno to errno1.
> > > * src_libgo_go_os_os_test.go.diff: Allow EFBIG return code to big
> > > seeks.
> > > * src_libgo_go_syscall_syscall_gnu_test.go: New file:
> > >   Define Type and Whence as 32bit in syscall.Flock_t
> > > * src_libgo_testsuite_gotest.diff: Remove comm option for ps -o.
> > > * add-gnu-to-libgo-headers.diff:
> > >   Add gnu to +build entry in file headers included in the build.
> > >   FIXME:  <add remaining headers>
> > 
> > I can't find these in the patches you sent.
> 
> It is in the last mail: patch 19-23(23)

Ah, sorry, mutt had pasted the content of the attached files without
putting the file names.

> > > Index: gcc-6-6.2.1-4.1/src/libgo/testsuite/gotest
> > > ===================================================================
> > > --- gcc-6-6.2.1-4.1.orig/src/libgo/testsuite/gotest
> > > +++ gcc-6-6.2.1-4.1/src/libgo/testsuite/gotest
> > > @@ -618,7 +618,11 @@ xno)
> > >  		wait $pid
> > >  		status=$?
> > >  		if ! test -f gotest-timeout; then
> > > -		    sleeppid=`ps -o pid,ppid,comm | grep "
> > > $alarmpid " | grep sleep | sed -e 's/ *\([0-9]*\) .*$/\1/'`
> > > +		    if test "$goos" = "gnu"; then
> > > +			sleeppid=`ps -o pid,ppid | grep "
> > > $alarmpid " | grep sleep | sed -e 's/ *\([0-9]*\) .*$/\1/'`
> > > +		    else
> > > +			sleeppid=`ps -o pid,ppid,comm | grep "
> > > $alarmpid " | grep sleep | sed -e 's/ *\([0-9]*\) .*$/\1/'`
> > > +		    fi
> > >  		    kill $alarmpid
> > >  		    wait $alarmpid
> > >  		    if test "$sleeppid" != ""; then
> > 
> > 
> > We could rather just implement the comm field in ps, AIUI it's just
> > an
> > alias for the command field.
> 
> Your choice. When implemented this patch wouldn't bee needed.

Then please do implement it :)

Samuel


Reply to: