Fix CC detection #334
+18
−8
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -287,6 +287,9 @@ setup_kernels_arches() | |
| fi | ||
| fi | ||
| fi | ||
| if [[ ! $arch ]]; then | ||
| die 12 $"Could not determine architecture." | ||
| fi | ||
| fi | ||
anbe42 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| # If only one arch is specified, make it so for all the kernels | ||
|
|
@@ -1061,7 +1064,19 @@ prepare_build() | |
| $"Check $build_dir for more information." | ||
| done | ||
|
|
||
| if [ -f "$kernel_source_dir/.kernelvariables" ]; then | ||
| export CC=$(echo -e "show-%:\n\t@echo \$(\$*)\ninclude $kernel_source_dir/.kernelvariables" | make -f - show-CC) | ||
| else | ||
| unset CC | ||
| fi | ||
|
|
||
| if [[ -e "${kernel_config}" ]]; then | ||
anbe42 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| local cc=$(sed -n 's|^CONFIG_CC_VERSION_TEXT="\([^ ]*\) .*"|\1|p' "${kernel_config}") | ||
| if command -v "$cc" >/dev/null; then | ||
| export CC="$cc" | ||
| export KERNEL_CC="$cc" | ||
| fi | ||
|
|
||
| if grep -q 'CONFIG_CC_IS_CLANG=y' "${kernel_config}"; then | ||
|
There was a problem hiding this comment. The hunk was removed by @hadogenes with 4729aef. *Reintroducing it seems correct at a glance. @hadogenes can you confirm this doesn't regress on your end? Which Suse variant/version are you using - SLES, Leap, Tumbleweed? Thanks There was a problem hiding this comment. clang detection is probably wrong if versioned clang-xx is to be used, but as long as we don't have a clang compiled kernel in Debian, I'm not going to investigate |
||
| local cc=clang | ||
| if command -v "$cc" >/dev/null; then | ||
|
|
@@ -1094,12 +1109,6 @@ actual_build() | |
| echo $"" | ||
| echo $"Building module:" | ||
|
|
||
| invoke_command "$clean" "Cleaning build area" background | ||
| echo $"DKMS make.log for $module-$module_version for kernel $kernelver ($arch)" >> "$build_log" | ||
| date >> "$build_log" | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Interesting - Arch and Fedora (IIRC) don't pull the compiler stack, when the headers are installed.
Should we do the same change in the Ubuntu case below?
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.
Already tried, the gcc dependency is required at least for older Ubuntu releases.
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.
Ack, please mention that in the commit message. For non Debian/Ubuntu experts such variations are non-obvious. 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.
jfyi @xnox is the above variation, something Ubuntu team is willing to address? I don't have a strong opinion, just making sure devs in both distros are aware.
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.
even i don't know or understand if and when and why we have compiler depedency. i think in ubuntu we add "Package: dkms Depends: gcc" because we don't have dependency from headers to the compiler either (similarish to fedora). But we also have different compilers used for different kernels, thus maybe our headers should pull in correct compiler in.....