Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generating css from stylus fails on Windows #294

Closed
budziq opened this issue May 22, 2017 · 1 comment · Fixed by #295
Closed

generating css from stylus fails on Windows #294

budziq opened this issue May 22, 2017 · 1 comment · Fixed by #295
Labels
A-Infrastructure Area: CI, Releases C-bug Category: A bug, incorrect or unintended behavior E-Easy Experience: Easy
Milestone

Comments

@budziq
Copy link
Contributor

budziq commented May 22, 2017

Hi,

I wanted to test my code fixing https://github.com/azerupi/mdBook/issues/293
And when testing the solution on Windows I have noticed that
cargo build --features=regenerate-css will always fail (with or without my changes).
It looks like Rust Command PATH finding is is broken rust-lang/rust#37519 or at least works differently than expected.

Namely stylus is represented on Windows as stylus.cmd while Command::spawn will append ".exe" but does not test with other executable extensions like cmd, thus Command::new("stylus") will expand to Command::new("stylus.exe") instead of Command::new("stylus.cmd"). Fortunately explicitly stating .

This should be relatively easy to workaround with #[cfg(windows)]
Nevertheless, the CI probably should be updated to test if stylus works too.

@azerupi azerupi added A-Infrastructure Area: CI, Releases E-Easy Experience: Easy C-bug Category: A bug, incorrect or unintended behavior labels May 22, 2017
@azerupi azerupi added this to the 0.1.0 milestone May 22, 2017
@azerupi
Copy link
Contributor

azerupi commented May 22, 2017

Thank you for the report.
As I have no Windows machine, support for it is sometimes lagging a little behind. I rely very strongly on AppVeyor, and contributions from Windows users for that part.

Nevertheless, the CI probably should be updated to test if stylus works too.

Indeed, we could add the build script to the CI to assure it always works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Infrastructure Area: CI, Releases C-bug Category: A bug, incorrect or unintended behavior E-Easy Experience: Easy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants