-
Notifications
You must be signed in to change notification settings - Fork 186
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
Configurable build (update of pull request #42) #66
Conversation
fe56973
to
ff17a69
Compare
This PR was not completely merged. setup.gmk file changed are not included, therefore the build cannot be done by pre-defining the compiler as expected. |
CC ?= ${TOOLPREFIX}gcc | ||
LD ?= ${TOOLPREFIX}ld | ||
AR ?= ${TOOLPREFIX}ar | ||
OD ?= ${TOOLPREFIX}objdump | ||
OC ?= ${TOOLPREFIX}objcopy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please describe what the intended outcome of this change is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current makefile gives some constraints:
- The toolchain must be in a
/toolchain-${platform} folder
- Toolchain binaries must reside in a
bin
folder - All toolchain binaries must reside in the same folder
Constraints 2-3 are generally met by most environments. Constraint 1 is problematic. E.g. in yocto, 1 is not met, 2.3 are.
- The proposed change:
TOOLCHAIN?=${HOME}/toolchain-${platform}
removes constraint 1. - The one you linked removes constraint 2-3.
In my case (yocto), removing constraint 1 is mandatory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed that yocto does not produce a tree compatible with constraint 2. So both changes are necessary.
Also, note that yocto will by default set CC, LD, ... vars in the build setup process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is a problem because GNU make populates CC and AR with default values. Hence ?=
has no effect for those variables unless make is called with -R
flag. That's why we cannot simply accept with change.
Thanks for the description of your use case. We will take this into account for future releases.
Our pull-request #42 was based on our master branch. We had to base this new pull request on feature branch to move forward on the master.
The commit is exactly the same as #42