I noticed my builds on Debian unstable were using BFD ld, and tried to configure with --enable-gold, but: 0:01.20 DEBUG: Executing: `/usr/lib/ccache/gcc -std=gnu99 -print-prog-name=ld.gold` 0:01.20 DEBUG: Executing: `/usr/lib/ccache/gcc -std=gnu99 -B /home/jld/src/obj.gecko-dev/obj-x86_64-pc-linux-gnu/build/unix/gold -Wl,--version` 0:01.20 ERROR: Could not find gold The first print-prog-name command just prints "ld.gold" (if given "ld" it prints an absolute path). The second command, I set up the .../build/unix/gold/ld symlink and ran it manually under strace; GCC doesn't look for ld in that directory, but it does look for collect2 (and doesn't find it); GCC runs collect2 which runs ld. This *does* work with GCC 6.4.0 or with Clang, but not GCC 7.3.0 or a GCC 8 build dated 2018-04-14. Both GCC 6 and 7 pass the -B path to collect2 as a -L option (and nowhere else among its arguments). However, skipping the -B stuff and running gcc with "-fuse-ld=gold" seems to work on all versions.

Sylvestre, do you remember what versions of gcc/clang support -fuse-ld? Since we bumped to gcc 6, maybe we can use -fuse-ld=gold everywhere now?

for clang, it is supported since 3.8. for gcc, at least 4.7 let me propose a patch

Comment on attachment 8971181 [details] Bug 1455767 - As we have gcc 6 as requirement, use -fuse-ld everywhere instead of the -B trick https://reviewboard.mozilla.org/r/239982/#review246612 ::: build/moz.configure/toolchain.configure:1527 (Diff revision 2) > - os.symlink(goldFullPath, os.path.join(targetDir, 'ld')) > > - linker = ['-B', targetDir] > + if (enable_gold_option or developer_options) and linker_name != 'bfd': > - cmd = cmd_base + linker + version_check > if 'GNU gold' in check_cmd_output(*cmd).decode('utf-8'): > # We have detected gold, will build with the -B workaround This comment needs an update. ::: build/moz.configure/toolchain.configure:1571 (Diff revision 2) > extra_toolchain_flags, when=is_linker_option_enabled) > @checking('for linker', lambda x: x.KIND) > def select_linker(linker, c_compiler, developer_options, build_env, toolchain_flags): > linker = linker[0] if linker else 'other' > if linker in ('gold', 'bfd', 'other'): > return enable_gnu_linker(linker == 'gold', c_compiler, developer_options, With all the code you're removing from enable_gnu_linker, it seems it would become simpler if you merged everything in this function.

Comment on attachment 8971181 [details] Bug 1455767 - As we have gcc 6 as requirement, use -fuse-ld everywhere instead of the -B trick https://reviewboard.mozilla.org/r/239982/#review248092 ::: build/moz.configure/toolchain.configure:1528 (Diff revision 3) > + if linker in ('gold', 'bfd', 'other'): > + # Used to check the kind of linker > + linker, cmd = get_linker_fuse_syntax(c_compiler, linker) You're doing: if linker in ('gold', 'bfd', 'other'): linker, cmd = get_linker_fuse_syntax(c_compiler, linker) (...) if linker == 'lld': lld, cmd = get_linker_fuse_syntax(c_compiler, linker) And that covers all the possible values of linker, so, you could move the get_linker_fuse_syntax call before the tests. And even better, inline it.

Comment on attachment 8971181 [details] Bug 1455767 - As we have gcc 6 as requirement, use -fuse-ld everywhere instead of the -B trick https://reviewboard.mozilla.org/r/239982/#review248142 ::: build/moz.configure/toolchain.configure:1516 (Diff revision 4) > - # Used to check the kind of linker > + def get_linker_fuse_syntax(c_compiler, linker): > + # Generate two commands: > + # - The call to the version of the linker to know which one we are actually getting > + # - The call to the compiler with the right syntax to call the linker > - version_check = ['-Wl,--version'] > + version_check = ['-Wl,--version'] > - cmd_base = c_compiler.wrapper + [c_compiler.compiler] + c_compiler.flags > + cmd_base = c_compiler.wrapper + [c_compiler.compiler] + c_compiler.flags > - if toolchain_flags: > - cmd_base += toolchain_flags > + compiler_arg = ["-fuse-ld=" + linker] if linker != "other" else [] > + cmd = cmd_base + compiler_arg + version_check > + return compiler_arg, cmd It seems rather pointless to keep this as a separate function. ::: build/moz.configure/toolchain.configure:1533 (Diff revision 4) > - if os.path.exists(targetDir): > - shutil.rmtree(targetDir) > - os.makedirs(targetDir) > - os.symlink(goldFullPath, os.path.join(targetDir, 'ld')) > + # Used to check the kind of linker > + linker, cmd = get_linker_fuse_syntax(c_compiler, linker) > + if toolchain_flags: > + cmd += toolchain_flags > > - linker = ['-B', targetDir] > + if (linker == 'gold' or developer_options) and linker != 'bfd': when you reach here, you've overwritten linker with ["-fuse-ld=..."], so this can't work. ::: build/moz.configure/toolchain.configure:1566 (Diff revision 4) > - else: > + else: > + if linker == 'lld': elif

Comment on attachment 8971181 [details] Bug 1455767 - As we have gcc 6 as requirement, use -fuse-ld everywhere instead of the -B trick https://reviewboard.mozilla.org/r/239982/#review248092 > You're doing: > > if linker in ('gold', 'bfd', 'other'): > linker, cmd = get_linker_fuse_syntax(c_compiler, linker) > (...) > if linker == 'lld': > lld, cmd = get_linker_fuse_syntax(c_compiler, linker) > > And that covers all the possible values of linker, so, you could move the get_linker_fuse_syntax call before the tests. And even better, inline it. Bien vu! merci beaucoup!

Comment on attachment 8971181 [details] Bug 1455767 - As we have gcc 6 as requirement, use -fuse-ld everywhere instead of the -B trick https://reviewboard.mozilla.org/r/239982/#review248478 I think at some point we should write a test case for all this. ::: build/moz.configure/toolchain.configure:1540 (Diff revision 5) > - elif enable_gold_option: > + if linker == 'gold': > die('Could not find gold') > # Else fallthrough. > > - cmd = cmd_base + version_check > cmd_output = check_cmd_output(*cmd).decode('utf-8') Food for followup: this could move before all the checks, to avoid it running twice when gold is not found in the default case.

Comment on attachment 8971181 [details] Bug 1455767 - As we have gcc 6 as requirement, use -fuse-ld everywhere instead of the -B trick https://reviewboard.mozilla.org/r/239982/#review248478 Do you have examples on how we do that currently? > Food for followup: this could move before all the checks, to avoid it running twice when gold is not found in the default case. Sure, I will open a follow up bug about that.

Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7c7118c12864 As we have gcc 6 as requirement, use -fuse-ld everywhere instead of the -B trick r=glandium

Backed out changeset 7c7118c12864 (bug 1455767) for failures in /python/mozbuild/mozbuild/test/configure/lint.py on a CLOSED TREE Problematic push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7c7118c128642433047df82ec98ca480e0b33176&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified Failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=177621933 Log: https://treeherder.mozilla.org/logviewer.html#?job_id=177621933&repo=autoland&lineNumber=35569 Backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7858e8a22a6835142fc7aab232d1455c5eb8f5ea&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified [task 2018-05-09T09:02:12.560Z] 09:02:12 INFO - ============================= test session starts ============================== [task 2018-05-09T09:02:12.560Z] 09:02:12 INFO - platform linux2 -- Python 2.7.9, pytest-3.1.3, py-1.4.34, pluggy-0.4.0 -- /builds/worker/workspace/build/src/obj-firefox/_virtualenvs/init/bin/python [task 2018-05-09T09:02:12.560Z] 09:02:12 INFO - rootdir: /builds/worker/workspace/build/src/python/mozbuild, inifile: [task 2018-05-09T09:02:12.560Z] 09:02:12 INFO - collecting ... collected 6 items [task 2018-05-09T09:02:12.560Z] 09:02:12 WARNING - ../python/mozbuild/mozbuild/test/configure/lint.py::Lint::test_browser TEST-UNEXPECTED-FAIL [task 2018-05-09T09:02:12.561Z] 09:02:12 WARNING - ../python/mozbuild/mozbuild/test/configure/lint.py::Lint::test_embedding_ios TEST-UNEXPECTED-FAIL [task 2018-05-09T09:02:12.562Z] 09:02:12 WARNING - ../python/mozbuild/mozbuild/test/configure/lint.py::Lint::test_extensions TEST-UNEXPECTED-FAIL [task 2018-05-09T09:02:12.563Z] 09:02:12 WARNING - ../python/mozbuild/mozbuild/test/configure/lint.py::Lint::test_js TEST-UNEXPECTED-FAIL [task 2018-05-09T09:02:12.563Z] 09:02:12 WARNING - ../python/mozbuild/mozbuild/test/configure/lint.py::Lint::test_memory TEST-UNEXPECTED-FAIL [task 2018-05-09T09:02:12.564Z] 09:02:12 WARNING - ../python/mozbuild/mozbuild/test/configure/lint.py::Lint::test_mobile_android TEST-UNEXPECTED-FAIL [task 2018-05-09T09:02:12.565Z] 09:02:12 INFO - =================================== FAILURES ===================================

Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a42df29b0d3d As we have gcc 6 as requirement, use -fuse-ld everywhere instead of the -B trick r=glandium

the error was caused by a parameter being useless (which was true) the interdiff is pretty clear: https://reviewboard.mozilla.org/r/239982/diff/5-6/