already reverted here https://codereview.chromium.org/2732023004/, should land any second

Brad, who owns pnacl these days? Does non-pnacl nacl ld support --build-id? Should I just put the --build-id behind !is_nacl in the gn file?

Derek mentioned to me he believes --build-id should be supported.
Any idea why this is failing Derek?

The issue here is that the pnacl compiler supports --build-id but not --build-id=<type>. So if you want to just omit the sha1 type (it's the default), it should work today (pnacl also does default to using build id even when not specified, so omitting it should work too). I also don't mind updating the PNaCl driver to accept the id type and pass that through to the linker as well if you'd prefer that. 

sha1 is the default in gold but not in e.g. lld.

Updating the pnacl driver seems like the most maintainable thing if you don't mind doing that. What's the latency for that?

I think I could get it done (with the requisite rolls) within a couple of days.

Cool, I'll wait for that then.

What's next here?

PNaCl toolchain roll is blocked on a goma issue (follow the chain of blocking bugs). Hopefully will get that resolved today.

Update: goma issue is resolved. However all NaCl toolchain rolls are currently blocked: for details, see https://bugs.chromium.org/p/chromium/issues/detail?id=701142

Basically we'll have to update the way we build the toolchain to avoid the dependence on new libc, or infra will have to update (or downgrade) some bots.

dschuff: I tried relanding my patch. It still breaks pnacl-ld on Windows: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.chrome%2FGoogle_Chrome_Win%2F16241%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

pnacl-ld: "--build-id=sha1" affects translation. To allow, specify --pnacl-allow-native

I reverted it again for now. Isn't pnacl platform-independent? Why does it complain about this on Windows?

If it complains on Windows, it should complain the same way on the other platforms too. 

What flags are passed to the compile command before and after this patch?

It only failed on windows:

https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Linux%20x64
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Mac
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20ChromeOS

The change is https://codereview.chromium.org/2756713002/diff/1/build/config/compiler/BUILD.gn , the effect is that -Wl,--build-id=sha1 is passed to non-mac posix linkers (which includes pnacl-ld, independent of host platforms, as far as I can tell)

https://bugs.chromium.org/p/chromium/issues/detail?id=702349#c5 -- Looks like Mac and Linux are affected too after all.

You can repro by patching in https://codereview.chromium.org/2756713002/diff/1/build/config/compiler/BUILD.gn  and changing that file so that the flag is added in non-official builds too. Then just building normally should repro.

OK, so it looks like the --build-id flag is being passed to all build, including those that produce portable pexe files (previously it was not). So that error (which isn't new) is basically there because --build-id doesn't really have any meaning for (portable) pexe files, only (native) nexe files. We could just make the compiler driver ignore that flag in that case instead of rejecting it (IIRC there are other flags we do that for). At this point I think I'd be OK with that, it doesn't seem too likely to cause problems for users.

Oh I forgot to mention the other option is just to change the gn files to not pass --build-id when building pexe files, only nexe files. Not sure how much work that would be. Also pnacl-translate (which creates a nexe from a pexe) produces a build ID by default, so no special flags are required.

Does the nacl-clang toolchain (to produce nexes) have a different gn toolchain description from the pnacl toolchain?

Ignoring the flag sounds fine to me. I don't know how pnacl works in the gn build. I could not pass the flag if (is_nacl) which I think will omit it for both nexe and pexe links. It sounds like that's cool too and I might do it if rolling back is blocked for another two weeks, but else making pnacl not err out on the flag seems nicer to me.

The flag is now recognized, but now I get

FAILED: newlib_pnacl/irt_exception_test_newlib_pnacl.pexe newlib_pnacl/exe.unstripped/irt_exception_test_newlib_pnacl.pexe 
python "../../build/toolchain/gcc_link_wrapper.py" --output="newlib_pnacl/irt_exception_test_newlib_pnacl.pexe" --strip="../../native_client/toolchain/linux_x86/pnacl_newlib/bin/pnacl-finalize" --unstripped-file="newlib_pnacl/exe.unstripped/irt_exception_test_newlib_pnacl.pexe" -- ../../native_client/toolchain/linux_x86/pnacl_newlib/bin/pnacl-clang++ -pthread -Wl,--fatal-warnings -Wl,--build-id=sha1 -Werror -o "newlib_pnacl/exe.unstripped/irt_exception_test_newlib_pnacl.pexe" -Wl,--start-group @"newlib_pnacl/irt_exception_test_newlib_pnacl.pexe.rsp"  -Wl,--end-group  -lnacl_exception 

pnacl-ld: "--build-id=sha1" affects translation. To allow, specify --pnacl-allow-native


(as mentioned elsewhere already). Is the plan to fix that nacl-side?

Yes, I'll make a change to ignore the flag when it's not relevant.

nativeclient:4391 is fixed and I tried out the patch from #19 and it seems to work, so I'll go ahead and close this one too.