Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1840633002:
tools: fix tickprocessor Cpp symbols on mac (Closed)
|
Created:
4 years, 9 months ago by fedor.indutny Modified:
4 years, 8 months ago CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Descriptiontools: fix tickprocessor Cpp symbols on mac
Despite man page documentation:
-f Display the symbol table of a dynamic library flat (as one
file not separate modules).
`nm` on mac treats `-f` as a shorthand for `-format`. The `-f` argument
does not seem to be required, so just remove it completely.
(For `-format` documentation - see `nm --help` on mac).
BUG=
Committed: https://crrev.com/1383d001ffe1d13fa71192da156b530dd0be779f
Cr-Commit-Position: refs/heads/master@{#35445}
Patch Set 1 #Patch Set 2 : add reviwer? #
Total comments: 4
Messages
Total messages: 17 (6 generated)
Description was changed from
==========
tools: fix tickprocessor Cpp symbols on mac
Despite man page documentation:
-f Display the symbol table of a dynamic library flat (as one
file not separate modules).
`nm` on mac treats `-f` as a shorthand for `-format`. The `-f` argument
does not seem to be required, so just remove it completely.
(For `-format` documentation - see `nm --help` on mac).
BUG=
==========
to
==========
tools: fix tickprocessor Cpp symbols on mac
Despite man page documentation:
-f Display the symbol table of a dynamic library flat (as one
file not separate modules).
`nm` on mac treats `-f` as a shorthand for `-format`. The `-f` argument
does not seem to be required, so just remove it completely.
(For `-format` documentation - see `nm --help` on mac).
BUG=
==========
fedor@indutny.com changed reviewers: + machenbach@chromium.org
ofrobots@google.com changed reviewers: + jkummerow@chromium.org
https://codereview.chromium.org/1840633002/diff/20001/tools/tickprocessor.js File tools/tickprocessor.js (right): https://codereview.chromium.org/1840633002/diff/20001/tools/tickprocessor.js#... tools/tickprocessor.js:762: // "flat" display option flag. It sounds like you were picking up the GNU version of nm, not Apple's. I'd drop the comment and tweak the commit log.
https://codereview.chromium.org/1840633002/diff/20001/tools/tickprocessor.js File tools/tickprocessor.js (right): https://codereview.chromium.org/1840633002/diff/20001/tools/tickprocessor.js#... tools/tickprocessor.js:762: // "flat" display option flag. On 2016/03/28 18:32:24, noordhuis wrote: > It sounds like you were picking up the GNU version of nm, not Apple's. I'd drop > the comment and tweak the commit log. It could be this, but sounds very unlikely considering that `nm --version` says: LLVM (http://llvm.org/): LLVM version 7.3.0 Optimized build. Built Mar 8 2016 (10:26:42). Default target: x86_64-apple-darwin15.4.0 Host CPU: haswell Also, I have just installed xcode tools on my wife's laptop, and it is absolutely the same. Could have been changed with latest updated, hard to tell!
Kindly reminding about this CL.
FYI: I'm a lousy reviewer for this file as I never wrote a single line and need to learn everything from scratch. I can rubberstamp stuff if I'm convinced that it doesn't break for anybody. Could you provide exact repro steps (for dummies) that show the undesired behavior and steps that show the desired behavior after the fix? I'll repro on a mac. I just want to make sure that dropping the -f is not leading to another problem. https://codereview.chromium.org/1840633002/diff/20001/tools/tickprocessor.js File tools/tickprocessor.js (right): https://codereview.chromium.org/1840633002/diff/20001/tools/tickprocessor.js#... tools/tickprocessor.js:762: // "flat" display option flag. On 2016/03/28 21:20:46, fedor.indutny wrote: > On 2016/03/28 18:32:24, noordhuis wrote: > > It sounds like you were picking up the GNU version of nm, not Apple's. I'd > drop > > the comment and tweak the commit log. > > It could be this, but sounds very unlikely considering that `nm --version` says: > > LLVM (http://llvm.org/): > LLVM version 7.3.0 > Optimized build. > Built Mar 8 2016 (10:26:42). > Default target: x86_64-apple-darwin15.4.0 > Host CPU: haswell > > Also, I have just installed xcode tools on my wife's laptop, and it is > absolutely the same. Could have been changed with latest updated, hard to tell! I logged onto a random mac 10.9.5 bot and typed: which nm /usr/bin/nm nm --version error: /Applications/Xcode5.1.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/nm: invalid argument -- Usage: /Applications/Xcode5.1.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/nm [-agnopruUmxjlfAP[s segname sectname] [-] [-t format] [[-arch <arch_flag>] ...] [file ...] The usage output has exactly the same flags as in the description here: https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPag...
> Could you provide exact repro steps (for dummies) that show the undesired > behavior and steps that show the desired behavior after the fix? I'll repro on a > mac. I just want to make sure that dropping the -f is not leading to another > problem. Well, on my mac I have: $ nm -f `which node` nm: for the -format option: Cannot find option named '/Users/findutnyy/.node/5.7.0/bin/node'! While if I will do: $ nm -f bsd `which node` | wc -l 32177 So basically it looks like things are different for nm shipped with Xcode, and nm shipped with command-line tools. Guess your buildbot has env setup for the build with xcode's clang, otherwise I can't see how `nm` could default to that value. https://codereview.chromium.org/1840633002/diff/20001/tools/tickprocessor.js File tools/tickprocessor.js (right): https://codereview.chromium.org/1840633002/diff/20001/tools/tickprocessor.js#... tools/tickprocessor.js:762: // "flat" display option flag. On 2016/04/06 06:53:52, Michael Achenbach wrote: > On 2016/03/28 21:20:46, fedor.indutny wrote: > > On 2016/03/28 18:32:24, noordhuis wrote: > > > It sounds like you were picking up the GNU version of nm, not Apple's. I'd > > drop > > > the comment and tweak the commit log. > > > > It could be this, but sounds very unlikely considering that `nm --version` > says: > > > > LLVM (http://llvm.org/): > > LLVM version 7.3.0 > > Optimized build. > > Built Mar 8 2016 (10:26:42). > > Default target: x86_64-apple-darwin15.4.0 > > Host CPU: haswell > > > > Also, I have just installed xcode tools on my wife's laptop, and it is > > absolutely the same. Could have been changed with latest updated, hard to > tell! > > I logged onto a random mac 10.9.5 bot and typed: > which nm > /usr/bin/nm > > nm --version > error: > /Applications/Xcode5.1.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/nm: > invalid argument -- > Usage: > /Applications/Xcode5.1.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/nm > [-agnopruUmxjlfAP[s segname sectname] [-] [-t format] [[-arch <arch_flag>] ...] > [file ...] > > The usage output has exactly the same flags as in the description here: > https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPag... I guess there are some differences between `nm` supplied by Xcode and /usr/bin/nm . It seems to me that (at least) on the latest OS X versions, `which nm` defaults to `/usr/bin/nm`. Sorry to ask you about this, but since I don't have any machines like this at hand... does the output differ without `-f` flag on that Xcode nm? What is the difference?
> Sorry to ask you about this, but since I don't have any machines like this at > hand... does the output differ without `-f` flag on that Xcode nm? What is the > difference? Hmm, somehow the system nm seems broken on that bot. LGTM to unblock you. In case this bothers anybody else we can revert/adapt again later.
Thank you, Michael!
The CQ bit was checked by fedor@indutny.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1840633002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1840633002/20001
Message was sent while issue was closed.
Description was changed from
==========
tools: fix tickprocessor Cpp symbols on mac
Despite man page documentation:
-f Display the symbol table of a dynamic library flat (as one
file not separate modules).
`nm` on mac treats `-f` as a shorthand for `-format`. The `-f` argument
does not seem to be required, so just remove it completely.
(For `-format` documentation - see `nm --help` on mac).
BUG=
==========
to
==========
tools: fix tickprocessor Cpp symbols on mac
Despite man page documentation:
-f Display the symbol table of a dynamic library flat (as one
file not separate modules).
`nm` on mac treats `-f` as a shorthand for `-format`. The `-f` argument
does not seem to be required, so just remove it completely.
(For `-format` documentation - see `nm --help` on mac).
BUG=
==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from
==========
tools: fix tickprocessor Cpp symbols on mac
Despite man page documentation:
-f Display the symbol table of a dynamic library flat (as one
file not separate modules).
`nm` on mac treats `-f` as a shorthand for `-format`. The `-f` argument
does not seem to be required, so just remove it completely.
(For `-format` documentation - see `nm --help` on mac).
BUG=
==========
to
==========
tools: fix tickprocessor Cpp symbols on mac
Despite man page documentation:
-f Display the symbol table of a dynamic library flat (as one
file not separate modules).
`nm` on mac treats `-f` as a shorthand for `-format`. The `-f` argument
does not seem to be required, so just remove it completely.
(For `-format` documentation - see `nm --help` on mac).
BUG=
Committed: https://crrev.com/1383d001ffe1d13fa71192da156b530dd0be779f
Cr-Commit-Position: refs/heads/master@{#35445}
==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/1383d001ffe1d13fa71192da156b530dd0be779f Cr-Commit-Position: refs/heads/master@{#35445} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
