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

npm run build fails on Windows #336

Closed
neilsutcliffe opened this issue Dec 20, 2017 · 12 comments
Closed

npm run build fails on Windows #336

neilsutcliffe opened this issue Dec 20, 2017 · 12 comments

Comments

@neilsutcliffe
Copy link
Contributor

neilsutcliffe commented Dec 20, 2017

Is this a bug report?

Yes

The 'npm run build' process does not work on Windows. Specifically we use string concatenation and regex with UNIX specific patterns in generate.js

Have you read the Contributing Guidelines?

Yes

Environment

WINDOWS

Steps to Reproduce

With a working install on Windows

npm run build

Expected Behavior

A nice build file with documents, blogs, pages and static files

Actual Behavior

all 4 fail in different ways.

@neilsutcliffe
Copy link
Contributor Author

This is mostly for my reference, as I seem to be the first to try it on local storage.

I have fixes for this in my massive insane PR as well, but I have streamlined the code at the same time.

#326

@steveklabnik
Copy link

I was seeing this, but wasn't sure that it was a bug. Guess I know now!

@neilsutcliffe
Copy link
Contributor Author

Aye. I think that issues like this can generally be avoided if the 'path' functions are used for anything when working with filePaths.

It results in slightly less readable code, but is guaranteed to be cross platform.

@Deterus
Copy link

Deterus commented Jan 3, 2018

I am eagerly awaiting this PR to be merged. As soon as this goes through were going to convert most of our internal wikis to Docusaurus. Our build servers are windows based, and this is our last major hurdle. Thanks for working on this its much appreciated!

@JoelMarcey
Copy link
Contributor

Hi all - I am back after some holiday and end of year work that I needed to get taken care of.

Since I see this is a popular issue that needs fixing, I will focus on this now.

@neilsutcliffe Hi! It looks like #360 is your PR that would fix this issue, correct?

@neilsutcliffe
Copy link
Contributor Author

#360 Will fix it. It's as the build process uses the incorrect path seperation character. The refactor uses the NPM path tools all over to ensure that it works cross platform.

#360 is a big change, but will only affect the built files. Should be relatively easy to test, although I haven't tested with translated docs or versions myself. Would it be worth creating a release branch?

@JoelMarcey
Copy link
Contributor

Those previous comments that I deleted were made for another repo -- too many tabs open. Sorry.

@JoelMarcey
Copy link
Contributor

@neilsutcliffe Would it be worth creating a smaller commit/PR to fix this particular issue now since #360 may require a bunch of testing?

@neilsutcliffe
Copy link
Contributor Author

I'm gonna say it's not worth it, as the PR is too different from the existing codebase to copy and paste lines. It's a complete refactor.

Any fixes would have to be made from scratch as the code just is not comparable.

@neilsutcliffe
Copy link
Contributor Author

I'm going to try and write some extra tests in #360 for versioning, amongst other things.

The hardest bit of tests are determining behavour.

I wonder if it is possible to write a test for windows path issues on a linux box? Honestly, I'm just glad they both treat extensions the same way.

@JoelMarcey
Copy link
Contributor

See #381 for a hopefully targeted fix to the Windows build problem.

@JoelMarcey
Copy link
Contributor

Fixed by #381.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants