Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Add u-root and tmpfs to the bootstrap #970

Merged
merged 1 commit into from
Aug 20, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions bootstrap.sh
Original file line number Diff line number Diff line change
@@ -1,10 +1,22 @@
#!/bin/sh

set -e

export GOBIN=$(pwd)/$(go env GOHOSTOS)_$(go env GOHOSTARCH)/bin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's set in this script, for this script and its children. That's not global, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

echo GOBIN is now $GOBIN

echo Building the build tool...
GO111MODULE=on go get ./util/src/harvey/cmd/...

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=on go get github.com/u-root/u-root

echo Fetch harvey-os.org commands and build them into $GOBIN
GO111MODULE=on go get harvey-os.org/cmd/...

echo FIXME -- once we get more architectures, this needs to be done in sys/src/cmds/build.json
echo Build tmpfs command into amd64 plan 9 bin
GO111MODULE=on GOOS=plan9 GOARCH=amd64 go build -o amd64/bin/tmpfs harvey-os.org/cmd/tmpfs

# this will make booting a VM easier
mkdir -p tmp
Expand All @@ -15,12 +27,16 @@ export ARCH=amd64
# You also need to export your C compiler flavor (gcc, clang, gcc-7...)
export CC=gcc
# And build:
./util/build
$GOBIN/build
# See \`build -h' for more information on the build tool.

To enable access to files, create a harvey and none user:
sudo useradd harvey
sudo useradd none

none is only required for drawterm/cpu access

Also:
export HARVEY=$(pwd)
add $GOBIN to your path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you add $GOBIN to your path, why are you calling $GOBIN/build on line 30?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because I did not add GOBIN to the path, the script just recommends it?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup. I was reviewing it using the mobile app missed the folded lines. my bad folks. I apologize

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those folded things can be hell.

Thanks for the reviews everyone, I did set modules on, what else needs to change?

EOF