Docker cli slows bash init

[guss77]

- What I did

Instead of caching the docker plugins paths during bash init, put it in a function to be called as needed.

• FixesBash completion slows down bash login sessions #3889

- How I did it

This is a very simply patch that should be self evident.

- How to verify it

Check that it doesn't affect completion results with plugins installed

- Description for the changelog

• Lookup plugins path for completion only on demand instead of caching it during bash initialization (which may be slow).

- A picture of a cute animal (not mandatory but encouraged)

 ʌ___ʌ
(O.O)
/)__)
--''--''--

[cpuguy83]

LGTM

I think the CLI could still improve here because it looks like it is making an API call to the _ping endpoint, but lazily evaluating the plugin path looks like the right thing to do here.

[cpuguy83]

This needs a DCO sign-off.

git commit --amend -s --no-edit && git push -f origin docker-cli-slows-bash-init

[codecov-commenter]

Codecov Report

Merging #4505 (ff94163) into master (cdabfa2) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@ Coverage Diff @@## master #4505 +/- ##
=======================================
Coverage 59.38% 59.38% =======================================
Files 288 288 Lines 24783 24783 =======================================
Hits 14717 14717 Misses 9179 9179 Partials 887 887 

[thaJeztah]

I think the CLI could still improve here because it looks like it is making an API call to the _ping endpoint

Oh! I see where that one comes from; needs a similar fix as 006c946 (#3904)

Opening a PR

[vvoland]

One complain from the shellcheck:

#11 5.639 In contrib/completion/bash/docker line 1147:
#11 5.639 echo ${DOCKER_PLUGINS_PATH//:/ }
#11 5.639 ^-------------------------^ SC2086: Double quote to prevent globbing and word splitting.
#11 5.639 #11 5.639 Did you mean: #11 5.639 echo "${DOCKER_PLUGINS_PATH//:/ }"
#11 5.639 

[thaJeztah]

• Opened cmd/docker: areFlagsSupported: don't Ping if not needed #4506 to fix the redundant /_ping

[guss77]

One complain from the shellcheck:
#11 5.639 ^-------------------------^ SC2086: Double quote to prevent globbing and word splitting.

I understand the general sentiment, but it is not correct generally and specifically in this case it is wrong:

1. It doesn't actually matter: I'm echoing the result and whoever receives it is going to see multiple words regardless.
2. The entire reason for converting : to spaces is to cause word spitting, which is then used in for plugin_path in $(__docker_plugins_path); do to iterate over the words.

I can change it to have the double quotes just to silence shellcheck, but that is just pasting over the real problem, which is shellcheck being over enthusiastic about recommendations that are not universally correct.

[albers]

Thanks very much for the contribution.
Functionally LGTM, but I'd like a naming change, see comment.

[thaJeztah]

For the linting error (assuming the current code is technically correct (I'm always confused by quotes in bash)); I think it'd be ok to add a # shellcheck disable=SC2086 in the code-block.

(unless there's another option that's both "good' and keeps the linter at bay of course)

[vvoland]

I don't think shellcheck is being completely incorrect here - the ${DOCKER_PLUGINS_PATH//:/ } is intended to be passed as a STRING argument of echo (message to be printed) and not as a whole command line to the echo.

This won't affect the word-split that we want to happen in thefor because we don't have quotes here (and shellcheck doesn't force us to change that):

forplugin_pathin$(__docker_plugins_path);do

[guss77]

I don't think shellcheck is being completely incorrect here - the ${DOCKER_PLUGINS_PATH//:/ } is intended to be passed as a STRING argument of echo (message to be printed) and not as a whole command line to the echo.

OK

[vvoland]

LGTM. Could you also squash your commits into one please?

[guss77]

LGTM. Could you also squash your commits into one please?

Can do.

[albers]

LGTM, thanks.
I'm looking forward to this change becoming GA.

[thaJeztah]

LGTM, thank you!

[thaJeztah]

Opened backports:

• [24.0 backport] Stop slowing bash init by caching plugins path slowly #4517
• [23.0 backport] Stop slowing bash init by caching plugins path slowly #4518
• [20.10 backport] Stop slowing bash init by caching plugins path slowly #4519