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

Use autoconf variable for C preprocessor #8180

Merged
merged 1 commit into from Dec 5, 2018
Merged

Use autoconf variable for C preprocessor #8180

merged 1 commit into from Dec 5, 2018

Conversation

lopsided98
Copy link
Contributor

Motivation and Context

ZFS uses a hardcoded name for the C preprocessor when detecting the kernel version, which may not be available when cross-compiling in an environment without a native compiler.

Description

This PR changes the command to use the correct prefixed preprocessor that was found with autoconf.

The trailing dash is needed because autoconf will normally set CPP to gcc -E, which requires - to read from stdin. This is not necessary, but harmless, if CPP is set to cpp.

I have a similar fix for 0.7; should I open a PR against the zfs-0.7-release branch?

How Has This Been Tested?

I built ZFS with this patch, both natively for x86_64 and cross compiled for aarch64.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

This fixes the build when cross-compiling, where the preprocessor might
be prefixed.

Signed-off-by: Ben Wolsieffer <benwolsieffer@gmail.com>
@tonyhutter tonyhutter added this to To do in 0.7.13 Dec 4, 2018
@tonyhutter
Copy link
Contributor

I have a similar fix for 0.7; should I open a PR against the zfs-0.7-release branch?

No need - I added it to the 0.7.13 patchlist.

@lopsided98
Copy link
Contributor Author

lopsided98 commented Dec 4, 2018

There is more needed for 0.7, though, because a similar cpp invocation is used to detect the SPL version. This check no longer exists in master since the SPL was merged.

@tonyhutter
Copy link
Contributor

@lopsided98 ok, that's fine then if you want to do an 0.7 PR as well.

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Nice find. Thanks!

@behlendorf behlendorf added Type: Building Indicates an issue related to building binaries Status: Code Review Needed Ready for review and testing labels Dec 4, 2018
@lopsided98
Copy link
Contributor Author

See #8183 and openzfs/spl#711.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 5, 2018
@codecov
Copy link

codecov bot commented Dec 5, 2018

Codecov Report

Merging #8180 into master will decrease coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #8180     +/-   ##
=========================================
- Coverage   78.46%   78.36%   -0.1%     
=========================================
  Files         378      378             
  Lines      114826   114765     -61     
=========================================
- Hits        90095    89933    -162     
- Misses      24731    24832    +101
Flag Coverage Δ
#kernel 78.65% <ø> (-0.25%) ⬇️
#user 67.37% <ø> (-0.15%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5eea0a...3c7eaa6. Read the comment docs.

@behlendorf behlendorf merged commit 2aa398f into openzfs:master Dec 5, 2018
@lopsided98 lopsided98 deleted the master-cpp-variable branch December 21, 2018 22:08
GregorKopka pushed a commit to GregorKopka/zfs that referenced this pull request Jan 7, 2019
This fixes the build when cross-compiling, where the preprocessor might
be prefixed.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Signed-off-by: Ben Wolsieffer <benwolsieffer@gmail.com>
Closes openzfs#8180
@tonyhutter tonyhutter moved this from To do to In progress in 0.7.13 Jan 28, 2019
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Jan 30, 2019
This fixes the build when cross-compiling, where the preprocessor might
be prefixed.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Signed-off-by: Ben Wolsieffer <benwolsieffer@gmail.com>
Closes openzfs#8180
tonyhutter pushed a commit that referenced this pull request Mar 4, 2019
This fixes the build when cross-compiling, where the preprocessor might
be prefixed.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Signed-off-by: Ben Wolsieffer <benwolsieffer@gmail.com>
Closes #8180
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested) Type: Building Indicates an issue related to building binaries
Projects
No open projects
0.7.13
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

4 participants