search

sass / dart-sass

The reference implementation of Sass, written in Dart.
https://sass-lang.com/dart-sass
MIT License
3.9k stars 352 forks source link

Upload releases for musl-libc and android #2149

Closed ntkme closed 9 months ago

ntkme commented 9 months ago

This PR creates release assets for linux-musl and android using unofficial dart-sdk builds that I'm maintaining:

This PR also does a refactor of CI job compositions to make it easier to understand and maintain. A behavior difference is that now the GitHub Release is only created if all the builds are successful, which avoid the issue we had in the past that some of the release artifact were missing due to random issues. In other words, the release assets are either all or none.

Closes #2148

ntkme commented 9 months ago

Sample run (some release steps are mocked): https://github.com/ntkme/dart-sass-test/actions/runs/7216265257 Sample release: https://github.com/ntkme/dart-sass-test/releases/tag/1.69.6

Screenshot 2023-12-14 at 3 04 27 PM
Screenshot 2023-12-13 at 9 13 33 PM
farnetto commented 9 months ago

The musl version works well for us. Thanks!

stof commented 9 months ago

should we also add support for musl-libc in the embedded-host-node repository so that the npm package sass-embedded can have this executable among its dependencies as well ?

ntkme commented 9 months ago

@stof I’m planning to look into that after this change goes out. We don’t necessarily need to support both at the same time.

nex3 commented 8 months ago

@ntkme Looks like the release is failing to find /.../dart/bin/snapshots/gen_kernel_aot.dart.snapshot for linux-musl 386 and android amd64. There's also a failure in the website deploy, which seems like npm isn't making the new package version available quickly enough. Not sure what we can do there other than add some additional wait time.

ntkme commented 8 months ago

@nex3 It looks like there might be a change in dart SDK causing cli_pkg to regress... for any ia32 platform it should not attempt to generate aot-snapshot...

ntkme commented 8 months ago

@nex3 I think the issue was this change: https://github.com/google/dart_cli_pkg/commit/3c5a2ccf0af48f128596c08a7da4081fbacc2e13

nex3 commented 8 months ago

Oh, I see... I skimmed https://github.com/dart-lang/sdk/issues/47177 and just noticed the fact that it was closed as completed and my comment that I was testing against the wrong SDK and assumed it was fixed. I'll re-add the exemption.

ntkme commented 8 months ago

The previous comment in the cli_pkg was indeed a bit misleading about all 32-bit. Dart SDK do have arm 32-bit aot support, but it never had ia32 aot support (see comments in https://github.com/dart-lang/sdk/issues/49969). In fact, the implementation was correct that it only exempt ia32.