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

usrsctp: fix build with clang 15+ #262809

Merged
merged 1 commit into from
Oct 24, 2023
Merged

Conversation

reckenrode
Copy link
Contributor

@reckenrode reckenrode commented Oct 22, 2023

Description of changes

  • Add missing function prototypes; and
  • Remove set but unused variables.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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.

@misuzu
Copy link
Contributor

misuzu commented Oct 23, 2023

I don't think it's a good idea to update this library to some random commit. Maybe there's a way to fix this clang16 issue while still using a tagged release.
Also this change should go to the staging

@reckenrode
Copy link
Contributor Author

reckenrode commented Oct 23, 2023

I don't think it's a good idea to update this library to some random commit.

Ideally, but that letter assumes projects doing regular releases, which is not the case here. This one doesn’t. It only has three releases, and the last release was very minor (just cleaning up .so versioning). In the issue requesting what would become the 0.9.4.0 release, it was questioned why people couldn’t just use snapshots instead. The release notes for that version are simply, “Current status as requested in #561.”

Maybe there's a way to fix this clang16 issue while still using a tagged release. Also this change should go to the staging

Sure, I can write and vendor a patch in nixpkgs. I normally like the to cherry-pick commits from upstream that fix the issue, but those here are mixed in with changes other than just the fixes. I rebased my staging branch last night, so it’s going to be a little while before I have a patch, but I should be able to update this PR by this evening EDT.

@reckenrode reckenrode changed the title usrsctp: 0.9.5.0 -> unstable-2023-09-13 usrsctp: fix build with clang 15+ Oct 23, 2023
@reckenrode reckenrode changed the base branch from master to staging October 23, 2023 20:20
* Add missing function prototypes; and
* Remove set but unused variables.
@reckenrode
Copy link
Contributor Author

reckenrode commented Oct 23, 2023

I updated the PR with a patch to remove the unused variables and add the missing prototypes. I initially tried applying the commits from upstream FreeBSD that did this, but they didn’t apply cleanly, so I made the changes manually to create the included patch. I also retargeted to staging.

@reckenrode reckenrode merged commit 58a63d4 into NixOS:staging Oct 24, 2023
24 checks passed
@reckenrode reckenrode deleted the usrsctp-update branch October 24, 2023 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants