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

Win CI: Bootstrap Crystal, build things *on* Windows, publish the binary #9123

Merged
merged 3 commits into from Apr 21, 2020

Conversation

oprypin
Copy link
Member

@oprypin oprypin commented Apr 18, 2020

Example:
First run: https://github.com/crystal-lang/crystal/runs/598199987
With cache: https://github.com/crystal-lang/crystal/runs/598303791

Sadly, LLVM here, if not cached, takes 1 hour to build. Although future builds will not need to build it, this might be too much reliance on the cache. Ideally Crystal org would provide a pre-built LLVM download. There is no official download that has the necessary "*.lib" files. Zig lang provides LLVM downloads but surely we can't just use it.

In the future, if there is also a permanent place to grab a "crystal.exe" from, we can just drop the Linux step from this.

@bcardiff
Copy link
Member

bcardiff commented Apr 18, 2020

We can move de GitHub Action to build LLVM for windows to distributions-script repo. That is already done for the omnibus osx packages using CircleCI.

Once built we can drop it in the same S3 bucket.

@waj
Copy link
Member

waj commented Apr 18, 2020

Did you try with binaries released at https://releases.llvm.org/download.html#10.0.0 or https://github.com/llvm/llvm-project/releases?
Otherwise, we could fork the llvm repository within crystal-lang org and include the build process there, releasing the binary within the GitHub releases.

@oprypin
Copy link
Member Author

oprypin commented Apr 18, 2020

@waj Yes. From my post:

There is no official download that has the necessary "*.lib" files.

@bcardiff's suggestion seems good.

This will work as is anyway and can be merged. The building and caching is very stable.
I think we should keep things like this until Crystal 0.35 release, then both llvm and crystal.exe can be tackled.

@oprypin
Copy link
Member Author

oprypin commented Apr 18, 2020

I was just thinking, maybe this approach to LLVM isn't even bad. Everything is declared explicitly, which is nice. But even better, with the cache this doesn't have any outbound traffic as far as GitHub is concerned :)

@oprypin
Copy link
Member Author

oprypin commented Apr 20, 2020

Well, the Mac CI failure is obviously unrelated.
I just double-checked that this action still passes with latest master (which wasn't a certainty...)
Please review, this is ready to go :>

@asterite
Copy link
Member

@oprypin rebasing on top of master will make CI pass

@oprypin
Copy link
Member Author

oprypin commented Apr 20, 2020

Yes but that's a heavy-handed move, as reviewers would need to double check that the commits have the same content.
I'm really against this practice of rebasing over nothing. Usually I'd merge master, but as I want the commits to look nice for a non-squash merge, that's also not a good option.
There's no point in re-running CI anyway, as this addition has no way to affect anything existing.

@oprypin
Copy link
Member Author

oprypin commented Apr 20, 2020

Ah well, not a big deal here 😬

@oprypin
Copy link
Member Author

oprypin commented Apr 20, 2020

Oh it is a big deal apparently. Does GitHub now dismiss reviews on force push? Nice.

@asterite
Copy link
Member

Does GitHub now dismiss reviews on force push? Nice.

It's a setting we flipped on.

@bcardiff bcardiff added this to the 0.35.0 milestone Apr 20, 2020
@oprypin
Copy link
Member Author

oprypin commented Apr 21, 2020

please :s

@straight-shoota straight-shoota merged commit 625884b into crystal-lang:master Apr 21, 2020
@straight-shoota
Copy link
Member

As you wish =) Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants