https://bugzilla.wikimedia.org/show_bug.cgi?id=64595

--- Comment #17 from Daniel Friesen <[email protected]> ---
((Edit conflicted, but I'll post it anyways))

(In reply to Krinkle from comment #12)
> So removing it is imho not a logical fix and won't solve the actual problem
> (if you'd have a file that has the same name as one of the other import
> paths it would cause the same bug).
MediaWiki has worked fine for a few releases with LESS in place. There is an
underlying bug, but it has existed ever since we introduced LESS support. It
only turned into a regression in 1.23 when Vector was added to the default path
(previously being dedicated solely to mixins that always had .mixins in their
filename). So removing the path won't fix the bug, but it will return us to the
status quo (the same behavior as 1.22) allowing us to release 1.23 (which is
already branched for release as AFAIK under a feature freeze) without a
regression and work on adding proper long-term fixes to the underlying bugs to
1.24.

I mentioned my long-term ideas earlier:
(Daniel Friesen said in comment #4)
> Long-term we'll probably want to fix the issue in multiple ways:
> - Maybe let resource loader module definitions define custom import paths.
> - Fix lessc or our use of it so that the current directory is searched first
> instead of last.

I'm working on the first right now. I also peaked at the second but there are
multiple ways of fixing that, each with tradeoffs that should be evaluated.

> I think there is a bug in how we interpret less that causes the path to be
> appended instead of prepended (like /my:$PATH vs $PATH:/my)

Actually it's not our fault at all, it's lessc's (leafo/lessphp) fault.

Instead of prepending the current directory to the import dir when compiling,
lessc appends it:
https://github.com/leafo/lessphp/blob/2cc77e3c7b9e84cf30ca64fccf2a45a84fbcf0d3/lessc.inc.php#L1918

lessc has only made two changes between the the copy we have in master and now,
and the behavior is still the same.

Also on our end in core we set our include path in a static
ResourceLoader::getLessCompiler() that doesn't know what the path being
compiled is, so there is no one simple workaround.

We can:
A) Fix lessc upstream by making that line use array_unshift. (Assuming they are
responsive enough and don't consider this existing behavior a feature or
something they want to wait before fixing)
B) Patch lessc locally in the same way (this would end up forking and making it
harder to maintain)
C) Make getLessCompiler return a subclass of lessc that messes with its import
dir handling on compile to work around the issue.
D) Explicitly work around the bug in every piece of code that does less
compilation by manually messing with the include path before compile. (doesn't
scale well)
E) Just drop lessc (leafo/lessphp) and switch to oyejorge/less.php, since this
is just one of the many issues in lessc that less.php lacks.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are on the CC list for the bug.
_______________________________________________
Wikibugs-l mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l

Reply via email to