Page MenuHomePhabricator

There should be a way to specify phutil library dependencies
Closed, WontfixPublic

Description

In the .arcconfig for www, we specify two phutil libraries, facebook-www-arcanist (contained within the www repo) and facebook-phabricator (a dependency of facebook-www-arcanist). It turns out that the facebook-phabricator library has a dependency on phabricator, but the only way that dependency is specified is in the .arcconfig file for the repository containing facebook-phabricator. We should have some way of specifying dependent libraries for a phutil library so that when a library is loaded, its dependencies are loaded first. For example, if repository R contains library A (and lists A in its .arcconfig), but A depends on B (and B depends on C), running arc in R should load A and also automatically load B and C without me having to specify B or C in the .arcconfig for R. (Note that this example doesn't quite match what I described above. If we modify it so A and B are in the .arcconfig for R, then R = www, A = facebook-www-arcanist, B = facebook-phabricator, and C = phabricator).

(In this particular setup, it's stupid that the phabricator library is a dependency of a library we're loading to run arc. My plan is to split facebook-phabricator into two parts: facebook-libphutil and facebook-phabricator (which depends on facebook-libphutil and phabricator), so facebook-www-arcanist only depends on facebook-libphutil. However, there will still be dependencies that need to be loaded.)

Event Timeline

I don't think we actually need to do this, once we remove __init__.php.

The theory is that you tell libphutil about all the libraries up front, and then it can load from any of them, so circular dependencies and out-of-order dependencies should work fine (even if they're not advisable).

D2545 broke this, but the intent was fine (load all functions when we load a library). The problem was that currently we incur some unavoidable side effects when we do this (load all dependencies as defined by __init__.php). Once __init__.php is gone, these side effects should go away too.

The only cases that could still break are files that, e.g., define a function and a class, but this is already nonstandard given the enforced "one class per file rule". We can encode it strictly if necessary.

Specifically, the libphutil rules that do need to be adhered to are:

  • One class per file.
  • If a file defines a class, it defines nothing else (i.e., don't put classes and functions in the same file).
  • No side effects of any kind ever in library code (i.e., you can not run any code alongside definitions of classes and functions). Run code explicitly from the toplevel only, never as a side effect of inclusion.

I think everything already follows these, though?

The theory is that you tell libphutil about all the libraries up front, and then it can load from any of them, so circular dependencies and out-of-order dependencies should work fine (even if they're not advisable).

What I wanted to avoid was having to explicitly list in the www .arcconfig the dependencies of other libraries, i.e. libraries that www does not use directly, but only through libraries it uses.

Oh! Yeah, that makes perfect sense.

Forgive my ignorance, but isn't this what Composer does?

Yes. This project does not use Composer. See T6012. See also T5055.

chad changed the visibility from "All Users" to "Public (No Login Required)".Sep 18 2015, 9:20 PM

Thanks for the response. I have similar concerns about security and using Composer in my own personal projects.
Perhaps similar logic to bower could be leveraged (which I think is more security conscious).
I'd hate to have to reinvent the wheel for library dependencies.

We haven't seen other installs encounter difficulty here. T5055 will eventually build some flavor of first-party package manager which should address a large superset of this problem class.