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.

Sign up for GitHub

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

Jump to bottom

darwin.system_cmds: fix build with clang 16 #235487

Merged
merged 1 commit into from
Jun 11, 2023
Merged

darwin.system_cmds: fix build with clang 16 #235487

merged 1 commit into from
Jun 11, 2023

Conversation

reckenrode
Copy link
Contributor

@reckenrode reckenrode commented Jun 2, 2023 •
edited
Loading

Description of changes

Clang 16 makes implicit declarations an error by default. The headers
are available, so include them.

getline was renamed to get_line to avoid a name clash. util.h
includes stdio.h, which defines getline.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@reckenrode reckenrode mentioned this pull request Jun 2, 2023
Closed
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jun 2, 2023
@ofborg ofborg bot requested review from shlevy and matthewbauer June 2, 2023 04:06
@reckenrode
darwin.system_cmds: fix build with clang 16
cd4ab1d 
Clang 16 makes implicit declarations an error by default. The headers
are available, so include them.

`getline` was renamed to `get_line` to avoid a name clash. `util.h`
includes `stdio.h`, which defines `getline`.
toonn
toonn reviewed Jun 11, 2023
View reviewed changes
Copy link
Contributor

@toonn toonn left a comment

Choose a reason for hiding this comment

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

Hmm, testing this by passing -Werror=strict-prototypes I get a ton more locations that need to be patched. This wasn't the case with Clang 16? I wonder why.

0001-Fix-implicit-declarations.patch.txt

toonn
toonn approved these changes Jun 11, 2023
View reviewed changes
Copy link
Contributor

@toonn toonn left a comment

Choose a reason for hiding this comment

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

I was enabling the wrong warning as error, should've been implicit-function-declaration >.<

LGTM now!

@toonn toonn merged commit b7ed52e into NixOS:staging Jun 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@toonn toonn toonn approved these changes

@shlevy shlevy Awaiting requested review from shlevy

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@reckenrode @toonn