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

portability: Avoid glibc and linux mount.h conflict #364

Closed
wants to merge 2 commits into from
Closed

portability: Avoid glibc and linux mount.h conflict #364

wants to merge 2 commits into from

Conversation

kraj
Copy link
Contributor

@kraj kraj commented Aug 14, 2022

With glibc 2.36+ linux/mount.h> and <sys/mount.h> headers are
no longer directly compatible

Signed-off-by: Khem Raj raj.khem@gmail.com

With glibc 2.36+ linux/mount.h> and <sys/mount.h> headers are
no longer directly compatible

Signed-off-by: Khem Raj <raj.khem@gmail.com>
Fixes
toys/other/lsusb.c:116:16: warning: variable 'tick' set but not used [-Wunused-but-set-variable]                                                                                                                                                                int fd = -1, tick = 0;                                                                                                                                                                                                                                                     ^
Signed-off-by: Khem Raj <raj.khem@gmail.com>
@landley
Copy link
Owner

landley commented Aug 15, 2022

No. This is a glibc bug. It goes in portability.h in an #ifdef glibc version >= such-and-such, and the fix is to not #include the broken glibc header but instead copy whatever structure defenitions and function prototypes we need out of it inside that #ifdef (but only for glibc, not for musl or bionic, or older glibc versions that weren't broken for the past 15 years).

We don't pollute a dozen separate commands for gnu clowns being stupid.

@landley landley closed this Aug 15, 2022
@landley
Copy link
Owner

landley commented Aug 15, 2022

I don't have a build environment for freshly broken gnu/stallman libraries: does moving the #include <linux/fs.h> before the #include <sys/mount.h> fix it, or did they not bother to have ANY guard define checking or #ifdefs at all?

@kraj
Copy link
Contributor Author

kraj commented Aug 15, 2022

I don't have a build environment for freshly broken gnu/stallman libraries: does moving the #include <linux/fs.h> before the #include <sys/mount.h> fix it, or did they not bother to have ANY guard define checking or #ifdefs at all?

This does not work either :(. Because linux/fs.h now includes linux/mount.h which comes in direct conflict with sys/mount.h [1] and sys/mount.h added same stuff with different types than kernel.

[1] https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E

@kraj
Copy link
Contributor Author

kraj commented Aug 15, 2022

No. This is a glibc bug. It goes in portability.h in an #ifdef glibc version >= such-and-such, and the fix is to not #include the broken glibc header but instead copy whatever structure defenitions and function prototypes we need out of it inside that #ifdef (but only for glibc, not for musl or bionic, or older glibc versions that weren't broken for the past 15 years).

yeah, I did think of doing that first but thought of avoiding bunch of ifdefs. but I guess thats what portability.h is for so, let me try it.

We don't pollute a dozen separate commands for gnu clowns being stupid.

yeah

@kraj
Copy link
Contributor Author

kraj commented Aug 15, 2022

I don't have a build environment for freshly broken gnu/stallman libraries: does moving the #include <linux/fs.h> before the #include <sys/mount.h> fix it, or did they not bother to have ANY guard define checking or #ifdefs at all?

try archlinux it uses glibc 2.36, fun !

@landley
Copy link
Owner

landley commented Aug 16, 2022

I've committed something locally that builds on glibc without including sys/mount.h but I need to finish my in-progress diff.c changes before I can push my current branch. Working on it...

@landley
Copy link
Owner

landley commented Aug 19, 2022

I'm told glibc fixed it in bminor/glibc@774058d72942 and bminor/glibc@2955ef4b7c9b so I can revert my local fix. (Still needs testing, but they've at least acknowledged the problem.)

@kraj
Copy link
Contributor Author

kraj commented Aug 19, 2022

I'm told glibc fixed it in bminor/glibc@774058d72942 and bminor/glibc@2955ef4b7c9b so I can revert my local fix. (Still needs testing, but they've at least acknowledged the problem.)

yeah, but 2.36 is just released so we have to live with the broken release for another 6 months or so atleast if not more.
I wonder if these should be backported to release branch.

@landley
Copy link
Owner

landley commented Aug 19, 2022

Two Red Hat guys and the Android Bionic maintainer are cc'ing me on an email discussion about that, which is always frustrating because I can't just link you to the relevant post in a web archive and let you join in their conversation without me being a middleman.

http://www.jowaltonbooks.com/poetry/whimsy/the-lurkers-support-me-in-email/

@landley
Copy link
Owner

landley commented Aug 20, 2022

A couple emails back Florian Wermer said "Right now, we want to give what we have for 2.37 a little bit more soak time, but I think the fix is ready for backporting to 2.36, at which point it will begin to make it into distributions. Cc:ing Carlos to see if he wants to restart doing point releases."

And I just got cc'd on Carlos O'Donnel's reply which starts "I do not plan to restart doing point releases" and then talks about "rolling releases" because two different people seeing consistent predictable behavior is against gnu or something?

So it looks like the official .36 is going to stay broken forever but .37 should work, and I guess every distro everywhere is more or less expected to locally add the two backported patches since glibc has abandoned the responsibility of acknowledging brown-paper-bag bugs.

Again, you'd think there would be a project mailing list for this with a web archive instead of private email random people are cc'd on because somebody else cc'd them out of the blue earlier... but no.

@enh-google
Copy link
Collaborator

So it looks like the official .36 is going to stay broken forever but .37 should work, and I guess every distro everywhere is more or less expected to locally add the two backported patches since glibc has abandoned the responsibility of acknowledging brown-paper-bag bugs.

yeah, i'm surprised that's how it's supposed to work, but, meh, "not my libc".

Again, you'd think there would be a project mailing list for this with a web archive instead of private email random people are cc'd on because somebody else cc'd them out of the blue earlier... but no.

well, that part's my fault. i assume they do have mailing lists; i just don't happen to be on them/know which would be appropriate (and had seen florian making positive statements somewhere on github).

i also assume there's some way to see how far through that process they are, but it looks like debian doesn't even have an affected glibc yet? https://packages.debian.org/search?keywords=glibc

i'm guessing that the arch linux user should file an arch linux bug or point out on https://bugs.archlinux.org/task/71741?project=1&string=glibc that this has been fixed in upstream and arch should backport the fixes? (glibc commit 774058d72942249f71d74e7f2b639f77184160a6 and commit 2955ef4b7c9b56fcd7abfeddef7ee83c60abff98.)

@kraj
Copy link
Contributor Author

kraj commented Aug 23, 2022

yeah and fedora 37 is using it as well see https://koji.fedoraproject.org/koji/buildinfo?buildID=2042380
gentoo is perhaps on its way as well as it seems - https://packages.gentoo.org/packages/sys-libs/glibc
We switched to 2.36 in yocto already. perhaps best to backport those two patches, but are there any fallouts because of them ? well need to see.

@landley
Copy link
Owner

landley commented Aug 23, 2022

I sympathize with the predicament gnu/redhat is putting you in, but if I merge a workaround for a bug they already fixed (almost immediately after the release) when do I get to remove it? A distro carrying a local patch to toybox can remove the workaround as soon as they upgrade to a non-broken glibc. But I've got an explicit 7 year rule for environments I choose to support, so I'd be stuck with the workaround until 2030.

I also admit glibc's (temporary) position that everybody should copy symbols out of kernel headers rather than including them was reopening old wounds for me ( https://lkml.iu.edu/hypermail/linux/kernel/0412.0/0866.html ). I fought that battle already, they were on the wrong side of it here, and I'm very happy they acknowledged that by fixing it. It's likely I'm a bit more "kill it with fire" about this issue than I would be otherwise, for which I apologize. I'm trying to compensate for that bias and I still think not taking a patch for something that's already fixed is the right thing to do, but if you really, really, really, really, really need the workaround merged into toybox... I will be unhappy, but at least they fscking FIXED it upstream. Grrr.

As for glibc refusing to have bugfix releases, they're saying "we are better developers than linux-kernel". Linus NAMED the concept of the "brown paper bag" release. I've done them a couple times in toybox myself, such as https://landley.net/toybox/#25-06-2012 and https://landley.net/toybox/#23-07-2015 . These days I have a reasonable test suite and a release checklist, but I reserve the right to acknowledge being stupid as necessary. Nobody's infallible. A gnu project staffed by the company that gave us systemd saying they don't backport bug fixes is not my idea of fun.

Glibc fixing git but refusing to backport the fix is a choice their development team is making. Distros shipping the known broken version without backporting the fix would be another choice. Me, I think there's a process missing there. Pointing people at a "rolling release" branch doesn't seem very helpful of glibc, and if they stick to that I highly doubt this is the only time it's ever going to come up. "It's ok, we only broke llvm without noticing, not like it was something important" does not inspire confidence in an "eyes forward never look back" release approach. (Am I being asked to take this patch for any reason OTHER than to help paper over glibc's broken process?)

If I don't merge a workaround for the .36-specific bug, it should start working again in February either way[1], this doesn't affect bionic or musl (which should not be ENCOURAGED to tell us to start block copying crap out of linux/loop.h again), my own devuan laptop is still on .28 plus debian's fixes du jour (so the main glibc test environment I develop against is unaffected and there's no way debian stable WOULDN'T backport a fix for something like this), I already ship prebuilt binaries each release (statically linked against musl), and as I mentioned a distro is welcome to carry a local patch against toybox for 6 months if they don't want to carry the fix against glibc?

That said, if you really can't cope otherwise tell me. I lump it when Elliott needs something, yocto's big enough I'm willing to be overruled when it's actually important. But as Seanan McGuire writes over doorways, "Be sure".

1 - modulo I haven't actually tested .37 either because I'm not set up to build glibc from source and incorporate it into toolchains and root filesystems here; I do that for musl, but it doesn't need perl as a build prerequisite.

@kraj
Copy link
Contributor Author

kraj commented Aug 23, 2022

I am looking towards backporting the glibc fixes into yocto. Perhaps thats the best course forward until 2.37 release. I think you have valid points.

@paulwratt
Copy link

since its already been fixed upstream, and since they are not doing point releases (is there any way to force/shame them into doing at least one) then there will inevitably be distro with broken glibc 2.36.

if the patch is available that works around broken 2.36, apply it, branch and tag it, then "un-fix" it (as opposed to rolling back). Thats ugly, but the patch is then "obviously" in the repo so it can be found and used if needed. Alternately add broken_glibc_236.patch file into the project, and let the builder decide it they need it or not.

The only other solution I can see is in via build configuartion that 1st checks version, then if glibc 2.36, do a check to see if its been patch. At least this way its a cleaner build.

Are there any other similar fixes in the past, that may or may not still be present, if so how were they handled..

For LTS I would not bother with 2.36 at all, its not even in wide spread use and yet look at the amount of damage and heart ache its already caused .. just skip to 2.37

@landley
Copy link
Owner

landley commented Aug 23, 2022

No.

@paulwratt
Copy link

paulwratt commented Aug 23, 2022 via email

@paulwratt
Copy link

paulwratt commented Aug 23, 2022

then what a about (up front in the readme) something like :

glic 2.36 is not supported without a patch

and leave it to others to decide what needs patching, and more importantly, discuss amongst themselves why a patch is needed in the first place

@kraj
Copy link
Contributor Author

kraj commented Aug 24, 2022

relevant patches have been backported into 2.36 release branch [1]. Phew !!

[1] https://sourceware.org/git/glibc.git/log/3bd3c612e98a53ce60ed972f5cd2b90628b3cba5...4dad97e2a2e510c6b53a0add29a2188714fcf4ab

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants