Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign up[WIP] cli-plugins: look for plugins in experimental folder when experimental is enabled #1902
Conversation
tiborvass
force-pushed the
tiborvass:plugin_experimental2
branch
from
849209a
to
ca44e4b
Codecov Report
@@ Coverage Diff @@
## master #1902 +/- ##
==========================================
- Coverage 56.75% 56.73% -0.03%
==========================================
Files 309 309
Lines 21680 21701 +21
==========================================
+ Hits 12305 12311 +6
- Misses 8476 8489 +13
- Partials 899 901 +2 |
|
LGTM |
| dirs := make([]string, len(pluginDirs), len(pluginDirs)*2) | ||
| copy(dirs, pluginDirs) | ||
| for _, dir := range dirs { | ||
| pluginDirs = append(pluginDirs, dir+"-experimental") |
ijc
May 23, 2019
Contributor
This puts all experimental paths after all non-experimental paths, rather than interleving them, is that as intended?
IOW this is:
~/.docker/cli-plugins
/usr/local/lib/docker/cli-plugins
/usr/lib/docker/cli-plugins
~/.docker/cli-plugins-experimental
/usr/local/lib/docker/cli-plugins-experimental
/usr/lib/docker/cli-plugins-experimental
and not
~/.docker/cli-plugins
~/.docker/cli-plugins-experimental
/usr/local/lib/docker/cli-plugins
/usr/local/lib/docker/cli-plugins-experimental
/usr/lib/docker/cli-plugins
/usr/lib/docker/cli-plugins-experimental
I think it's deliberate but just wanted to check that the implications had been considered i.e. As a user I cannot now install an experimental update to something shipped in /usr/lib/docker/cli-plugins in my home dir.
This puts all experimental paths after all non-experimental paths, rather than interleving them, is that as intended?
IOW this is:
~/.docker/cli-plugins
/usr/local/lib/docker/cli-plugins
/usr/lib/docker/cli-plugins
~/.docker/cli-plugins-experimental
/usr/local/lib/docker/cli-plugins-experimental
/usr/lib/docker/cli-plugins-experimental
and not
~/.docker/cli-plugins
~/.docker/cli-plugins-experimental
/usr/local/lib/docker/cli-plugins
/usr/local/lib/docker/cli-plugins-experimental
/usr/lib/docker/cli-plugins
/usr/lib/docker/cli-plugins-experimental
I think it's deliberate but just wanted to check that the implications had been considered i.e. As a user I cannot now install an experimental update to something shipped in /usr/lib/docker/cli-plugins in my home dir.
thaJeztah
May 23, 2019
Member
Was thinking about this; if someone installs both an "experimental" and "stable" version of a plugin; which one should take precedence?
Was thinking about this; if someone installs both an "experimental" and "stable" version of a plugin; which one should take precedence?
ijc
May 23, 2019
Contributor
it should be precisely the first enabled plugin found in the search path, which is why the above distinction on the ordering might matter.
With the current layout I think there is no way to install an experimental version "over" an non-experimental one, other than by pretending it is non-experimental
it should be precisely the first enabled plugin found in the search path, which is why the above distinction on the ordering might matter.
With the current layout I think there is no way to install an experimental version "over" an non-experimental one, other than by pretending it is non-experimental
tiborvass
May 23, 2019
Author
Contributor
We could do either, I thought this was not only easier to implement, but also made more sense. But happy to change it.
We could do either, I thought this was not only easier to implement, but also made more sense. But happy to change it.
| } | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| fmt.Println("toto", tc.c.Experimental()) |
ijc
May 23, 2019
Contributor
left over deubg?
left over deubg?
|
Code looks ok, I left some minor comments/nits. I think the stuff about e2e testing from #1902 (comment) applies to this variant applies here too but you were going to do that in a follow up so that's fine. |
thaJeztah
changed the title
|
I renamed this "WIP" for now; as there might be things to discuss w.r.t. Docker Desktop integration and some other concerns; lets do so on #1897 |
…l is enabled This patch expands the cli plugin search paths by adding `-experimental` to each, only if the CLI has experimental mode enabled. To test that experimental plugins do not work by default: ``` $ ln -s plugins-linux-amd64 build/plugins-linux-amd64-experimental $ vim ~/.config/docker.json # add $(pwd)/build/plugins-linux-amd64 to "cliPluginsExtraDirs" $ make plugins $ make binary $ docker helloworld ``` To show it enabled: ``` DOCKER_CLI_EXPERIMENTAL=enabled docker helloworld ``` Signed-off-by: Tibor Vass <tibor@docker.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
tiborvass
force-pushed the
tiborvass:plugin_experimental2
branch
from
ca44e4b
to
e39aa2d
|
@ijc Updated with fixes. |
|
I can't retrigger validate, but validate passed locally for me. |
|
restarted CI (you can restart after connecting GitHub in the user-settings in CircleCI) |
|
closing as #1898 was merged |
|
I'm reopening because I thought this was still desired. |
Yeah, I was about to say I don't think the two approaches need to be mutually exclusive... |
This is an alternative to #1898
Closes #1897 #1898
Related to docker/docker-ce-packaging#332
This patch expands the cli plugin search paths by adding
-experimentalto each,only if the CLI has experimental mode enabled.
To test that experimental plugins do not work by default:
To show it enabled:
Signed-off-by: Tibor Vass tibor@docker.com