Add u-root and tmpfs to the bootstrap #970
Conversation
rminnich
force-pushed
the
bootstrap_uroot_tmpfs
branch
2 times, most recently
from
3cda2ee to
3b2956f
Compare
|
why is llvm.org such a clusterfuck? Should we just start our own GitHub repo with the build artifacts in it? |
|
@rminnich can you link the error messsage? I think when you force pushed, that log went away. or atleast I can't find it |
rminnich
force-pushed
the
bootstrap_uroot_tmpfs
branch
from
3b2956f to
0b3655d
Compare
bootstrap.sh
Outdated
| GOBIN="$(pwd)/util" go get -u harvey-os.org/cmd/ufs | ||
| GO111MODULE=on GOBIN="$(pwd)/util" go get ./util/src/harvey/cmd/... | ||
| echo Fetching u-root and building it... | ||
| GO111MODULE=off go get github.com/u-root/u-root |
There was a problem hiding this comment.
why are we turning off modules? GO111MODULE doesn't require the target package to have a .mod file.
If it's all the same to you I think we should use modules and pin specific versions of external packages.
There was a problem hiding this comment.
Also note that Harvey's go.mod can influence what version of external dependency is used. Which is why I suggest running go get inside a temporary directory that's not part of a module. See golang/go#40276
There was a problem hiding this comment.
But in this case that is precisely what we want, we always want a known and tested version of external packages to be downloaded and installed. I feel this is important since we don't have testing of u-root packages on a plan9 system. It's very likely that someone might break a u-root package and not even realize it's broken for plan9.
There was a problem hiding this comment.
Sorry, I meant dependency of u-root can be influenced by harvey's dependency when doing the minimum version selection. For example, u-root uses an older version golang.org/x/tools package when installed from the harvey module:
$ export GO111MODULE=on
$ cd $HARVEY
$ GOBIN=$(pwd)/util/ go get github.com/u-root/u-root@latest
...
$ go version -m util/u-root
util/u-root: go1.15
path github.com/u-root/u-root
mod github.com/u-root/u-root v6.0.0+incompatible h1:YqPGmRoRyYmeg17KIWFRSyVq6LX5T6GSzawyA6wG6EE=
dep github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
dep golang.org/x/sys v0.0.0-20190412213103-97732733099d h1:+R4KGOnez64A81RvjARKc4UT5/tI9ujCIVX+P5KiHuI=
dep golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 h1:5Beo0mZN8dRzgrMMkDp0jc8YXQKx9DiJ2k1dkvGsn5A=
$ mkdir /tmp/foo
$ cd /tmp/foo/
$ GOBIN=$(pwd) go get github.com/u-root/u-root@latest
...
$ go version -m u-root
u-root: go1.15
path github.com/u-root/u-root
mod github.com/u-root/u-root v6.0.0+incompatible h1:YqPGmRoRyYmeg17KIWFRSyVq6LX5T6GSzawyA6wG6EE=
dep github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
dep golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
dep golang.org/x/sys v0.0.0-20200819171115-d785dc25833f h1:KJuwZVtZBVzDmEDtB2zro9CXkD9O0dpCv4o2LHbQIAw=
dep golang.org/x/tools v0.0.0-20200820010801-b793a1359eac h1:DugppSxw0LSF8lcjaODPJZoDzq0ElTGskTst3ZaBkHI=
dep golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
There was a problem hiding this comment.
I don't think that's a problem for us, as golang/go#40276 states
The
@latestsuffix [...] implies that the command is time-dependent and not reproducible.
One of our goals was (is #319?) to make harvey builds reproducible.
I feel like using @latest in the bootstrap process would cause us to back slide in trying to achieve that goal. As you've demonstrated it's easy to verify what ever it is we're checking in is correct, if you don't think it's correct, update the mod file so it does the correct thing. It requires a bit more diligence on our part but I'd rather have that than go get @latest and have builds randomly break because some external dependency updated.
There was a problem hiding this comment.
u-root and modules still don't play well. If you've tested this with modules turned on and it works, I'm interested to hear it.
There was a problem hiding this comment.
did you try it first? I'd like to know that it works. Would be appreciated if you could try a thing before advocating changing that thing.
There was a problem hiding this comment.
yeah, I did.
case in point with with the modules turned off I get
GOBIN is now /home/sevki/src/test/harvey/linux_amd64/bin
Building the build tool...
Fetching u-root and building it...
Fetch harvey-os.org commands and build them into /home/sevki/src/test/harvey/linux_amd64/bin
# harvey-os.org/internal/ufs
../../../go/src/harvey-os.org/internal/ufs/filesystem.go:443:45: undefined: protocol.ListenerOpt
../../../go/src/harvey-os.org/internal/ufs/filesystem.go:443:69: undefined: protocol.Listener
../../../go/src/harvey-os.org/internal/ufs/filesystem.go:458:12: undefined: protocol.NewListener
because my $GOPATH had 511a25aa7f1542a74d755af15e02503a1d87c71a checked out which was broken and turning on go modules for all fixed the bootstrap script.
There was a problem hiding this comment.
thanks, good to know it worked for you, I just pushed it with modules on
| @@ -1,10 +1,22 @@ | |||
| #!/bin/sh | |||
|
|
|||
| set -e | |||
|
|
|||
| export GOBIN=$(pwd)/$(go env GOHOSTOS)_$(go env GOHOSTARCH)/bin | |||
There was a problem hiding this comment.
I don't think we should be setting globals that we haven't declared. this is going to confuse the hell out of people when their binaries start showing up under harvey/linux_amd64/bin, even after they stop playing with harvey.
IMHO HARVEYBIN would be better and wouldn't have as many side effects.
There was a problem hiding this comment.
it's set in this script, for this script and its children. That's not global, right?
There was a problem hiding this comment.
GOBIN is for use by the go command.
I'm a bit lost on the problem of setting a variable in a script that's inherited by its children. This is not a global. export is for a process and its children. I don't see the issue here.
There was a problem hiding this comment.
Do you mean that it might cause confusion at some point later on when extending the script @sevki? I.e., global from the script's perspective and all its child processes inheriting it?
There was a problem hiding this comment.
No, I screwed up. I was reviewing this on my phone and saw the entire change as a single block. I missed the folded lines.
|
|
||
| Also: | ||
| export HARVEY=$(pwd) | ||
| add $GOBIN to your path |
There was a problem hiding this comment.
if you add $GOBIN to your path, why are you calling $GOBIN/build on line 30?
There was a problem hiding this comment.
because I did not add GOBIN to the path, the script just recommends it?
There was a problem hiding this comment.
yea it's just a long cat-eof-style output comment - confused me at first as well when not looking closely 👀
starting here https://github.com/Harvey-OS/harvey/pull/970/files#diff-3af436e6760a0c5f1e3d16a3384853a8L12
which GitHub cuts off (you need to click the expander for the full context; can be a bit annoying here sometimes, and helpful in other cases)
There was a problem hiding this comment.
yup. I was reviewing it using the mobile app missed the folded lines. my bad folks. I apologize
There was a problem hiding this comment.
those folded things can be hell.
Thanks for the reviews everyone, I did set modules on, what else needs to change?
This ensures that we have u-root and tmpfs for inclusion in kernels. Build tools are now placed in $GOHOSTOS_$GOHOSTARCH/bin Signed-off-by: Ronald G Minnich <rminnich@gmail.com>
rminnich
force-pushed
the
bootstrap_uroot_tmpfs
branch
from
0b3655d to
5bc88f7
Compare
This ensures that we have u-root and tmpfs for inclusion in
kernels.
Signed-off-by: Ronald G Minnich rminnich@gmail.com