-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
linux-cachyos-hardened: 6.12.5-1 #362
Conversation
Signed-off-by: SM9() <me@sm9.dev>
We are currently waiting that linux-hardened gets updated in archlinux - I know the patchset exists, but the config is also an important part |
This seems like a very lousy attempt at rebasing the PKGBUILD.
|
Why?, its not cool to to be a cunt, it is better to be clear and constructive, that way I can improve any commits later on.
?
Not sure what you mean, why does .SRCINFO matter, are you submitting this to the aur?
Whats wrong with ThinLTO, can you be a little clearer?
Obviously this part is wrong as ptr1337 has already mentioned, once the correct config was available I was going to amend my commit.
I look forward to hearing your (none cuntish) reply that hopefully clears things up a little more. Thanks |
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.
Here are the reviews if I haven't made myself clear.
@@ -106,18 +105,17 @@ _use_auto_optimization=${_use_auto_optimization-y} | |||
# "full: uses 1 thread for Linking, slow and uses more memory, theoretically with the highest performance gains." | |||
# "thin: uses multiple threads, faster and uses less memory, may have a lower runtime performance than Full." | |||
# "none: disable LTO | |||
_use_llvm_lto=${_use_llvm_lto-none} | |||
_use_llvm_lto=${_use_llvm_lto-thin} |
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.
Only the default kernel defaults to ThinLTO, the rest stick to GCC
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.
Why is this, can you explain? thanks.
# This was added to facilitate https://github.com/CachyOS/linux-cachyos/issues/286 | ||
_use_gcc_suffix=${_use_gcc_suffix-} | ||
# Enabled by default to show the difference between LTO kernels and GCC kernels | ||
_use_gcc_suffix=${_use_gcc_suffix-y} |
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 is only meant to be used by the default kernel to not break the naming scheme for the rest of the kernels.
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.
Understood, I guess the whole concept here can be improved on.
_pkgsuffix="cachyos-${_cpusched}-lto" | ||
elif [ "$_use_llvm_lto" = "none" ] && [ -z "$_use_kcfi" ] && [ "$_use_gcc_suffix" = "y" ]; then | ||
_pkgsuffix="cachyos-${_cpusched}-gcc" | ||
# Enable AUTOFDO_CLANG for the first compilation to create a kernel, which can be used for profiling |
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.
AutoFDO isn't used by any other kernel except for the default and rc so there's not reason to add this.
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.
Whether its used or not is the decision of whoever is building the kernel, also, keeping it there keeps things consistent.
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.
If you want to add it in the name of consistency, then you should add it for the other PKGBUILDs too. The default kernel doesn't need to follow the other PKGBUILDs since it's already different in many ways (ThinLTO default, AutoFDO, more soon)
if _is_lto_kernel && [ "$_use_lto_suffix" = "y" ]; then | ||
_pkgsuffix=cachyos-hardened-lto | ||
elif ! _is_lto_kernel && [ "$_use_gcc_suffix" = "y" ]; then | ||
_pkgsuffix=cachyos-hardened-gcc |
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.
Why are you even hardcoding hardened
here?
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.
Because it was not tagged with "-hardened" when I built it, so I added this, also, this is the PKGBUILD for the hardened kernel, why does it matter?
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.
It doesn't really matter much because -hardened uses its own configs, but ideally there should only be one PKGBUILD. We have much more in this repo to facilitate the building process to our repositories. Just to add another point, it keeps consistency :) with the rest of the PKGBUILDs that does this.
if _is_lto_kernel; then | ||
depends+=(clang llvm lld) | ||
fi |
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.
.SRCINFO doesn't respect any conditionals and will add this regardless. Since the other kernels don't default to LTO, we've decided to not add this for the other PKGBUILDs. Please see #346.
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.
Understood, although again, I thought all kernels default to LTO / Clang now, if not, would you mind explaining why not? Thanks
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.
#286. IMO it's to reduce the resources used when building kernels as the other kernel variants aren't used as much if at all compared to the default kernel. Other than that, another mass migration would need to take place by adding provides() replaces() to the PKGBUILDs temporarily which we're not quite fond of doing.
Yes, and
Nothing, it's just that the other kernels will default to GCC, and the changes you've made in this PR obviously goes against that decision.
There's a feature to draft pull requests in GitHub. How am I (or any other person other than you) supposed to know if this PR is ready or if there will be any other future changes? |
Understood.
I thought clang was the new default for all kernels, excuse me if I am wrong here.
You are correct, I should have submitted a draft. |
I noticed that the hardened kernel was outdated but there are patches for 6.12.5.