Switch to path autocompletion after -- for bazel run commands.

[quval]

Closes #11292.

[quval]

Friendly ping :)

[quval]

@philwo: another friendly ping?

[philwo]

Sorry for the delay. I'll import this now. 😊

[quval]

Thank you! May I add this to the 4.1 issue, or is it too late by now? 😁

[philwo]

@quval Please feel free to add it there.

[philwo]

Unfortunately I'm getting a single test failure in our internal presubmit testing system:

test_complete_pattern:

Expected 'video/streamer2/BUILD video/streamer2/names/ video/streamer2/stuff/ video/streamer2/testing/', was 'video/streamer2/BUILD video/streamer2/names/ video/streamer2/testing/'
-- Test log: -----------------------------------------------------------
------------------------------------------------------------------------
test_complete_pattern FAILED: Expected 'video/streamer2/BUILD 
video/streamer2/names/
video/streamer2/stuff/
video/streamer2/testing/', was 'video/streamer2/BUILD 
video/streamer2/names/
video/streamer2/testing/' .
third_party/bazel/scripts/bash_completion_test.sh:176: in call to assert_expansion_function
third_party/bazel/scripts/bash_completion_test.sh:397: in call to test_complete_pattern

The same test passes fine on Bazel's external CI.

I'm still looking for possible causes. As far as I can see, the internal test should basically do the exact same thing as the external one, but maybe there's some slight difference somewhere. 😞 Unfortunately I don't know anything about bash completion, so this makes it a bit hard. I don't understand what this particular function and added bit is actually testing, too. (The test_filename_expansion_after_double_dash method below is clear though.)

It's evening here, so I'm checking out and will look again tomorrow. In case you look at this error message and think "Ohhh, well this looks like it doesn't actually use my patch from file XYZ in this PR when you run it internally?" or anything else that catches your eye in the meantime, let me know. 😊

[quval]

@philwo Oy, I'm sorry to hear this! The purpose of that added assert is to test the path mode of _bazel__complete_pattern, which was previously untested, and which I modified in this PR to support both regular files and directories - basically just testing that video/streamer2/<tab><tab> behaves as one would expect. I have no idea why stuff/ would be missing from the autocompletion: other assertions in the same test seem to find it (so it should exist), and in general subdirectories seem to be discoverable by this specific assertion. I would love to debug this, but nothing comes to mind. :( Thanks for taking care of this!

[philwo]

Ahhhhh found it 😀 See how the stuff directory is created with a PACKAGE_PATH_PREFIX in case it is set here: https://cs.opensource.google/bazel/bazel/+/master:scripts/bash_completion_test.sh;l=143;drc=5e5ee0db8f1925bcf9ac39232faa0622966d7e2a

In the normal Bazel variant of the test, PACKAGE_PATH_PREFIX is undefined, so it ends up in the same directory as the other directories testing and names, but in our internal variant of the test, the PACKAGE_PATH_PREFIX is actually set to a path, so it'll be missing during the assert later.

Easy to fix, I think this should solve it. :)

[quval]

Thanks so much!