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

Remove Windows from CI #413

Closed

Conversation

keithmattix
Copy link
Contributor

@keithmattix keithmattix commented Aug 23, 2024

Due to a general lack of ecosystem support for Windows (e.g. Envoy, Bazel) and our inability to get Windows working with newer toolchains, we've decided to remove Windows from CI until the project has active contributors with sufficient expertise to make it work

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Copy link
Member

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

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

Thanks! You could also add WAMR on Windows in this PR and remove Wasmtime on Windows as part of #406, since it still works until the update.

@keithmattix
Copy link
Contributor Author

Good idea! I'll do that

.github/workflows/test.yml Outdated Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
@keithmattix
Copy link
Contributor Author

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

@PiotrSikora
Copy link
Member

PiotrSikora commented Aug 23, 2024

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

It looks like CMake issue, so it might be worth updating rules_foreign_cc to a more recent version, since the one we use now is from ~2.5 years ago.

If that doesn't automagically fix the build, then I agree it's not worth pursuing further.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix keithmattix force-pushed the remove-wasmtime-windows-from-ci branch from de48c52 to 4b834df Compare August 24, 2024 14:15
@keithmattix
Copy link
Contributor Author

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

@PiotrSikora
Copy link
Member

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix
Copy link
Contributor Author

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Ah, I didn't push up my latest changes; I played around with both of those files, but I think there's an import cycle since we pull the wavm repositories and stuff in the same function we import the bazel rules. I suppose I could try to split them up?

keithmattix and others added 4 commits August 25, 2024 21:55
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix
Copy link
Contributor Author

Ok I figured out the bazel issues, and Linux builds fine, but looks like there's still some pathing issues on Windows

@PiotrSikora
Copy link
Member

Linux builds fine, but looks like there's still some pathing issues on Windows

ACK, thanks for trying! Feel free to drop Windows support.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix keithmattix changed the title Remove wasmtime windows from CI and test other engines Remove Windows from CI Aug 26, 2024
@PiotrSikora
Copy link
Member

As mentioned before, removal of Wasmtime on Windows from the CI should be done as part of #406, since it works fine until the Wasmtime update, so this PR can be closed. Thanks!

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.

2 participants