Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Make the path to .pulumi folder configurable with an ENV variable #3300

Merged
merged 5 commits into from
Oct 8, 2019

Conversation

mikhailshilkov
Copy link
Member

@mikhailshilkov mikhailshilkov commented Oct 4, 2019

Introduces PULUMI_HOME environment variable which points to a path to the path to .pulumi folder. Defaults to <user's home dir> + ".pulumi" if not specified.

Fixes #2966. In addition to plugins, it "moves" the credentials file, templates, workspaces.

bin folder is intact: to move it, we need to adjust all installation scripts to respect PULUMI_HOME and put executables in the proper bin folder.

@lukehoban
Copy link
Member

If this change is okay, we also need to adjust all installation scripts to respect the same variable and put executables in the proper bin folder.

I do think we should do this - but I believe this is technically orthogonal (the PR as is will work without this, and there is no reason the binaries need to be in the same .pulumi folder as the rest of these artifacts).

Copy link
Member

@lukehoban lukehoban left a comment

Choose a reason for hiding this comment

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

This LGTM - but would love @pgavlin eyes on it as well just in case there are other corner cases we might need to cover here.


// PulumiBookkeepingLocationEnvVar is a path to the folder where '.pulumi' folder is stored.
// The path should not include '.pulumi' itself. It defaults to the user's home dir if not specified.
PulumiBookkeepingLocationEnvVar = "PULUMI_BOOKKEEPING_LOCATION"
Copy link
Member

Choose a reason for hiding this comment

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

The name "bookkeeping" feels a bit unusual here for a public facing thing. Should it be PULUMI_HOME_DIR? or PULUMI_HOME?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I copied it from an internal var, but I do realize it's ugly.
PULUMI_HOME would sound like ~/.pulumi to me, so including .pulumi. Is there a name to indicate that it's one level above?

Copy link
Contributor

@nesl247 nesl247 Oct 4, 2019

Choose a reason for hiding this comment

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

I feel like it shouldn't append .pulumi. If you set PULUMI_HOME, it would be the full path (e.g. PULUMI_HOME=/workspace/pulumi pulumi up).

See https://getcomposer.org/doc/03-cli.md#composer-home for an example.

Copy link

Choose a reason for hiding this comment

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

I also think it shouldn't add .plumi, just use PULUMI_HOME value as is

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this makes sense. I renamed to PULUMI_HOME and now it's assumed to be the full path, so no .pulumi is appended. The default is still ~/.pulumi.

@lukehoban lukehoban requested a review from pgavlin October 4, 2019 18:54
@lukehoban
Copy link
Member

cc also @nesl247 and @geekflyer who had input on scenarios that would need this as part of #2966.

@mikhailshilkov
Copy link
Member Author

I do think we should do this - but I believe this is technically orthogonal (the PR as is will work without this, and there is no reason the binaries need to be in the same .pulumi folder as the rest of these artifacts).

There is just one spot - getUpgradeCommand - where this change affects the bin folder. Maybe I can adjust it to check both locations.

@mikhailshilkov
Copy link
Member Author

There is just one spot - getUpgradeCommand - where this change affects the bin folder. Maybe I can adjust it to check both locations.

I reverted this check until we change the installation scripts (if we want to).

Another point to discuss:

Local login files still default to ~/.pulumi; this location is configurable via pulumi login itself. We could respect PULUMI_HOME as the default for the --local option. This might become a breaking change if we do it later, then folks introduce PULUMI_HOME while we still use ~/.pulumi for local logins, and then we change the default.

@@ -353,7 +353,7 @@ func getUpgradeCommand() string {
return "$ brew upgrade pulumi"
}

if filepath.Dir(exe) != filepath.Join(curUser.HomeDir, ".pulumi", "bin") {
if filepath.Dir(exe) != filepath.Join(curUser.HomeDir, workspace.BookkeepingDir, "bin") {
return ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Why bookkeepingdir here but homedir elsewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

We would need to adjust the installation scripts first. This PR assumes the bin is still in its old place.

pkg/workspace/paths.go Outdated Show resolved Hide resolved
@nesl247
Copy link
Contributor

nesl247 commented Oct 8, 2019

What's left to get this merged?

@pgavlin pgavlin merged commit 69743fe into master Oct 8, 2019
@pulumi-bot pulumi-bot deleted the mikhailshilkov/config-pulumi-dir branch October 8, 2019 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

change or customize plugin install location
5 participants