Skip to content

Always use POSIX paths for generated CMake files#65493

Merged
stuartmorgan-g merged 1 commit intoflutter:masterfrom
stuartmorgan-g:linux-plugin-cmake-on-windows
Sep 9, 2020
Merged

Always use POSIX paths for generated CMake files#65493
stuartmorgan-g merged 1 commit intoflutter:masterfrom
stuartmorgan-g:linux-plugin-cmake-on-windows

Conversation

@stuartmorgan-g
Copy link
Contributor

Description

The Windows plugin CMake generation had code to ensure that the paths
written to it used POSIX separators, but the Linux version didn't; that
meant that plugin updates run on Windows machines would corrupt the
generated (but checked in) Linux CMake file.

This change shares that code so that both will use POSIX paths
regardless of what OS they are generated on.

Related Issues

Fixes #64591

Tests

I added the following tests: Ensures that both Windows and Linux files have the correct separators when run with a Windows-style filesystem.

Checklist

Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

  • No, no existing tests failed, so this is not a breaking change.
  • Yes, this is a breaking change.

The Windows plugin CMake generation had code to ensure that the paths
written to it used POSIX separators, but the Linux version didn't; that
meant that plugin updates run on Windows machines would corrupt the
generated (but checked in) Linux CMake file.

This change shares that code so that both will use POSIX paths
regardless of what OS they are generated on.

Fixes flutter#64591
@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 9, 2020
@stuartmorgan-g
Copy link
Contributor Author

The way I got the test setup working to allow running a test with the Windows filesystem is kind of ugly; if you have a cleaner suggestion that would be great. (I couldn't find obvious precedent for this in other tests; some set the test-fixture-wide fs object's style based on the host OS, but I really don't want to do that since it would cause failures to only show up when run on Windows, for no good reason).

@jonahwilliams
Copy link
Contributor

Generally what I've been doing in tests that don't use the context is to just create the filesystem in each test case (if the test actually depends on the context), or just use the posix context in setup if the test is filesystem/context agnostic.

Unfortunately I think the plugins tests are a complete mess right now, but what you've got is a reasonable addition IMO.

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM but make sure the tests pass once the infrastructure issue is resolved

@stuartmorgan-g stuartmorgan-g merged commit 6d1c244 into flutter:master Sep 9, 2020
@stuartmorgan-g stuartmorgan-g deleted the linux-plugin-cmake-on-windows branch September 9, 2020 23:08
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Building on Windows corrupts Linux plugin registrant

3 participants