servo / stylo

74 stars 16 forks source link

Install mako on CI #55

Closed mrobinson closed 2 months ago

mrobinson commented 3 months ago

Previous version of main had modifications to the build.rs from upstream that manually added mako to the Python path. That difference from upstream was eliminated in order to reduce the diff we have on top of Gecko's version of Stylo. This means that mako must be available when compiling Stylo on CI.

nicoburns commented 2 months ago

Hmm... this also affects local builds (see https://servo.zulipchat.com/#narrow/stream/263398-general/topic/Build.20Issues/near/453845092 - reproduced locally). I'm inclined to think we ought to re-add the build.rs change (could we formulate it in such a way that we could be upstream it?): it's not usual for a Rust crate to require a python module to be installed globally.

mrobinson commented 2 months ago

Hmm... this also affects local builds (see https://servo.zulipchat.com/#narrow/stream/263398-general/topic/Build.20Issues/near/453845092 - reproduced locally). I'm inclined to think we ought to re-add the build.rs change (could we formulate it in such a way that we could be upstream it?): it's not usual for a Rust crate to require a python module to be installed globally.

The original change was a bit of a hack, because it presupposed that you were building stylo as a part of Servo. I think we should likely figure out how to make this more general.

nicoburns commented 2 months ago

This is also breaking my build if I try to use Stylo as a git dependency in Blitz.