Code review - Issue 152138: code review 152138: Build changes to support work on the BSDs.https://codereview.appspot.com/2023-04-23T15:33:30+00:00rietveld
Message from unknown
2009-11-14T04:40:38+00:00dhourn:md5:e6cb7db17e1b10248e3da4f36ccf79e1
Message from unknown
2009-11-14T05:41:08+00:00dhourn:md5:dab5628925b7446cc178f9fa68dae27f
Message from unknown
2009-11-14T05:43:55+00:00dhourn:md5:9111745e7f3e2b06baa998299174fd33
Message from unknown
2009-11-14T05:50:39+00:00dhourn:md5:7f0175b11429fe879ff4b17da8e130bb
Message from devon.odell@gmail.com
2009-11-14T05:51:27+00:00dhourn:md5:d285458d4b1eaa69378634148ba1cb9e
Hello rsc,
I'd like you to review the following change.
Message from rsc@golang.org
2009-11-14T06:18:53+00:00rscurn:md5:bc4c6618ba1ab2b7ae5ccc5225176534
Thanks for doing this!
Some comments on the integration aspects.
I haven't looked carefully at the FreeBSD code.
http://codereview.appspot.com/152138/diff/1061/1066
File src/all.bash (right):
http://codereview.appspot.com/152138/diff/1061/1066#newcode7
src/all.bash:7: sh make.bash
bash please.
next line too
http://codereview.appspot.com/152138/diff/1061/1067
File src/clean.bash (right):
http://codereview.appspot.com/152138/diff/1061/1067#newcode14
src/clean.bash:14: /usr/bin/env bash clean.bash
no need for /usr/bin/env here, only in #!
http://codereview.appspot.com/152138/diff/1061/1067#newcode16
src/clean.bash:16: sanemake clean
what shall we do about sanemake?
http://codereview.appspot.com/152138/diff/1061/1070
File src/cmd/8a/Makefile (right):
http://codereview.appspot.com/152138/diff/1061/1070#newcode31
src/cmd/8a/Makefile:31: yacc $(YFLAGS) $(YFILES)
bison -y please
http://codereview.appspot.com/152138/diff/1061/1071
File src/cmd/8l/asm.c (right):
http://codereview.appspot.com/152138/diff/1061/1071#newcode39
src/cmd/8l/asm.c:39: char dragonflydynld[] = "/usr/libexec/ld-elf.so.2";
why does this say dragonflydynld and the other said freebsd?
http://codereview.appspot.com/152138/diff/1061/1072
File src/cmd/8l/obj.c (right):
http://codereview.appspot.com/152138/diff/1061/1072#newcode52
src/cmd/8l/obj.c:52: * -H5 -T0x80100020 -R4096 is ELF32
tabs after *
http://codereview.appspot.com/152138/diff/1061/1072#newcode56
src/cmd/8l/obj.c:56: * -HD -Tx -Rx is DragonFly BSD elf-exec
H13 (or H0xD but that's longer and weird)
presumably
http://codereview.appspot.com/152138/diff/1061/1073
File src/cmd/cc/Makefile (right):
http://codereview.appspot.com/152138/diff/1061/1073#newcode39
src/cmd/cc/Makefile:39: yacc $(YFLAGS) $(YFILES)
bison please
http://codereview.appspot.com/152138/diff/1061/1076
File src/cmd/gc/Makefile (right):
http://codereview.appspot.com/152138/diff/1061/1076#newcode52
src/cmd/gc/Makefile:52: yacc $(YFLAGS) $(YFILES)
bison please
http://codereview.appspot.com/152138/diff/1061/1079
File src/cmd/make.bash (right):
http://codereview.appspot.com/152138/diff/1061/1079#newcode8
src/cmd/make.bash:8: sh clean.bash
bash
http://codereview.appspot.com/152138/diff/1061/1079#newcode17
src/cmd/make.bash:17: sh mkenam
bash
http://codereview.appspot.com/152138/diff/1061/1090
File src/pkg/exp/eval/test.bash (right):
http://codereview.appspot.com/152138/diff/1061/1090#newcode12
src/pkg/exp/eval/test.bash:12: make
sanemake?
http://codereview.appspot.com/152138/diff/1061/1104
File src/pkg/syscall/mkall.sh (right):
http://codereview.appspot.com/152138/diff/1061/1104#newcode1
src/pkg/syscall/mkall.sh:1: #!/bin/sh
/usr/bin/env bash (why not)
http://codereview.appspot.com/152138/diff/1061/1104#newcode149
src/pkg/syscall/mkall.sh:149: echo "$mkerrors >zerrors_$GOOSARCH.go"
put |gofmt back
http://codereview.appspot.com/152138/diff/1061/1116
File src/sanemake.bash (right):
http://codereview.appspot.com/152138/diff/1061/1116#newcode9
src/sanemake.bash:9: # Figure out which make to run; this is set by make.bash.
I don't understand why this script is so complicated.
Shouldn't it be one of
#!/bin/sh
exec make "$@"
#!/bin/sh
exec gmake "$@"
and then be set by make.bash appropriately?
Instead of "sanemake", I'd rather call it "gomake"
to be clear that we installed it.
http://codereview.appspot.com/152138/diff/1061/1116#newcode15
src/sanemake.bash:15: # If this is a 64-bit machine, compile 64-bit versions of
This comment looks wrong.
Message from unknown
2009-11-14T14:17:40+00:00dhourn:md5:d1e197d26f8ae0f4a1471b6edfd3cd99
Message from unknown
2009-11-14T14:21:30+00:00dhourn:md5:1f68052f821859fa5b15281b198ae078
Message from unknown
2009-11-14T14:28:33+00:00dhourn:md5:b082ab1735c36924c571a09092c450b4
Message from devon.odell@gmail.com
2009-11-14T14:29:11+00:00dhourn:md5:aa9b68da8cd8c806142288c3487cc512
Hello rsc,
I'd like you to review the following change.
Message from devon.odell@gmail.com
2009-11-14T14:31:15+00:00dhourn:md5:135648c02952eef273fe11b52db73a62
On 2009/11/14 06:18:53, rsc wrote:
> Thanks for doing this!
No problem; it's fun!
> Some comments on the integration aspects.
> I haven't looked carefully at the FreeBSD code.
I think I've taken care of all these points in the latest revision. Sorry for all the noise in this patchset; I'm still trying to figure out the codereview extensions to some extent.
I got the stuff through gofmt, but, though it compiles, it doesn't work properly in FreeBSD. Best guess is locking stuff is still broken and / or no working channels. Working on getting gofmt to work next.
--dho
Message from rsc@golang.org
2009-11-14T16:00:30+00:00rscurn:md5:c79ab85994d13e9520d6f5ccd3b1823f
I suggest deleting this file and just writing it directly from the script.
Message from rsc@golang.org
2009-11-14T16:08:38+00:00rscurn:md5:13db7f55b015397b3ad5fa909730e0ba
Some comments about make.
Try running "hg sync --local" (or just hg sync
if you want to pull changes in too) and it will
drop any files with no actual changes from the change.
http://codereview.appspot.com/152138/diff/1336/1355
File src/make.bash (right):
http://codereview.appspot.com/152138/diff/1336/1355#newcode33
src/make.bash:33: MAKE=make
This isn't really a function of $GOOS, since you
might be building nacl binaries on freebsd,
or even linux binaries.
Instead, should probably do something like
MAKE=make
if ! make --version 2>/dev/null | grep 'GNU Make' >/dev/null; then
MAKE=gmake
fi
This should probably be after quietgcc and after the check of $GOBIN.
http://codereview.appspot.com/152138/diff/1336/1355#newcode48
src/make.bash:48: rm -f $GOBIN/gomake
Move block after quietgcc check.
http://codereview.appspot.com/152138/diff/1336/1355#newcode49
src/make.bash:49: MAKE=${MAKE:-make}
(echo '#!/bin/sh'; echo 'exec '$MAKE' "$@"') >$GOBIN/gomake
Message from rsc@golang.org
2009-11-14T16:12:48+00:00rscurn:md5:e29ea36dbec41ce78e332185261d6596
I'm happy with the build fixes and they seem like a good logical
piece to split off from the FreeBSD stuff.
I suggest running "hg change" to delete the freebsd-specific
files from the list and edit the change description to say that
it's build fixes for running on BSD machines.
Then run "hg diff @152138" to double-check for freebsd-specific files,
and run hg upload. I'll apply that one and we can keep iterating on
the FreeBSD stuff in a new CL.
hg sync --local will automatically take out any files that
are no longer modified.
Message from rsc@golang.org
2009-11-14T16:27:12+00:00rscurn:md5:4189d5db9d22e7a2cc0e17686b80cc59
http://codereview.appspot.com/152138/diff/1336/1372
File src/pkg/runtime/freebsd/thread.c (right):
http://codereview.appspot.com/152138/diff/1336/1372#newcode14
src/pkg/runtime/freebsd/thread.c:14: if(psema != 0) return;
split return onto next line
s/psema/*psema/
http://codereview.appspot.com/152138/diff/1336/1372#newcode15
src/pkg/runtime/freebsd/thread.c:15: if(ksem_init(&sema, 1) != 0) return;
split return onto next line
http://codereview.appspot.com/152138/diff/1336/1372#newcode47
src/pkg/runtime/freebsd/thread.c:47: if(l->sema == 0) initsema(&l->sema);
split onto next line
http://codereview.appspot.com/152138/diff/1336/1372#newcode59
src/pkg/runtime/freebsd/thread.c:59: ksem_destroy(l->sema);
ksem_post, probably
Message from unknown
2009-11-14T17:43:10+00:00dhourn:md5:b1172fa774b0b0d16139427e5522fb8e
Message from unknown
2009-11-14T17:45:41+00:00dhourn:md5:393dacb49970a4434fab844bc8a6fc80
Message from unknown
2009-11-14T17:49:18+00:00dhourn:md5:29b27134c1eb85608e2725d79fa5829f
Message from devon.odell@gmail.com
2009-11-14T17:49:37+00:00dhourn:md5:337334d287fa2ef8a5cf102af638791f
Hello rsc,
I'd like you to review the following change.
Message from rsc@golang.org
2009-11-14T19:56:22+00:00rscurn:md5:128addd8748f0074d4facf0a1d058e72
Thanks, looks good.
cd $GOROOT/src
hg file -d 152138 cmd/cov/Makefile cmd/prof/Makefile libcgo/Makefile
Have you done the copyright stuff at
http://golang.org/doc/contribute.html#copyright?
http://codereview.appspot.com/152138/diff/436/1489
File src/cmd/cov/Makefile (right):
http://codereview.appspot.com/152138/diff/436/1489#newcode27
src/cmd/cov/Makefile:27: install-freebsd: install-default
This file doesn't belong in this CL.
http://codereview.appspot.com/152138/diff/436/1493
File src/cmd/prof/Makefile (right):
http://codereview.appspot.com/152138/diff/436/1493#newcode27
src/cmd/prof/Makefile:27: install-freebsd: install-default
This file doesn't belong in this CL.
http://codereview.appspot.com/152138/diff/436/1494
File src/libcgo/Makefile (right):
http://codereview.appspot.com/152138/diff/436/1494#newcode19
src/libcgo/Makefile:19: LDFLAGS_freebsd=-pthread -shared -lm
This change doesn't belong in this CL.
http://codereview.appspot.com/152138/diff/436/1496
File src/make.bash (right):
http://codereview.appspot.com/152138/diff/436/1496#newcode32
src/make.bash:32: darwin | linux | nacl | freebsd)
I'll take freebsd off the list when I submit this.
Message from unknown
2009-11-14T23:16:21+00:00dhourn:md5:50039967cd7d3bf4bcf7b385c1db67ff
Message from unknown
2009-11-14T23:16:52+00:00dhourn:md5:2e10379746e54bbe8985902149191b6a
Message from devon.odell@gmail.com
2009-11-14T23:17:05+00:00dhourn:md5:a3b76978b98ee8440014de9f88630be4
Hello rsc,
I'd like you to review the following change.
Message from rsc@golang.org
2009-11-14T23:29:11+00:00rscurn:md5:b5534c348c6e1da4464d44c7c5a0a837
*** Submitted as http://code.google.com/p/go/source/detail?r=692ac2ced6f7 ***
Build changes to support work on the BSDs.
This does still contain some FreeBSD-specific bits, but
it's a pain to do partial diffs.
R=rsc
http://codereview.appspot.com/152138
Committer: Russ Cox <rsc@golang.org>
Message from myexamples04@gmail.com
2023-04-23T15:31:49+00:00myexamples04urn:md5:9467de9527b81afeed47d17812c31421
Message from myexamples04@gmail.com
2023-04-23T15:33:30+00:00myexamples04urn:md5:64395a671ac69a5dd4ba347f7338cf30
Hello hello hello kopi dan milo