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

Please allow us to set Makefile variables from outside #233

Merged
merged 2 commits into from Nov 28, 2016

Conversation

Projects
None yet
2 participants
@danrl
Contributor

danrl commented Nov 27, 2016

Small change that helps a lot in automated cross-compiling tasks.

danrl added some commits Nov 27, 2016

stateful: Respect existing variables
This makes life easier for automated build systems and cross compiling. E.g. for LEDE/OpenWRT.
stateless: Respect existing variables
This makes life easier for automated build systems and cross compiling. E.g. for LEDE/OpenWRT.
@danrl

This comment has been minimized.

Show comment
Hide comment
@danrl

danrl Nov 27, 2016

Contributor

Not part of this PR, but somehow related: Can we have these variables set in the mod/Makefile, so that it propagates down to the other two makefiles? Currently I have to patch the makefiles and call them separately, would prefer to call the parent makefile once and setting KERNEL_DIR via export before calling the makefile.

Contributor

danrl commented Nov 27, 2016

Not part of this PR, but somehow related: Can we have these variables set in the mod/Makefile, so that it propagates down to the other two makefiles? Currently I have to patch the makefiles and call them separately, would prefer to call the parent makefile once and setting KERNEL_DIR via export before calling the makefile.

@ydahhrk

This comment has been minimized.

Show comment
Hide comment
@ydahhrk

ydahhrk Nov 28, 2016

Member

Ok, cool. I used to do it like this:

make KERNEL_DIR=/path/to/kernel

In fact, it also seems to work in mod/. Is this not acceptable?

But this does look better so approving.

On the other hand, you seem to be working with a very old version (something between 3.3.0 and 3.3.1), which has known critical bugs. I suggest an upgrade.

Member

ydahhrk commented Nov 28, 2016

Ok, cool. I used to do it like this:

make KERNEL_DIR=/path/to/kernel

In fact, it also seems to work in mod/. Is this not acceptable?

But this does look better so approving.

On the other hand, you seem to be working with a very old version (something between 3.3.0 and 3.3.1), which has known critical bugs. I suggest an upgrade.

@ydahhrk ydahhrk merged commit a37b795 into NICMx:master Nov 28, 2016

@danrl

This comment has been minimized.

Show comment
Hide comment
@danrl

danrl Nov 28, 2016

Contributor

Thanks for merging. And yes, you are right, setting via make command line is possible, too. It was just less desirable in a specific use case.

On the other hand, you seem to be working with a very old version (something between 3.3.0 and 3.3.1), which has known critical bugs. I suggest an upgrade.

I fetched v3.5.1 via release page on this repo. Maybe I checked out the wrong branch for the pull request?!

Contributor

danrl commented Nov 28, 2016

Thanks for merging. And yes, you are right, setting via make command line is possible, too. It was just less desirable in a specific use case.

On the other hand, you seem to be working with a very old version (something between 3.3.0 and 3.3.1), which has known critical bugs. I suggest an upgrade.

I fetched v3.5.1 via release page on this repo. Maybe I checked out the wrong branch for the pull request?!

@ydahhrk

This comment has been minimized.

Show comment
Hide comment
@ydahhrk

ydahhrk Nov 28, 2016

Member

OK

But wait a second: Is this finished? I'm not sure what you meant by

Can we have these variables set in the mod/Makefile, so that it propagates down to the other two makefiles?

It seems like the patch you sent already allows me to export KERNEL_DIR and cross-compile via the parent makefile.

Member

ydahhrk commented Nov 28, 2016

OK

But wait a second: Is this finished? I'm not sure what you meant by

Can we have these variables set in the mod/Makefile, so that it propagates down to the other two makefiles?

It seems like the patch you sent already allows me to export KERNEL_DIR and cross-compile via the parent makefile.

@danrl

This comment has been minimized.

Show comment
Hide comment
@danrl

danrl Nov 28, 2016

Contributor

Mea culpa, it is finished. Late night pull request /o\

Everything works via export now as expected.

Contributor

danrl commented Nov 28, 2016

Mea culpa, it is finished. Late night pull request /o\

Everything works via export now as expected.

@ydahhrk ydahhrk modified the milestone: 3.5.2 Dec 2, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment