@ntkme Thanks for opening this WIP PR.
sass-embedded requires a minimum Ruby version of 2.6.0. So before further developments, please update the requirement in the gemspec and CI config files.

@ntkme For future reference, you need not rebase unless there are merge conflicts, especially in a draft PR. We always squash all commits while merging a pull request.

Appveyor with ruby 2.7 and 3.0 is having some issue that I don't quite understand.
Appveyor with ruby 2.6 is stuck installing sass-embedded. - This step is just downloading three artifacts from GitHub: protoc, embedded-protocol, dart-sass-embedded. Not sure how could that stuck for an hour. I think it is likely an environment setup issue, but I don't know how to debug this.

sass-embedded gem package has been tested on windows using GitHub actions. For example: https://github.com/ntkme/sass-embedded-host-ruby/actions/runs/1240509193

So, it should work in windows with the right setup, not sure what's wrong with Appveyor.

Shall we try debug Appveyor or maybe we can move windows tests to use GitHub actions?

So, it should work in windows with the right setup, not sure what's wrong with Appveyor.
Shall we try debug Appveyor or maybe we can move windows tests to use GitHub actions?

@ntkme I've pushed a commit to migrate tests for Windows to GH Actions (just for this branch). While the installation succeeds, the test suite fails anyways since the expected output is not the same for sourcemaps for Linux and Windows for the two implementations.
Hopefully, you can fix it or is that for dart-sass to handle?

In the meantime, I'll think of another way to run the specs for two implementations without indenting the existing spec (to improve maintainability, ease running a single failing spec and to reduce the overall diff on this pull request.)

The source map issue has to do with dart-sass converting D:/... to D/... for windows, and that breaks the native directory handling in ruby. It should to be trivial to fix and I will take a look.

I fixed the the source map issue on windows. Tests now passes for ruby 2.7/3.0 on windows.

However, Ruby 2.6 on windows would get stuck during dependencies installation.

@ntkme that looks like a classic "rerun and succeed" CI issue...

Attempted to run bundle install outside of setup-ruby action, and it worked. But bundle install inside setup-ruby with ruby 2.6 does not work, even with a new cache-version set. Not sure why.

@ntkme I don't know what the issue is with Ruby 2.6 and Windows.
Since this was evident on AppVeyor as well, I have removed sass-embedded as a runtime dependency. Users will have to manually include the gem in their dev environment either via Gemfile or direct install.

Currently, the loading is within the :convert method. It is less than ideal but adequate for the time being.

Now that the CI has passed (I removed Windows-Ruby2.6 from the matrix), let us focus on the documentation aspect.

@ashmaroli Sure. Let's do that for now. I opened ruby/setup-ruby#218, hope we can get some help.

@ashmaroli Manged to get access to a windows machine and fixed ruby 2.6 on windows. Test now passes. Details is here if anyone is interested: ruby/setup-ruby#218 (comment)

That is wonderful investigative work @ntkme. 🎉
Thanks for fixing things so quickly.

@ntkme I tested this on an old system running on Windows 7 SP1. Latest versions of Ruby and Jekyll installs and runs fine on that system but it doesn't have a newer version of Powershell. (Windows 7 ships with just PS 2.0; PS 5+ is needed for running Expand-Archive cmdlet).

Therefore, in the interest of Jekyll users still on Windows 7, I suggest not having sass-embedded as a runtime dependency of this plugin.

It looks like rubyinstaller2 provides gunzip command from msys2. I can check if gunzip is available and fallback to Expand-Archive.

Never mind. Forgot that gunzip does not work with zip that contains more than one files.

I looked at how sass/embedded-host-node handles this problem and it runs out they have added a dependency just to handle zip files. So, we can do the same here.

@ntkme I have decided to not include sass-embedded as a runtime dependency of the upcoming release of this plugin.
Users willing to try dart-sass are welcome to make the switch willingly; so your work here will not go to waste.

Slowly, as the ecosystem stabilizes, we'll probably phase out sassc as the primary implementation and migrate to dart-sass out-of-the-box.

@ashmaroli That's fine with me. Please feel free to push changes to do that.

@ashmaroli By the way, excluding it from runtime dependencies means that we can have this feature released even with dart-sass-embedded being in beta, as ultimately it is end users' choice to manually install this unstable dependency. Is that what you meant by "upcoming release"?

have this feature released even with dart-sass-embedded being in beta..

Yes, that is the plan.
I only mentioned "upcoming" in the lax sense. There will surely be a minor-version bump and this feature will be a nice addition.
However, there is no fixed date, so no hurry. You can contribute in your free time.

Windows 7 now works with some VBS magic to do the unzip work: sass-contrib/sass-embedded-host-ruby@73a66b2

Manually tested with Windows 7 VM from https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/ and rubyinstaller2 (2.6.8-1)

Windows 7 now works with some VBS magic

Whoa! That was a wrong move, @ntkme
Your project just lost a couple of security points.

Using gems with native extensions are always a security concern. In the current scope, the gem already downloads third-party archives from the internet without user intervention during the installation, installs a native executable (dart), (none of this is logged in the stdout) and now there is a custom non-Ruby script also bundled within the gem (irrespective of whether that script runs for 99% of user-base).

Agreed, that everything is open-source and verifiable.
Agreed, that each concern has workarounds:

• Compile gem manually from source.
• Download archives manually, validate them and point to them during installation.
• Install PowerShell 5.1+ (if on Windows)

Nevertheless, the default behavior has a negative impact on trustworthiness.

The bundled VBS script is very explicit that it can only be used to unzip, nothing else, and it is only used if user does not have powershell 5.0+. I don’t see any problem just because the common misconceptions of VBS being associated with malware. I think this is a fair use case since there is no other solution out of box.

Compile the dart code would require the whole dart tool chain. In addition, it will need git or unzip or otherwise bundle whole source code with the project that can introduce a whole lot more errors.

Ask user to install powershell 5.0 might be something fair, but the user experience is not as good as this.

Regardless of using VBS or not, users have to decide what gems they trust. I mean, even if a malicious gem does not have any extension it can still do harm at runtime as soon as a user requires it.

If people are so paranoid about VBS I can definitely revert it and just call it a day saying Windows 7 is not supported, as it is already end of support. If it wasn’t pointed out here that it wouldn’t work with windows 7, I won’t even bother try supporting an end of support system.

If people are so paranoid about VBS I can definitely revert it and just call it a day saying Windows 7 is not supported

It wasn't about VBS but having a custom script silently run as part of the installation. IMO, reverting the support would be best.
Also, if you cut a new release, you need not push a commit here to update the Gemfile. The version used in the action runner will get updated automatically (until you release a v0.8).

Also, I apologize for wasting your time by mentioning Windows 7.

It wasn't about VBS but having a custom script silently run as part of the installation.

Overall the problem is that native dependencies which are not native language binding libraries does not really work with native extension build system (mkmf.rb or node-gyp), as those building systems are designed only for building native language binding libraries. sass team made the choice to to use dart compiled native binary and stdio communication instead of native binding as there is no easy way to create a dart-to-other-language binding. For example, to implement a dart-compiled-native-code-to-ruby-binding, the whole Dart VM need to be embedded into the ruby native binding library, and then a remote call interface need to be written to handle serialization of method, parameters, and return values between Dart VM methods/types and ruby c-extension methods/types. - That is possible but the development cost is too expensive.

On the other hand, use a custom script to handle installation of a native dependency that is not a native language binding library is actually pretty common nowadays. For example, in nodejs world, the "postinstall" hook can be used to run any script to handle additional setup or configuration to provide out of box user experience, and many projects use this feature to run custom script. In the official nodejs equivalent project sass/embedded-host-node, you can see that they use a custom script to download files during installation: https://github.com/sass/embedded-host-node/blob/46d600d239698fab9f811c059469d89efaaf3355/package.json#L24

You do have a good point about "silently run", but that's a design of rubygems. By default, extconf would only print Building native extensions. This could take a while..., but nothing else unless gem install is run with --verbose option. It's rubygems's design choice to "silently run" by default, I cannot really do anything about it. If user run try to install it with --verbose, I did make sure that each non-ruby command executed is also printed to the console, so that users can be aware of what has happened. There is nothing intentionally hidden.

By design ruby's mkmf.rb uses Makefile, thus any commands available on the system can be used under that. Practically running a VBS is not really different from writing the same functionality in ruby code or other language. embedded-host-node has dependencies extract-zip and tar to handle compressed file in purely nodejs code. We could do the same for ruby by adding rubyzip. However, there is a clear benefit of VBS: it's tiny and concise. From a security audit perspective, it's 113KB of ruby code (only counting files in the lib folder) vs 0.5KB of VBS to be inspected.

Finally, I understand people have concerns because the custom script "download" artifacts from internet. sass-embedded only download from sass' and protobuf's official GitHub releases. For restricted environment that may have limit access for public internet, it does offer an alternative offline installation method where user has the full control of dependencies being used:

https://github.com/ntkme/sass-embedded-host-ruby/blob/f1b0e66dfefdeeb1a3578da19a963a9da47aee1a/ext/sass/extconf.rb#L11-L24

An alternative that does not involve downloading or unzipping would be building platform native gem for each platform and embedding the uncompressed platform native dependencies directly in the gem itself. However, in the end that's less transparent (need to verify a 8.7MB binary from non-official distribution channel), therefore make it even worse than downloading from official release channel.

@ashmaroli I did a rebase because merging conflict is getting crazy.

Also, I changed the way that test works so that now this PR does not indent the test cases, so the changes would be much easier to review. Now test script would run twice in the CI with the environment variable SASS_IMPLEMENTATION controls which implementation to be used in test. This environment variable is test only and won't affect users.

I did a rebase because merging conflict is getting crazy.

No issues, it was inevitable.

I changed the way that test works so that now this PR does not indent the test cases,

Glad that fog has been cleared.

test script would run twice in the CI with the environment variable SASS_IMPLEMENTATION controls which implementation to be used in test.

This is less than ideal, but an acceptable trade-off. I'll see if we can get the best of both methods (former and current) after this PR gets merged.
Now, onto the review phase (although documentation for this feature is still scant).

@parkr PTAL when you get a chance. Thanks!

Thank you @ntkme, for all the work put into this.
@jekyllbot: merge +minor

Thanks @ashmaroli @mattr- @parkr for your help!