Pkg: store results of Pkg.build and behave more intuitively on broken packages

[tkelman]

Pkg has some implementation issues (being worked on in #11196) and some usability issues. I recall seeing someone vent about a number of these non-intuitive behaviors recently, but can't remember whether it was on github or the mailing list or HN or reddit, sorry.

A big trouble spot in the process of installing packages is when you first run Pkg.add on some complicated package that has binary dependencies, and Pkg.build (which runs automatically when you first do Pkg.add) fails for one reason or another. It's not immediately obvious that Pkg.build is a separate thing that you have to run manually from then on, until someone asks for help and we tell them that. Trying to run Pkg.add a second time (which is the intuitive thing most people will probably try first) doesn't trigger Pkg.build, it's typically going to be a no-op.

Another issue is #7054. In response to trying Pkg.add a second time and that not working, the next step people will probably try is Pkg.rm. But Pkg.rm is misleadingly named, it isn't really a remove operation, instead it moves the package to the hidden .trash folder. Again, not intuitive.

So, I have a proposal for trying to make this better. We should save (somewhere, somehow, in some format, not getting too picky on the implementation details just yet) the results of whether the most recent call to Pkg.build succeeded or failed, for all installed packages. For pure-Julia packages that don't have a deps/build.jl file, treat this as always succeeding. When an installed package is in a broken state, I think calling Pkg.add or Pkg.rm on it (really Pkg.resolve, but most users don't see that) should behave differently - Pkg.add should automatically run Pkg.build again until it has succeeded, and Pkg.rm should just delete the package instead of sending it to .trash since it's unlikely we want to re-use the broken state.

tl;dr: RFC on title- good idea, bad idea? Can get into implementation questions and start working towards a PR if the idea is popular.

[wildart]

First, as I wrote in #7054, whole .trash idea should be abandoned. Period. it is unnecessary having .cache in place. We could check if package is "dirty" and inform user about that. Let's give user decisions about package "force" remove.
Second, we cannot run Pkg.build until it succeeds. What if it never does?

[tkelman]

I'm not suggesting running Pkg.build in an infinite loop. I'm suggesting calling Pkg.build every time Pkg.add (really resolve) gets run manually until build has succeeded once, not just the first time.

Common example scenario: user runs Pkg.add("Ipopt"), but it fails to build because the user doesn't have gfortran installed yet. If the error message from configure was clear enough that the user goes and does that, then they have to also be paying close enough attention to a different message suggestion to know that running Pkg.build is the key command they need, instead of trying again with the same Pkg.add command that they tried to use before - it's confusing that Pkg.add results in configure and a bunch of other steps getting called the first time you run it, but from then on you need a different command you haven't seen before. It can be a lot of overwhelming information all at once, if things go wrong the first time you try.

[wildart]

It would be more intuitive to suggest a user to build the package agian with Pkg.build rather then to call Pkg.add again and again.

[tkelman]

It would be more intuitive to suggest a user to build the package agian with Pkg.build

We already do suggest that. And yet this still confuses people. If the definition of insanity is trying the same thing twice and expecting different results, then Pkg.add qualifies.

(I'm being facetious of course, but this is confusing if you don't understand what's going on under the covers. The main proposal on the add/build side is that resolve should call build not only on the changed set of packages, but also on any existing not-yet-successfully-built packages.)

[wildart]

The problem is in a package definition of a dependency. A package has REQUIRE dependencies and other "strange" dependency mechanism through "deps\build.jl" file. The latter is completely separated from the package management. ( I mean from a package description. It is just a file is specific folder.)
This file based dependency provides very limited error handling mechanism for the package management (script was executed or not), and I am not surprised that build.jl errs often.

And then a BinDeps package which I think is a culprit in this discussion. This "beast" does not take any attempt to make a user life easier. Not even an error log, everything is dumped on screen - yeah, go figure. If developer wasn't diligent enough to check all possible cases of binary dependency installation across all platforms, OSes, distros then problem guaranteed. And because of above poor integration into the package management, the package manager has no control on whatever happens in build.jl script. So, proposing fixes to the package manager would not solve anything. It is just monkey patching.

We need either to bring some standard to build.jl script or fix whatever is executed in it.

[StefanKarpinski]

I think that external libraries that are currently build in deps/ with BinDeps should not live inside of packages – they should be a lower layer of dependency – libs open which packages depend. Those are the things that are hard to install correctly and that you don't want to change around much. Once you have those separated out, you can pretty much change package code as easily as you like.

[mlubin]

@StefanKarpinski, that's basically how WinRPM and Homebrew.jl work.

[tkelman]

I didn't want to bring up the "BinDeps needs a rewrite" discussion here, sure that's a large part of the problem but out of scope for this issue. Given that the problem BinDeps is trying to solve is a hard one, and it fails often right now, I'm proposing relatively minor changes to try to make what Pkg does in response make a little more sense to users.

[tkelman]

We could have a broader discussion about whether deps/build.jl is the right way to do things, and in the "executing arbitrary code" sense it probably isn't. It's useful to look at how Python is moving away from the python setup.py install model and towards a structured standard set of metadata, at least for conda. Conda needs a team to build the world for itself though instead of leveraging existing package managers (which also build the world of course, though all in incompatible ways), which we haven't wanted to get into too much - at least not on every possible flavor of Linux. (writing a MiniConda.jl binary provider is one of many items on my list of "things I would do if I were getting paid for it")

Even if we refactored so that a "SourceBuilds.jl" package centralized the locations of where other packages built to, a la WinRPM.jl and Homebrew.jl, things wouldn't be any less likely to fail in various ways. Right now when WinRPM.jl or Homebrew.jl go wrong, it usually needs me or Elliot or someone else who's seen the problem before to translate what the problem is and what the solution might be. And that can only happen if the person was patient enough to open an issue about the problem instead of just giving up immediately.

[tkelman]

I got a little rant-y here, sorry for the word count. I'd appreciate feedback on the core proposal, to store results of Pkg.build and use that information to change some add/build/rm behavior.

[timholy]

I think it's quite reasonable. The current problem is that if people don't take red text seriously during the initial install, they never get a reminder that something is broken.

[nalimilan]

+1 to @tkelman's proposal. It's particularly confusing to have packages that are supposed to be installed, but are not built. Pkg.info could also print something about them being in a broken state.

A higher standardization of dependency installation is also a good idea, but much beyond the scope of this issue.

[ScottPJones]

👍 also on this proposal

[tkelman]

Awesome, thanks for the input. 3 yeas is enough for me to start working on a PR then.

On format - how does plaintext, one line per package, pkgname true or pkgname false, sorted by name, sound? Save as joinpath(Pkg.dir(), "BUILT") maybe?

[wildart]

I have my doubts about not using .trash after I had some unpleasant experience with an automatic dependency resolution which removed a package that had a development branch in it. (it wasn't HEAD branch at the moment). I understand that this is very improbable scenario for a common user, but developers are potentially at risk to loose their work.
If packages are explicitly marked as under development, they can be handled appropriately during the dependency resolution. This could eliminate need to keep packages after deletion because they might have some changes. The developer will decide when (un)set a development flag on a package, to include it into the resolution cycle. But for ordinary user Pkg.add and Pkg.rm will behave as they should - add and remove package.
I would also suggest not to dump on screen any information from Pkg.build step, only warning about unssessesfull build attempt (during or/and at the end of the installation process) and point to a location of a saved build log for a problem resolution. This would eliminate noise on screen, and make user more receptive to errors which could happen during the package installation.

[KristofferC]

I would also suggest not to dump on screen any information from Pkg.build step, only warning about unssessesfull build attempt (during or/and at the end of the installation process) and point to a location of a saved build log for a problem resolution.

I agree with this. Many times when there are multiple packages to install (typically with a new installation of Julia) there are multiple pages with red text and it is hard to get an overview.

[tkelman]

The problem with so many configure scripts is there are non-errors that result in automatic disabling of optional features, which are often important. Logging the output by default is a good idea since it means there are standard files you can upload (like brew gist-logs with homebrew, we should make a keyword argument to do that), but I'm not so sure about exclusively directing the output only into a somewhat hard-to-find log file without showing anything by default.

[tkelman]

I'm going to close this for now. #12350 should hopefully help if it or something like it gets merged. Otherwise the "adding more state" is probably something that needs to be fixed within BinDeps, or a replacement thereof. It's really the source of almost all of the problems here, as @wildart said. There are a handful of packages that use deps/build.jl in their own ways either as homegrown replacements for BinDeps or for other purposes, but I'm less familiar with what the user experience has been with the reliability of those schemes.

Eventually we'll need to do something like #11955, and declarative build (or binary install, following the conda or R examples) instructions, rather than arbitrary code execution, should likely fit into that somehow.