Skip to content

Commit b34b5e9

Browse files
rullzerrullzer
committed
Lock SCSS so we only run 1 job at a time
This is bit hacky but a start to lock the SCSS compiler properly Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl> Retry during 10s then give up Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com> Properly get error message Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com> Do not clear locks and properly debug scss caching Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com> Update lib/private/Template/SCSSCacher.php Co-Authored-By: Daniel Kesselberg <mail@danielkesselberg.de> Update lib/private/Template/SCSSCacher.php Co-Authored-By: Daniel Kesselberg <mail@danielkesselberg.de> Update lib/private/Template/SCSSCacher.php
1 parent c193c0d commit b34b5e9

File tree

2 files changed

+77
-18
lines changed

2 files changed

+77
-18
lines changed

‎lib/private/Template/IconsCacher.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,9 @@ public function getCachedList() {
238238
}
239239
}
240240

241+
/**
242+
* Add the icons cache css into the header
243+
*/
241244
public function injectCss() {
242245
$mtime = $this->timeFactory->getTime();
243246
$file = $this->getCachedList();

‎lib/private/Template/SCSSCacher.php‎

Lines changed: 74 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
use Leafo\ScssPhp\Exception\ParserException;
3333
use Leafo\ScssPhp\Formatter\Crunched;
3434
use Leafo\ScssPhp\Formatter\Expanded;
35+
use OC\Memcache\NullCache;
3536
use OCP\AppFramework\Utility\ITimeFactory;
3637
use OCP\Files\IAppData;
3738
use OCP\Files\NotFoundException;
@@ -42,6 +43,7 @@
4243
use OCP\ICacheFactory;
4344
use OCP\IConfig;
4445
use OCP\ILogger;
46+
use OCP\IMemcache;
4547
use OCP\IURLGenerator;
4648
use OC\Files\AppData\Factory;
4749
use OC\Template\IconsCacher;
@@ -84,6 +86,9 @@ class SCSSCacher {
8486
/** @var ITimeFactory */
8587
private $timeFactory;
8688

89+
/** @var IMemcache */
90+
private $lockingCache;
91+
8792
/**
8893
* @param ILogger $logger
8994
* @param Factory $appDataFactory
@@ -111,8 +116,13 @@ public function __construct(ILogger $logger,
111116
$this->defaults = $defaults;
112117
$this->serverRoot = $serverRoot;
113118
$this->cacheFactory = $cacheFactory;
114-
$this->depsCache = $cacheFactory->createDistributed('SCSS-' . md5($this->urlGenerator->getBaseUrl()));
119+
$this->depsCache = $cacheFactory->createDistributed('SCSS-deps-' . md5($this->urlGenerator->getBaseUrl()));
115120
$this->isCachedCache = $cacheFactory->createLocal('SCSS-cached-' . md5($this->urlGenerator->getBaseUrl()));
121+
$lockingCache = $cacheFactory->createDistributed('SCSS-locks-' . md5($this->urlGenerator->getBaseUrl()));
122+
if (!($lockingCache instanceof IMemcache)) {
123+
$lockingCache = new NullCache();
124+
}
125+
$this->lockingCache = $lockingCache;
116126
$this->iconsCacher = $iconsCacher;
117127
$this->timeFactory = $timeFactory;
118128
}
@@ -137,10 +147,7 @@ public function process(string $root, string $file, string $app): bool {
137147

138148
if (!$this->variablesChanged() && $this->isCached($fileNameCSS, $app)) {
139149
// Inject icons vars css if any
140-
if ($this->iconsCacher->getCachedCSS() && $this->iconsCacher->getCachedCSS()->getSize() > 0) {
141-
$this->iconsCacher->injectCss();
142-
}
143-
return true;
150+
return $this->injectCssVariablesIfAny();
144151
}
145152

146153
try {
@@ -150,7 +157,35 @@ public function process(string $root, string $file, string $app): bool {
150157
$folder = $this->appData->newFolder($app);
151158
}
152159

153-
$cached = $this->cache($path, $fileNameCSS, $fileNameSCSS, $folder, $webDir);
160+
$lockKey = $webDir . '/' . $fileNameSCSS;
161+
162+
if (!$this->lockingCache->add($lockKey, 'locked!', 120)) {
163+
$retry = 0;
164+
sleep(1);
165+
while ($retry < 10) {
166+
if (!$this->variablesChanged() && $this->isCached($fileNameCSS, $app)) {
167+
// Inject icons vars css if any
168+
$this->lockingCache->remove($lockKey);
169+
$this->logger->debug('SCSSCacher: ' .$lockKey.' is now available after '.$retry.'s. Moving on...', ['app' => 'core']);
170+
return $this->injectCssVariablesIfAny();
171+
}
172+
$this->logger->debug('SCSSCacher: scss cache file locked for '.$lockKey, ['app' => 'core']);
173+
sleep($retry);
174+
$retry++;
175+
}
176+
$this->logger->debug('SCSSCacher: Giving up scss caching for '.$lockKey, ['app' => 'core']);
177+
return false;
178+
}
179+
180+
try {
181+
$cached = $this->cache($path, $fileNameCSS, $fileNameSCSS, $folder, $webDir);
182+
} catch (\Exception $e) {
183+
$this->lockingCache->remove($lockKey);
184+
throw $e;
185+
}
186+
187+
// Cleaning lock
188+
$this->lockingCache->remove($lockKey);
154189

155190
// Inject icons vars css if any
156191
if ($this->iconsCacher->getCachedCSS() && $this->iconsCacher->getCachedCSS()->getSize() > 0) {
@@ -180,19 +215,24 @@ public function getCachedCSS(string $appName, string $fileName): ISimpleFile {
180215
*/
181216
private function isCached(string $fileNameCSS, string $app) {
182217
$key = $this->config->getSystemValue('version') . '/' . $app . '/' . $fileNameCSS;
183-
if (!$this->config->getSystemValue('debug') && $cacheValue = $this->isCachedCache->get($key)) {
218+
219+
// If the file mtime is more recent than our cached one,
220+
// let's consider the file is properly cached
221+
if ($cacheValue = $this->isCachedCache->get($key)) {
184222
if ($cacheValue > $this->timeFactory->getTime()) {
185223
return true;
186224
}
187225
}
188226

227+
// Creating file cache if none for further checks
189228
try {
190229
$folder = $this->appData->getFolder($app);
191230
} catch (NotFoundException $e) {
192-
// creating css appdata folder
193-
$folder = $this->appData->newFolder($app);
231+
return false;
194232
}
195233

234+
// Checking if file size is coherent
235+
// and if one of the css dependency changed
196236
try {
197237
$cachedFile = $folder->getFile($fileNameCSS);
198238
if ($cachedFile->getSize() > 0) {
@@ -201,7 +241,7 @@ private function isCached(string $fileNameCSS, string $app) {
201241
if ($deps === null) {
202242
$depFile = $folder->getFile($depFileName);
203243
$deps = $depFile->getContent();
204-
//Set to memcache for next run
244+
// Set to memcache for next run
205245
$this->depsCache->set($folder->getName() . '-' . $depFileName, $deps);
206246
}
207247
$deps = json_decode($deps, true);
@@ -228,13 +268,11 @@ private function isCached(string $fileNameCSS, string $app) {
228268
*/
229269
private function variablesChanged(): bool {
230270
$injectedVariables = $this->getInjectedVariables();
231-
if ($this->config->getAppValue('core', 'scss.variables') !== md5($injectedVariables)) {
271+
if ($this->config->getAppValue('core', 'theming.variables') !== md5($injectedVariables)) {
232272
$this->resetCache();
233-
$this->config->setAppValue('core', 'scss.variables', md5($injectedVariables));
234-
273+
$this->config->setAppValue('core', 'theming.variables', md5($injectedVariables));
235274
return true;
236275
}
237-
238276
return false;
239277
}
240278

@@ -289,7 +327,7 @@ private function cache(string $path, string $fileNameCSS, string $fileNameSCSS,
289327
'@import "functions.scss";' .
290328
'@import "' . $fileNameSCSS . '";');
291329
} catch (ParserException $e) {
292-
$this->logger->error($e, ['app' => 'core']);
330+
$this->logger->logException($e, ['app' => 'core']);
293331

294332
return false;
295333
}
@@ -327,7 +365,11 @@ private function cache(string $path, string $fileNameCSS, string $fileNameSCSS,
327365
*/
328366
public function resetCache() {
329367
$this->injectedVariables = null;
330-
$this->cacheFactory->createDistributed('SCSS-')->clear();
368+
369+
// do not clear locks
370+
$this->cacheFactory->createDistributed('SCSS-deps-')->clear();
371+
$this->cacheFactory->createDistributed('SCSS-cached-')->clear();
372+
331373
$appDirectory = $this->appData->getDirectoryListing();
332374
foreach ($appDirectory as $folder) {
333375
foreach ($folder->getDirectoryListing() as $file) {
@@ -338,6 +380,7 @@ public function resetCache() {
338380
}
339381
}
340382
}
383+
$this->logger->debug('SCSSCacher: css cache cleared!');
341384
}
342385

343386
/**
@@ -358,7 +401,7 @@ private function getInjectedVariables(): string {
358401
$scss->compile($variables);
359402
$this->injectedVariables = $variables;
360403
} catch (ParserException $e) {
361-
$this->logger->error($e, ['app' => 'core']);
404+
$this->logger->logException($e, ['app' => 'core']);
362405
}
363406

364407
return $variables;
@@ -391,7 +434,7 @@ public function getCachedSCSS(string $appName, string $fileName): string {
391434
return substr($this->urlGenerator->linkToRoute('core.Css.getCss', [
392435
'fileName' => $fileName,
393436
'appName' => $appName,
394-
'v' => $this->config->getAppValue('core', 'scss.variables', '0')
437+
'v' => $this->config->getAppValue('core', 'theming.variables', '0')
395438
]), \strlen(\OC::$WEBROOT) + 1);
396439
}
397440

@@ -449,4 +492,17 @@ private function getWebDir(string $path, string $appName, string $serverRoot, st
449492

450493
return $webRoot . substr($path, strlen($serverRoot));
451494
}
495+
496+
/**
497+
* Add the icons css cache in the header if needed
498+
*
499+
* @return boolean true
500+
*/
501+
private function injectCssVariablesIfAny() {
502+
// Inject icons vars css if any
503+
if ($this->iconsCacher->getCachedCSS() && $this->iconsCacher->getCachedCSS()->getSize() > 0) {
504+
$this->iconsCacher->injectCss();
505+
}
506+
return true;
507+
}
452508
}

0 commit comments

Comments
 (0)