Skip to content

Allow external builds to use non-gnu compilers #955

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
merged 1 commit into from
Mar 11, 2016
Merged

Allow external builds to use non-gnu compilers #955

merged 1 commit into from
Mar 11, 2016

Conversation

franc0is
Copy link
Contributor

JerryScript-DCO-1.0-Signed-off-by: François Baldassari francois@pebble.com

@franc0is franc0is mentioned this pull request Mar 10, 2016
Merged
@LaszloLango LaszloLango added feature request Requested feature tools Related to the tooling scripts labels Mar 10, 2016
akosthekiss
akosthekiss reviewed Mar 10, 2016
View reviewed changes
build/configs/toolchain_external.cmake
@@ -17,4 +17,4 @@ include(CMakeForceCompiler)
set(CMAKE_SYSTEM_NAME EXTERNAL)
set(CMAKE_SYSTEM_PROCESSOR "${EXTERNAL_CMAKE_SYSTEM_PROCESSOR}")

CMAKE_FORCE_C_COMPILER(${EXTERNAL_CMAKE_C_COMPILER} GNU)
CMAKE_FORCE_C_COMPILER(${EXTERNAL_CMAKE_C_COMPILER} ${EXTERNAL_CMAKE_C_COMPILER_FAMILY})
Copy link
Member

Choose a reason for hiding this comment

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

Actually, the second parameter of CMAKE_FORCE_C_COMPILER is called < compiler-id > and sets the CMAKE_C_COMPILER_ID cmake internal variable. Thus, it is preferrable to call the external variant EXTERNAL_CMAKE_C_COMPILER_ID (instead of _FAMILY).

Moreover, the introduction of this new variable breaks all the current ports that use this external cmake file, since the Makefiles under the targets/ directory don't specify value for EXTERNAL_CMAKE_C_COMPILER_ID. Please, update those files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great suggestion! I'm still learning my way around cmake. Changed!

@franc0is franc0is force-pushed the external-platform-compilers branch from a1047d1 to 3858bad Compare March 11, 2016 01:49
@akosthekiss
Copy link
Member

LGTM (FWIW)
Please, rebase.

@zherczeg
Copy link
Member

LGTM

1 similar comment
@LaszloLango
Copy link
Contributor

LGTM

@franc0is franc0is force-pushed the external-platform-compilers branch from 3858bad to 2daaa7d Compare March 11, 2016 17:59
@franc0is
Copy link
Contributor Author

rebased

@akosthekiss
Copy link
Member

Let's do one more round of rebasing. (By landing #944, master moved one patch ahead of this.)

Allow external builds to use non-gnu compilers
05e7a83 
JerryScript-DCO-1.0-Signed-off-by: François Baldassari francois@pebble.com
@franc0is franc0is force-pushed the external-platform-compilers branch from 2daaa7d to 05e7a83 Compare March 11, 2016 18:13
@franc0is
Copy link
Contributor Author

done

@akosthekiss akosthekiss merged commit 05e7a83 into jerryscript-project:master Mar 11, 2016
@zherczeg zherczeg mentioned this pull request Apr 7, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
feature request Requested feature tools Related to the tooling scripts
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@franc0is @akosthekiss @zherczeg @LaszloLango