feat: add support for compiling open/r on 64-bit arm linux #95

cabelitos · 2021-05-03T13:37:36Z

Description:

Prior to this patch it was not possible to run open/r on linux running on ARM due to cmake and openssl being harded to download/compile for the x86_64 arch.

In order to prevent such problem this patch actively compiles cmake instead of using pre-compiled binaries and also allows the OpenSSLBuilder to provide the correct build args to openssl, thus not trying to compile or run x86_64 software.

Test Plan:

built the project by using ./build/build_openr.sh

wez · 2021-05-03T15:16:06Z

build/fbcode_builder/manifests/cmake

@@ -11,30 +11,17 @@ cmake
 [dependencies]
 ninja

-[download.os=windows]


I'm not properly awake yet, but I worry that these windows and darwin changes will break things for those platforms; in particular, Windows doesn't have a predictable make utility, so it is desirable to sidestep that headache if we can!

Hey @wez, thank you for the review.
At least on darwin (big sur) I had no problems with my current setup, however I do not have a windows machine. Do you prefer that we leave the dowload options for windows and darwin as they were?

IMO we should leave darwin just as linux, since apple now supports ARM archs, which will eventually cause problems for some users (like me).

But really sure if we can do conditionals at the manifest declaration, because if we can I can change in a way that it always downloads the binaries (for ARM/x86) for all platforms.

I'm 50:50 on making darwin build it: darwin's arm platforms can run intel binaries, so the build is probably faster if we have it download the binary pre-built.

Windows I think is important to download binaries because there's no guarantee that there is a working make at all, let alone one that targets native windows binaries.

The .os=windows stuff at the end of the key is the getdeps conditional syntax.
You could add arch as a context variable over here:

openr/build/fbcode_builder/getdeps/buildopts.py

Line 167 in f0e6d03

"os": host_type.ostype,

if you wanted to allow something like [download.all(os=darwin, arch=arm)]

Awesome, ty for the input.
Do you prefer that we allow linux to download the ARM binaries instead of compiling it?
I'll revert the changes for windows/darwin

if building it isn't dramatically slower than downloading it, then building seems fine on linux; generally nicer to have fewer conditions in the manifest!

@wez ty for your help, I've sent a new version which only compiles cmake on linux. I took the approach for letting the manifest with fewer conditions.

It did not take much time to compile cmake for me. (And I was running on a VM with 1 processor)

However if you prefer that we expose the arch type for the manifest. I can quickly implement this feature.

facebook-github-bot · 2021-05-05T16:08:09Z

@cooperlees has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

cooperlees

Also, if you could run your python change through black that would be awesome.

Sorry for not having CI to do this.

cooperlees · 2021-05-05T16:18:53Z

build/fbcode_builder/getdeps/builder.py

@@ -184,6 +184,11 @@ def run_tests(
        cmd = ["make"] + self.test_args + self._get_prefix()
        self._run_cmd(cmd, env=env)

+class CMakeBootStrapBuilder(MakeBuilder):
+    def _build(self, install_dirs, reconfigure):
+        env = self._compute_env(install_dirs)


Is this meant to be passed somewhere / used?

my bad, this can be deleted.

cabelitos · 2021-05-05T20:54:03Z

@cooperlees I'll fix the problems tonight. I did not know that you guys were using a linter. I'll fix the linter problems.

Btw, I can't see the linter warning via the github CI. since it requires a facebook employee login. Which I'm not. :/

cooperlees · 2021-05-05T21:16:22Z

@cooperlees I'll fix the problems tonight. I did not know that you guys were using a linter. I'll make the create the setup.

Btw, I can't see the linter warning via the github CI. since it requires a facebook employee login. Which I'm not. :/

Yeah, my bad. I didn't set it up here. One day I will.

If you just run black over the .py files it should auto fix all the linter errors. If not, I'll fix them post merge due to me sucking and not having CI running the linter here.

facebook-github-bot · 2021-05-06T00:25:57Z

@cabelitos has updated the pull request. You must reimport the pull request before landing.

cabelitos · 2021-05-06T00:26:55Z

@cooperlees btw, at all. I executed the black linter and also removed the env = self._compute_env(install_dirs)

Thank you very much for your support.

facebook-github-bot · 2021-05-06T00:27:37Z

@cabelitos has updated the pull request. You must reimport the pull request before landing.

Prior to this patch it was not possible to run open/r on linux running on ARM due to cmake and openssl being harded to download/compile for the x86_64 arch. In order to prevent such problem this patch actively compiles cmake instead of using pre-compiled binaries and also allows the OpenSSLBuilder to provide the correct build args to openssl, thus not trying to compile or run x86_64 software.

facebook-github-bot · 2021-05-06T00:29:25Z

@cabelitos has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-05-06T00:42:01Z

@cabelitos has updated the pull request. You must reimport the pull request before landing.

cabelitos · 2021-05-06T00:43:38Z

I guess that the "update branch" button created a merge commit :/

facebook-github-bot · 2021-05-06T14:32:38Z

@cooperlees has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

cabelitos · 2021-05-06T21:35:51Z

did i break something? :[

cooperlees · 2021-05-11T14:55:32Z

Yes, trying to workout what / how

CMake Error at cmake_install.cmake:559 (file):
  file INSTALL cannot copy file
  "/data/sandcastle/temp/fbcode_builder_getdeps/build/openr/rib_policy_test"
  to
  "/data/sandcastle/temp/fbcode_builder_getdeps/installed/openr/sbin/tests/openr/decision/rib_policy_test".
FAILED: CMakeFiles/install.util

My lack of cmake skills make this non obvious.

cabelitos · 2021-05-11T20:14:40Z

@cooperlees Maybe these tips can help you:

is this any space left on the disk?
did you check the permissions for the /data/sandcastle/temp/fbcode_builder_getdeps/installed/openr/sbin/tests/openr/decision/rib_policy_test folder?

The whole build flow worked on my VM.

cooperlees · 2021-05-26T19:45:19Z

Yeah, sorry about this. I haven't had time to dig in. I will try rebase it internally, and just fix and land.

cabelitos · 2021-05-31T14:55:34Z

@cooperlees np at all. please let me know if you need help.

cooperlees · 2021-06-01T14:09:26Z

@cooperlees np at all. please let me know if you need help.

Looking into this today:

CMake Error at cmake_install.cmake:559 (file):
  file INSTALL cannot copy file
  "/data/sandcastle/temp/fbcode_builder_getdeps/build/openr/rib_policy_test"
  to
  "/data/sandcastle/temp/fbcode_builder_getdeps/installed/openr/sbin/tests/openr/decision/rib_policy_test".
FAILED: CMakeFiles/install.util

Let me know if you can spot it.

Summary: Description: Prior to this patch it was not possible to run open/r on linux running on ARM due to cmake and openssl being harded to download/compile for the x86_64 arch. In order to prevent such problem this patch actively compiles cmake instead of using pre-compiled binaries and also allows the OpenSSLBuilder to provide the correct build args to openssl, thus not trying to compile or run x86_64 software. Pull Request resolved: facebook/openr#95 Test Plan: * built the project by using ./build/build_openr.sh Reviewed By: wez Differential Revision: D28224684 Pulled By: cooperlees fbshipit-source-id: 9de61dc6d7dcf7116ec5c67f3f165cd4a4bb5e5c

facebook-github-bot · 2021-06-01T18:40:31Z

This pull request has been merged in 752a556.

facebook-github-bot · 2021-06-01T18:40:31Z

This pull request has been merged in b744723.

Summary: Description: Prior to this patch it was not possible to run open/r on linux running on ARM due to cmake and openssl being harded to download/compile for the x86_64 arch. In order to prevent such problem this patch actively compiles cmake instead of using pre-compiled binaries and also allows the OpenSSLBuilder to provide the correct build args to openssl, thus not trying to compile or run x86_64 software. Pull Request resolved: facebook/openr#95 Test Plan: * built the project by using ./build/build_openr.sh Reviewed By: wez Differential Revision: D28224684 Pulled By: cooperlees fbshipit-source-id: 9de61dc6d7dcf7116ec5c67f3f165cd4a4bb5e5c

Summary: Description: Prior to this patch it was not possible to run open/r on linux running on ARM due to cmake and openssl being harded to download/compile for the x86_64 arch. In order to prevent such problem this patch actively compiles cmake instead of using pre-compiled binaries and also allows the OpenSSLBuilder to provide the correct build args to openssl, thus not trying to compile or run x86_64 software. Pull Request resolved: #95 Test Plan: * built the project by using ./build/build_openr.sh Reviewed By: wez Differential Revision: D28224684 Pulled By: cooperlees fbshipit-source-id: 9de61dc6d7dcf7116ec5c67f3f165cd4a4bb5e5c

Summary: Description: Prior to this patch it was not possible to run open/r on linux running on ARM due to cmake and openssl being harded to download/compile for the x86_64 arch. In order to prevent such problem this patch actively compiles cmake instead of using pre-compiled binaries and also allows the OpenSSLBuilder to provide the correct build args to openssl, thus not trying to compile or run x86_64 software. Pull Request resolved: facebook/openr#95 Test Plan: * built the project by using ./build/build_openr.sh Reviewed By: wez Differential Revision: D28224684 Pulled By: cooperlees fbshipit-source-id: 9de61dc6d7dcf7116ec5c67f3f165cd4a4bb5e5c

cabelitos · 2021-06-02T01:00:57Z

@cooperlees the only thing that I can think of is permissions or full disk.

cabelitos · 2021-06-02T01:01:18Z

Btw, ty very much for merging!

cooperlees · 2021-06-02T01:05:04Z

Btw, ty very much for merging!

Thanks for contributing, sorry it took me so long to dig into what we needed internally. Was caching the new tar to stop transient download failures and lints … error was very misleading.

Summary: Description: Prior to this patch it was not possible to run open/r on linux running on ARM due to cmake and openssl being harded to download/compile for the x86_64 arch. In order to prevent such problem this patch actively compiles cmake instead of using pre-compiled binaries and also allows the OpenSSLBuilder to provide the correct build args to openssl, thus not trying to compile or run x86_64 software. Pull Request resolved: facebook/openr#95 Test Plan: * built the project by using ./build/build_openr.sh Reviewed By: wez Differential Revision: D28224684 Pulled By: cooperlees fbshipit-source-id: 9de61dc6d7dcf7116ec5c67f3f165cd4a4bb5e5c

facebook-github-bot added the CLA Signed label May 3, 2021

cabelitos mentioned this pull request May 3, 2021

Compiling for aarch64 architecture #89

Closed

cooperlees requested a review from wez May 3, 2021 14:14

wez reviewed May 3, 2021

View reviewed changes

cabelitos force-pushed the aarch64 branch from 27f68b6 to 9b16515 Compare May 4, 2021 00:48

cooperlees requested changes May 5, 2021

View reviewed changes

cabelitos force-pushed the aarch64 branch from 9b16515 to 444e9fb Compare May 6, 2021 00:25

cabelitos force-pushed the aarch64 branch from b707023 to c89af66 Compare May 6, 2021 00:29

Merge branch 'master' into aarch64

d5c13f6

facebook-github-bot closed this in facebookincubator/fizz@b744723 Jun 1, 2021

facebook-github-bot closed this in facebookarchive/fbzmq@752a556 Jun 1, 2021

facebook-github-bot added Merged labels Jun 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for compiling open/r on 64-bit arm linux #95

feat: add support for compiling open/r on 64-bit arm linux #95

cabelitos commented May 3, 2021

wez May 3, 2021

cabelitos May 3, 2021

cabelitos May 3, 2021

wez May 3, 2021

cabelitos May 3, 2021 •

edited

wez May 3, 2021

cabelitos May 4, 2021

facebook-github-bot commented May 5, 2021

cooperlees left a comment

cooperlees May 5, 2021

cabelitos May 5, 2021

cabelitos commented May 5, 2021 •

edited

cooperlees commented May 5, 2021

facebook-github-bot commented May 6, 2021

cabelitos commented May 6, 2021

facebook-github-bot commented May 6, 2021

facebook-github-bot commented May 6, 2021

facebook-github-bot commented May 6, 2021

cabelitos commented May 6, 2021

facebook-github-bot commented May 6, 2021

cabelitos commented May 6, 2021

cooperlees commented May 11, 2021

cabelitos commented May 11, 2021 •

edited

cooperlees commented May 26, 2021

cabelitos commented May 31, 2021

cooperlees commented Jun 1, 2021

facebook-github-bot commented Jun 1, 2021

facebook-github-bot commented Jun 1, 2021

cabelitos commented Jun 2, 2021

cabelitos commented Jun 2, 2021

cooperlees commented Jun 2, 2021

feat: add support for compiling open/r on 64-bit arm linux #95

feat: add support for compiling open/r on 64-bit arm linux #95

Conversation

cabelitos commented May 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cabelitos May 3, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented May 5, 2021

cooperlees left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cabelitos commented May 5, 2021 • edited

cooperlees commented May 5, 2021

facebook-github-bot commented May 6, 2021

cabelitos commented May 6, 2021

facebook-github-bot commented May 6, 2021

facebook-github-bot commented May 6, 2021

facebook-github-bot commented May 6, 2021

cabelitos commented May 6, 2021

facebook-github-bot commented May 6, 2021

cabelitos commented May 6, 2021

cooperlees commented May 11, 2021

cabelitos commented May 11, 2021 • edited

cooperlees commented May 26, 2021

cabelitos commented May 31, 2021

cooperlees commented Jun 1, 2021

facebook-github-bot commented Jun 1, 2021

facebook-github-bot commented Jun 1, 2021

cabelitos commented Jun 2, 2021

cabelitos commented Jun 2, 2021

cooperlees commented Jun 2, 2021

cabelitos May 3, 2021 •

edited

cabelitos commented May 5, 2021 •

edited

cabelitos commented May 11, 2021 •

edited