mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
28.08k stars 2.88k forks source link

waf should look into "/usr/local" on Unix-like systems #2710

Closed czarkoff closed 8 years ago

czarkoff commented 8 years ago

Currently waf doesn't search headers or libraries under "/usr/local". It is a known and widely used location, so waf should add -I/usr/local/include to compiler flags and -L/usr/local/lib to linker flags.

mia-0 commented 8 years ago

It uses pkg-config for most of the detections, so make sure your PKG_CONFIG_PATH is set correctly.

czarkoff commented 8 years ago

It uses pkg-config long after it matters. Before trying pkg-config it fails during iconv test.

avih commented 8 years ago

This could have negative effects when cross compiling.

czarkoff commented 8 years ago

Sure. That's why I file an issue instead of pull request - it should be done by someone who understands waf and its use cases in this project better then me.

ghost commented 8 years ago

It should look in the compiler's default search path. If the platform doesn't include /usr/local in its default search path (like BSDs), then that's not our problem. Why should every single project on this planet hardcode this path?

czarkoff commented 8 years ago

Probably because it is industry standard to hardcode this path? FWIW mpv even installs to this location by default.

ghost commented 8 years ago

Probably because it is industry standard to hardcode this path?

Maybe from your BSD perspective. On Linux, /usr/local is part of the toolchain default paths by default. On Windows, the most used desktop OS, there isn't even a /usr/local.

czarkoff commented 8 years ago

It is not just BSD perspective: same is true for almost all Unix-like systems with several Linux distros being notable exceptions. Provided that mpv relies on stuff that is by no standard supposed to be part of operating system, doesn't it make sense to actually search for that stuff in location where it is supposed to be found?

ghost commented 8 years ago

If it's "standard", why would a program explicitly have to add these to the search paths? This makes literally absolutely no sense.

The systems on which /usr/local is not in the compiler search path probably normally expect that things are installed to /usr, so just install stuff there.

czarkoff commented 8 years ago

/usr/local is a standard location for things that are not parts of operating systems. It is absolutely logical that operating system's tools don't look there by default - it is outside their scope of responsibility. It is absolutely logical that software which relies on stuff outside operating system should manage that stuff on its own accord. It is absolutely logical that stuff which doesn't come with operating system is not installed to operating system's destination parts. One may wonder if including directories for external stuff in default search paths makes any sense.

That is probably why most Unix software out there indeed hardcodes /usr/local in search paths without relying on operating system doing things it is not supposed to do.

ghost commented 8 years ago

It is absolutely logical that operating system's tools don't look there by default

Well, then you sure don't mind that mpv by default doesn't look for components not part of the operating system. Why should it?

czarkoff commented 8 years ago

Because it is not a part of operating system and relies on software that is also not a part of operating system.

See, if the problem was that libc wasn't reachable via default library path, it would not be mpv's problem: libc is a part of operating systems by all standards. Iconv is not. And it should never be assumed to be reachable without looking up places where external software resides.

olifre commented 8 years ago

libc is a part of operating systems by all standards. Iconv is not.

For reference, I don't even have anything in /usr/local/ on my Gentoo desktop system - and staying with the example of iconv, on Gentoo it is part of glibc for almost all arches, and pulled in by virtual/libc, so at least this distribution (which has a ports system, so it's somewhat closer to BSD) disagrees very much on this statement.

I also do not see why mpv should check for any external software - any sensible packaging (ports, ebuilds, rpms, whatever) installs mpv to /usr and "makes it part of the operating system" in a clean way.

What would CLEARLY happen though if mpv started to look in random non-standard locations like /usr/local or even /opt/ is that Linux distributions would have to patch mpv's build system downstream on the distribution side to make it NOT look there, since that severely interferes with packaging (no external locations should be checked) and thus violates all packaging guidelines I know of, just pick any distribution and read the guidelines.

czarkoff commented 8 years ago

On Unix-like systems mpv installs itself into /usr/local by default. Even on your Gentoo desktop system. Fortunately, there is a flag to change prefix, so that distributors can avoid patching and may install wherever they want. Obviously, there is enough flexibility in waf to handle this issue. What makes library/headers search paths so special that they can't be handled the same way?

olifre commented 8 years ago

On Unix-like systems mpv installs itself into /usr/local by default.

That's the waf-default AFAIK, mpv does not hardcode anything here. This default is likely to prevent accidental installation into system directories when "trying out" the software - for permanent installation I would expect one uses (or just writes) a port / build a package for it, which indeed uses --prefix to install to system paths. Every sensible build system offers prefix (or something similar) nowadays, luckily.

What makes library/headers search paths so special that they can't be handled the same way?

The simple fact that looking in any explicit path by default breaks relocatability of the package. Simple example: Application XY links against library /usr/local/lib64/libXYZ.so (explicitly with full path, since that's not in the OS's library path) on machine B. The user of machine B builds an RPM-file of that and distributes it. Nobody will be able to use that RPM, except other people with exactly the same manual non-standard setup in /usr/local/, which is why all packaging rules forbid any automagic pickup of non-standard locations. Let's not even talk about the funny mixing between library versions and ABIs you can do when having several libraries in non-standard locations. With the advent of PCMs and PCHs, these problems fully propagate to headers.

So in my opinion, every build-system should ONLY look in the system library paths and include directories, NEVER anywhere else. If all packages did that, all distributions would be patch-free, and a lot of obscure issues would be gone.

So for your case, my advice would be: To get directories such as /usr/local/lib64/ in the overall library path, the only correct way is to extend /etc/ld.so.conf on Unix-like systems - since that is then something specific to your system. In fact, it usually lists at least /usr/local/lib64/ or /usr/local/lib32/ by default. For the includes: If you are using gcc, it's even sufficient to NOT patch it since then it will look in /usr/local/include/ by default: https://gcc.gnu.org/onlinedocs/cpp/Search-Path.html Maybe you have a distribution-specific patch to gcc which prevents it from looking there?

For reference, Gentoo, as a very vanilla meta-distribution, left /usr/local/include in the list of paths in which gcc searches - and also has /usr/local/lib64/ or /usr/local/lib32/ in /etc/ld.so.conf. It just does not install anything there, since there's no good reason to do so.

czarkoff commented 8 years ago

@olifre: What you say is true on Linux and is false on other Unix-like systems, where there is strong distinction between operating system and third-party software. So users of those other systems don't install software to /usr. Now, you instruct me to break conventions of my platform because they diverge from your opinion and you don't like them?

olifre commented 8 years ago

Now, you instruct me to break conventions of my platform because they diverge from your opinion and you don't like them?

No, that's not my point - if that's a valid and common location for installation on your platform, it's surely possible to instruct the toolchain (gcc and dynamic linker) to also look in these paths. As I told you, gcc (and also clang, by the way) actually look into these places ("/usr/local") by default, unless the distribution you are on explicitly patches them to not do it. A usual Linux distribution does not do it, which means that mpv would would iconv.h even in /usr/local/include/ on my Linux system without any changes to mpv or waf. If that does not work in your Unix environment, apparently you are using a patched version of gcc / clang which has been made incompatbile with /usr/local/ on purpose by your distribution, so why install software to a place your distribution forbids?

czarkoff commented 8 years ago

@olifre Please, just stop assuming that you know my platform better then me. As I've told you before, search paths for both C preprocessor and linker are limited to locations where no third-party software should ever get installed. When you install iconv from packages, it is put under /usr/local. Because this location specifically exists for stuff that does not come with operating system and should not be present in operating system's header and library search locations. Because it does not belong to operating system.

olifre commented 8 years ago

@czarkoff 0k, I give up - just out of imterest, could you tell me the name of this Unix environment (and also compiler) since I am apparently unaware of these? Maybe then I can learn something about in in the future.

I doubt, though, that mpv will be extended to look in any non-standard location, but of course that's @wm4 and not my decision. For reference, also /usr/include is not explicitly mentioned in waf to find iconv and pkgconfig for the rest, it purely relies on gcc to look in the system include path (anything else by default would also create problems on Linux as I outlined, so it should be hidden behind a user-configurable flag or a distribution-specific check).

rr- commented 8 years ago

What is so difficult about

PATH=/usr/local/:$PATH ./waf configure && ./waf

(or whatever actually works for you) that is worth over 1500 words that have been spoken here?

ghost commented 8 years ago

The BSD logic still escapes me. Programs shouldn't hardcode include/library search paths, because 1. it's not necessary as the compiler configuration already cares about this, and 2. it should be up to pkg-config to configure non-standard paths.

Also, the main reason why there is /usr/local on Linux at all is because this is for circumventing the package manager. This is also the reason why this is the installation path.

I don't see why BSD would separate "system" and "user" software, and that physically by path. The separation doesn't make too much sense, and if you want it, there's still /lib vs. /usr/lib (but note how even BSD probably includes both in the linker default search paths).

It sounds more like this was a historical mistake of GNU software defaulting to /usr/local on BSD, even though /usr/local was no concept on BSD. And somehow it was reinterpreted into some weird anti-feature.

@rr-: probably not PATH, but some other switch.

chaoskagami commented 8 years ago

To the people who don't get what @czarkoff is talking about; yes, both FreeBSD and OpenBSD install packages via the package tree to /usr/local. This is not some GNU tools quirk (I wish it was.)

It's all a case of semantics because BSDs do ship a kernel-bound toolset, whereas Linux does not. These 'operating system tools' for a FSB-compiant system go in /usr. With Linux there's no distinction of /usr and /usr/local, since the kernel does not ship with utilities bound to it. 'Operating system tools' therefore refers to whatever the distro ships, and /usr/local takes on the meaning of 'root dumped it here' because of this. On BSD, 'user software' refers to anything NOT bound to the kernel, thus /usr/local is user software as in 'the user installed it and it is not part of the core system'. On both Linux and BSD / is only for early boot software and certain essential tools like ls.

Yes, it's odd. They are different, but it's because Linux is just a kernel that we don't make this distinction. I used to use FreeBSD a few years ago. This is operating system convention on BSD, and installing to /usr is indeed incorrect and potentially OS-fucking at worst. Please don't suggest this to him.

In all honesty, /usr/local should be searched by default. This is the case for autotools and cmake. Why is waf being so different and dumb here?

ghost commented 8 years ago

I don't get why you blame waf, if on BSD cc program.c fails and you need to hardcode arbitrary platform-specific paths for no reason. Why is BSD so different and dumb here?

olifre commented 8 years ago

In all honesty, /usr/local should be searched by default.

Is this really the correct place to search for? Even for non-external components like iconv? As far as I learned in my first steps with FreeBSD, iconv is part of system and thus is in /usr/. Though that seems to be different for different BSD flavours... So it's a "distribution specific" specialty.

Why is waf being so different and dumb here?

Waf is not explicitly looking anywhere. It relies on the compiler to find everything from its standard search paths. It would feel strange to me to hardcode any paths in a build-system - but you are right, cmake is hardcoding these lookup paths and you can't even turn them off easily. I personally consider this a bug (there has also been mailing list discussion for cmake), since it's pretty hard to make cmake NOT look in /usr/local/ which makes it harder to build redistributable packages with it (on Linux).

So currently, I know of two ways to get mpv compile on BSDs:

export CPPFLAGS=${CPPFLAGS}:-I/usr/local/include
export LDFLAG=${LDFLAGS}:-L/usr/local/lib

That's also what many portfiles do, and I can't see what's wrong with doing that downstream on the distribution side (since accomodating the different BSD flavours on which stuff is sometimes external and sometimes system should probably not be the build system's or upstream's job).

ghost commented 8 years ago

use any recent FreeBSD on which iconv is not external. MPV just compiles without any hacks.

Is this because all other deps use pkg-config?

Btw. I don't understand why iconv is in /usr/local, as iconv is part of POSIX.

olifre commented 8 years ago

Is this because all other deps use pkg-config?

It seems so - for reference, I just used a plain vanilla FreeBSD image from http://ftp.freebsd.org/pub/FreeBSD/snapshots/VM-IMAGES/11.0-CURRENT/amd64/Latest/ put it into qemu, updated the port-snapshot, did a port install mpv and port uninstall mpv to pull in all dependencies for mpv, and then git clone of mpv master and the regular waf configure / waf cycle. I did not check whether each single optional dependency was found, though, but it configured and compiled just fine. Of course, on FreeBSD I would still suggest to use the available port / (or even the binary pkg) instead of manually compiling unless one is doing development. For some reason I fail to understand, the FreeBSD-port still sets the CPPFLAGS / LDFLAGS to enforce looking into the /usr/local directories explicitly. I am unsure whether this is a bug or maybe needed for some dependency (or it is to be compatible with other BSDs...).

Btw. I don't understand why iconv is in /usr/local, as iconv is part of POSIX.

It's really strange to me, too. For FreeBSD: https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/using-iconv.html iconv only became part of system in 2013. That page also describes the hacking needed inside a portfile to patch buildsystems to pick up the "correct" iconv depending on the FreeBSD version.

chaoskagami commented 8 years ago

Sorry @wm4. Was a bit tired and my choice of words was bad. Didn't mean "dumb" in a mean sense, more just an "I don't understand why" sense. :( And yeah, I don't get why BSD is so different and dumb either. That's why I now run Linux.

@olifre So then maybe @czarkoff just borked up his system somehow or the port build is actually bugged. Or he's using something that isn't a modern BSD or is a server mainframe, in which case, he's doing it wrong. Either way, I think it's safe to say based on what everyone is saying that this isn't mpv's fault unless he mentions which BSD he's actually using (which was surprisingly omitted from this discussion?)

On linux, iconv is part of glibc. Glibc is not the C library used on BSD, so they didn't actually have iconv in their own standard library until recently, apparently.

See, it's all these random differences that made me quit using BSD. Anyways, I'll stop derailing now. I don't feel I'm actually adding much at this point.

ghost commented 8 years ago

Fairly sure he uses OpenBSD.

avih commented 8 years ago

Well, all that being said, it would still be useful for a build system to support custom inc/lib dirs, even if it shouldn't use arbitrary ones by default.

pigoz commented 8 years ago

It does: CFLAGS/LDFLAGS

czarkoff commented 8 years ago

@pigoz: CFLAGS=/usr/local/include waf (or actually CPPFLAGS=/usr/local/include waf if using intended tool) prepends /usr/local/include, which may shadow headers from /usr/include. Same story with LDFLAGS. What I ask for is a way to append flags.

pigoz commented 8 years ago

I can add something like LAST_LDFLAGS / LAST_CFLAGS to the build system. Would that be ok?

czarkoff commented 8 years ago

I guess it would. Ideally they should default to /usr/local/lib and /usr/local/include if TARGET or DEST_OS are not set.

olifre commented 8 years ago

I can add something like LAST_LDFLAGS / LAST_CFLAGS to the build system. Would that be ok?

@pigoz : Appending to the existing CFLAGS / LDFLAGS will not solve the shadowing issue : @czarkoff wants to add fallback include paths which are checked after the system include paths. The system include paths are not part of CFLAGS but compiled into g++ / clang++ unless overridden with a custom spec file. Apart from using --nostdinc and rebuilding the full include path list, manually changing the order in the process, I wouldn't know how that could be done (maybe there's an easier way and I just don't know it). All upstream ports I found for openBSD / freeBSD seem to live with the "header shadowing" which of course violates the BSD principle as @czarkoff pointed out correctly.

czarkoff commented 8 years ago

@olifre: true, but this way I can do LAST_CPPFLAGS="-I/usr/include -I/usr/local/include" LAST_LDFLAGS="-L/usr/lib -L/usr/local/lib" waf ... and get the desired effect.

All upstream ports I found for openBSD / freeBSD seem to live with the "header shadowing" which of course violates the BSD principle as @czarkoff pointed out correctly.

Not really. You may notice CFLAGS="-I. -I.." quirk which is needed because of a (now solved) shadowing issue. In other words, as of mpv 0.15.0 there is no real problem to solve. But there will be problem next time new header file will be added to the project.

olifre commented 8 years ago

@olifre: true, but this way I can do LAST_CPPFLAGS=-I/usr/include -I/usr/local/include LAST_LDFLAGS=-L/usr/lib -L/usr/local/lib waf ... and get the desired effect.

At least for g++ this trick does not work, it just ignores the -I/usr/include in that case. From the documentation, that's a feature: You can add to this list with the -Idir command-line option. All the directories named by -I are searched, in left-to-right order, before the default directories. The only exception is when dir is already searched by default. In this case, the option is ignored and the search order for system directories remains unchanged. https://gcc.gnu.org/onlinedocs/cpp/Search-Path.html

I did not find the correct snippet of documentation for clang, but I would guess it does the same... At least on my Linux here I can confirm that:

$ clang++ -E -x c++ - -I/usr/include -I/usr/local/include -v < /dev/null
ignoring duplicate directory "/usr/include"
  as it is a non-system directory that duplicates a system directory

and include-order is unchanged.

Not really. You may notice CFLAGS=-I. -I.. quirk which is needed because of a...

Ah, right! Yeah, indeed I missed that quirk...

czarkoff commented 8 years ago

Indeed. Then CPPFLAGS=--nostdinc LAST_CPPFLAGS=-I/usr/include -I/usr/local/include LAST_LDFLAGS=-L/usr/lib -L/usr/local/lib waf ... There seems to be no such problem with linker:

ddc@e325:~ $ doas touch /usr/local/lib/libm.so.9.0
ddc@e325:~ $ cc -L/usr/lib -L/usr/local/lib -lm 1.o -o 1
ddc@e325:~ $ ldd 1                                      
1:
        Start            End              Type Open Ref GrpRef Name
        000012533a800000 000012533ac01000 exe  1    0   0      1
        0000125628899000 0000125628cc1000 rlib 0    1   0      /usr/lib/libm.so.9.0
        00001255a6039000 00001255a6502000 rlib 0    1   0      /usr/lib/libc.so.84.2
        0000125567300000 0000125567300000 rtld 0    1   0      /usr/libexec/ld.so
olifre commented 8 years ago

There seems to be no such problem with linker

Indeed, I think that's an include-path specialty only.

CPPFLAGS=--nostdinc LAST_CPPFLAGS=-I/usr/include -I/usr/local/include LAST_LDFLAGS=-L/usr/lib -L/usr/local/lib waf ...

I'm not sure that will do what you want, consider the following situation:

--nostdinc -I/usr/local/include/ -I/usr/include -I/usr/local/include

then the second -I/usr/local/include is ignored:

ignoring duplicate directory "/usr/local/include"

and lookup order will be to check in /usr/local/include first and only then in /usr/include (since the first specification wins due to the include-deduplication I mentioned before).

So I would guess that for CFLAGS actually it is better not to use LAST_CPPFLAGS since the first, not the last, specification wins.

czarkoff commented 8 years ago

See, the problem I have is that CFLAGS=-I/usr/local/include shadows mpv's own headers. This is just silly. This issue can be solved by prepending local search path (-I/path/to/mpv/distribution) to CPPFLAGS and putting everything user supplies or waf finds during configure stage after that. This way I can just say CPPFLAGS="-I/usr/local/include" LDFLAGS="-L/usr/local/lib" waf configure, and everything magically works. I don't really care about shadowed headers/libs under /usr, because I am on BSD and here we deal with such issues by moving conflicting stuff out of the way into subdirectories.

Another problem I have is that I have to use waf. I am told that waf is used because it automagically does all configuration, so that I don't need to fiddle with flags. Regardless, here is a huge discussion about the way I have to fiddle with flags in order to make waf pick the right things. LAST_CPPFLAGS defaulting to -I/usr/local/include and LAST_LDFLAGS defaulting to -L/usr/local/lib, together with -I/path/to/mpv/distribution prepended to CPPFLAGS would solve that problem as well. I am told that mpv is supposed to be cross-compilable outside container/chroot on system with all sort of stuff installed under /usr/local, so I suggest to enable aforementioned defaults only when TARGET and DEST_OS variables are empty (which, as I gather, is impossible if cross-compilation is enabled).

Now, did I express my problem and possible solution? Does it break anything? Is there any reason not to implement it?

czarkoff commented 8 years ago

To make it crystal clear, ideal solution for both my problems would be:

  1. Set waf's CFLAGS to -I/path/to/mpv/source/distribution
  2. If TARGET and DEST_OS are unset, set LAST_CFLAGS=-I/usr/local/include and LAST_CFLAGS=-L/usr/local/lib. Let user override these defaults via environment or flags.
  3. Append contents of CFLAGS and LDFLAGS environment variables to corresponding waf variables.
  4. Run waf configuration tests and put necessary preprocessor flags after -I/path/to/mpv/source/distribution in waf's CFLAGS and prepend necessary linker flags to waf's LDFLAGS. LAST_CFLAGS and LAST_LDFLAGS should be visible to pkg-config and tests throughout the process, but should not be put into waf's CFLAGS and LDFLAGS yet.
  5. Append LAST_CFLAGS to waf's CFLAGS and LAST_LDFLAGS to waf's LDFLAGS.
olifre commented 8 years ago

See, the problem I have is that CFLAGS=-I/usr/local/include shadows mpv's own headers. This is just silly.

Now I see your point... indeed.

shadowed headers/libs under /usr, because I am on BSD and here we deal with such issues by moving conflicting stuff out of the way into subdirectories.

For reference, that' s also true for some Linuxes.

Another problem I have is that I have to use waf. I am told that waf is used because it automagically does all configuration, so that I don't need to fiddle with flags. Regardless, here is a huge discussion about the way I have to fiddle with flags in order to make waf pick the right things.

I think the main problem here is that there is no automagic being done for iconv - for all other dependencies I think it should work on any BSD since pkg-config is used, which provides the underlying automagic. So another option might be to implement a some-BSD-flavour-specific automagic for iconv? Wouldn't that also solve the problem?

To make it crystal clear, ideal solution for both my problems would be: [...]

Isn't that something that would be a generic solution for any waf-project? Maybe it's worth to ask the waf-people whether LAST_CFLAGS / LAST_LDFLAGS could be implemented in a generic way for any waf-project.

Regardless, I personally don't see anything that would break by this change. Of course it's up to @pigoz / @wm4 to decide and implement.

pigoz commented 8 years ago

Yes, I think it would be better to implement some custom OpenBSD detection of iconv inside our wscript since that looks less invasive (we tested this on FreeBSD when we rewrote the build system and FreeBSD switched just then to expose a .pc file for iconv).

All the other important software is picked up with PKG_CONFIG_PATH (except OSS which is a major clusterfuck even worse than Lua).

The question is now how to detect iconv on OpenBSD.

czarkoff commented 8 years ago

FWIW if tests involving pkg-config are run before other tests, no specific detection code is necessary any more - something will definitely pick up right locations.

pigoz commented 8 years ago

@czarkoff Could you test if the following branch fixes iconv detection on openbsd? https://github.com/mpv-player/mpv/tree/openbsd-iconv

czarkoff commented 8 years ago

Yup, works for me. That said, I'd rather go with setting policy "pkg-config" first - special cases may arise in future.

pigoz commented 8 years ago

But what should we look for in this early pkg-config check? Also we want to fail the configure as soon as possible if key components like iconv or pthreads are missing from the system.

czarkoff commented 8 years ago

My idea was to make tests that use pkg-config first - mpv depends on lots of stuff, and something will definitely bring /usr/local in before other checks are run. That doesn't sit well with "fail early if there is no iconv" idea.

pigoz commented 8 years ago

Thanks, I pushed the fix to master (it will be in the 0.16.0 release).

czarkoff commented 8 years ago

Thanks!