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: add a OFFLINE var to Makefile #16686

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Nov 2, 2017

We have a build environment that restricts our internet connection and
currently building fails due to the lint-md target needs to be able to
install npm modules.

This commit suggests adding a OFFLINE variable to the Makefile which
can be used like in the following examples:

$ env OFFLINE=true make -j8 test
$ make -j8 test OFFLINE=true
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Nov 2, 2017
@gibfahn
Copy link
Member

gibfahn commented Nov 2, 2017

All the other flags in the Makefile appear to be ALL_CAPS, so for consistency, how about OFFLINE=true?

General idea seems like a good idea to me though.

Putting a line at the top would probably be good for visibility too.

node/Makefile

Lines 3 to 16 in 841e305

BUILDTYPE ?= Release
PYTHON ?= python
DESTDIR ?=
SIGN ?=
PREFIX ?= /usr/local
FLAKY_TESTS ?= run
TEST_CI_ARGS ?=
STAGINGSERVER ?= node-www
LOGLEVEL ?= silent
OSTYPE := $(shell uname -s | tr '[A-Z]' '[a-z]')
COVTESTS ?= test-cov
GTEST_FILTER ?= "*"
GNUMAKEFLAGS += --no-print-directory
GCOV ?= gcov

cc/ @nodejs/build

@gibfahn
Copy link
Member

gibfahn commented Nov 2, 2017

Out of interest how do you get node onto your machines? Seems like the ideal solution would be to have a git clone && make fetch-all or something, that downloads everything you need. Allows us to keep the git repo smaller, but still make it easy for people to download everything.

@danbev
Copy link
Contributor Author

danbev commented Nov 2, 2017

All the other flags in the Makefile appear to be ALL_CAPS, so for consistency, how about OFFLINE=true?

Yeah that makes sense and I like OFFLINE which sounds much better. I'll update, thanks!

Out of interest how do you get node onto your machines?

It is placed on the machine by an internal system that packages node as a tar.gz file.

@gibfahn
Copy link
Member

gibfahn commented Nov 2, 2017

It is placed on the machine by an internal system that packages node as a tar.gz file.

So it does a git clone then tars it up? In that case a make fetch-all type command would solve the issue for you right?

@danbev
Copy link
Contributor Author

danbev commented Nov 2, 2017

So it does a git clone then tars it up? In that case a make fetch-all type command would solve the issue for you right?

The machine that does this is internal and has the same restrictions I'm afraid.

@danbev danbev changed the title build: add a no-internet var to Makefile build: add a OFFLINE var to Makefile Nov 2, 2017
@gibfahn
Copy link
Member

gibfahn commented Nov 2, 2017

The machine that does this is internal and has the same restrictions I'm afraid.

So is there a firewall exception for https://github.com/nodejs/node ?

@danbev
Copy link
Contributor Author

danbev commented Nov 2, 2017

So is there a firewall exception for https://github.com/nodejs/node ?

I don't know how it is configured, but there is no way to access anything external to this environment, github or anything else.

@richardlau
Copy link
Member

I think #16635 will stop the lint-md target from needing to install modules (since they'll be there already in the source tree).

@danbev
Copy link
Contributor Author

danbev commented Nov 2, 2017

I think #16635 will stop the lint-md target from needing to install modules (since they'll be there already in the source tree).

Nice, I was not aware of that. Thanks @gibfahn for feedback on this. I'll close this once #16635 lands.

@joyeecheung
Copy link
Member

By the way, does that build machine need to build docs and run linters? If not, then it can just do make test-only, which only build the binary and run the tests.

@danbev
Copy link
Contributor Author

danbev commented Nov 3, 2017

By the way, does that build machine need to build docs and run linters?

Thanks for the suggestion but we also have to build the docs, but running the linters would be alright to skip.

We have a build environment that restricts our internet connection and
currently building fails due to the lint-md target needs to be able to
install npm modules.

This commit suggests adding a OFFLINE variable to the Makefile which
can be used like in the following examples:

$ env OFFLINE=true make -j8 test
$ make -j8 test OFFLINE=true
@danbev danbev force-pushed the no-internet-lint-md branch from e1b025c to c6ffc7a Compare November 3, 2017 05:28
@danbev
Copy link
Contributor Author

danbev commented Nov 14, 2017

Closing as this PR is out of date.

@danbev danbev closed this Nov 14, 2017
@danbev danbev deleted the no-internet-lint-md branch November 16, 2017 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants