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.

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

Support tool-specific standard flags in makefile #862

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

jpf91
Copy link
Contributor

@jpf91 jpf91 commented Mar 24, 2024

This introduces the following new makefile variables: CFLAGS for C, CXXFLAGS for C++, ASFLAGS for Assembler and LDFLAGS for the linker. Names follow conventions from https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html

These variables can then be set either in the user application makefile (before including common.mk) or on the command line when calling make.

Fixes #856

jpf91 pushed a commit to jpf91/neorv32 that referenced this pull request Mar 24, 2024
@stnolting stnolting added enhancement New feature or request SW Software-related labels Mar 24, 2024
@stnolting
Copy link
Owner

stnolting commented Mar 24, 2024

This looks great! Thanks for implementing this!

You really are one of the few who also note the version number in their PRs. 👍 Anyway, I do not think we need to set a new "version" here. This is just a minor change that is seems to be fully backwards compatible.

It would also be nice if perhaps two lines could be added to the makefile's documentation 😅.

This introduces the following new makefile variables: `CFLAGS` for C, `CXXFLAGS` for C++, `ASFLAGS` for Assembler and `LDFLAGS` for the linker. Names follow  conventions from  https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html

These variables can then be set either in the user application makefile (before including `common.mk`) or on the command line when calling make.
@jpf91
Copy link
Contributor Author

jpf91 commented Mar 24, 2024

This looks great! Thanks for implementing this!

Thanks :-)

Anyway, I do not think we need to set a new "version" here.

👍 I was a bit confused as all changelog entries have a new version. I guess I shouldn't add a changelog entry then? Or is it ok to have an entry but keep the old version?

It would also be nice if perhaps two lines could be added to the makefile's documentation 😅.

Good point, I totally forgot about that 😇 I've pushed a new revision which adds the documentation and removes the version change 😄

@stnolting
Copy link
Owner

I guess I shouldn't add a changelog entry then? Or is it ok to have an entry but keep the old version?

Well, that's a good point... The changelog describes the "version increments" as follows: "The identifier is incremented for each core hardware change and also for major software and/or general project changes."

But this doesn't really clarify how a "major change" is actually defined... 😅

In this case, it is only a minor change that is also fully backwards compatible. I therefore think that it is okay not to assign a new version number for this. 😉

@stnolting stnolting merged commit 7110944 into stnolting:main Mar 26, 2024
7 checks passed
@jpf91
Copy link
Contributor Author

jpf91 commented Mar 29, 2024

Thanks!

@jpf91 jpf91 deleted the makefile branch March 29, 2024 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SW Software-related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature request] Makefile: Additional CXX_USER_FLAGS only for C++ Compilation
2 participants