sass-bundle icon indicating copy to clipboard operation
sass-bundle copied to clipboard

Incorrect Sass binary path construction results in repetitive download of the binary

Open FlyingDR opened this issue 1 year ago • 1 comments

The current implementation of the construction of the path for Sass binary relies on the expectation that the main entry point is always named sass:

https://github.com/SymfonyCasts/sass-bundle/blob/aa47da8ae43136a21a764eedb1471f2a168359cb/src/SassBinary.php#L162-L165

But this is not true for Windows, where it is a sass.bat. Effectively it results in a re-download of the Sass distributive for each build due to this logic:

https://github.com/SymfonyCasts/sass-bundle/blob/aa47da8ae43136a21a764eedb1471f2a168359cb/src/SassBinary.php#L37-L40

Correct implementation of the getDefaultBinaryPath method would be:

private function getDefaultBinaryPath(): string
{
    return $this->binaryDownloadDir.'/dart-sass/sass'.(str_contains(strtolower(\PHP_OS), 'win') ? '.bat' : '');
}

There is also one more related and Windows-specific issue:

Normally after download bundle tests that download results in the appearance of the Sass binary and throws an exception if it is not so:

https://github.com/SymfonyCasts/sass-bundle/blob/aa47da8ae43136a21a764eedb1471f2a168359cb/src/SassBinary.php#L109-L112

However, for Windows, no such check is applied because there is a separate branch for archive extraction that ends with unconditional return.

https://github.com/SymfonyCasts/sass-bundle/blob/aa47da8ae43136a21a764eedb1471f2a168359cb/src/SassBinary.php#L87-L97

Because of this, in case if Sass binary download will not cause the Sass compiler to actually appear in the filesystem it will not be reported.

FlyingDR avatar Apr 07 '24 12:04 FlyingDR

Hey @FlyingDR ,

Thank you for reporting these Windows-specific bugs! Would you like to create PRs for them? It would be much appreciated.

bocharsky-bw avatar Apr 11 '24 13:04 bocharsky-bw