rad-exe: top-level rad CLI

[FintanH]

This is the first draft of the top-level rad CLI library -- to become the rad executable.

In its current form, it defines the global arguments and performs the sanitisation of those arguments, as per the RFC. It has mock profile subcommand that does nothing. It allows for an external rad-<command> to be called spawning a subprocess to execute it.

Signed-off-by: Fintan Halpenny fintan.halpenny@gmail.com

[FintanH]

It seems like the test::integration::git_helpers::remote::smoke test is consistently failing. It will fail locally for me if I run all the tests but passes if I run it as a standalone test. Seems like that would mean a concurrency issue. Did someone say concurrency was hard recently? :thinking:

[FintanH]

It seems like the test::integration::git_helpers::remote::smoke test is consistently failing. It will fail locally for me if I run all the tests but passes if I run it as a standalone test. Seems like that would mean a concurrency issue. Did someone say concurrency was hard recently?

So the error that comes up is No key found and always happens during the clone portion of the smoke test. This indicates that the path doesn't exist when it comes around to calling get_key. I wonder if the temp dir gets dropped somehow

[kim]

So the error that comes up is No key found

I can't reproduce locally. Do you have a test run log with RUST_LOG=debug?

[FintanH]

I can't reproduce locally.

Are you running the whole test suite, i.e. cargo test? If you run the test individually it will work fine, or even just a subset of the tests.

Do you have a test run log with RUST_LOG=debug?

Here's the output from the logs when it fails https://gist.github.com/FintanH/8eb6de5e47c5038f26931ebbd5cddde6

[kim]

Are you running the whole test suite, i.e. cargo test?

Yes

Here's the output from the logs when it fails [1]https://gist.github.com/FintanH/8eb6de5e47c5038f26931ebbd5cddde6

Mkay, that's not too helpful. Perhaps add some GITTRACE* vars?

[FintanH]

Yes

Huh... Doesn't Work On My Machine:tm:

Mkay, that's not too helpful. Perhaps add some GITTRACE* vars?

I updated the gist with the output trace outputs. I don't see anything suspicious :eyes:

[kim]

Something is not right with the profile selection logic:

https://buildkite.com/monadic/radicle-link-seedling/builds/3300#401f270a-f190-4ac9-82d8-7a51890d740c

The key file the helper looks for is /tmp/.tmpPaeAgS/ghi/keys/librad.key, but the one the test thinks it has created is /tmp/.tmpPaeAgS/461d13f9-ba32-4617-b6c1-7477caf7eee2/keys

[FintanH]

The key file the helper looks for is /tmp/.tmpPaeAgS/ghi/keys/librad.key, but the one the test thinks it has created is /tmp/.tmpPaeAgS/461d13f9-ba32-4617-b6c1-7477caf7eee2/keys

Whoa! Where did that specific file path come from :o Was trying to get that to print before :sweat_smile: Thanks for the spot here, I'll look into this. Weird that it only happens on occasion :thinking:

[kim]

Afaiu ghi is not actually a valid profile identifier. Perhaps some environmental variable gets overwritten, and is not validated?

[FintanH]

Afaiu ghi is not actually a valid profile identifier.

Any String is a valid profile identifier, but the default ones generated are UUIDs.

Perhaps some environmental variable gets overwritten, and is not validated?

This sounds like a good hunch :detective:

[FintanH]

To summarise the issue, the latest tests for sanitising rad arguments involved setting the RAD_PROFILE environment variable. This would result in the argument being read during the test of the git remote helper (yay concurrency!)

The fix I've provided here is to use rusty-fork[0] which spawns those test cases into a subprocess, thus not interfering with the environment variables in the rest of the tests.

[kim]

Seems sensible.

[kim]

You might be interested in adapting 9d51e6c06ff81b281c44890030237685cdf15928

The error renders as:

Error: key file: /tmp/.tmpPaeAgS/ghi/keys/librad.key

Caused by:
    No key found

so perhaps it needs a different copy.

[FintanH]

so perhaps it needs a different copy.

How about changing the error variant here[0] to include the path?

[kim]

Yeah sure, that works, too