From 5e2c3df905cc3a224265aedb8c2213dfa8faed15 Mon Sep 17 00:00:00 2001
From: Lukas 'ai-pi' Eipert <leipert@gitlab.com>
Date: Wed, 28 Feb 2024 15:01:12 +0000
Subject: [PATCH] Switch from sassc to cssbundling gem
We already chose to switch to cssbundling per default about a week ago:
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144479
We haven't seen any regressions in production (apart from the UTF-8 fix
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/145363 needed). So
it is probably safe to move forward with the removal of the old SCSS
compiler.
---
.gitlab-ci.yml | 4 ---
Gemfile | 3 +-
Gemfile.checksum | 3 --
Gemfile.lock | 9 ------
config/application.rb | 20 ++-----------
lib/tasks/gitlab/assets.rake | 3 +-
scripts/frontend/compare_css_compilers.sh | 35 -----------------------
scripts/frontend/lib/compile_css.mjs | 23 ---------------
scripts/generate-e2e-pipeline | 1 -
9 files changed, 4 insertions(+), 97 deletions(-)
delete mode 100755 scripts/frontend/compare_css_compilers.sh
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 301695a2ff38..8e58120dc6f0 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -254,10 +254,6 @@ variables:
BUILD_ASSETS_IMAGE: "true" # Set it to "false" to disable assets image building, used in `build-assets-image`
SIMPLECOV: "true"
- # Temporary variable to enable new CSS compiler
- # See: https://gitlab.com/gitlab-org/gitlab/-/issues/438278
- USE_NEW_CSS_PIPELINE: "true"
-
include:
- local: .gitlab/ci/_skip.yml
rules:
diff --git a/Gemfile b/Gemfile
index 1e8a82688210..aad8a14a733a 100644
--- a/Gemfile
+++ b/Gemfile
@@ -341,8 +341,7 @@ gem 'gitlab_chronic_duration', '~> 0.12' # rubocop:todo Gemfile/MissingFeatureCa
gem 'rack-proxy', '~> 0.7.7' # rubocop:todo Gemfile/MissingFeatureCategory
-gem 'sassc-rails', '~> 2.1.0', feature_category: :shared, require: false
-gem 'cssbundling-rails', '1.3.3', feature_category: :shared, require: false
+gem 'cssbundling-rails', '1.3.3', feature_category: :shared
gem 'autoprefixer-rails', '10.2.5.1' # rubocop:todo Gemfile/MissingFeatureCategory
gem 'terser', '1.0.2' # rubocop:todo Gemfile/MissingFeatureCategory
diff --git a/Gemfile.checksum b/Gemfile.checksum
index 538260c54904..dff4fc77e42b 100644
--- a/Gemfile.checksum
+++ b/Gemfile.checksum
@@ -591,9 +591,6 @@
{"name":"safe_yaml","version":"1.0.4","platform":"ruby","checksum":"248193992ef1730a0c9ec579999ef2256a2b3a32a9bd9d708a1e12544a489ec2"},
{"name":"safety_net_attestation","version":"0.4.0","platform":"ruby","checksum":"96be2d74e7ed26453a51894913449bea0e072f44490021545ac2d1c38b0718ce"},
{"name":"sanitize","version":"6.0.2","platform":"ruby","checksum":"48c4eb8e92bb1699056b6000986ac50fc9df82f458a941abf2c4d6759bccd5cf"},
-{"name":"sassc","version":"2.4.0","platform":"ruby","checksum":"4c60a2b0a3b36685c83b80d5789401c2f678c1652e3288315a1551d811d9f83e"},
-{"name":"sassc","version":"2.4.0","platform":"x64-mingw32","checksum":"8773b917cb52c7e92c94d4bf324c1c0be3e50d9092f9f5ed4c3c6e454b451c5e"},
-{"name":"sassc-rails","version":"2.1.2","platform":"ruby","checksum":"5f4fdf3881fc9bdc8e856ffbd9850d70a2878866feae8114aa45996179952db5"},
{"name":"sawyer","version":"0.9.2","platform":"ruby","checksum":"fa3a72d62a4525517b18857ddb78926aab3424de0129be6772a8e2ba240e7aca"},
{"name":"sd_notify","version":"0.1.1","platform":"ruby","checksum":"cbc7ac6caa7cedd26b30a72b5eeb6f36050dc0752df263452ea24fb5a4ad3131"},
{"name":"seed-fu","version":"2.3.7","platform":"ruby","checksum":"f19673443e9af799b730e3d4eca6a89b39e5a36825015dffd00d02ea3365cf74"},
diff --git a/Gemfile.lock b/Gemfile.lock
index 3ed9cfa4b4ea..57d5d0b156d7 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1546,14 +1546,6 @@ GEM
sanitize (6.0.2)
crass (~> 1.0.2)
nokogiri (>= 1.12.0)
- sassc (2.4.0)
- ffi (~> 1.9)
- sassc-rails (2.1.2)
- railties (>= 4.0.0)
- sassc (>= 2.0)
- sprockets (> 3.0)
- sprockets-rails
- tilt
sawyer (0.9.2)
addressable (>= 2.3.5)
faraday (>= 0.17.3, < 3)
@@ -2103,7 +2095,6 @@ DEPENDENCIES
rubyzip (~> 2.3.2)
rugged (~> 1.6)
sanitize (~> 6.0.2)
- sassc-rails (~> 2.1.0)
sd_notify (~> 0.1.0)
seed-fu (~> 2.3.7)
selenium-webdriver (~> 4.18, >= 4.18.1)
diff --git a/config/application.rb b/config/application.rb
index 270a8260bc1c..013d2403fe01 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -255,19 +255,6 @@ class Application < Rails::Application
config.active_record.has_many_inversing = false
config.active_record.belongs_to_required_by_default = false
- # Switch between cssbundling-rails and sassc-rails conditionally
- # To activate cssbundling-rails you can set `USE_NEW_CSS_PIPELINE=1`
- # For more context, see: https://gitlab.com/gitlab-org/gitlab/-/issues/438278
- # Need to be loaded before initializers
- config.before_configuration do
- if Gitlab::Utils.to_boolean(ENV["USE_NEW_CSS_PIPELINE"], default: true)
- require 'cssbundling-rails'
- else
- require 'fileutils'
- require 'sassc-rails'
- end
- end
-
# Enable the asset pipeline
config.assets.enabled = true
@@ -624,11 +611,8 @@ class Application < Rails::Application
end
# Add `app/assets/builds` as the highest precedence to find assets
- # This is required if cssbundling-rails is used, but should not affect sassc-rails. it would be empty
- if defined?(Cssbundling)
- initializer :add_cssbundling_output_dir, after: :prefer_specialized_assets do |app|
- app.config.assets.paths.unshift("#{config.root}/app/assets/builds")
- end
+ initializer :add_cssbundling_output_dir, after: :prefer_specialized_assets do |app|
+ app.config.assets.paths.unshift("#{config.root}/app/assets/builds")
end
# We run the contents of active_record.clear_active_connections again
diff --git a/lib/tasks/gitlab/assets.rake b/lib/tasks/gitlab/assets.rake
index ad70bc06e341..fa294761b43f 100644
--- a/lib/tasks/gitlab/assets.rake
+++ b/lib/tasks/gitlab/assets.rake
@@ -52,7 +52,6 @@ module Tasks
puts "Generating the SHA256 hash for #{asset_files.size} Webpack-related assets..." if verbose
assets_sha256 = asset_files.map { |asset_file| Digest::SHA256.file(asset_file).hexdigest }.join
- assets_sha256 += ENV.fetch('USE_NEW_CSS_PIPELINE', 'true')
Digest::SHA256.hexdigest(assets_sha256).tap { |sha256| puts "=> SHA256 generated in #{Time.now - start_time}: #{sha256}" if verbose }
end
@@ -113,7 +112,7 @@ namespace :gitlab do
# app/assets/javascripts/locale/**/app.js are pre-compiled by Sprockets
Gitlab::TaskHelpers.invoke_and_time_task('gettext:compile')
# Skip Yarn Install when using Cssbundling
- Rake::Task["css:install"].clear if defined?(Cssbundling)
+ Rake::Task["css:install"].clear
Gitlab::TaskHelpers.invoke_and_time_task('rake:assets:precompile')
log_path = ENV['WEBPACK_COMPILE_LOG_PATH']
diff --git a/scripts/frontend/compare_css_compilers.sh b/scripts/frontend/compare_css_compilers.sh
deleted file mode 100755
index faf208716782..000000000000
--- a/scripts/frontend/compare_css_compilers.sh
+++ /dev/null
@@ -1,35 +0,0 @@
-#!/bin/bash
-set -euo pipefail
-IFS=$'\n\t'
-
-if [ ! -d "glfm_specification" ] || [ ! -f "GITALY_SERVER_VERSION" ]; then
- echo 'Please run this from the gitlab root folder with `./scripts/frontend/compare_css_compilers.sh`'
- exit 1
-fi
-
-function clean_up {
- rm -rf public/assets
- rm -rf app/assets/builds/*
- rm -rf tmp/cache/assets
-}
-
-rm -rf tmp/css_compare
-clean_up
-
-export SKIP_YARN_INSTALL=1
-
-echo "Compiling with sassc-rails"
-export USE_NEW_CSS_PIPELINE=0
-time bin/rails assets:precompile
-scripts/frontend/clean_css_assets.mjs public/assets tmp/css_compare/sassc-rails
-
-clean_up
-
-export USE_NEW_CSS_PIPELINE=1
-echo "Compiling with dart-sass"
-time bin/rails assets:precompile
-scripts/frontend/clean_css_assets.mjs public/assets tmp/css_compare/cssbundling
-
-clean_up
-
-echo 'You now can run `diff -u tmp/css_compare/sassc-rails tmp/css_compare/cssbundling` to diff the two'
diff --git a/scripts/frontend/lib/compile_css.mjs b/scripts/frontend/lib/compile_css.mjs
index 8b59c17bab3a..056db6ec784c 100644
--- a/scripts/frontend/lib/compile_css.mjs
+++ b/scripts/frontend/lib/compile_css.mjs
@@ -1,7 +1,6 @@
import { mkdir, writeFile } from 'node:fs/promises';
import { fileURLToPath } from 'node:url';
import path from 'node:path';
-import { env } from 'node:process';
import { compile, Logger } from 'sass';
import glob from 'glob';
/* eslint-disable import/extensions */
@@ -223,26 +222,7 @@ export async function compileAllStyles({ shouldWatch = false }) {
return fileWatcher;
}
-// Mirroring gems/gitlab-utils/lib/gitlab/utils.rb
-function shouldUseNewPipeline(defaultValue = true) {
- if ([true, false, 0, 1].includes(env?.USE_NEW_CSS_PIPELINE)) {
- return env.USE_NEW_CSS_PIPELINE;
- }
-
- if (/^(true|t|yes|y|1|on)$/i.test(`${env.USE_NEW_CSS_PIPELINE}`)) {
- return true;
- }
- if (/^(false|f|no|n|0|off)$/i.test(`${env.USE_NEW_CSS_PIPELINE}`)) {
- return false;
- }
-
- return defaultValue;
-}
-
export function viteCSSCompilerPlugin({ shouldWatch = true }) {
- if (!shouldUseNewPipeline()) {
- return null;
- }
let fileWatcher = null;
return {
name: 'gitlab-css-compiler',
@@ -256,9 +236,6 @@ export function viteCSSCompilerPlugin({ shouldWatch = true }) {
}
export function simplePluginForNodemon({ shouldWatch = true }) {
- if (!shouldUseNewPipeline()) {
- return null;
- }
let fileWatcher = null;
return {
async start() {
diff --git a/scripts/generate-e2e-pipeline b/scripts/generate-e2e-pipeline
index 1f32a33e6c1c..e8efcaee7405 100755
--- a/scripts/generate-e2e-pipeline
+++ b/scripts/generate-e2e-pipeline
@@ -49,7 +49,6 @@ variables:
QA_SUITES: "$QA_SUITES"
QA_TESTS: "$QA_TESTS"
KNAPSACK_TEST_FILE_PATTERN: "$KNAPSACK_TEST_FILE_PATTERN"
- USE_NEW_CSS_PIPELINE: "true"
YML
)
--
GitLab