Pants native client: --concurrent does not work

[Eric-Arellano]

To reproduce, apply this diff:

diff --git a/build-support/bin/BUILD b/build-support/bin/BUILD
index 799ded3ea..9b143d221 100644
--- a/build-support/bin/BUILD
+++ b/build-support/bin/BUILD
@@ -15,3 +15,5 @@ pex_binary(name="generate_github_workflows", entry_point="generate_github_workfl
 pex_binary(name="generate_docs", entry_point="generate_docs.py", dependencies=[":docs_templates"])
 pex_binary(name="release_helper", entry_point="release_helper.py")
 pex_binary(name="reversion", entry_point="reversion.py")
+
+pex_binary(name="debug_concurrent", entry_point="debug_concurrent.py")
diff --git a/build-support/bin/debug_concurrent.py b/build-support/bin/debug_concurrent.py
index e69de29bb..8f629257c 100644
--- a/build-support/bin/debug_concurrent.py
+++ b/build-support/bin/debug_concurrent.py
@@ -0,0 +1,3 @@
+import subprocess
+
+subprocess.run(["./pants", "--concurrent", "--version"], check=True)

Then run USE_NATIVE_PANTS=0 ./pants run build-support/bin/debug_concurrent.py vs USE_NATIVE_PANTS=1 ./pants run build-support/bin/debug_concurrent.py.

❯ USE_NATIVE_PANTS=1 ./pants run build-support/bin/debug_concurrent.py
Another pants invocation is running. Will wait up to 60.0 seconds for it to finish before giving up.
If you don't want to wait for the first run to finish, please press Ctrl-C and run this command with PANTS_CONCURRENT=True in the environment.

Waiting for invocation to finish (waited for 5s so far)...

Waiting for invocation to finish (waited for 10s so far)..

Note that when using ctrl-c w/ USE_NATIVE_PANTS=1, it cancels out of the outer debug_concurrent.py session and then the inner ./pants --version run executes, but it doesn't terminate.

fyi @jsirois

[jsirois]

OK. I don't repro on Linux:

Python client:

$ USE_NATIVE_PANTS=0 ./pants run build-support/bin/debug_concurrent.py
12:58:00.12 [INFO] Initialization options changed: reinitializing scheduler...
12:58:00.54 [INFO] Scheduler initialized.
Another pants invocation is running. Will wait up to 60.0 seconds for it to finish before giving up.
If you don't want to wait for the first run to finish, please press Ctrl-C and run this command with PANTS_CONCURRENT=True in the environment.

^CInterrupted by user.

$ 12:58:05.80 [INFO] Initialization options changed: reinitializing scheduler...
12:58:06.40 [INFO] Scheduler initialized.
2.7.0.dev0


$ 

Native client:

$ USE_NATIVE_PANTS=1 ./pants run build-support/bin/debug_concurrent.py
12:57:43.23 [INFO] Initialization options changed: reinitializing scheduler...
12:57:43.70 [INFO] Scheduler initialized.
Another pants invocation is running. Will wait up to 60.0 seconds for it to finish before giving up.
If you don't want to wait for the first run to finish, please press Ctrl-C and run this command with PANTS_CONCURRENT=True in the environment.

^CInterrupted by user.

$ 12:57:50.08 [INFO] Initialization options changed: reinitializing scheduler...
12:57:50.98 [INFO] Scheduler initialized.
2.7.0.dev0


$

In both cases I need to hit <enter> after the version prints to get my command prompt back.

[jsirois]

This is likely the same root cause as in #12285 - some issue with console handling / signal handling in either the nails client, the nailgun crate that wraps its use, or both.

[stuhood]

So, independent of Ctrl+C behavior and etc, the implementation of the --concurrent flag is here:

pants/src/python/pants/bin/pants_runner.py

Lines 44 to 49 in b8f6b9f

	# If we want concurrent pants runs, we can't have pantsd enabled.
	return (
	global_bootstrap_options.pantsd
	and not terminate_pantsd
	and not global_bootstrap_options.concurrent
	)

... i.e., it's used to ask the client not to use pantsd.

AFAICT, the native client does not implement the --concurrent flag.

[jsirois]

Aha ! Ok. That's a much nicer bug to deal with.

[stuhood]

(Backstory on this flag: on the surface, --concurrent and --no-pantsd are equivalent, but we wanted to separate them in order to differentiate usecases which would use pantsd if they could (but can't because of #7654), from usecases that will never want pantsd (some docker usecases, etc). Once #7654 is fixed, we'll deprecate the --concurrent flag.)

[jsirois]

Ok, gotcha - thanks. That fix appears to have 0 impact on my non-repro of this issue, but I'll get it in regardless since that's its own bug.

[jsirois]

Oh wait - @Eric-Arellano USE_NATIVE_PANTS=0 does not mean what you think. The bash test in pants is just that the env var is defined - the value is not checked. So USE_NATIVE_PANTS=0 == USE_NATIVE_PANTS=1 == USE_NATIVE_PANTS=Fred.
That said, I was wrong about the --concurrent bug fix not being relevant, I had a boolean sense switch in my initial impl. With --concurrent support implemented neither USE_NATIVE_PANTS=1 ./pants run build-support/bin/debug_concurrent.py nor ./pants run build-support/bin/debug_concurrent.py needs CTRL-C at all and just runs to completion.