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

Please slow down and coordinate the move from dart-sass-embedded #2014

Closed
bep opened this issue Jun 12, 2023 · 6 comments · Fixed by sass/homebrew-sass#54
Closed

Please slow down and coordinate the move from dart-sass-embedded #2014

bep opened this issue Jun 12, 2023 · 6 comments · Fixed by sass/homebrew-sass#54
Assignees

Comments

@bep
Copy link

bep commented Jun 12, 2023

I apologise in advance if I'm not hitting the right target audience here, but ...

I maintain godartsass and hugo, both use Dart Sass Embedded. I am in the process of upgrading these to the new protocol wire format, but you need to give upstream maintainers some time (this is not my day job) to do the work and testing needed -- also considering that there seem to be some critical issues (e.g. #2004) with this new version.

This issue mostly comes from the fact that you seem to have retired the old dart-sass-embedded Homebrew binary, which creates more noise than I appreciate:

https://discourse.gohugo.io/t/dart-sass-embedded-deprecated-no-longer-works-with-homebrew/44775

A footnote here: It would also be good if you in the future somehow add the major version to your proto files/packages, to allow libraries to use both old and new version side-by-side.

@ntkme
Copy link
Contributor

ntkme commented Jun 12, 2023

Old homebrew binary can be installed with brew install sass/sass/[email protected].

@nex3
Copy link
Contributor

nex3 commented Jun 13, 2023

We went through the usual design and request-for-feedback process with the change to the embedded protocol, and we've left all the old versions of dart-sass-embedded available everywhere (including on Homebrew, as @ntkme points out). I'm not sure what "giving maintainers some time" would look like if not one of those two things. At some point we needed to release the new binaries for you to test against.

@jmooring
Copy link
Contributor

In CI/CD environments, a script to build a Hugo site may include:

brew install sass/sass/dart-sass-embedded

Which, after recent changes, doesn't install Embedded Dart Sass. It installs Dart Sass. So... broken Hugo builds.

We will update our documentation to include the version number as suggested above.

@ntkme
Copy link
Contributor

ntkme commented Jun 14, 2023

Due to the possibility of future compatibility issues or unindented bugs, it would better for host implementations to self-contain the sass embedded binaries and maintain a one-to-one version mapping between compiler and host. Official embedded-host-node, my embedded-host-ruby and embedded-host-java are all following this approach, so that any new release of the compiler won't break any released version of host. This would also make it easier to triage issues as it won't have infinite number of possibility in the compatibility matrix.

With that said, I understand that properly distribute prebuilt binaries is difficult. For example, I maintain 6 projects just to get ruby host releases running and even had to contribute code to official Dart SDK.

Here are all the projects I maintain for sass-embedded-host-ruby, they may help if you choose to bundle pre-built binaries:

These take quite a lot effort to maintain, but the effort was well worth it. For reference, about 20% of downloads for ruby host are from alpine linux, so it was extremely critical to provide musl support so that I don't get spammed by frustrated users.

Hope it helps.

@nex3
Copy link
Contributor

nex3 commented Jun 15, 2023

@ntkme Do you know if it would be possible to continue supporting the unversioned dart-sass-embedded Homebrew tap with some sort of explicit deprecation warning?

@bep
Copy link
Author

bep commented Jun 15, 2023

@ntkme my 50 cents about this;

  • dart-sass-embedded and sass (or dart-sass) are 2 major versions and not in any way compatible; renaming the old binary having both share the same protobuf filename/package is not the way to go about it. If you had spent some time in the original issue discussing "how to do the upgrade", I would have told you that.
  • I have so far spent 10 hours on this upgrade, hours I need to take out of my own pocket (time I now wish I had spent on trout fishing). I was initially positive to this upgrade (as it initially talked about improved performance), now my mood is fading.
  • Any talk about "you should embed the binary" etc. is all well and good, but that's not currently an option for me, and that is not only budget concerns (re. the 10 hours already spent on this). Also, adding that now would not have retrospectively unbroken things.

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 a pull request may close this issue.

4 participants