Switch to sassc-rails

What does this MR do?

It replaces the sass-rails gem with the sassc-rails gem for faster compilation.

Are there points in the code the reviewer needs to double check?

No, the diff is straightforward.

Why was this MR needed?

It was requested in issue #18432 (closed), and generally faster compilation is a good thing :)

Does this MR meet the acceptance criteria?

• Changelog entry added
• Documentation created/updated
• API support added
• Tests
  • Added for this feature/bug
  • All builds are passing
• Conform by the merge request performance guides
• Conform by the style guides
• Branch has no merge conflicts with master (if it does - rebase it please)
• Squashed related commits together

What are the relevant issue numbers?

Closes #18432 (closed)

More information

Without attempting to be very scientific here are some numbers I got:

Using sassc-rails:

[1] pry(main)> Benchmark.bm { |bm| bm.report { Rails.application.assets["application.css"] } }
user system total real
1.430000 0.380000 1.810000 ( 1.830753)

Using sass-rails:

[1] pry(main)> Benchmark.bm { |bm| bm.report { Rails.application.assets["application.css"] } }
user system total real
12.320000 0.530000 12.850000 ( 12.909684)

The result is faster page loads when changing CSS in development and faster precompilation.

added 1 commit

• b4623c5a - Switch to sassc-rails

Compare with previous version

mentioned in issue #18432 (closed)

Thanks @rmacklin!

@rymai would you like to take a look?

added ~164274 label

assigned to @rymai

marked the task All builds are passing as completed

@rmacklin Thanks! Looks good to me, passing to @rspeicher since you requested/advised to use this gem.

added ~506179 ~112478 labels

changed milestone to %8.16

assigned to @rspeicher

So the trade-off is that because this is building a native extension, the bundle install process will take longer. The gem will speed up asset recompilation on developers' local machines, so it's a good win for Frontend devs in particular, but it might be a wash, or net loss, on CI.

Thank you guys for having a look at this.

@rspeicher Yeah, when I switched to sassc at work the native extension building was a one time cost per CI node. I'm not familiar with your CI setup, are the nodes not fixed machines (whose gemsets would persist)? If not, is this something that could be included in a base image? Again, please excuse my lack of familiarity with your CI

Anyway, I could also change the PR to only use sassc in development if we don't think the trade-off is worth it.

added 1 commit

• d5e996d1 - Switch to sassc-rails

Compare with previous version

resolved all discussions

enabled an automatic merge when the pipeline for d5e996d1 succeeds

@rmacklin Great, thanks!

merged

mentioned in commit e7fdb1aa

mentioned in issue #26762 (closed)

mentioned in issue omnibus-gitlab#1861 (closed)

This is causing build failures on centos 6: https://gitlab.com/gitlab-org/gitlab-ce/issues/26762

Ah sorry, wasn't aware of the broad OS support. We can revert this to get it passing and then discuss how to proceed.

Ok, so looking at https://gitlab.com/gitlab-org/gitlab-development-kit/blob/562538829153823ff628b376e09243b0460bc473/doc/prepare.md it seems GitLab supports a pretty wide variety of linux distros. Some questions from an outsider (totally fine if we don't have all the answers):

Do we have any kind of automated way to test all supported environments against infrastructure changes like this one?

If not, do we know (or have documented somewhere) what supported distros use older packages, and therefore are likely more susceptible to issues like this?

From https://github.com/sass/libsass/pull/848#issuecomment-70639304, the libass maintainers mention that:

Notably older CentOS users have very old compilers, lower than even we support.

so is it possible that CentOS 6 is the only one we need to worry about for sassc?

If that's the case, how would we feel about updating CentOS requirements to include installing a newer GCC (if on CentOS 6; it seems CentOS 7 isn't affected)?

If we're against that, how would we feel about conditionally using sassc-rails and falling back to sass-rails in environments that can't compile libsass?

As mentioned in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8556#note_21458451 I could also make it such that sassc-rails is only used in development; however if it's really only one distro that doesn't work with sassc-rails I'd probably lean towards making that environment the exception (but of course I'm not claiming to know best).

As mentioned, it's okay if we don't have answers to all of this right now; just trying to share the questions that came to mind. Sorry for breaking builds :(

@twk3 I haven't picked this into stable yet (so it may not go into rc4) - let me know if I should? I see the new issue is still open. /cc @rymai @dbalexandre @rspeicher

mentioned in merge request !8642 (merged)

For cross-reference, see https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1065#note_21687686

@jameslopez No need to pick if you follow the process I suggest in the linked comment.