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 <fcntl.h> instead of <sys/fcntl.h> #15925

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

thesamesam
Copy link
Contributor

@thesamesam thesamesam commented Feb 23, 2024

Motivation and Context

<sys/fcntl.h> is deprecated and triggers a warning (-> error) on musl.

Bug: https://bugs.gentoo.org/925235

Description

When building on musl, we get:

In file included from tests/zfs-tests/cmd/getversion.c:22:
/usr/include/sys/fcntl.h:1:2: error: #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h> [-Werror=cpp]
    1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h>

In file included from module/os/linux/zfs/vdev_file.c:36:
/usr/include/sys/fcntl.h:1:2: error: #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h> [-Werror=cpp]
    1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h>                                                                                                                     

How Has This Been Tested?

Tested on musl and glibc on amd64.

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)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@thesamesam thesamesam force-pushed the musl-fcntl branch 3 times, most recently from 17c9384 to 68419c7 Compare February 23, 2024 06:40
@thesamesam thesamesam changed the title tests: use <fcntl.h> instead of <sys/fcntl.h> Use <fcntl.h> instead of <sys/fcntl.h> Feb 23, 2024
@thesamesam thesamesam marked this pull request as draft February 24, 2024 19:30
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Feb 27, 2024
@behlendorf
Copy link
Contributor

@thesamesam is there any reason this is still marked as a draft?

@thesamesam
Copy link
Contributor Author

Completely forgot. I've got a busy few days but I'll put this on my list to come back to.

I need to double check if there's any other missing includes and whether they need a guard - there was one I dropped which it turns out is needed for the kernel build but is wrong for the userland build, or something like that.

@behlendorf
Copy link
Contributor

@thesamesam if this is still needed can you rebase this and mark it ready for review.

@thesamesam
Copy link
Contributor Author

The snag when I last looked was something about getting the include right for the module vs the userland side, I'll look on the weekend - sorry for keeping you waiting.

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.

Makes sense. Can you just remove the draft status from this PR and update the commit comment to address the make checkstyle warnings.

@thesamesam
Copy link
Contributor Author

thesamesam commented Nov 1, 2024

Whoops -- something about this PR is cursed :)

Thank you for your patience again. I'm sorry for being a snail on this -- my only musl system wasn't very accessible and I ended up making some bad assumptions about something else I'd seen when it turns out it was unrelated.

@thesamesam thesamesam marked this pull request as ready for review November 1, 2024 04:01
@thesamesam thesamesam force-pushed the musl-fcntl branch 3 times, most recently from 33e1f4a to 333b422 Compare November 1, 2024 04:27
When building on musl, we get:
```
In file included from tests/zfs-tests/cmd/getversion.c:22:
/usr/include/sys/fcntl.h:1:2: error: #warning redirecting incorrect
 #include <sys/fcntl.h> to <fcntl.h> [-Werror=cpp]
 1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h>

In file included from module/os/linux/zfs/vdev_file.c:36:
/usr/include/sys/fcntl.h:1:2: error: #warning redirecting incorrect
 #include <sys/fcntl.h> to <fcntl.h> [-Werror=cpp]
 1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h>
```

Bug: https://bugs.gentoo.org/925235
Signed-off-by: Sam James <sam@gentoo.org>
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Nov 7, 2024
@behlendorf behlendorf merged commit 4a7a0a0 into openzfs:master Nov 7, 2024
21 of 24 checks passed
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Nov 7, 2024
When building on musl, we get:

```
In file included from tests/zfs-tests/cmd/getversion.c:22:
/usr/include/sys/fcntl.h:1:2: error: #warning redirecting incorrect
 #include <sys/fcntl.h> to <fcntl.h> [-Werror=cpp]
 1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h>

In file included from module/os/linux/zfs/vdev_file.c:36:
/usr/include/sys/fcntl.h:1:2: error: #warning redirecting incorrect
 #include <sys/fcntl.h> to <fcntl.h> [-Werror=cpp]
 1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h>
```

Bug: https://bugs.gentoo.org/925235
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Sam James <sam@gentoo.org>
Closes openzfs#15925
@thesamesam thesamesam deleted the musl-fcntl branch November 8, 2024 03:41
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants