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

Fix 'pulumi stack ls' on Windows #4094

Merged
merged 2 commits into from
Mar 18, 2020
Merged

Conversation

chrsmith
Copy link
Contributor

@chrsmith chrsmith commented Mar 17, 2020

The command pulumi stack ls didn't work on Windows when using the local file backend. The reason boils down to the blob.Bucket type we use, to abstract file IO for the { local disk, Azure, GCP, S3, etc. }

By design, it only supports forward slashes as directory separator characters. So we canonicalize file paths using file path.ToSlash before we call its methods, using the wrappedBucket interface.

However, we missed the canonicalization for the data we pass to ListObjects(...). Which was why pulumi stack ls didn't work on Windows. (But pulumi stack did, because we use a different method, which does the path canonicalization.)

This PR also runs the pkg/backend/ unit tests on Appveyor too, since previously we didn't have any Windows-coverage for these file bucket methods.

Fixes #3504

@leezen
Copy link
Contributor

leezen commented Mar 17, 2020

🎉 for more Windows test coverage

Copy link
Contributor

@jkisk jkisk left a comment

Choose a reason for hiding this comment

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

LGTM

// view of file paths. Since it will assume that backslashes (file separators on Windows) are part of
// file names, and this causes "problems".
func TestWrappedBucket(t *testing.T) {
// wrappedBucket will only massage file paths IFF it is needed, as filepath.ToSlash is a noop.
Copy link
Contributor

Choose a reason for hiding this comment

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

IFF > IF

// Perform basic file operations using wrappedBucket and verify that it will
// successfully handle both "/" and "\" as file separators. (And probably fail in
// exciting ways if you try to give it a file on a system that supports "\" or "/" as
// a valid character in a filename.)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there systems that do that? Just curious...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not to my knowledge.

assert.False(t, exists, "Deleted file still found?")
})

// Verify ListObjects / listBucket works with regard to differeing file separators too.
Copy link
Contributor

Choose a reason for hiding this comment

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

differeing > differing

Copy link
Contributor

@EvanBoyle EvanBoyle left a comment

Choose a reason for hiding this comment

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

🚢

@chrsmith chrsmith merged commit 8efd992 into master Mar 18, 2020
@pulumi-bot pulumi-bot deleted the chrsmith/3504/stack-ls-incorrect branch March 18, 2020 20:57
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.

Pulumi Stack ls behavior is incorrect
4 participants