-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add Windows build workflow #6360
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
Conversation
12db3b8 to
ba7c56a
Compare
|
@compnerd, @roman-bcny: Anything else to consider for this basic workflow? |
|
Nice! Maybe you could even start with |
8ffcc50 to
19181d8
Compare
|
@SimplyDanny this is weird, in the server 2025 I see the same exact log appear 24 or 25 times in those logs with different timestamps, repeatedly with different command lines. No idea what's going on there, looks like some kind of toolchain glitch?.. Wondering if --filter with just one test would work, maybe --skip is messing things up (but I'm just guessing). I'll see if I can repro. |
|
I now see it's just the build step that's failing so test filters aren't relevant. It seems I'm able to get it to run for much longer with --jobs 1 but still looking |
|
@SimplyDanny a few things from my experiments (roman-bcny#1)
lmk if there is anything I can help with! |
|
I would recommend against that workaround - the long paths are an issue, and not all users will be able to enable that. We should instead be fixing those scenarios. |
|
Agree, just listed what I noticed. |
|
Very unclear to me, what causes the failure. One of the first builds in this branch succeeded building at least and I haven't changed anything significant since then. Another option I might try is to build it with Bazel. This requires #6364 with This also has the benefit of better caching. However, I need to find out whether the static linking would work with it. Long paths are my favorite issue on Windows. 😅 However, on my test system, the base path is even longer than the one on the GitHub runners. So that might not be the problem. Thanks for your support, though. That's much appreciated! |
7117c84 to
4fe8410
Compare
Static linking should work properly with bazel or CMake. I made sure to properly enable that on Windows for those build systems. |
334cad2 to
51c7d2a
Compare
|
@SimplyDanny seems like bazel isn't trivial, if you want to give the previous version another try it might work better with the dev toolchain and is just a few line change: (we'll need the dev toolchain for statically linked releases anyways) |
39a6535 to
946e661
Compare
|
The failure is windows error 206: the filename or extension is too long - interesting. I wonder if using |
|
This seems to fail similarly 🤔 I think that we might be generating a file with an invalid character? Note that Foundation is long path aware and will protect you against issues due to path length when ever possible. |
|
Latest build seems to be a success! |
|
I don't want to give up on Other than that, long file paths seem to be an issue indeed. However, that doesn't seem to be caused by a single source file being too long, but by a generated file in the test process. Because actually, The Bazel build doesn't quite work on Windows. There are still too many issues that require fixes upstream. |
032401c to
95cdc16
Compare
95cdc16 to
d0f9128
Compare
|
@SimplyDanny I'm trying to fix test on #6365 and the remaining issue is with buildkite, seems to be related to /var symlink getting resolved to /private/var on macOS, causing path mismatches so glob tests etc fail, but it might be a pre-existing issue. I see that you have a green build here which is a bit puzzling. Maybe you also ran into /var vs /private/var and somehow this is no longer an issue because of the bazel config tweaks you made? |
I don't think it's due to my changes. |
|
The workflows basically work now, however, not reliably. Sometimes they still get stuck during compilation (like in the last run). |
|
@SimplyDanny thanks
|
Just fear. 😅 Every time we try to improve performance in this area, there were some edge cases we didn't have considered and changes had to be reverted. So I want to be really sure this doesn't break anything. But I might merge it shortly and offer a prerelease to let people try.
You are right. Let's merge it and observe how it behaves. In parallel, I try to get it building with Bazel for the reasons mentioned above. If we are lucky, it might also be more stable. |
So at least the relevant parts on the testing side got merged. You can now build on them in the other PR. |
|
Thank you! Ugh, I just ran into the same issue we saw before with builds crashing, on the developer snapshot, so that didn't magically resolve the issue :/ https://github.com/realm/SwiftLint/actions/runs/20031956981/job/57443150646?pr=6365 |
Yeah, both Windows builds fail randomly. It's still good we have them now. However, that will cause many confusions over time. It would be important to get them stable, but I have no idea at the moment what even causes the error. |
|
I poke at a few dev snapshots in my fork and this one didn't crash on me yet so there is hope! |
|
Ugh spoke too soon. That one doesn't seem to work on ARM. Sad. |
|
Updated to the latest (Dec 1) snapshot and it's green on both x64 and arm! #6371 I'm told there were some issues with arm that are being looked into but Dec 1 was worked around so hopefully it just works. |
Resolves #6350.