Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: build armhf on GNU/Linux #1482

Merged
merged 1 commit into from Aug 28, 2017
Merged

chore: build armhf on GNU/Linux #1482

merged 1 commit into from Aug 28, 2017

Conversation

jviotti
Copy link
Contributor

@jviotti jviotti commented May 30, 2017

This commit makes use of the resin/armv7hf-debian Docker image to
test and generate armhf builds.

We needed to add a slash before build in .gitignore given that git
was refusing to include any changes on scripts/build otherwise.

Signed-off-by: Juan Cruz Viotti jviotti@openmailbox.org

@lurch
Copy link
Contributor

lurch commented May 31, 2017

Could you add -t docker support to scripts/build/architecture-convert.sh and then update scripts/build/docker/run-command.sh to use it?

I've not tested myself yet, but does changing armv7l to armv7hf in various places now mean that Etcher won't build so easily on a RPi3 as it did before? (which isn't something we officially support yet, but apparently some users have tried it and found it works)

We needed to add a slash before build in .gitignore given that git was refusing to include any changes on scripts/build otherwise.

Lol, good catch!

EDIT: Once this is all working, should we also start publishing armv7hf builds to bintray?

@jviotti
Copy link
Contributor Author

jviotti commented May 31, 2017

Could you add -t docker support to scripts/build/architecture-convert.sh and then update scripts/build/docker/run-command.sh to use it?

Done!

I've not tested myself yet, but does changing armv7l to armv7hf in various places now mean that Etcher won't build so easily on a RPi3 as it did before? (which isn't something we officially support yet, but apparently some users have tried it and found it works)

This whole ARM architecture mess confuses me, but as far as I understand, both armv7l and armv7hf builds are compatible with each other. In fact, Debian only seems to support armhf (https://wiki.debian.org/SupportedArchitectures), so we'd have to coerce both to that anyway.

@jviotti
Copy link
Contributor Author

jviotti commented May 31, 2017

EDIT: Once this is all working, should we also start publishing armv7hf builds to bintray?

Yeah, let's try to sort out snapshot Bintray builds first.

@lurch
Copy link
Contributor

lurch commented May 31, 2017

This whole ARM architecture mess confuses me

...and lots of other people too! ;-)

In fact, Debian only seems to support armhf

https://wiki.debian.org/ArmHardFloatPort says "Currently the Debian armhf port requires at least an ARMv7 CPU with Thumb-2 and VFP3D16." so I guess the armhf in Debian is actually armv7hf.

I'm not sure what armv7l indicates though :-S

@jviotti
Copy link
Contributor Author

jviotti commented May 31, 2017

Once we get this running, I'll give the compiled package a go in an armv7l Pi and we'll see how that goes.

IMAGE_ID="etcher-build-$ARGV_ARCHITECTURE"
DOCKER_ARCHITECTURE=$(./scripts/build/architecture-convert.sh -r "$ARGV_ARCHITECTURE" -t docker)
DOCKERFILE="$HERE/Dockerfile-$DOCKER_ARCHITECTURE"
IMAGE_ID="etcher-build-$DOCKER_ARCHITECTURE"
Copy link
Contributor

Choose a reason for hiding this comment

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

Much nicer 🎉
Although does changing IMAGE_ID from etcher-build-$ARGV_ARCHITECTURE to etcher-build-$DOCKER_ARCHITECTURE mean that any existing Docker images will become invalidated / redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just on your build environment, so that's fine, they will get re-created the next time.

@jviotti jviotti force-pushed the armv7hf-builds branch 2 times, most recently from 312d5bd to d87baee Compare May 31, 2017 14:58
@jviotti jviotti changed the title chore: build armv7hf on GNU/Linux chore: build armhf on GNU/Linux May 31, 2017
Makefile Outdated
@@ -73,7 +73,7 @@ else
HOST_ARCH = x86
endif
ifeq ($(shell uname -m),armv7l)
HOST_ARCH = armv7l
HOST_ARCH = armhf
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO we should keep this 'top-level' ARCH string as armv7hf, as that's much more specific, and then convert it as necessary inside the other scripts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought about that, however I think it'd be even more confusing that various armv7 similar architectures (like armv7l and armv7hf) generate the same armhf package. What do you think?

Since they all result in the same generic architecture, maybe its worth to convey that very clearly from the start?

Copy link
Contributor

@lurch lurch Jun 1, 2017

Choose a reason for hiding this comment

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

I think "generate the same armhf package" is an OS-specific assumption. E.g. just because armv7hf actually maps to armhf on Debian, there might be separate armv6hf and armv7hf packages on some other OS. So IMHO we should keep the Makefile as specific as possible, and then let architecture-convert.sh map that to whatever arch-strings different systems / platforms / OSes / tools need, as necessary.

EDIT: but yeah, I agree that we should probably treat armv7hf and armv7l as equivalent - I've asked for more info about it in flowdock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. Check my latest changes!

@@ -51,17 +51,25 @@ if [ "$ARGV_TYPE" == "node" ]; then
RESULT=ia32
elif [ "$ARGV_ARCHITECTURE" == "x64" ]; then
RESULT=x64
elif [ "$ARGV_ARCHITECTURE" == "armv7l" ]; then
elif [ "$ARGV_ARCHITECTURE" == "armhf" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be armv7hf ;-)

Copy link
Contributor

Choose a reason for hiding this comment

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

It might also be worth this script checking that the supplied $ARGV_ARCHITECTURE is in the list [x86, x64, armv7hf] of 'recognised arches' before then checking the $ARGV_TYPE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I missed that one!

It might also be worth this script checking that the supplied $ARGV_ARCHITECTURE is in the list [x86, x64, armv7hf] of 'recognised arches' before then checking the $ARGV_TYPE?

What would be the difference? Unless I'm missing something, if the architecture is not set by the specified type, then the script will complain when it finds out that RESULT is not defined.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it's just a "belt and braces" approach, but you're right in that it's probably not necessary. As long as we're always sure that the type-blocks are always checking for the identical ARCH strings ;)

@alexandrosm
Copy link
Contributor

alexandrosm commented Jun 1, 2017 via email

@jviotti
Copy link
Contributor Author

jviotti commented Jun 1, 2017 via email

@alexandrosm
Copy link
Contributor

alexandrosm commented Jun 1, 2017 via email

@jviotti jviotti force-pushed the armv7hf-builds branch 3 times, most recently from 90dd633 to 3403c8c Compare June 10, 2017 17:40
@jviotti
Copy link
Contributor Author

jviotti commented Jun 12, 2017

@petrosagg @nghiant2710 Sorry to bother again, but can someone help me out with this? The builds don't work, I have no clue why, and my Docker knowledge is far from enough to debug this issue.

The Travis CI builds just silently stop without any errors, and I get some weird Go stacktraces on misc1.dev.resin.io.

@lurch
Copy link
Contributor

lurch commented Jun 12, 2017

The Travis CI builds just silently stop without any errors

Nope, it says Timeout (20 minutes) reached. Terminating "./scripts/ci/install.sh -o linux -r armv7hf"
Which is obviously because the resin base-image being used is so minimal, that installing all the dependencies we require is just taking too long (and of course everything is being run through qemu so that'll be slowing things down too).
Perhaps we could get a resin/armv7hf-debian-qemu-with-etcher-deps:latest ;-)

I get some weird Go stacktraces on misc1.dev.resin.io.

Maybe that's because the docker on misc1 has these qemu patches already built-in, and so gets confused when you try doing the 'manual workaround' for non-patched docker? (just a guess!)

@jviotti
Copy link
Contributor Author

jviotti commented Jun 12, 2017

Nope, it says Timeout (20 minutes) reached. Terminating "./scripts/ci/install.sh -o linux -r armv7hf"

Oh, good observation. I increased the timeout limit to 40 minutes just to see if another error is thrown, but we should investigate why it takes so long, since that's definitely not expected (it takes very little time on misc1 with the non qemu base image).

@lurch
Copy link
Contributor

lurch commented Jun 12, 2017

"(and of course everything is being run through qemu so that'll be slowing things down too)"

And misc1 probably has many more spare CPU cycles (and disk bandwidth) than the overloaded Travis VMs ;)

@jviotti
Copy link
Contributor Author

jviotti commented Jun 15, 2017

I now get a scary go stacktrace in the builds. Any ideas @petrosagg ?

panic: standard_init_linux.go:175: exec user process caused "exec format error" [recovered]

	panic: standard_init_linux.go:175: exec user process caused "exec format error"

goroutine 1 [running, locked to thread]:

panic(0x7ddea0, 0xc820145500)

	/usr/local/go/src/runtime/panic.go:481 +0x3e6

github.com/urfave/cli.HandleAction.func1(0xc8201032e8)

	/go/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/urfave/cli/app.go:478 +0x38e

panic(0x7ddea0, 0xc820145500)

	/usr/local/go/src/runtime/panic.go:443 +0x4e9

github.com/opencontainers/runc/libcontainer.(*LinuxFactory).StartInitialization.func1(0xc820102bf8, 0xc82001a090, 0xc820102d08)

	/go/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/opencontainers/runc/libcontainer/factory_linux.go:259 +0x136

github.com/opencontainers/runc/libcontainer.(*LinuxFactory).StartInitialization(0xc820059450, 0x7f505cbb1650, 0xc820145500)

	/go/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/opencontainers/runc/libcontainer/factory_linux.go:277 +0x5b1

main.glob.func8(0xc820076a00, 0x0, 0x0)

	/go/src/github.com/opencontainers/runc/main_unix.go:26 +0x68

reflect.Value.call(0x7448a0, 0x8f0bd0, 0x13, 0x839998, 0x4, 0xc820103268, 0x1, 0x1, 0x0, 0x0, ...)

	/usr/local/go/src/reflect/value.go:435 +0x120d

reflect.Value.Call(0x7448a0, 0x8f0bd0, 0x13, 0xc820103268, 0x1, 0x1, 0x0, 0x0, 0x0)

	/usr/local/go/src/reflect/value.go:303 +0xb1

github.com/urfave/cli.HandleAction(0x7448a0, 0x8f0bd0, 0xc820076a00, 0x0, 0x0)

	/go/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/urfave/cli/app.go:487 +0x2ee

github.com/urfave/cli.Command.Run(0x83c828, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x8d0180, 0x51, 0x0, ...)

	/go/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/urfave/cli/command.go:191 +0xfec

github.com/urfave/cli.(*App).Run(0xc820001800, 0xc82000a100, 0x2, 0x2, 0x0, 0x0)

	/go/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/urfave/cli/app.go:240 +0xaa4

main.main()

	/go/src/github.com/opencontainers/runc/main.go:137 +0xe24

@jviotti jviotti force-pushed the armv7hf-builds branch 3 times, most recently from 0c36617 to 70c88a0 Compare June 26, 2017 15:25
@lurch
Copy link
Contributor

lurch commented Aug 14, 2017

@jviotti I assume we'll pick this up again when the local CI machines that @brownjohnf is setting up for us are live?

@jviotti
Copy link
Contributor Author

jviotti commented Aug 14, 2017

@lurch What do you think about merging the changes to the Makefile, etc, but delay making the CI services actually build this target?

@lurch
Copy link
Contributor

lurch commented Aug 14, 2017

Haha, I was just pondering exactly the same thing myself... :-D

@jviotti
Copy link
Contributor Author

jviotti commented Aug 14, 2017

Haha, check my latest changes! I think this should already be merge-able

@lurch
Copy link
Contributor

lurch commented Aug 14, 2017

If we're only changing the Makefile as you suggested, then I'm not sure if we should be changing .travis.yml too or the Dockerfiles?

@jviotti
Copy link
Contributor Author

jviotti commented Aug 14, 2017

Ah, there was an extra .travis.yml change I missed. Regarding the Dockerfiles, I guess we can keep that so people can actually build it without using an actual ARM device. What do you think?

@lurch
Copy link
Contributor

lurch commented Aug 14, 2017

Are we sure the Dockerfiles actually work locally? (I could test myself, but quicker to ask @jviotti 😉 ) Remember that none of the instructions we provide actually refer to Docker.
I'll do some testing this evening, if I remember / get time.

...although if people don't have an actual ARM device, why would they want to build the ARM version of Etcher? 😆

This commit makes use of the `resin/armv7hf-debian` Docker image to
test and generate armhf builds.

We needed to add a slash before `build` in `.gitignore` given that git
was refusing to include any changes on `scripts/build` otherwise.

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
@jviotti
Copy link
Contributor Author

jviotti commented Aug 21, 2017

I can confirm it works on misc1, but its slow as hell.

...although if people don't have an actual ARM device, why would they want to build the ARM version of Etcher?

I was thinking people would want to build it on a more powerful host, but ARM on Docker seems to be painfully slow anyway :P

@@ -138,6 +82,62 @@ $(error Can't build $(TARGET_ARCH) binaries on a $(HOST_ARCH) host)
endif

# ---------------------------------------------------------------------
# Application configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, why this massive change to the Makefile? Or is it just GH going crazy again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This snippet uses TARGET_ARCH, which is declared a bit above, so we needed to move down the whole section

Copy link
Contributor

@brownjohnf brownjohnf left a comment

Choose a reason for hiding this comment

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

Looks fine to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants