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

Adjusted the linker flags for kconfig mconf (menuconfig) #332

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

echelonxray
Copy link

For some reason, this breaks on my Gentoo systems. There,
libcurses does not contain nodelay, so the linking fails
with symbol resolution errors regarding the nodelay symbol.

I have added -ltinfo to fix this issue.

Signed-off-by: Michael T. Kloos [email protected]

@echelonxray
Copy link
Author

Rob. If you prefer, I can close this and "git send-email" this as a patch up to the mailing list.

@landley
Copy link
Owner

landley commented Apr 9, 2022

That's not the hold up.

This kconfig fork is more than 10 years old, comes from the Linux kernel, and has built on every system I've ever thrown it at. Suddenly and uniquely, gentoo broke. Now you want to add an extra library which isn't probed: if that library isn't there on some system, the build will break.

I have a todo item to replace this subdir with a fresh implementation. If gentoo did uniquely break its curses implementation so that toybox stopped building on gentoo but still builds on every other Linux system, then the fix for gentoo is for toybox's build to stop using curses.

@echelonxray
Copy link
Author

I know that it comes from the Linux kernel and menuconfig works fine for me from a recent Linux kernel. The recent Linux kernels are using "pkg-config --libs ncursesw" or "pkg-config --libs ncurses" to determine the libraries to link. On my Gentoo system that resolves to "-lncurses -ltinfo".

I remember working with Toybox years before, but I can't recall if I was using Gentoo at the time. It is possible it was broken in an update in the last few years. I'll try to do some more digging into exactly what is up with curses. I was able to find some threads on the subject from the Gentoo forums but they are almost 10 years old and reference USE Flags that no longer seem to exist. Other distros seem to be symlinking libtinfo to libncurses. This would seem to indicate that Gentoo is using a different build configuration for curses that splits it's libraries across these separate files.

For some reason, this breaks on my Gentoo systems.  There,
libcurses does not contain nodelay, so the linking fails
with symbol resolution errors regarding the nodelay symbol.

I have changed it to use pkg-config --libs ncurses, which,
in my case, adds -ltinfo to fix this issue.

Signed-off-by: Michael T. Kloos <[email protected]>
@echelonxray
Copy link
Author

I agree that a new self-contained build system is needed. After all, the one currently being used is GPLv2. What is your vision for this new build system? Did you intend to write something that mimics kbuild, something similar but more minimal, or something different. I suppose it could be an interesting project. I might consider starting work on something if you want.

In the meantime, I have force-pushed a change to my pull request to make the linking more dynamic using pkg-config.

@landley
Copy link
Owner

landley commented Apr 10, 2022

So now it won't build on a system that hasn't got pkg-config installed. Which was not previous a dependency...

@landley
Copy link
Owner

landley commented Apr 10, 2022

And my new kconfig should read the existing config syntax and do defconfig, allnoconfig (including KCONFIG_ALLCONFIG), allyesconfig, oldconfig, and menuconfig (which needs to show help, do forward slash searches...) It's maybe 2 weeks focus?

@echelonxray
Copy link
Author

I've started work on a Kconfig replacement. I'd like to be able to use it in one of my projects as well. I can't make any promises to when it will be complete though.

@thesamesam
Copy link

thesamesam commented Feb 10, 2024

The ncurses headers are being used, so ncurses should be linked against using its proper name.

We dropped the libcurses symlink for https://bugs.gentoo.org/836696 because it confused CMake and there was no real reason to keep it. Are there any systems that toybox runs on where ncurses is used where the library is not named libncurses?

Now, as for nodelay and tinfo, some distributions build ncurses with a split tinfo because it affects ABI. Hence the need for asking pkg-config for flags, as it may return -lncurses -ltinfo.

Is there no way in the current toybox setup to try a command and fallback if it fails?

@landley
Copy link
Owner

landley commented Feb 10, 2024

If it was just "change -lcurses to -lncurses which has been in all sy stems for 7 years" I'd happily do it. But "also probe for this other random library because we split one library into two libraries, and now you have a new package build dependency on this whole other pkgconfig thing that didn't need to be installed before"...

I'm sorry you broke your system.

@thesamesam
Copy link

We've had split tinfo for ages (since about 2013). Anyway, happy to engage constructively if there's interest in doing so. I don't think I've come in and blamed you for anything, just asked some questions to hopefully get towards a solution.

@echelonxray
Copy link
Author

Might I suggest that Gentoo apply the diff from this PR as a patch with an ebuild? It would fix the problem downstream until this subsystem is replaced.

@thesamesam
Copy link

Yeah, I will do. Thanks! Was just hoping to discuss it and see if we could find some compromise but that doesn't seem to be happening.

@landley
Copy link
Owner

landley commented Feb 11, 2024

I acknowledge there is a problem, and that I need to address it, I just dunno how to do your suggested fix without installing and regression testing a dozen distro images (current and old versions of fedora, debian, suse, ubuntu, alpine, popos, elementaryos, arch, and gentoo) plus testing on current versions of freebsd and macos, or adding a new build dependency package.

I need to replace kconfig with a fresh implementation, including a working allyesconfig, allnoconfig, defconfig, miniconfig, and menuconfig. I've bumped that up the priority list.

I think the best way to do a new menuconfig is just have a big bullet point list you scroll up/down, that maintains an indentation level instead of navigating into full-screen menus. (That wouldn't scale to the dozen level deep Linux menus, but toybox menus only go 2 or 3 levels deep.) And I can do the "help include" syntax while I'm at it...

@eli-schwartz
Copy link

Maybe I'm missing something, but what's wrong with:

cc ... $(pkg-config --libs ncurses || echo -lcurses)

as a replacement for cc ... -lcurses?

It is an industry-wide standard used by Makefiles that try checking pkg-config and fall back to a statically coded common version... it's very common because pkg-config is very common.

No need to hard block everything on extensively rewriting your entire build system. No need to hard block everything on adding regression testing of "a dozen distro images plus testing on current versions of freebsd and macos".

@landley
Copy link
Owner

landley commented Feb 11, 2024

See "or adding a new build dependency package", above.

@eli-schwartz
Copy link

HOW is it a new dependency to not require something and use the existing -lcurses when it doesn't exist? Please explain.

@landley
Copy link
Owner

landley commented Feb 11, 2024

A build dependency is a package required to build another package. The pkg-config package is a GPLv2 program from http://pkg-config.freedesktop.org packaged by debian at https://packages.debian.org/pkg-config which the existing toybox build does not require.

Toybox does not implement pkg-config, nor is it one of the 7 packages that aboriginal linux required to build a musl+uclibc minimal native development environment capable of rebuilding linux under itself and building linux from scratch under the result via https://landley.net/aboriginal/control-images/ . (Linux from scratch packages that required pkg-config could build and install it from source under the existing system the same way build dependencies like perl and python were handled.)

Using a finished toysh and built with an llvm+musl or llvm+bionic toolchain instead of gcc+glibc, in theory toybox can be built without any gpl prerequisite packages (outside the kernel). This would be a requirement for such a development environment to eventually be considered for addition to Android itself, allowing Android to build under Android. (It's also why there's a git stub in pending: that's a GPL package and building AOSP requires git.)

Repository owner deleted a comment from eli-schwartz Feb 14, 2024
@joecool1029
Copy link

A build dependency is a package required to build another package.

@landley I get your concern but is it possible there's a simple miscommunication here? Eli's earlier suggestion does not require pkg-config since it prints the same -lcurses it always has for systems that don't have pkg-config installed.

If seeing stderr print out that pkg-config is missing on those systems is intrusive add something like:

cc ... $(pkg-config --libs ncurses 2>/dev/null || echo -lcurses)

Just to reiterate: Doing it this way in no way requires pkg-config to be installed on the build system It only calls it if it's there and falls back to the old behavior if it isn't.

@eli-schwartz
Copy link

eli-schwartz commented Apr 18, 2024

There's no miscommunication. @landley does not like systems he isn't interested in and if you argue about it your comments are prone to being deleted.

When you're convinced that pkg-config is "evil", the problem isn't whether or not it is required. The problem is that, apparently, accepting a patch to optionally use it would involve the ethical compromise of admitting that there are circumstances where it could be useful. Better to delete convincing arguments.

If it was a simple miscommunication, then there wouldn't be coverups.

@joecool1029
Copy link

I'm not here to make assumptions or complain. Just saw a claim that was not accurate so I wanted to know why so this very real problem Rob acknowledges can get a fix that works everywhere until he ships his new build system. If there's a serious issue with doing it that way then just say it, the stated problem however is not the case.

Remember, this could have been closed and all of Eli's comments could have been deleted, but they weren't. Wizard could have been casting spells in the deleted one for all we know.

@oliverkwebb
Copy link

Remember, this could have been closed and all of Eli's comments could have been deleted, but they weren't. Wizard could have been casting spells in the deleted one for all we know.

It's something I heavily doubt, the context around that comment makes it seem like it's actual technical discussion. I of course could be wrong and am getting my pitchfork out too early, but just deleting a comment with no beforehand warning or reason as to why afterwards looks BAD. And it gets rid of technical discussion that could be linked too.

As Eli said: "If it was a simple miscommunication, then there wouldn't be coverups." it would be good to at least get a reason for github comments mysteriously disappearing in technical discussion where the other party has a valid argument, and there being none is... Concerning

@eli-schwartz
Copy link

eli-schwartz commented Apr 18, 2024

I don't recall the exact wording I used. For the record, GitHub does contain the required functionality for repository owners to mark abusive comments as "disruptive" and hide them by default, which is... frequently a better option than outright deletion if you want people to be able to verify after the fact what exactly was going on.

What I do recall is that I tried to point out that the suggestion to use

pkg-config ... || echo "fallback value if pkg-config cannot be executed because it's not installed"

(in this case echoing the statically encoded "-lcurses")

is per definition not a build dependency because the || shell construct means it is not required, and on Aboriginal or Android it will simply skip right over to the echo that is used today.

On systems where only POSIX functionality (i.e. not pkg-config) is available all you need to support is POSIX || and POSIX echo.

@eli-schwartz
Copy link

And I don't understand what the red herring is about "ackshually pkg-config is gplv2 and toybox doesn't implement pkg-config nor require it to bootstrap a musl+uclibc minimal native development environment capable of rebuilding linux under itself and building linux from scratch under the result via https://landley.net/aboriginal/control-images/ [...] in theory toybox can be built without any gpl prerequisite packages (outside the kernel). This would be a requirement for such a development environment to eventually be considered for addition to Android itself, allowing Android to build under Android. (It's also why there's a git stub in pending: that's a GPL package and building AOSP requires git.)".

It's a license copypasta that misses the point that no one ever tried to force a mandatory dependency on gplv2 software to begin with, due to it being, well, optional.

Looks to me more like some kind of "license politics" instead of minimalism.

If it's not required then the license shouldn't matter as it won't be used by Android anyway. But if one is reluctant to add even optional checking for a GPLv2 program and executing it if it exists, because one dislikes GPLv2...

No comment.

@oliverkwebb
Copy link

It's a license copypasta that misses the point that no one ever tried to force a mandatory dependency on gplv2 software to begin with,

Funnily enough, Rob makes the code depend on GPLv3 bash (Yes there is a replacement in the works, but for now the build depends on there being GPLv3 code on the system). But discourages any other dependence non-userspace GPL code.

(<tangent>
The dependence on bash never personally made sense to me, and seems to mainly derive from a 15 year old technical disagreement with Ubuntu. When shell portability is a larger problem (The same way C doesn't need replacing as much as C++ does, but people confuse the 2 anyway).

Rob's argument for bash is usually a appeal to tradition with a "#include "story-of-our-people.txt"" at the end. And while the story of the 2400 baud modem and the sun manuals and whatnot is nice to hear. Appealing to tradition never made since to me because times have changed and almost every build script ever is POSIX/dash compliment.

(Another way to think of this is that if Linus Torvalds created linux by bootstrapping a kernel to run csh, would it mean that we should set out to implement it?)
</tangent>)

shrug, anyway

due to it being, well, optional.

Looks to me more like some kind of "license politics" instead of minimalism.

If it's not required then the license shouldn't matter as it won't be used by Android anyway. But if one is reluctant to add even optional checking for a GPLv2 program and executing it if it exists, because one dislikes GPLv2...

My impression was that Rob disliked GPLv3 and the FSF, and doesn't use GPLv2 anymore because of corporate politics with google and android.

Still, given the info you gave, I have absolutely no idea why Rob decided to get rid of your thread, and from my view was probably just outright wrong in doing so. Which is why I was so angry

Thanks

@enh-google
Copy link
Collaborator

Funnily enough, Rob makes the code depend on GPLv3 bash (Yes there is a replacement in the works, but for now the build depends on there being GPLv3 code on the system). But discourages any other dependence non-userspace GPL code.

nonsense --- toybox builds fine on macOS, which remained on the last GPLv2 bash.

@oliverkwebb
Copy link

oliverkwebb commented Apr 18, 2024

Funnily enough, Rob makes the code depend on GPLv3 bash (Yes there is a replacement in the works, but for now the build depends on there being GPLv3 code on the system). But discourages any other dependence non-userspace GPL code.

nonsense --- toybox builds fine on macOS, which remained on the last GPLv2 bash.

Ah, the support horizon is 10 years and the last GPLv2 bash was 16 years ago, assumed that you needed GPLv3 bash (Especially since MacOS doens't even use bash anymore, replacing it for zsh)

I imagine that toybox would build fine under a gsed from 2007, don't know though

@enh-google
Copy link
Collaborator

Ah, the support horizon is 10 years and the last GPLv2 bash was 16 years ago, assumed that you needed GPLv3 bash (Especially since MacOS doens't even use bash anymore, replacing it for zsh)

my mac disagrees with "replace":

~$ bash --version
GNU bash, version 3.2.57(1)-release (arm64-apple-darwin23)
Copyright (C) 2007 Free Software Foundation, Inc.
~$ 

only the default has changed to zsh.

@eli-schwartz
Copy link

eli-schwartz commented Apr 18, 2024

The funny thing here is that the GPLv2 pkg-config program hasn't been developed in 7 years, and about 5 years ago distros started switching to an alternative implementation that is ISC licensed.

Even if pkg-config was mandatory to build toybox it still wouldn't be a gpl prerequisite package, and android could absolutely include that ISC implementation as part of a development environment included with Android itself.

It would still be an additional dependency, which is the original objection to making it mandatory, so that's not going to happen. But the license copypasta is just uninformed.

I don't think I mentioned this in my deleted comment, but I... can't remember. I know I knew it at the time, and I might have mentioned it, but I might have been focused on other aspects and after having my comment deleted I would have anyways lost interest in further discussion.

However since we're all back to mentioning the license anyway... :)

@landley
Copy link
Owner

landley commented Apr 19, 2024

Sorry, I've been ignoring this thread ever since I actually started work on the kconfig replacement code. I accepted the problem report, but not the proposed solution, and would rather work on the fix than argue after the fact about why I'm not doing a different fix.

Re support horizon: if MacOS 14 "sarcoma" shipped last year, and the bash version available in that is 3.2, it doesn't matter when bash 3.2 came out unless macos ships an update for the package that all the sarcoma users can apply, or toybox's prereq build includes a shell that can drive the rest of the build. (Yes it's a judgement call whether a large distro prominently shipping an old package refreshes the timer, or in the case of centos had a support horizon that extended the timer.)

Looks like I did delete a comment from eli-schwartz some months ago? Checking my back email, he was replying to my statement that I will not add the build dependency by basically saying I don't get to make that decision and then reading the posix page for the shell || operator at me because clearly I didn't know about it, and it probably seemed more polite to me at the time to just delete it than any reply I could make would have been to borderline sealioning from someone who jumped into a thread a dozen replies in to ignore a repeated "no".

@joecool1029
Copy link

joecool1029 commented Apr 19, 2024

@landley The obvious solution is to close this thread since the PR and alternatives presented you rejected. It's for the best.

It's clear you and the others would rather now go on about tangentially related things. My single goal in commenting was to ask why a fix that appears to work in all scenarios and that did not fall afoul of your stated concerns was rejected or ignored. The thread was still open and miscommunications happen, so I tried. However, it's clear this isn't a technical discussion and must be some kind of strange performance.

I'm out.

@eli-schwartz
Copy link

Looks like I did delete a comment from eli-schwartz some months ago? Checking my back email, he was replying to my statement that I will not add the build dependency by basically saying I don't get to make that decision

The comment of mine that comes before the comment you deleted unambiguously proves that I said no such thing.

But more generally, "telling the maintainer they don't get to make that decision" is pure nonsense, and not something I'd ever say.

In fact, in other projects I have been extremely happy to explain to people how they can reduce that build dependency down to an optional feature. And I'm a core committer for an Apache build system that goes to great lengths to make pkg-config completely optional in this precise scenario, which is very much not easy but we do it because people should be able to build software in environments that don't have pkg-config, if possible (and ncurses does make it possible).

So, please don't make up lies about me. Thanks.

then reading the posix page for the shell || operator at me because clearly I didn't know about it

That sounds like the kind of thing that might have occurred due to exasperated sarcasm. However GitHub definitely doesn't send me emails for my own comments, so I'm unable to verify and given that you're making up things about me I'm disinclined to take your word on it...

than any reply I could make would have been to borderline sealioning from someone who jumped into a thread a dozen replies in to ignore a repeated "no".

  • citation definitely needed if you're going to make that accusation
  • you have absolutely no right whatsoever to tell me that I'm not allowed to leave a comment on an issue/PR that affects me, just because I'm not the person who originally opened it and there have been a dozen comments already. Regardless of whether I ignored a repeated "no" -- but I didn't ignore it, I very politely tried to offer a compromise, to which you replied with an unusual dismissiveness.

Rob, it's within your right as a maintainer to say "no, I don't want to". You don't need to make up lies about what people say while hiding behind deleted comments, then pretend you care about their problems but the solutions "don't work" despite fulfilling every one of your publicly stated criteria.

It's extremely off-putting, and if we're going to discuss "borderline anything" behavior then what you're doing is borderline libel.

Please reconsider your interaction style.

@oliverkwebb
Copy link

The comment of mine that comes before the comment you deleted unambiguously proves that I said no such thing.

"HOW is it a new dependency to not require something and use the existing -lcurses when it doesn't exist? Please explain" Doesn't prove anything, nor the comment above it you made.

Can we get the comment Eli posted back, at least copy-pasted somewhere for the record? I know it doesn't matter, but I'd like to come to my own judgement on whether or not Rob's or Eli's description of it was accurate as opposed to blindly believing one or the other.

In fact, in other projects I have been extremely happy to explain to people how they can reduce that build dependency down to an optional feature.

Even if this commit gets through, in a few weeks (A few months at most) you won't need "optional feature". There will be a different configure system without ncurses.

than any reply I could make would have been to borderline sealioning from someone who jumped into a thread a dozen replies in to ignore a repeated "no".

citation definitely needed if you're going to make that accusation

I second this.

you have absolutely no right whatsoever to tell me that I'm not allowed to leave a comment on an issue/PR that affects me,

But to be fair, "you have absolutely no right whatsoever" makes Eli's "I'd never tell a Project manager what they are and aren't allowed to do on their own project" a lot less credible....

just because I'm not the person who originally opened it and there have been a dozen comments already. Regardless of whether I ignored a repeated "no" -- but I didn't ignore it, I very politely tried to offer a compromise, to which you replied with an unusual dismissiveness.

The kconfig rewrite is coming, change the makefile yourself, and wait 2 weeks if it's that vital of a problem.

Rob, it's within your right as a maintainer to say "no, I don't want to". You don't need to make up lies about what people say while hiding behind deleted comments, then pretend you care about their problems but the solutions "don't work" despite fulfilling every one of your publicly stated criteria.

It's extremely off-putting, and if we're going to discuss "borderline anything" behavior then what you're doing is borderline libel.

Please reconsider your interaction style.

Because yours is perfect and well gauged to a problem this minor that can be fixed on your end in 30 seconds...

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.

7 participants