I've attached two diffs (D9941 and D9942) which I think close this task, although it seemed too simple? Let me know if anything is missing.

I think doing this is probably premature. In particular, the real goal isn't just "move Aphront to libphutil", but "make Aphront a general-purpose web framework". Moving all of the code in its current state probably doesn't help much, since it's far away from satisfying that goal for various other reasons, and it will make it slightly harder to move toward that goal (by requiring changes in two libraries), and might make some people think they can use Aphront today when they probably won't be successful.

(Do you want to reuse Aphront to build other applications, or are you just tackling this as a general quality/cleanup task?)

On the technical side, D9941 and D9942 create circular dependencies between libphutil and Phabricator. For example, require_celerity_resource() and javelin_tag() are defined in Phabricator, but a lot of the moved code depends on them (I'm surprised arc lint didn't complain about this). Although we can normally survive circular dependencies because we don't do anything as a side effect of inclusion, they're not desirable, and the code can't be used for anything general-purpose as long as you need to include both libphutil and Phabricator. There's other stuff like calls to Javelin::initBehavior(...) for behaviors which aren't defined in libphutil, too.

On the path between here and Aphront being general-purpose, I think we'll get rid of all the Aphront...View UI stuff and call it "PHUI" instead. I'm not sure if we'd ship any of "PHUI" as general web framework or not -- I'd lean toward not, although I haven't looked at it much. We have some components (like "workflow" and dialogs) that we'd need to be able to render some kind of HTML to support and which might be useful to make part of the general framework. But I imagine "Aphront" being fairly limited in featureset (roughly: request, response, routing, controllers, domains-in-the-future, and maybe not even "applications"), and then developers being able to add pieces on top of that if they want a more-Phabricator-like environment (like the "applications" infrastructure, celerity, authentication, maybe PHUI).

While I think the code as written mostly isn't too entangled, a lot of it has small dependency problems.

I think the path forward probably should look more like:

One at a time, separate dependencies on components and move them to libphutil -- probably in blocks like "requests", "responses", "controllers", and "routing".

When a component moves, apply any breaking API changes or generalizations that it should have to be general-purpose. An example is that I think willProcessRequest(array $data) + processRequest() was probably a mistake. A better signature would be processRequest(AphrontRequest $request, array $route_data) -- or even making the route data a property of $request -- because 99% of willProcessRequest() methods just copy route data to properties, and 90% of processRequest() methods begin with $request = $this->getRequest(). Another example is that AphrontRequest->getUser() should probably be AphrontRequest->getViewer().

AphrontRequest is probably the easiest thing to move, but even that isn't very easy. It has a setUser($user) method without a typehint, but calls a specific methods on that object, notably validateCSRFToken(). CSRF probably needs to move elsewhere before we can port this, and maybe we need a more general "middleware" concept to support it. We also probably need an AphrontUser interface. It has things like a hard-coded X-Phabricator-Csrf header and hard-coded request types, and domain management, too. Given that it depends on domains, maybe we need to build and port more general domain handling first. It also has some odd API choices like setCookie() being part of $request. Getting all of this stuff right will take time and consideration, and I think it's better to generalize/separate/isolate components before we move them, rather than dump a lot of not-really-working code into libphtuil/ and plan to clean it up later.

Thanks for the feedback. I figured that it was probably-not-quite-right but wasn't sure how to tackle this exactly (you're feedback now provides a better understanding of the work involved.

I have some sort of plan to maybe attempt to use Aphront outside of Phabricator, but nothing concrete.

In T1806#11, @epriestley wrote:

For example, require_celerity_resource() and javelin_tag() are defined in Phabricator, but a lot of the moved code depends on them (I'm surprised arc lint didn't complain about this).

I think that this is possible (at least in theory) but probably messy in the general sense. We could add this to ArcanistXHPASTLinter but it would involve processing include and require statements (as well as consulting php_compat_info.json to determine if the class/function is built-in or provided by some extension. This wouldn't work at all for libphutil modules because of the autoloader magic which happens.

Adding this sort of functionality to ArcanistPhutilXHPASTLinter is possible as well (simpler too as you would just need to check __phutil_library_map__.php and php_compat_info.json).

In general, I think that this is a good idea to implement, but seems tricky to get correct.

I think that Aphront should probably be in it's own module, rather than being added to libphutil. Celerity would also be in that module.

I can see at some point in the far future phutil being used as a module and package system for building sites or command line tools (which only need libphutil and don't want a full web stack).

In T1806#14, @hach-que wrote:

I think that Aphront should probably be in it's own module, rather than being added to libphutil. Celerity would also be in that module.

I can see at some point in the far future phutil being used as a module and package system for building sites or command line tools (which only need libphutil and don't want a full web stack).

I somewhat agree... I think if we continue down that path then we could end up with an arbitrary number of different (small) repositories with dependencies between them. Possibly a better approach would be to allow loading of submodules? i.e. in the .arcconfig file you could specify "load": ["phutil/aphront"]. T1273 is partially related.

We could add this to ArcanistXHPASTLinter but it would involve processing include and require statements (as well as consulting php_compat_info.json to determine if the class/function is built-in or provided by some extension. This wouldn't work at all for libphutil modules because of the autoloader magic which happens.

This code already exists (in ArcanistPhutilLibraryLinter), I think it was just broken by D9864. Previously, $symbols was the raw output of phutil_symbols.php. After D9864, it's the processed output of PhutilLibraryMapBuilder, which no longer has the need key. I'll send you a diff.

I don't think submodules would be the best option here. Git submodules work when you have a strict hierarchy, but I can see libphutil modules being more like a graph.

I'd expect that the library map or initialization might contain a list of external libraries it depends on, and there'd be a tool inside libphutil that would clone / update dependencies within the same layout that Phabricator uses (with libphutil, Arcanist, Phabricator and any other modules sitting next to each other).

In T1806#67907, @epriestley wrote:

When a component moves, apply any breaking API changes or generalizations that it should have to be general-purpose. An example is that I think willProcessRequest(array $data) + processRequest() was probably a mistake. A better signature would be processRequest(AphrontRequest $request, array $route_data) -- or even making the route data a property of $request -- because 99% of willProcessRequest() methods just copy route data to properties, and 90% of processRequest() methods begin with $request = $this->getRequest(). Another example is that AphrontRequest->getUser() should probably be AphrontRequest->getViewer().

D10698 enables some less junk APIs for these common cases.