tools: update tooling to work with new macOS CLI #21173

@@ -667,7 +667,7 @@ def get_llvm_version(cc):

 def get_xcode_version(cc):
  return get_version_helper(
-    cc, r"(^Apple LLVM version) ([5-9]\.[0-9]+)")
+    cc, r"(^Apple LLVM version) ([1-9]?[0-9]+\.[0-9]+)")


Isn't the [1-9]? part redundant? Was the intention to use 2 digits? If so it should be [1-9]?[0-9] without the +, I guess.

Trott · Jun 6, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/15293/

lpinca · Jun 6, 2018

lpinca approved these changes Jun 6, 2018

View changes

Trott · Jun 7, 2018

Hmmm....make succeeds with this but not make test-addons....

Trott · Jun 7, 2018

$ make test-addons
/Library/Developer/CommandLineTools/usr/bin/make -C out BUILDTYPE=Release V=1
  touch c6d5e3a8cbc4c6925abc7773ea7a8b5d20bd67b3.intermediate
  LD_LIBRARY_PATH=/Users/trott/io.js/out/Release/lib.host:/Users/trott/io.js/out/Release/lib.target:$LD_LIBRARY_PATH; export LD_LIBRARY_PATH; cd ../.; mkdir -p /Users/trott/io.js/out/Release/obj/gen/src/node/inspector/protocol; python deps/v8/third_party/inspector_protocol/CodeGenerator.py --jinja_dir deps/v8/third_party/inspector_protocol/.. --output_base "/Users/trott/io.js/out/Release/obj/gen/src/" --config "/Users/trott/io.js/out/Release/obj/gen/node_protocol_config.json"
  touch bca5cfa02b0b2280915e4521c79df9165beadac7.intermediate
  LD_LIBRARY_PATH=/Users/trott/io.js/out/Release/lib.host:/Users/trott/io.js/out/Release/lib.target:$LD_LIBRARY_PATH; export LD_LIBRARY_PATH; cd ../deps/v8/gypfiles; mkdir -p /Users/trott/io.js/out/Release/obj/gen/src/inspector/protocol /Users/trott/io.js/out/Release/obj/gen/include/inspector; python ../third_party/inspector_protocol/CodeGenerator.py --jinja_dir ../third_party --output_base "/Users/trott/io.js/out/Release/obj/gen/src/inspector" --config ../src/inspector/inspector_protocol_config.json
rm bca5cfa02b0b2280915e4521c79df9165beadac7.intermediate c6d5e3a8cbc4c6925abc7773ea7a8b5d20bd67b3.intermediate
if [ ! -r node -o ! -L node ]; then ln -fs out/Release/node node; fi

Building addon /Users/trott/io.js/test/addons/01_function_arguments/
xcode-select: error: tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance

xcode-select: error: tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance

Traceback (most recent call last):
  File "/Users/trott/io.js/deps/npm/node_modules/node-gyp/gyp/gyp_main.py", line 16, in <module>
    sys.exit(gyp.script_main())
  File "/Users/trott/io.js/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/__init__.py", line 545, in script_main
    return main(sys.argv[1:])
  File "/Users/trott/io.js/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/__init__.py", line 538, in main
    return gyp_main(args)
  File "/Users/trott/io.js/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/__init__.py", line 523, in gyp_main
    generator.GenerateOutput(flat_list, targets, data, params)
  File "/Users/trott/io.js/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/generator/make.py", line 2170, in GenerateOutput
    part_of_all=qualified_target in needed_targets)
  File "/Users/trott/io.js/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/generator/make.py", line 795, in Write
    self.Pchify))
  File "/Users/trott/io.js/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/generator/make.py", line 1190, in WriteSources
    cflags = self.xcode_settings.GetCflags(configname)
  File "/Users/trott/io.js/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py", line 551, in GetCflags
    archs = self.GetActiveArchs(self.configname)
  File "/Users/trott/io.js/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py", line 420, in GetActiveArchs
    xcode_archs_default = GetXcodeArchsDefault()
  File "/Users/trott/io.js/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py", line 118, in GetXcodeArchsDefault
    xcode_version, _ = XcodeVersion()
  File "/Users/trott/io.js/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py", line 1265, in XcodeVersion
    version = re.match(r'(\d\.\d\.?\d*)', version).groups()[0]
AttributeError: 'NoneType' object has no attribute 'groups'
make: *** [test/addons/.buildstamp] Error 1

Trott · Jun 7, 2018

Argh...the bad regexp is inside deps/npm/node_modules/node-gyp so it will have to be fixed upstream.

Trott · Jun 7, 2018

Copying xcode_emulation.py in this PR to the one in the deps/npm/node_modules fixes things. I'll submit a PR upstream and mark this as blocked on that (although I suppose it's not really since it's not broken any worse without that--still doesn't build either way).

Trott · Jun 7, 2018

Looks like someone already has a PR open for it, as @richardlau noted above: nodejs/node-gyp#1455

Trott · Jun 7, 2018

Hmmm...since it's checked into the repo, the version in deps/npm/node_modules/node-gyp can be updated here, I suppose, but that's probably not the "right" thing to do, especially as there are more changes than just the one line.

Trott · Jun 8, 2018

We can unblock this as node-gyp 3.6.3 has the fix now.

richardlau · Jun 8, 2018

We can unblock this as node-gyp 3.6.3 has the fix now.

Are we waiting for npm to update to node-gyp 3.6.3 or float a patch ourselves via this PR?

Trott · Jun 8, 2018

So, now that there's a new node-gyp that fixes build issues on macOS and smartOS, what's the correct way to update it in the tools directory and the deps/npm directory? @nodejs/npm @bnoordhuis

Trott · Jun 8, 2018

Are we waiting for npm to update to node-gyp 3.6.3 or float a patch ourselves via this PR?

I'd prefer to float a patch to get things fixed right away for folks here. Not sure the right way to update the tools/gyp directory though.

mistydemeo · Jun 13, 2018

@Trott It looks like this patch didn't make it into v10.x, and hence into 10.4.1, unlike the other two Xcode 10-related patches. Any chance you could backport this into any branches that are still getting releases?

Trott · Jun 13, 2018

@Trott It looks like this patch didn't make it into v10.x, and hence into 10.4.1, unlike the other two Xcode 10-related patches. Any chance you could backport this into any branches that are still getting releases?

Argh! Yeah, the other three patches made it but not this one. I was wondering why I was having trouble building on 10.x.

Trott · Jun 13, 2018

The patch is on v10.x-staging so it should be in the next v10.x release. It looks like it only landed on the staging branch yesterday which is probably why it wasn't in the release this week.

Trott added build macos labels Jun 6, 2018

nodejs-github-bot added build tools labels Jun 6, 2018

Trott force-pushed the Trott:fix-configure branch from cda8ff4 to 7f4dba0 Jun 6, 2018

This was referenced Jun 6, 2018

Closed

build: fix version check for xcode 10 #21166

Closed

configure: clang version is misdetected for double-digit clang versions #21175

mistydemeo referenced this pull request Jun 6, 2018
Closed
configure: fix comparing double-digit versions #21183
2 of 2 tasks complete

Trott added the blocked label Jun 7, 2018

Trott force-pushed the Trott:fix-configure branch from 56d0213 to ade91c9 Jun 7, 2018

apapirovski referenced this pull request Jun 7, 2018
Closed
Fix configure script to work with Apple Clang 10 #21200
2 of 2 tasks complete

Trott removed the blocked label Jun 8, 2018

targos referenced this pull request Jun 19, 2018
Merged
v10.5.0 proposal #21400

This was referenced Jun 20, 2018

Closed

2018-06-20 Version 10.5.0 (Current) @targos beevelop/docker-android-nodejs#216

Closed

2018-06-20 Version 10.5.0 (Current) @targos beevelop/docker-nodejs#201

MylesBorins referenced this pull request Jul 9, 2018
Merged
v8.12.0 proposal #21593

nodejs/node

Join GitHub today

tools: update tooling to work with new macOS CLI #21173

Conversation

Trott commented Jun 6, 2018 • edited

Checklist

Trott added build macos labels Jun 6, 2018

nodejs-github-bot added build tools labels Jun 6, 2018

This comment has been minimized.

Trott commented Jun 6, 2018

Trott force-pushed the Trott:fix-configure branch from cda8ff4 to 7f4dba0 Jun 6, 2018

This comment has been minimized.

Trott commented Jun 6, 2018

This comment has been minimized.

richardlau commented Jun 6, 2018

richardlau approved these changes Jun 6, 2018 View changes

richardlau left a comment

This comment has been minimized.

Trott commented Jun 6, 2018

lpinca reviewed Jun 6, 2018 View changes

This comment has been minimized.

lpinca Jun 6, 2018 • edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Trott commented Jun 6, 2018

lpinca approved these changes Jun 6, 2018 View changes

This was referenced Jun 6, 2018

mistydemeo referenced this pull request Jun 6, 2018

This comment has been minimized.

Trott commented Jun 7, 2018

This comment has been minimized.

Trott commented Jun 7, 2018

This comment has been minimized.

Trott commented Jun 7, 2018

This comment has been minimized.

Trott commented Jun 7, 2018

This comment has been minimized.

Trott commented Jun 7, 2018

Trott added the blocked label Jun 7, 2018

Trott force-pushed the Trott:fix-configure branch from 56d0213 to ade91c9 Jun 7, 2018

This comment has been minimized.

Trott commented Jun 7, 2018

apapirovski referenced this pull request Jun 7, 2018

This comment has been minimized.

Trott commented Jun 8, 2018

Trott removed the blocked label Jun 8, 2018

This comment has been minimized.

richardlau commented Jun 8, 2018

This comment has been minimized.

Trott commented Jun 8, 2018

This comment has been minimized.

Trott commented Jun 8, 2018

shisama added a commit to shisama/node that referenced this pull request Jun 12, 2018

This comment has been minimized.

mistydemeo commented Jun 13, 2018

This comment has been minimized.

Trott commented Jun 13, 2018

This comment has been minimized.

Trott commented Jun 13, 2018

Trott added a commit to Trott/io.js that referenced this pull request Jun 13, 2018

Trott added a commit to Trott/io.js that referenced this pull request Jun 13, 2018

targos added a commit that referenced this pull request Jun 13, 2018

MylesBorins added a commit that referenced this pull request Jun 14, 2018

MylesBorins added a commit that referenced this pull request Jun 14, 2018

shisama added a commit to shisama/node that referenced this pull request Jun 17, 2018

targos referenced this pull request Jun 19, 2018

This was referenced Jun 20, 2018

calebeby added a commit to Pigmice2733/scouting-frontend that referenced this pull request Jun 20, 2018

This was referenced Jun 20, 2018

MylesBorins referenced this pull request Jul 9, 2018

rvagg added a commit that referenced this pull request Aug 16, 2018

rvagg added a commit that referenced this pull request Aug 16, 2018

rvagg added a commit that referenced this pull request Nov 24, 2018

rvagg added a commit that referenced this pull request Nov 24, 2018

rvagg added a commit that referenced this pull request Nov 28, 2018

MylesBorins added a commit that referenced this pull request Dec 26, 2018

rvagg added a commit that referenced this pull request Feb 28, 2019

Trott commented Jun 6, 2018 •

edited

Trott force-pushed the Trott:fix-configure branch from `cda8ff4` to `7f4dba0` Jun 6, 2018

richardlau approved these changes Jun 6, 2018

View changes

lpinca reviewed Jun 6, 2018

View changes

lpinca Jun 6, 2018 •

edited

lpinca approved these changes Jun 6, 2018

View changes

Trott force-pushed the Trott:fix-configure branch from `56d0213` to `ade91c9` Jun 7, 2018