code review 183142: Replicate signal names from syscall in os/signal.

Replicate signal names from syscall in os/signal.

[dsymonds, 2010-01-07 06:31:16]

Hello rsc (cc: golang-dev@googlegroups.com),

I'd like you to review the following change.

[rsc, 2010-01-07 16:30:06]

good idea, thanks.
one more thing. add a line in the generated file saying

var _ = syscall.Syscall

to silence the imported and not used if there are no signals.

http://codereview.appspot.com/183142/diff/6/1012
File src/pkg/os/signal/Makefile (right):

http://codereview.appspot.com/183142/diff/6/1012#newcode10
src/pkg/os/signal/Makefile:10: unix_signals.go\
Might as well call it unix.go.  You're already in package signal.

http://codereview.appspot.com/183142/diff/6/1012#newcode12
src/pkg/os/signal/Makefile:12: CLEANFILES=unix_signals.go
probably CLEANFILES+=unix.go
(s/=/+=/) just in case

http://codereview.appspot.com/183142/diff/6/1012#newcode17
src/pkg/os/signal/Makefile:17: @echo "// Automatically generated. Do not edit!"
> $@
Please drop all the @.
It's important to see what's breaking, and echoing everything is the only way to
do that.

Also move this into a shell script, so that this is just

./mkunix $^ >$@ || rm -f $@

Then you're not limited to the silly one-line-per-command nonsense of Makefiles.

Having done that, make this warning look like the others in syscall:
same phrasing and a separate line giving the exact command line.

http://codereview.appspot.com/183142/diff/6/1012#newcode21
src/pkg/os/signal/Makefile:21: @gawk '/^[ \t]*SIG/ { print "\t" $$1 "\t=
UnixSignal(syscall." $$1 ")" }' $< >> $@
Please drop all the @.
It's important to see what's breaking, and echoing everything is the only way to
do that.

Please also move this into a shell script, so that this is just

./mkunix $^ >$@ || rm -f $@

Then you're not limited to the silly one-line-per-command nonsense of Makefiles.

In the script, please avoid gawk, which my Mac doesn't have.
Not everyone has awk either, but you can rely on sed:

sed -n '/^	SIG/ {
	s/ .*//
	s/SIG.*/& = UnixSignal(syscall.&)/p
}' zerrors_${GOOS}_${GOARCH}.go

[dsymonds, 2010-01-07 22:58:17]

http://codereview.appspot.com/183142/diff/6/1012
File src/pkg/os/signal/Makefile (right):

http://codereview.appspot.com/183142/diff/6/1012#newcode10
src/pkg/os/signal/Makefile:10: unix_signals.go\
On 2010/01/07 16:30:06, rsc wrote:
> Might as well call it unix.go.  You're already in package signal.
> 

Done.

http://codereview.appspot.com/183142/diff/6/1012#newcode12
src/pkg/os/signal/Makefile:12: CLEANFILES=unix_signals.go
On 2010/01/07 16:30:06, rsc wrote:
> probably CLEANFILES+=unix.go
> (s/=/+=/) just in case
> 

Done.

http://codereview.appspot.com/183142/diff/6/1012#newcode17
src/pkg/os/signal/Makefile:17: @echo "// Automatically generated. Do not edit!"
> $@
On 2010/01/07 16:30:06, rsc wrote:
> Please drop all the @.
> It's important to see what's breaking, and echoing everything is the only way
to
> do that.
> 
> Also move this into a shell script, so that this is just
> 
> ./mkunix $^ >$@ || rm -f $@
> 
> Then you're not limited to the silly one-line-per-command nonsense of
Makefiles.
> 
> Having done that, make this warning look like the others in syscall:
> same phrasing and a separate line giving the exact command line.
> 

Done.

http://codereview.appspot.com/183142/diff/6/1012#newcode21
src/pkg/os/signal/Makefile:21: @gawk '/^[ \t]*SIG/ { print "\t" $$1 "\t=
UnixSignal(syscall." $$1 ")" }' $< >> $@
On 2010/01/07 16:30:06, rsc wrote:
> Please drop all the @.
> It's important to see what's breaking, and echoing everything is the only way
to
> do that.
> 
> Please also move this into a shell script, so that this is just
> 
> ./mkunix $^ >$@ || rm -f $@
> 
> Then you're not limited to the silly one-line-per-command nonsense of
Makefiles.
> 
> In the script, please avoid gawk, which my Mac doesn't have.
> Not everyone has awk either, but you can rely on sed:
> 
> sed -n '/^	SIG/ {
> 	s/ .*//
> 	s/SIG.*/& = UnixSignal(syscall.&)/p
> }' zerrors_${GOOS}_${GOARCH}.go
> 
> 

Done.

[rsc, 2010-01-08 08:03:08]

LGTM

[rsc, 2010-01-08 08:19:47]

http://codereview.appspot.com/183142/diff/1018/1021
File src/pkg/os/signal/mkunix.sh (right):

http://codereview.appspot.com/183142/diff/1018/1021#newcode22
src/pkg/os/signal/mkunix.sh:22: sed -n 's/^[ \t]*\(SIG[A-Z0-9]\+\).*/\t\1\t=
UnixSignal(syscall.\1)/p' "$1"
\+ is a GNUism.
please change [A-Z0-9]+ to [A-Z0-9][A-Z0-9]*
so that it works on BSD sed too.

\t is also a GNUism.
use literal tab characters
(in the replacement, spaces would be fine)

[dsymonds, 2010-01-08 11:14:34]

http://codereview.appspot.com/183142/diff/1018/1021
File src/pkg/os/signal/mkunix.sh (right):

http://codereview.appspot.com/183142/diff/1018/1021#newcode22
src/pkg/os/signal/mkunix.sh:22: sed -n 's/^[ \t]*\(SIG[A-Z0-9]\+\).*/\t\1\t=
UnixSignal(syscall.\1)/p' "$1"
On 2010/01/08 08:19:47, rsc wrote:
> \+ is a GNUism.
> please change [A-Z0-9]+ to [A-Z0-9][A-Z0-9]*
> so that it works on BSD sed too.
> 
> \t is also a GNUism.
> use literal tab characters
> (in the replacement, spaces would be fine)
> 

Done.

[dsymonds, 2010-01-11 04:44:54]

ping.

[rsc, 2010-01-11 19:23:52]

*** Submitted as http://code.google.com/p/go/source/detail?r=88aa77c7b9e3 ***

Replicate signal names from syscall in os/signal.

R=rsc
CC=golang-dev
http://codereview.appspot.com/183142

Committer: Russ Cox <rsc@golang.org>