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

Build script dies when using npm prefix. #3905

Closed
aaronpowell opened this issue Dec 19, 2018 · 12 comments
Closed

Build script dies when using npm prefix. #3905

aaronpowell opened this issue Dec 19, 2018 · 12 comments

Comments

@aaronpowell
Copy link
Contributor

If you're using a tool to manage multiple versions of node.js on a machine (like ps-nvm, nodist or nvm-windows) it's likely that the build.ps1 script will blow up when trying to run gulp.

The problem is because while the script attempts to create a sandboxed environment to run node.js it doesn't cater to the npm folder prefix.

Instead of doing a global install of gulp it would be better to use npx to run it from the node_modules folder of Belle.

Expect a PR soon.

@aaronpowell
Copy link
Contributor Author

Ok, turns out this is messier than I thought I'm getting a bunch of errors throughout it related to different node modules, the gulpfile, etc.

For instance this commit 704a1ee updated the devDependencies, one of which was gulp-imagemin, but gulp-imagemin has a peerDependence of gulp@4.0.0, which isn't installed (gulp is pinned at 3.9.1). Now I think it survives that, but I'm not 100% sure.

/me keeps on digging

@aaronpowell
Copy link
Contributor Author

Alright, pretty much sorted, but a few things:

  • I've had to roll gulp-imagemin back, otherwise it raises an error (as described above)
  • The version of node.js used in the build script is really quite old and will be going into maintenance old mode 1st Jan 2019. This is a problem because the npx version shipped with it has a bug ("Path must be a string. Received undefined" zkat/npx#144) that writes our a message that PowerShell thinks is an error
    • I'd like to upgrade Umbraco.Build's scripts to use a current Node.js version (10.x), but I can't find the source of that

Because of the 2nd issue I've not created a PR yet, but I have a working branch with my changes, that can be found https://github.com/umbraco/Umbraco-CMS/compare/temp8...aaronpowell:temp8-3905?expand=1

@dawoe
Copy link
Contributor

dawoe commented Dec 20, 2018

Hi @aaronpowell

First of all thanks for raising this issue.

In V7 the node is downloaded by this powershell script : https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/build/Modules/Umbraco.Build/Get-UmbracoBuildEnv.ps1#L97

Is that what you are after ?

Dave Community Pull Request team

@aaronpowell
Copy link
Contributor Author

I haven't looked at v7, only V8 (which is where my branch is from).

Is that script published to the NuGet package V8 uses? If so I'll PR that too.

@aaronpowell
Copy link
Contributor Author

Just had a look at that file @dawoe and no, it's not the same as is used in v8, v8 using the NuGet package Umbraco.Build which is on the MyGet feed (https://www.myget.org/F/umbracocore/api/v3/index.json), but I can't find the source code for it anywhere. @nul800sebastiaan @zpqrtbnk @Shazwazza do you know where the source of that package is?

@zpqrtbnk
Copy link
Contributor

Hint: don't look at v7, the build script is entirely different in v8.

The build tooling is at https://github.com/umbraco/Umbraco-Build but private at the moment - no idea why, looking at opening it today.

As for the versions of Node etc, I remember dealing with them when creating the script but I know little of Node so can totally have messed things up. Welcoming fixes.

@zpqrtbnk
Copy link
Contributor

https://github.com/umbraco/Umbraco-Build is now public - happy hacking!

@aaronpowell
Copy link
Contributor Author

Thanks @zpqrtbnk I'll get on that when I'm sober 😂

@zpqrtbnk
Copy link
Contributor

Umbraco.Build 0.2.0 with latest Node (from your PR) has been pushed to MyGet. Happy to see a PR to build.ps1 that fixes the Node path issue.

@aaronpowell
Copy link
Contributor Author

Ace - I'll test it locally and then send through a PR.

@aaronpowell
Copy link
Contributor Author

PR linked, tested locally using the new Umbraco.Build and it seems all good 😄;

@nul800sebastiaan
Copy link
Member

Fixed in #3938

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

No branches or pull requests

5 participants