Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

packer: error multiple paths in PACKER_PLUGIN_PATH #12967

Merged
merged 1 commit into from
May 10, 2024

Conversation

lbajolet-hashicorp
Copy link
Contributor

When a user defines PACKER_PLUGIN_PATH in their environment, we need to error if their path defines multiple directories separated by :.

This used to be supported, but this is removed with 1.11 as we're simplifying the loading process for plugins, so we opted to fall-back to only one plugin directory supported.


"github.com/hashicorp/packer-plugin-sdk/pathing"
)

// PluginFolder returns the known plugin folder based on system.
func PluginFolder() (string, error) {
if packerPluginPath := os.Getenv("PACKER_PLUGIN_PATH"); packerPluginPath != "" {
if strings.Contains(packerPluginPath, ":") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The operator on windows is ";" you can use os.PathListSeparator here to determine if there are multiple paths defined.

When a user defines PACKER_PLUGIN_PATH in their environment, we need to
error if their path defines multiple directories separated by `:`.

This used to be supported, but this is removed with 1.11 as we're
simplifying the loading process for plugins, so we opted to fall-back to
only one plugin directory supported.
@lbajolet-hashicorp lbajolet-hashicorp force-pushed the error_on_multiple_paths_plugin_envvar branch from d9b5c4d to f7b41b6 Compare May 10, 2024 14:44
Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! LGTM

Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add a test for this in the testing branch. For now.

@lbajolet-hashicorp lbajolet-hashicorp merged commit f80ba28 into main May 10, 2024
11 checks passed
@lbajolet-hashicorp lbajolet-hashicorp deleted the error_on_multiple_paths_plugin_envvar branch May 10, 2024 19:11
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 10, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants