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

fix: Make postinstall script work on Windows #364

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AtkinsSJ
Copy link
Collaborator

@AtkinsSJ AtkinsSJ commented May 1, 2024

Split each cd call into one per directory so that we don't have any directory separators, because separators are different on Windows than Mac/Linux.

This is intended to fix the issue in #362 but I'm waiting for confirmation on that.

@marco00petrucci
Copy link

I replaced the line 30 as you said from:
"postinstall": "cd packages/phoenix && cd packages/contextlink && npm install && cd - && cd packages/strataparse && npm install && cd - && cd packages/pty && npm install"
to:
"postinstall": "cd packages && cd phoenix && cd packages && cd contextlink && npm install && cd - && cd strataparse && npm install && cd - && cd pty && npm install"

but it seems it doesn't work.
I've typed npm start and it says:
image

Thank you a lot for the answer! ♥️

@AtkinsSJ
Copy link
Collaborator Author

AtkinsSJ commented May 1, 2024

Thanks for trying it out, did you run npm install after changing the line? I should have mentioned that.

@marco00petrucci
Copy link

marco00petrucci commented May 1, 2024

Yes, I made it:
image

Split each `cd` call into one per directory so that we don't have any
directory separators, because separators are different on Windows than
Mac/Linux.

Also, Windows' cd doesn't support `cd -` as going back to the last
directory. Now that we navigate one dir at a time, we can just use
`cd ..` instead.
@AtkinsSJ
Copy link
Collaborator Author

AtkinsSJ commented May 2, 2024

Ah... I think cd - isn't a thing on Windows. I've changed it again, hopefully it'll work now. Can you try applying my changes here and then running npm install and npm start again?

@marco00petrucci
Copy link

Now npm install worked.
But if I make a npm start it says:

"export" is not recognized as an internal or external command, operable program or batch file

image

@AtkinsSJ
Copy link
Collaborator Author

AtkinsSJ commented May 2, 2024

Thanks again. Seems like making it work on Windows is going to be more work than I anticipated. 😅 Until then, you could try using the Docker build. Setting up Docker on Windows might not be straightforward depending on your system though. (Link)

@marco00petrucci
Copy link

Thank you for now, I really appreciate your help.
I already tried using Docker but it gives me an error:
image
Can you help me with it please? Thank again.

@AtkinsSJ
Copy link
Collaborator Author

AtkinsSJ commented May 2, 2024

Oh whoops, I forgot our instructions for that assume Linux. I don't actually use Docker so I can't help you unfortunately.

@o0101
Copy link
Contributor

o0101 commented May 2, 2024

Hey @AtkinsSJ not sure if it's related, but to get my postinstall scripts to work on windows for BrowserBox I needed to do this:

    "postinstall": "node exec.js \"./scripts/postinstall.sh\"",

Seems tricky at first, and looked like overkill to me when I first thought of it but worked really well and no more windows problems.

The issue is due to paths, and others have tripped over this with npm basically saying they will never fix this because people shouldn't be using windows or sth - that issue thread was a real doozy but I can't find it now haha 🤣

@AtkinsSJ AtkinsSJ marked this pull request as draft May 3, 2024 16:21
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

Successfully merging this pull request may close these issues.

None yet

3 participants