mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.52k stars 1.6k forks source link

meson --reconfigure or ninja configure should force dependency checks #6180

Open Jehan opened 4 years ago

Jehan commented 4 years ago

If a previous configure discovered a valid dependency, it won't check for new versions of said dependency. For instance, I had a libheif 1.5.1 (distribution package) which is a valid version in my project. I installed libheif 1.6.0 (source build) but meson did not discovered it, even though it is first in $PKG_CONFIG_PATH, because it relied on cache:

Dependency libheif found: YES (cached)
meson.build:716: WARNING:

        libheif version 1.5.0 and 1.5.1 are known to crash when
        exporting (bug #4185). Please update.

Obviously I discovered it didn't catch the right version because we have a warning in our meson configuration on buggy versions of libheif (otherwise it would have been been unknown, which is bad too! cf. #6181).

Now I realize that you cache for optimization reason. Not sure how I feel about this (seriously how faster is the configuration step by using cached dependencies rather than redoing a pkg-config test?) but at the very least, the dependencies should be re-checked when we force a reconfigure by running either meson --reconfigure or ninja reconfigure which are explicit requests to reconfigure the whole project.

Right now, even if I uninstall the distribution libheif-devel package, ninja/meson reconfigure still rely on cache (which is wrong), and the actual build fails because it can't find the files anymore. Even a ninja distclean did not allow me to reconfigure. In the end, only solution I found was to wipe out the whole build just for this one dependency.

scivision commented 4 years ago

Nice to have the PR adding the clear option.

In general Meson projects using CMake to find dependencies would benefit most from Meson cache as CMake can be relatively slow

Jehan commented 4 years ago

In general Meson projects using CMake to find dependencies would benefit most from Meson cache as CMake can be relatively slow

How about pkg-config dependencies? Are they much slower? I don't remember if I specifically timed a first meson configuration vs a cached one on our project (GIMP, which uses mostly pkg-config dependencies and I don't think we have any CMake one), but I don't remember the first configuration feeling that much slower.

A new option is cool and better than current situation, but I am just brain-storming to see if current --reconfigure cannot be improved too (because each new option makes the tool a bit more complex to use, and debugging builds possibly more difficult too).

mensinda commented 4 years ago

Yes, rescanning the pkg-config dependencies does add up. Especially because we have to execute pkg-config more than once for each dependency (2-4 times if I remember correctly). So for each dependency there is (2-4) * process spawning overhead * pkg-config parsing + meson post-processing.

For GIMP specifically, I have measured around 2.6 seconds for --reconfigure, 4.4 seconds for --clearcache and 3.6 seconds if only the package cache is cleared.

Jehan commented 4 years ago

Ok on my machine, I computed 9.9s for a first configure and 4.7s for a --reconfigure (--clearcache untested as I have not tried the patch) for GIMP.

So ok it is faster, though a big use case of reconfiguring a project is when you changed a dependency and you want to make sure it is taken into account (actually it looks like the main use case to me). So I still think not using cache by default makes sense.

Cache is still useful when the project is automatically reconfigured (you touch one of the meson.build file or whatever else automatically re-runs the configuration). In these case, yeah doing it as quick as possible seems fit (it was not explicit request, just triggered). But when you request an explicit --reconfigure, doesn't it mean most of the time that you made some external change and you want the build to take it into account?

jpakkane commented 4 years ago

But when you request an explicit --reconfigure, doesn't it mean most of the time that you made some external change and you want the build to take it into account?

I'm guessing most cases are of the type "I added a new dependency and want Meson to pick it up". This works without cache clearing and is fast. Going the other way, i.e. deleting or updating dependencies, is rarer.

These optimizations were put in place at the request of GStreamer developers (I think). On Windows their reconfigure setups take tens of seconds (minutes?) without caching.

mensinda commented 4 years ago

We could also better document meson configure --clearcache.

Some random ideas what could be done:

xclaesse commented 4 years ago

Persistent cache makes reconfigure significantly faster. However the downside is cache invalidation is basically impossible... I personally use --wipe a lot more than --reconfigure for this reason, and rely on ccache to rebuild fast enough. But I agree we should have something like --reconfigure --clearcache

Jehan commented 4 years ago

I had again issues today about the not-catching-up-with-dependencies issue. I find it very annoying. I find especially annoying the fact that meson is not able to do the right thing automatically. It forces us to first encounter unexplained issues, then understand where the issue is from, and finally forcefully wipe our build, which is bad (and waste even more time in the end).

I really wonder if there is any way I could convince that configure should pick up updated dependencies by itself. If really some project don't want this (because they really have crazy long configure time, like the GStreamer example), maybe we could set up a per-project setting? Like there could be some persistent configure settings where you tell meson not to check updated versions of already configured libraries.

mensinda commented 4 years ago

The main problem is that automatic cache invalidation is far from trivial. Dependencies may depend on multiple files (even for pkg-config), there are multiple search paths, supporting all dependency backends, etc. It is not that we don't want to get cached dependencies right, it's just that it is hard to get cache invalidation right and managing the increased maintenance burden (while also being faster than without any caching).

Adding a new option to disable caching may also be a good compromise...

It is currently also possible to use meson configure --clearcache <build dir> to reset all dependencies. This won't trigger a full rebuild (like --wipe) and just resets the dependency cache.

Jehan commented 4 years ago

It is currently also possible to use meson configure --clearcache

The problem is that it is still an explicit command to add (hence implies to be aware of the issue, or to run it all the time, which makes it useless). Typically what happened to me right now was: I ran a ninja command, configure happens automatically because meson files changed, the configure then the build completed without showing any issue, but then I try and run GIMP but it fails with an error telling me I am running on a version of the dependency too old. Note well that the dependency version cached in meson was not only older than the version actually installed, but worse it was even older than the newly configured minimum requirement in meson.build! This is bad.

Seriously I am not sure if we can have a middle ground with a cache and a better way to invalidate the cache. But if we can't, even no dependency cache would be better. Currently yes, configure is extra fast, but it is based on a logics which is not robust at all. For development (where you often play with dependencies), this is really not reliable.

If cost of caching is unreliable builds, then it is not worth it. Reliable slower configure is the way to go (unless we find the magics for reliable + fast of course, not saying there is not a solution, but in the meantime, I vote for reliable over fast 1000 times).

xclaesse commented 4 years ago

The middle ground could be to disable persistent cache by default, but have an option to turn it on when the user knows what he's doing?

For reference, gst-build takes 55.6s to configure without cache, and 34.6s with the cache on my laptop. So that's pretty significant.

Jehan commented 4 years ago

The middle ground could be to disable persistent cache by default, but have an option to turn it on when the user knows what he's doing?

I agree. Reliability should be the default. Speed optimization can be enabled for people who know what they are doing.

Jehan commented 3 years ago

No progress on this? πŸ™‚

This is just happening to me too often but still not often for me to remember which is the cache-wiping command to run. So I usually end up wasting 15 min not understanding why a change in my dependency doesn't work; then realizing that it was still using an older version of the deps and remembering that meson pretends to reconfigure in such cases; then 15 min to try and remember the command to run to actually wipe the cache, ending up searching the web and finding my own report always and again.

Really I believe that a reconfigure step in a sane build system (a tool for devs primarily, whose build environment is necessarily always changing and evolving) should imply cache reset.

nielsdg commented 2 years ago

The middle ground could be to disable persistent cache by default, but have an option to turn it on when the user knows what he's doing?

I'm very much in favor of this idea. I really prefer not having a cache and making sure I have correct builds, then being able to compile a few times faster. Having to explain this kind of thing to newcomers who say their builds suddenly don't work anymore is not an ideal scenario.

jpakkane commented 2 years ago

The obvious problem is that if we would have added all the "simple" toggle options people have requested of us, there would be over 100 of them by now. That is also infeasible.

nielsdg commented 2 years ago

Sure, I agree that adding options might not be the way to go.

However: then I'd carefully say that adding the dependency cache introduced a regression in correctness in favor of performance, and that it as such should be disabled.

jpakkane commented 2 years ago

A better solution is to make it work correctly out of the box without sacrificing speed. In the case of pkg-config you should be able to store the locations and timestamps of the actual .pc files and then check those and rerun the pc command if any change has happened. This is more work, of course, and I don't know if this information is easily obtainable from pkg-config (or CMake or ...).

eli-schwartz commented 2 years ago

From pkg-config, it is... doable if not easy.

You just need to have pkgconf rather than pkg-config, and add another invocation: pkgconf --path <dep>, although recursing down the dependency tree might be a bit more work. ;)

keszybz commented 2 years ago

Yes please.

I think that it might be easier to pull the following trick: just take a hash of the timestamps of all the pkgconfig directories. (A hash so that it's just a single number. A hash instead of a min or max to make it sensitive to modification of any of the directories.) And then when reconfiguring, recalculate this hash, and assume all pkgconfig files changed and that part needs to be done again. Reading mtimes of a few directories is essentially free, and this way we avoid any complicated logic. I think that the case where you add new pkgconfig files is relatively rare, so not handling that case with utmost efficiency isn't too important.

Jehan commented 2 years ago

I think that the case where you add new pkgconfig files is relatively rare, so not handling that case with utmost efficiency isn't too important.

Euh… no. This is one of the main cases for which I reported this issue. From what I understand, you propose to only monitor the pkg-config paths (hence only reconfigure when the paths change), is that it? I'm sure this is ok for some use cases. But the current meson also breaks for many use cases without changing pkg-config paths.

Typically the issue I often encounter: I have a custom build prefix and the normal system prefixes; I use system packages for dependencies when possible; one day I realize a bug happens in a dep, so I build the dep from git, into my custom prefix, to check if it is fixed in more recent code or to fix the bug myself β‡’ no paths changed, and meson currently fails to pick up the dependency change.

There are even cases where a reconfigure should fail but it doesn't! For instance, nowadays we consider that being able to load remote files is a base feature so we test for glib-networking on configure. So if I install a new glib in my custom dir, without glib-networking, the configure is supposed to fail. But it doesn't because of caching. Once again, no paths changed.

Anyway bottom line: we need a reliable reconfigure. When we ask to reconfigure the project, it means we want it to be really reconfigured. I can accept shortcuts on the automatically-detected reconfigure (when you just run ninja and it detects some changes, hence make a partial and very quick reconfigure) but when I explicitly ask a reconfigure, that means I want the real deal.

Of course, if meson is able to be even faster than pkg-config itself and detect very quickly the no-change case reliably, then it's double-win. Sure. I am not against optimization. That's why I didn't comment regarding the few comments above. If you are able to make this work, I'm happy and all.

But this last comment: nope. We want reliability/correctness first, and efficiency second. Shortcuts are only acceptable is they don't break correctness. If they do, then no thanks. πŸ˜›

Jehan commented 2 years ago

Another example of brokenness brought by non-redone configuration today. Someone broke the pkg-config file of GEGL (one of GIMP's main dependency) nearly a week ago, but none of the developers were seeing it on local builds because meson was just using cached configuration. Only our CI would fail, unless we'd --wipe the full config hence redo a full build (or do a from-scratch build which is basically the same thing).

Really the more I go, the more I think that this whole "never reconfigure" thing of meson is conceptually wrong. I understand that you care a lot about speed of configuration, but I think some middle-ground between optimization and correctness needs to be found. Or if some projects really need to be over-optimized and don't care at all about the correctness of their build, it should be an explicit option (as said earlier). The default should be a "correct configuration" (and for me, I would not switch this default ever because I am always in my code and nothing is more annoying to me than when the bug is in the build system rather than in-code). Please! πŸ™

xclaesse commented 2 years ago

You just need to have pkgconf rather than pkg-config, and add another invocation: pkgconf --path <dep>, although recursing down the dependency tree might be a bit more work. ;)

With https://github.com/mesonbuild/meson/pull/9021 we get a native python implementation of pkgconfig that could do that easily. The extra bonus advantage is if we rely exclusively on that pkgconfig implementation, it is many orders of magnitude faster and cache becomes completely useless at that point. The reason is it does not have to spawn pkg-config process multiple times for each lookup reparsing every pc files every time.

xclaesse commented 2 years ago

Someone broke the pkg-config file of GEGL (one of GIMP's main dependency) nearly a week ago

Oops, my fault actually, sorry. That said, GEGL does not have CI, how is it not constantly broken? (offtopic)

Jehan commented 2 years ago

Oops, my fault actually, sorry.

Ahahah yeah, no prob, it happens. I am not pointing fingers. :-)

That said, GEGL does not have CI, how is it not constantly broken? (offtopic)

Indeed it should have a CI. This being said, we build babl and GEGL HEAD in GIMP's CI. So basically every push in GIMP is actually like a GEGL CI job too. Then when something breaks in GEGL or babl (happens of course from time to time), we fix it. That's probably why it's less of a priority.

Though it's true it should have its own CI too. (let's finish the off-topic here, I guess! 😜)