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: mac OBJ_DIR should point to obj.target #11857

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Mar 15, 2017

I think there might be an issue with the value of OBJ_DIR when
using a "mac" os. The value is currently specified in common.gypi
which is included by node.gyp:

'OBJ_DIR': '<(PRODUCT_DIR)/obj',

In the generated Makefile (out/Makefile) the object output directory
is:

obj := $(builddir)/obj

And in the included node.target.mk we have the OBJS declared:

OBJS := \
         $(obj).target/$(TARGET)/src/async-wrap.o \
         $(obj).target/$(TARGET)/src/cares_wrap.o \

If OBJ_DIR is used in node.gyp to point to generated object files
on mac they will not be found.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build

I think there might be an issue with the value of OBJ_DIR when
using a "mac" os. The value is currently specified in common.gypi
which is included by node.gyp:
'OBJ_DIR': '<(PRODUCT_DIR)/obj',

In the generated Makefile (out/Makefile) the object output directory
is:
obj := $(builddir)/obj

And in the included node.target.mk we have the OBJS declared:
OBJS := \
         $(obj).target/$(TARGET)/src/async-wrap.o \
         $(obj).target/$(TARGET)/src/cares_wrap.o \

If OBJ_DIR is used in node.gyp to point to generated object files
on mac they will not be found.
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Mar 15, 2017
@danbev
Copy link
Contributor Author

danbev commented Mar 15, 2017

danbev added a commit to danbev/node that referenced this pull request Mar 17, 2017
I think there might be an issue with the value of OBJ_DIR when
using a "mac" os. The value is currently specified in common.gypi
which is included by node.gyp:
'OBJ_DIR': '<(PRODUCT_DIR)/obj',

In the generated Makefile (out/Makefile) the object output directory
is:
obj := $(builddir)/obj

And in the included node.target.mk we have the OBJS declared:
OBJS := \
         $(obj).target/$(TARGET)/src/async-wrap.o \
         $(obj).target/$(TARGET)/src/cares_wrap.o \

If OBJ_DIR is used in node.gyp to point to generated object files
on mac they will not be found.

PR-URL: nodejs#11857
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@danbev
Copy link
Contributor Author

danbev commented Mar 17, 2017

Landed in f0d4237

@danbev danbev closed this Mar 17, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Mar 20, 2017
I think there might be an issue with the value of OBJ_DIR when
using a "mac" os. The value is currently specified in common.gypi
which is included by node.gyp:
'OBJ_DIR': '<(PRODUCT_DIR)/obj',

In the generated Makefile (out/Makefile) the object output directory
is:
obj := $(builddir)/obj

And in the included node.target.mk we have the OBJS declared:
OBJS := \
         $(obj).target/$(TARGET)/src/async-wrap.o \
         $(obj).target/$(TARGET)/src/cares_wrap.o \

If OBJ_DIR is used in node.gyp to point to generated object files
on mac they will not be found.

PR-URL: nodejs#11857
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
I think there might be an issue with the value of OBJ_DIR when
using a "mac" os. The value is currently specified in common.gypi
which is included by node.gyp:
'OBJ_DIR': '<(PRODUCT_DIR)/obj',

In the generated Makefile (out/Makefile) the object output directory
is:
obj := $(builddir)/obj

And in the included node.target.mk we have the OBJS declared:
OBJS := \
         $(obj).target/$(TARGET)/src/async-wrap.o \
         $(obj).target/$(TARGET)/src/cares_wrap.o \

If OBJ_DIR is used in node.gyp to point to generated object files
on mac they will not be found.

PR-URL: nodejs#11857
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

should this land on v6.x?

@danbev danbev deleted the common.gypi-mac-OBJ_DIR branch April 20, 2017 07:52
@MylesBorins
Copy link
Contributor

ping

@danbev
Copy link
Contributor Author

danbev commented May 8, 2017

Sorry about missing this one. I'll take a look at this tomorrow.

@danbev
Copy link
Contributor Author

danbev commented May 9, 2017

@MylesBorins Sorry about the delay on this. I don't think this is necessary to backport. It would not hurt to do so, but this was added when we were working on how to get the cctest to use object files. That is not done in the v6.x-staging branch which is why I think this one can be skipped.

@gibfahn
Copy link
Member

gibfahn commented May 9, 2017

So we'd want this once/if #11956 gets backported? @danbev I note that's backport-requested, any chance you could raise a backport for it?

@danbev
Copy link
Contributor Author

danbev commented May 9, 2017

@gibfahn Oh I was not aware of that. I'll take a look and raise a PR for that backport and a backport for this. @MylesBorins Ignore my earlier comment :)

@MylesBorins MylesBorins added the baking-for-lts PRs that need to wait before landing in a LTS release. label May 15, 2017
@refack refack added the embedding Issues and PRs related to embedding Node.js in another project. label Aug 17, 2017
@MylesBorins MylesBorins removed the baking-for-lts PRs that need to wait before landing in a LTS release. label Aug 17, 2018
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. embedding Issues and PRs related to embedding Node.js in another project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants