Don't pass --allow-shlib-undefined to lld if it's not supported

[Akaricchi]

Fixes builds with llvm-mingw

[dcbaker]

looking at the LLVM code --allow-shlib-undefined has been implemented for ELF (so Linux, *BSD, etc) for a long time, but isn't implemented for PE/COFF. So we should do one of two things here:

1. Push the has_link_argument method into the DynamicLinker class and invoke that instead of opencoding it with Popen.
2. Add the MachineInfo class into the Linker like Remove compiler type #5833 does and do somehting like:

if self.info.is_windows():
    return []
return self._apply_prefix('--allow-shlib-undefined'

[Akaricchi]

I personally prefer the first approach. In general I think it's better to make less assumptions about available features.

I'll see if I can do that tomorrow.

[jpakkane]

ping

[Akaricchi]

Yeah sorry, I got caught up in Taisei stuff. Now that we're done with a new release I'll go take a look into this now.

[Akaricchi]

Looks like this is basically implemented already; DynamicLinker has a has_multi_arguments method. Only problem is that it takes an Environment object, which I don't think is accessible in this context. I see a few other methods like get_soname_args and build_rpath_args take Environment as a parameter; would it be okay to do the same in get_allow_undefined_args? Is there any reason why we don't just instantiate linkers with a permanent reference to an Environment?

EDIT: nevermind the "already implemented" part. On closer inspection, it looks like no linker actually implements has_multi_arguments. I guess that's what @dcbaker was talking about.

[jon-turney]

looking at the LLVM code --allow-shlib-undefined has been implemented for ELF (so Linux, *BSD, etc) for a long time, but isn't implemented for PE/COFF.

I think it cannot be implemented for PE/COFF shared libraries, as they cannot contain undefined symbols (every imported symbol is resolved to a providing library at link time).

So any shared library which actually requires undefined symbols isn't going to be simply portable to PE/COFF (see test cases/common/121 shared module for an example of the required gymnastics)

However, since we don't mark shared libraries as having undefined symbols or not (c.f. libtool -no-undefined), I guess all we can do is not provide this flag where unsupported, and have the build fail on PE/COFF if it's actually needed.

[Akaricchi]

Reviving this since it's still relevant

@dcbaker unfortunately it doesn't seem easy to move the has_link_argument logic into DynamicLinker. It's too tightly coupled with the Compiler class (at least in clike case). What I could do is pass the Compiler object to the linker's get_allow_undefined_args method, and have the linker call the compiler's has_link_argument there. It's a shitty hack, but it would work without forcing me to refactor half the codebase.

Edit: I realized that wouldn't work either, since the compiler doesn't have access to Environment :/

[textshell]

Isn't --allow-shlib-undefined default on lld anyway? So maybe that allows to simplify this.

[dcbaker]

I still don't like calling external binaries when we really do know that in the real work there's just elf, mach-o and pe/coff. But it's not the end of the world and I can fix this later if I really want to.

[xclaesse]

This broke CI, s/typing/T/