[RFC] Removal of the lit external shell

This is essentially a follow-up to https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179. Now that the lit internal shell has been enabled by default in upstream LLVM. Some downstreams with large lit-based test suites have also started following suite, such as Swift in https://forums.swift.org/t/rfc-enabling-the-internal-shell-by-default-in-llvm-lit-tests/82148. Given this, there are no upstream users of the external shell, and downstream users are starting to migrate off, I think it makes sense to remove support for the external shell at this time.

The Plan

Assuming the community approves, I plan on adding a warning to lit when creating a test suite with the external shell enabled (potentially with an additional keyword argument that requires acknowledgement that this behavior is deprecated to have a warning instead of a hard error). This warning will be added before the LLVM 23 branch date. After the LLVM 23 branch date, removal of the external shell will commence, giving users working off of a release version ~6 months to migrate. Users at tip of tree have already had about six months since the internal shell was enabled by default and would have one or two more months at least until LLVM 23 branches.

Motivation

The external shell is essentially dead code. There might be some downstreams relying on it, but they are likely able to trivially migrate to the internal shell. Most downstreams that are not able to migrate trivially still seem to be doing so given the advantages of the internal shell (better error messaging, faster runtime, and better cross platform support). This makes maintenance on certain lit components easier. Adding new builtins now only requires work in one place. We can delete quite a few tests that are specific to the external shell. And we also lose some API users that might make further internal refactorings easier.

I have heard of some people using the external shell to aide in debugging. The need for this should hopefully be mostly gone now that the internal shell provides much more detailed debugging info by default. It’s possible there are significant use cases here that I’m not aware of, but I believe we should be able to fix anything that comes up without keeping the external shell around.

This is all contingent on the fact that there are no known issues with the internal shell as the last couple around resource leaks have been fixed recently. If there are still issues that are causing people to force use of the external shell for downstream testing, it would be helpful to know so we can fix any issues.

9 Likes

For Swift we didn’t see any noticeable speedup in testing, but the migration was still well worth it imo: I’m in favour of removing the external shell. When I added --update-tests the implemenation effort to support the external shell was significantly higher than for the internal shell, to the point where it wasn’t worth it – and this was before the internal shell was the default. If we don’t remove the external shell I suspect with time new features will be added only to the internal shell.

1 Like

I agree that this is a good thing to do. Having two ways of doing things seems silly if there are no clear benefits to having both (and it seems like that is now the case).

Since the other post is now quite old: I recently discovered an inconsistency with the internal lit shell in a downstream project. I was piping the output of a Python script that uses the rich library into FileCheck (which I think was bad style anyways).

On the system shell, rich determined that the pipe was not an interactive tty and thus output text without cropping to a set width. On the lit shell, rich deduced a terminal size of 80 and cropped all characters beyond this limit by default. I worked around this by disabling cropping everywhere, even for interactive shells. I have not looked into this further yet, but this could be on rich.