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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - Change C compiler to Zig #464

Closed
wants to merge 5 commits into from
Closed

[Merged by Bors] - Change C compiler to Zig #464

wants to merge 5 commits into from

Conversation

nacardin
Copy link
Contributor

@nacardin nacardin commented Nov 11, 2020 •
edited
Loading

In order to fix musl build issues on newer GCC, this PR changes C compiler to Zig and Linker to LLD 11.

Please test on various OSes.

@nacardin nacardin changed the title WIP: Fix musl build on newer GCC Change C compiler to Clang Nov 12, 2020
@nacardin nacardin marked this pull request as ready for review November 12, 2020 17:31
@sehz
Copy link
Contributor

sehz commented Nov 12, 2020

@sehz try this on Mac

@nacardin nacardin requested a review from sehz November 12, 2020 18:31
@sehz
Copy link
Contributor

sehz commented Nov 12, 2020

fails to compile on Mac:

   Compiling ring v0.16.15
   Compiling idna v0.2.0
   Compiling async-global-executor v1.4.3
   Compiling async-fs v1.5.0
   Compiling async-net v1.5.0
   Compiling tokio v0.2.22
   Compiling time-macros v0.1.1
   Compiling aes v0.6.0
   Compiling hkdf v0.10.0
   Compiling which v4.0.2
   Compiling futures-util v0.3.8
   Compiling der-parser v4.1.0
error: failed to run custom build command for `ring v0.16.15`
Caused by:
  process didn't exit successfully: `/fluvio/target/debug/build/ring-84ee798471bc431b/build-script-build` (exit code: 101)
  --- stdout
  OPT_LEVEL = Some("0")
  TARGET = Some("x86_64-unknown-linux-musl")
  HOST = Some("x86_64-apple-darwin")
  CC_x86_64-unknown-linux-musl = None
  CC_x86_64_unknown_linux_musl = None
  TARGET_CC = Some("clang")
  CFLAGS_x86_64-unknown-linux-musl = None
  CFLAGS_x86_64_unknown_linux_musl = None
  TARGET_CFLAGS = None
  CFLAGS = None
  CRATE_CC_NO_DEFAULTS = None
  DEBUG = Some("true")
11:41
 ENV __CF_USER_TEXT_ENCODING=0x1F5:0x0:0x0
  running "clang" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "--target=x86_64-unknown-linux-musl" "-I" "include" "-Wall" "-Wextra" "-std=c1x" "-Wbad-function-cast" "-Wnested-externs" "-Wstrict-prototypes" "-pedantic" "-pedantic-errors" "-Wall" "-Wextra" "-Wcast-align" "-Wcast-qual" "-Wconversion" "-Wenum-compare" "-Wfloat-equal" "-Wformat=2" "-Winline" "-Winvalid-pch" "-Wmissing-field-initializers" "-Wmissing-include-dirs" "-Wredundant-decls" "-Wshadow" "-Wsign-compare" "-Wsign-conversion" "-Wundef" "-Wuninitialized" "-Wwrite-strings" "-fno-strict-aliasing" "-fvisibility=hidden" "-fstack-protector" "-g3" "-U_FORTIFY_SOURCE" "-DNDEBUG" "-c" "-o/fluvio/target/x86_64-unknown-linux-musl/debug/build/ring-060568c9c9dc5f55/out/aes_nohw.o" "crypto/fipsmodule/aes/aes_nohw.c"

@nacardin
Copy link
Contributor Author

No longer needed since #521 remove ring dependency.

@nacardin nacardin closed this Nov 30, 2020
@nacardin nacardin mentioned this pull request May 3, 2021
Closed
@nacardin
Copy link
Contributor Author

nacardin commented May 5, 2021

We now have other dependencies that use CC

@nacardin nacardin reopened this May 5, 2021
@nacardin nacardin removed the request for review from sehz May 5, 2021 23:24
@nacardin nacardin marked this pull request as draft May 5, 2021 23:24
@nacardin
Copy link
Contributor Author

nacardin commented Jun 14, 2021 •
edited
Loading

Updated this to use zig/lld. Seems to work everywhere as long as ld.lld points to an LLVM 11 install. LLVM 12 does not work. I would like to add a pre-check to make sure that ld.lld is from LLVM 11. Also need to update dev docs to add zig/llvm installation instrctions.

@nacardin nacardin force-pushed the clang branch 3 times, most recently from 9258306 to 5fbfa43 Compare June 17, 2021 21:28
@nacardin nacardin marked this pull request as ready for review June 22, 2021 16:52
@nacardin nacardin changed the title Change C compiler to Clang Change C compiler to Zig Jun 22, 2021
@nacardin nacardin requested a review from sehz June 22, 2021 16:52
sehz
sehz approved these changes Jun 22, 2021
View reviewed changes
Copy link
Contributor

@sehz sehz left a comment

Choose a reason for hiding this comment

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

Very nice work! This finally fixes long standing cross compilation issues

sehz
sehz requested changes Jun 22, 2021
View reviewed changes
Copy link
Contributor

@sehz sehz left a comment

Choose a reason for hiding this comment

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

Can you add to changelog?

@ajhunyady
Copy link
Contributor

Does this help with the Raspberry Pi build?

@sehz
Copy link
Contributor

sehz commented Jun 22, 2021

Yes, it should cross compile to Raspberry on on any supported OS (Linux, Mac, etc)

@ajhunyady
Copy link
Contributor

Ok, I look forward to the Raspberry Pi build for my weekend project. Any plans to add that back to pre-built images?

@nacardin
Copy link
Contributor Author

Can you add to changelog?

Updated

@nacardin nacardin requested a review from sehz June 23, 2021 17:18
@sehz
Copy link
Contributor

sehz commented Jun 23, 2021

there is merge conflict

@nacardin
Copy link
Contributor Author

there is merge conflict

resolved

sehz
sehz approved these changes Jun 23, 2021
View reviewed changes
Copy link
Contributor

@sehz sehz left a comment

Choose a reason for hiding this comment

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

LGTM

@sehz
Copy link
Contributor

sehz commented Jun 23, 2021

bors r+

@bors
Copy link

bors bot commented Jun 23, 2021

Merge conflict.

@sehz
Copy link
Contributor

sehz commented Jun 23, 2021

need rebase again with other PR landed first

@sehz
Copy link
Contributor

sehz commented Jun 23, 2021

bors r+

bors bot pushed a commit that referenced this pull request Jun 23, 2021
@nacardin
Change C compiler to Zig (#464)
2756663 
In order to fix musl build issues on newer GCC, this PR changes C compiler to Zig and Linker to LLD 11.

Please test on various OSes.

Co-authored-by: Nick Cardin <nick@cardin.email>
@bors
Copy link

bors bot commented Jun 23, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title Change C compiler to Zig [Merged by Bors] - Change C compiler to Zig Jun 23, 2021
@bors bors bot closed this Jun 23, 2021
@nacardin nacardin deleted the clang branch June 24, 2021 11:59
@simlay simlay mentioned this pull request Jun 25, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sehz sehz sehz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nacardin @sehz @ajhunyady