Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Replace EXTRA_CFLAGS with ccflags-y #270

Merged
merged 1 commit into from
Mar 5, 2025
Merged

Conversation

C2Redhat
Copy link
Member

@C2Redhat C2Redhat commented Mar 4, 2025

Based on:
https://www.kernel.org/doc/Documentation/kbuild/makefiles.txt

EXTRA_FLAGS has been deprecated. We should be using ccflags-y. Replaced EXTRA_CFLAGS with ccflags-y in all our files.

Copy link

github-actions bot commented Mar 4, 2025

Unit Test Results

  11 files    95 suites   1h 7m 16s ⏱️
270 tests 269 ✔️ 0 💤 0  1 🔥
274 runs  273 ✔️ 0 💤 0  1 🔥

For more details on these errors, see this check.

Results for commit 6ae1458.

Copy link
Contributor

@sweettea sweettea left a comment

Choose a reason for hiding this comment

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

Mostly lgtm, just two files seem unneeded.

Copy link
Member

@lorelei-sakai lorelei-sakai left a comment

Choose a reason for hiding this comment

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

Some of these changes may not be strictly necessary, but it's probably better to change everything rather than try to sort out exactly which ones care about this change and which don't.

Also, what happened with the CI tests? They show a failure but I can't figure out what that failure is.

One question is when did the deprecation/changeover happen? Would it make sense for us to backport this to older versions? (I think probably not, there's no reason for the kernel to backport this kind of build change.)

Copy link
Contributor

@sweettea sweettea left a comment

Choose a reason for hiding this comment

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

Okay, I can buy the argument that it's consistent everywhere, so it's fine by me.

Copy link
Contributor

@sweettea sweettea left a comment

Choose a reason for hiding this comment

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

One tiny thing: the actual title of the change refers to EXTRA_FLAGS, not EXTRA_CFLAGS.

@C2Redhat C2Redhat changed the title Replace EXTRA_FLAGS with ccflags-y Replace EXTRA_CFLAGS with ccflags-y Mar 5, 2025
Based on:
https://www.kernel.org/doc/Documentation/kbuild/makefiles.txt

EXTRA_CFLAGS has been deprecated. We should be using ccflags-y.
Replaced EXTRA_CFLAGS with ccflags-y in all our files.
Copy link
Member

@lorelei-sakai lorelei-sakai left a comment

Choose a reason for hiding this comment

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

Looks good to me, looking forward to the build working again.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants