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

pkgsMusl.nginx: fix build #313836

Merged
merged 1 commit into from
May 23, 2024
Merged

Conversation

superherointj
Copy link
Contributor

pkgsMusl.nginx: fix build

Fixes:

error: #warning usage of non-standard #include <sys/cdefs.h> is deprecated [-Werror=cpp]

CC Musl: @yu-re-ka @alyssais @ulrikstrid
CC Nginx: @fpletz @RaitoBezarius @helsinki-systems

Fixes:

> error: #warning usage of non-standard #include <sys/cdefs.h> is deprecated [-Werror=cpp]
@superherointj superherointj added 0.kind: build failure A package fails to build 6.topic: musl Running or building packages with musl libc labels May 23, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels May 23, 2024
Copy link
Member

@alyssais alyssais left a comment

Choose a reason for hiding this comment

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

We should strongly consider just ditching these compatibility headers and fixing stuff properly upstream, because I have a feeling they cause more build failures (like this one) than they prevent.

But let's worry about that after #229439.

@alyssais
Copy link
Member

@ofborg build nginx pkgsMusl.nginx

@fpletz fpletz merged commit 2e62b5c into NixOS:master May 23, 2024
26 of 27 checks passed
@superherointj superherointj deleted the pkgsMusl.nginx-fix-build branch May 23, 2024 14:25
@SuperSandro2000
Copy link
Member

I think a very similar fix was already proposed half a year back in #263147 which was not accepted. Glad to see that we choose a churn friendly solution 😄

@superherointj
Copy link
Contributor Author

superherointj commented Jun 13, 2024

Glad to see that we choose a churn friendly solution 😄

Hi Sandro,
As always, nice to see you here.
Thanks for the reference.
I'm not sure what you mean by that. I didn't have a better solution at the time.
I proposed what I found to be working. I understand that it might not be a perfect solution.
But I don't have perfection information. I don't really know C/C++ well [haven't programmed in that]. So I'm not comfortable in fixing at upstream level.
Also, time is limited, and there's priorities too. It's just too much to be done.
There is also an information imbalance between different people. Sometimes people say something like that is obvious, while that isn't for others.
Anyway, if someone has a better solution, I'm all for it.
Thanks for your understanding.

@Mic92
Copy link
Member

Mic92 commented Jun 13, 2024

Glad to see that we choose a churn friendly solution 😄

Hi Sandro, As always, nice to see you here. Thanks for the reference. I'm not sure what you mean by that. I didn't have a better solution at the time. I proposed what I found to be working. I understand that it might not be a perfect solution. But I don't have perfection information. I don't really know C/C++ well [haven't programmed in that]. So I'm not comfortable in fixing at upstream level. Also, time is limited, and there's priorities too. It's just too much to be done. There is also an information imbalance between different people. Sometimes people say something like that is obvious, while that isn't for others. Anyway, if someone has a better solution, I'm all for it. Thanks for your understanding.

I don't think he meant to criticize your approach, but instead was a bit disgruntled that his pull request was rejected. I would have probably fixed it in the same way. -Werror is better left to the projects itself, that can pin point a specific compiler version. However in distributions, you want to be able to upgrade compilers without having to patch half the ecosystem.

@SuperSandro2000
Copy link
Member

But I don't have perfection information. I don't really know C/C++ well [haven't programmed in that]. So I'm not comfortable in fixing at upstream level.
Also, time is limited, and there's priorities too. It's just too much to be done.

Same, same for me. I totally didn't want to critique you and just say that I had the basically the same mindset a while ago and agree with the sentiment in this thread and that I am glad that I am not alone with this opinion.

@Ma27
Copy link
Member

Ma27 commented Jun 13, 2024

@SuperSandro2000 can you elaborate please? Seems like the PR was closed by the original author and it was your suggestion in review that got rejected.

@SuperSandro2000
Copy link
Member

I think the original author and me had the same idea, to make musl maintenance easy and reduce churn with every update and compiler change but it got declined by the maintainer for reasons I do not follow. But everything alright, I don't think we need to change anything here codewise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: build failure A package fails to build 6.topic: musl Running or building packages with musl libc 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants