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

@franc0is franc0is commented Mar 10, 2016

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 Outdated
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

@akosthekiss akosthekiss Mar 10, 2016

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

@franc0is franc0is Mar 11, 2016

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

akosthekiss commented Mar 11, 2016

LGTM (FWIW)
Please, rebase.

@zherczeg
Copy link
Member

zherczeg commented Mar 11, 2016

LGTM

1 similar comment
@LaszloLango
Copy link
Contributor

LaszloLango commented Mar 11, 2016

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

franc0is commented Mar 11, 2016

rebased

@akosthekiss
Copy link
Member

akosthekiss commented Mar 11, 2016

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

franc0is commented Mar 11, 2016

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