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

Error with NPM When Running make install During Contribution Set-Up #79

Closed
syke99 opened this issue May 22, 2022 · 18 comments
Closed

Error with NPM When Running make install During Contribution Set-Up #79

syke99 opened this issue May 22, 2022 · 18 comments
Labels
help wanted We'd love your help!

Comments

@syke99
Copy link
Contributor

syke99 commented May 22, 2022

I've been extremely impressed by and interested in but (great work @matthewmueller!!) and I wanted to begin contributing to an interesting bug that I saw in the issues. So I cloned bud and was following the instructions in Contributing.md, but I ran into an error when running make install.

OS: Windows 10
Go Version: 1.18.2
Node.js Version: 16.14.0

Bud_Make_NPM_Error

@syke99 syke99 changed the title Error with NPM When Running make install During Set-Up Error with NPM When Running make install Contribution During Set-Up May 22, 2022
@matthewmueller
Copy link
Contributor

Thanks for the kind words Quinn! Does running npm standalone work for you?

I'm not a windows user, so there's a good chance I'm doing something wrong. That would be something that would be super helpful to contribute! 😇

@012e
Copy link
Contributor

012e commented May 22, 2022

@matthewmueller Did you use the wrong slashes somewhere in the Makefile. The path is using forward slash (Linux), instead of back slash (Windows).

@syke99
Copy link
Contributor Author

syke99 commented May 22, 2022

You're super welcome!! Keep up the great work!! And yes, running npm works as expected. I also updated npm itself to 8.10.0, but I'm still getting that error 😢

@syke99
Copy link
Contributor Author

syke99 commented May 22, 2022

Ahh yeah, I forgot about that. Thanks @012e. That's most likely the issue @matthewmueller. I'd put in a PR, but you're probably much more familiar with Makefiles than I am. But if you want, I'll give it a shot!!

@012e
Copy link
Contributor

012e commented May 22, 2022

@syke99 Can you try changing this line in the Makefile

bud/Makefile

Line 12 in cf2a242

(cd livebud && npm install)

into

cd livebud 
npm install

@syke99
Copy link
Contributor Author

syke99 commented May 22, 2022

@012e yeah, let me give that a shot!

Edit: It worked perfectly!! Thanks you!! 😄

@012e
Copy link
Contributor

012e commented May 22, 2022

Congrats🎉!

Looks like that's a problem with your make binary. It somehow figured out npm's path but using the wrong slash.

@syke99
Copy link
Contributor Author

syke99 commented May 22, 2022

Yeah, I used Chocolatey to download and install make on Windows. So that's probably the issue. I also saw in the Makefile that v8go isn't supported yet, so that will be an issue. But I'm still pressing forward because I'm going to compile a list of changes that will be needed to support Windows development, (some of which can already be made and shouldn't break anything and I can test that through one of my WSL2 setups) and then submit them individually as either issues (if they can already be implemented), suggestions for future improvements (such as implementing whatever else is needed once v8go support is added to Windows), or PRs for additions to the README for Windows development, such as the need for the gcc compiler.

@matthewmueller
Copy link
Contributor

matthewmueller commented May 22, 2022

Accepting PRs! One reason I used (cd livebud && npm install) was to make sure the next command isn't inside livebud/, is there a way to do that in a Makefile on Windows? Otherwise, I guess we can do cd .. after.

@syke99
Copy link
Contributor Author

syke99 commented May 22, 2022

I made one small change in package/socket/child.go that's specific to Windows development that's ready to be merged in. But should I create another branch, or do you want me to fork bud and submit a PR that way? (Something that should probably be added to Contributing.md 😉 )

Also, I'm not sure about if there's a way to do that in a Makefile as I'm not very experienced with them at all. For now, I'm going to submit a PR to check if the OS type is Windows and if so, run

cd livebud
npm install
cd ..

else just run (cd livebud && npm install)

I've also got a small list of steps for setting up the gcc compiler for Windows, but I'm not sure how you wanted that addition (to the README or making another markdown file?) to be added since Windows development wont be possible until support for v8go is a thing.

@syke99
Copy link
Contributor Author

syke99 commented May 22, 2022

Also, it looks like Windows support of v8go is completed @matthewmueller could you possibly enlighten me on what v8go is and how's it's being implemented? I see that it's looking for, but not finding, -lv8. But I haven't looked through the source code enough to see where this is being implemented, so thought maybe you'd have a good idea of what the issue may be 😅

@syke99
Copy link
Contributor Author

syke99 commented May 22, 2022

I've opened this PR for this issue. I'll make a discussion about Windows development support to discuss the other issues 😋

@syke99 syke99 changed the title Error with NPM When Running make install Contribution During Set-Up Error with NPM When Running make install During Contribution Set-Up May 22, 2022
@matthewmueller matthewmueller added the help wanted We'd love your help! label May 23, 2022
@syke99
Copy link
Contributor Author

syke99 commented May 23, 2022

This PR was closed due to fork tying commits to other forks for some reason. It has been replaced with this PR instead.

@012e
Copy link
Contributor

012e commented May 24, 2022

Chocolatey's make is kinda outdated. We are not the first ones getting this error (see the comment section). Can you try some other like MinGW's?

@syke99
Copy link
Contributor Author

syke99 commented May 24, 2022

Chocolatey's make is kinda

Yeah, I got MinGW’s make to work and it’s added to my list of steps for setting up Windows development that I’m gonna but in a markdown file and put up a PR for whenever my other PR gets approved so I can make another fork

@012e
Copy link
Contributor

012e commented May 24, 2022

Uh, @syke99 you can just make a new branch to create PRs, doesn't need to be main.

@syke99
Copy link
Contributor Author

syke99 commented May 24, 2022

🤦‍♀️ I honestly can't believe I didn't even realize that... wow I feel dumb lol. But regardless, I think we're going to abandon native Windows development support and just make a note for Windows users to use WSL/WSL2. developing with bud on WSL/WSL2 works just fine and there would be a REALLY heavy lift in getting native Windows development supported, specifically whenever it comes to passing around file descriptors/handlers through processes, as well and needing to work in tandem with the v8go team to get Windows supported once again. And honestly, while that would be a huge contribution to both v8go and bud, at this time, it doesn't really make sense, at least in the context of bud, to attempt that lift.

@matthewmueller
Copy link
Contributor

matthewmueller commented Jun 3, 2022

Closing since we're already tracking in the Window Support discussion: #81

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted We'd love your help!
Projects
None yet
Development

No branches or pull requests

3 participants