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

Windows & MSVC support #16

Merged
merged 40 commits into from
Apr 28, 2023
Merged

Windows & MSVC support #16

merged 40 commits into from
Apr 28, 2023

Conversation

dcharkes
Copy link
Collaborator

@dcharkes dcharkes commented Apr 25, 2023

Compilation on Windows with visual studio.

Closes:

Some more details:

This PR contains a lot of .toFilePath() refactorings to ensure paths are handled correctly on Windows.

Also in this PR, the RelativePath tool resolver now takes globs instead of uri's so that we can resolve relative paths with /bla/*/ where * can be a version number for example.

@dcharkes dcharkes mentioned this pull request Apr 25, 2023
@coveralls
Copy link

coveralls commented Apr 25, 2023

Coverage Status

coverage: 99.919% (+0.02%) from 99.903%
when pulling f1702fe on windows
into ee1d836 on main.

@dcharkes dcharkes marked this pull request as ready for review April 26, 2023 09:50
@dcharkes
Copy link
Collaborator Author

dcharkes commented Apr 26, 2023

Something iffy is going on with the code-paths on the Windows bot:

image

edit: An older build did the coverage correctly: https://coveralls.io/builds/59308648
commit: 8aaae0d
There was no change in the workflow between that commit and the current one.

Both builds have checked out the code in D:\a\native\native.
Running native\pkgs\c_compiler> dart pub global run coverage:test_with_coverage on Windows locally leads to pkgs\c_compiler\coverage\lcov.info with absolute paths and a pkgs\c_compiler\coverage\coverage.json with relative paths.

Copy link
Member

@HosseinYousefi HosseinYousefi left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @dcharkes!

@dcharkes
Copy link
Collaborator Author

Thanks @HosseinYousefi

The current PR doesn't work on the Dart CI bots, because clang can't rely on std implementations being on path. We do have a std implementation in depot_tools and also an MSVC. However that MSVC doesn't have a vcvarsall.bat but some depot_tools specific script that sets the environment variables. I'll make a separate PR for that.

@dcharkes dcharkes merged commit d28a168 into main Apr 28, 2023
@dcharkes dcharkes deleted the windows branch April 28, 2023 07:08
HosseinYousefi pushed a commit that referenced this pull request Nov 16, 2023
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.

3 participants