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

Revamp build mechanism #504

Closed
am11 opened this issue Nov 1, 2014 · 4 comments · Fixed by #509
Closed

Revamp build mechanism #504

am11 opened this issue Nov 1, 2014 · 4 comments · Fixed by #509

Comments

@am11
Copy link
Contributor

am11 commented Nov 1, 2014

It involves, lib/build.js and scripts/republish.

Current behavior

For contributor:

When we do npm install, it:

  • downloads dependencies.
  • tests the current binary exits and if it passes few tests, it skips the build process and continues. Otherwise:
  • builds binary and copies into bin/<os-name>-<architecture>/binding.node
  • clones into node-sass-binaries
  • copies binaries of every OS into bin
    • then it overwrites the binary it just built using build.js script..

For consumers (via nodejs.org):

  • downloads dependencies.
  • copies binaries of every OS into bin
  • tests the current binary exists and passes few tests, it skips the build process and continue. Otherwise:
  • builds binary and copies into bin/<os-name>-<architecture>/binding.node

Preferred behavior

For contributor:

It should:

  • download dependencies.
  • test the current binary, if it passes few tests, it skips the build process and continue. Otherwise:
  • build binary and copy into bin/<os-name>-<architecture>/binding.node

For consumer (via nodejs.org):

It should:

  • download dependencies.
  • copy only the required binary into bin and remove others from the disk.
  • test the current binary passes few tests; skip the build process and continue. Otherwise:
  • build binary (if python, C++ compilers and other tools are present) and copy into bin/<os-name>-<architecture>/binding.node
@nschonni
Copy link
Contributor

nschonni commented Nov 1, 2014

copy only the required binary into bin and remove others from the disk.

I get this from the "keep it clean" point of view, but for those that actually check node_modules into their builds would then break for teams working on more than a single OS or platform. I don't think the benefit of a slightly smaller node_modules directory is worth it.

@nschonni
Copy link
Contributor

nschonni commented Nov 1, 2014

We may be able to abuse the GitHub repo (sorry @andrew 👅 ) with https://github.com/mapbox/node-pre-gyp
The difference with that approach is that it never ships with a binary (similar to what @kevva uses in imagemin), which means the entire lookup flow is different

@andrew
Copy link
Contributor

andrew commented Nov 1, 2014

I wonder if @ErisDS can shed some light on how pre-gyp is working for the ghost team?

@ErisDS
Copy link

ErisDS commented Nov 1, 2014

I ❤️ pre-gyp so much. It has unquestionably completely solved the biggest install problem we had with Ghost, the only time people have issues with sqlite3 now is if they're on a system so obscure there's no binary available AND no the system is missing build tools - which is quite rare - and people in that situation usually have the savvy to resolve the problem.

As proof this isn't just hyperbole, check out our installation forum. Go back to the first pages and the majority of issues are with people getting stuck on the npm install step where installing sqlite3 would fail for a number of varied reasons. Take a look at the more recent pages, there are no sqlite3 issues in sight :)

@am11 am11 closed this as completed in #509 Nov 5, 2014
jiongle1 pushed a commit to scantist-ossops-m2/node-sass that referenced this issue Apr 7, 2024
Accept multiple - characters before interpolant
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants