Support mingw clang in configure scripts.

RESOLVED FIXED in Firefox -esr60

Status

enhancement
RESOLVED FIXED
Last year
11 months ago

People

(Reporter: jacek, Assigned: jacek)

Tracking

(Blocks 1 bug)

60 Branch
mozilla63
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 fixed, firefox63 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

Last year
There are a few minor changes needed for clang:

- clang for mingw targets doesn't support nor need -fPIC
- since clang toolchain has only static libc++, it doesn't need -static option. This option also causes invalid linker command line (which is a clang bug)
- we need -Wno-incompatible-ms-struct to silence a problematic (and inaccurate, I think) warnings/errors
- clang doesn't support -fno-keep-inline-dllexport. We have it for GCC to speed up linking time, build it's not a problem for clang+lld anyway. Linking is pretty fast anyway.
- clang doesn't support -mnop-fun-dllimport which we use for folding libraries on GCC. clang can be made to support folding libraries without any special flags
Comment hidden (mozreview-request)

Comment 2

Last year
mozreview-review
Comment on attachment 8988136 [details]
Bug 1471556 - Support mingw clang in configure scripts.

https://reviewboard.mozilla.org/r/253388/#review260282

::: js/src/old-configure.in:457
(Diff revision 1)
> -    DSO_PIC_CFLAGS='-fPIC'
> +        DSO_PIC_CFLAGS='-fPIC'
> -    ASFLAGS="$ASFLAGS -fPIC"
> +        ASFLAGS="$ASFLAGS -fPIC"

I assume -fPIC is ignored by mingw-gcc? Because otherwise this changes the mingw-gcc build.

::: js/src/old-configure.in:663
(Diff revision 1)
> +            # Use static libgcc and libstdc++
> +            LDFLAGS="$LDFLAGS -static"

I don't see a particular reason we should treat gcc and clang differently for this.
Attachment #8988136 - Flags: review?(mh+mozilla)
Assignee

Comment 3

Last year
Thanks for the review.

(In reply to Mike Hommey [:glandium] from comment #2)
> Comment on attachment 8988136 [details]
> Bug 1471556 - Support mingw clang in configure scripts.
> 
> https://reviewboard.mozilla.org/r/253388/#review260282
> 
> ::: js/src/old-configure.in:457
> (Diff revision 1)
> > -    DSO_PIC_CFLAGS='-fPIC'
> > +        DSO_PIC_CFLAGS='-fPIC'
> > -    ASFLAGS="$ASFLAGS -fPIC"
> > +        ASFLAGS="$ASFLAGS -fPIC"
> 
> I assume -fPIC is ignored by mingw-gcc? Because otherwise this changes the
> mingw-gcc build.

-fPIC is accepted by mingw GCC, but I think it's a noop. All code is position independent. I may change the patch to leave GCC alone anyway.

> ::: js/src/old-configure.in:663
> (Diff revision 1)
> > +            # Use static libgcc and libstdc++
> > +            LDFLAGS="$LDFLAGS -static"
> 
> I don't see a particular reason we should treat gcc and clang differently
> for this.

It doesn't work with current clang/lld. -static option causes clang to pass --start-group/--end-group to linker. In case of mingw target, lld effectively translates that to invoke lld-link (msvc link.exe replacement). Translation of --start-group/--end-group is not yet implemented. It should be probably noop in this case, but it causes error instead.

clang toolchain has currently only static version of libc++, so -static is not really needed anyway.
Comment hidden (mozreview-request)

Comment 5

Last year
mozreview-review
Comment on attachment 8988136 [details]
Bug 1471556 - Support mingw clang in configure scripts.

https://reviewboard.mozilla.org/r/253388/#review261514

::: js/src/old-configure.in:454
(Diff revision 2)
>          fi
>      fi
>      WARNINGS_AS_ERRORS='-Werror'
>      DSO_CFLAGS=''
> +
> +    if test "$OS_ARCH" != "WINNT" -o -z "$CLANG_CC"; then

stopping at OS_ARCH != WINNT should be enough as long as gcc effectively ignores -fPIC, which is a reasonable assumption.
Attachment #8988136 - Flags: review?(mh+mozilla) → review+
Comment hidden (mozreview-request)

Comment 7

Last year
Pushed by jacek@codeweavers.com:
https://hg.mozilla.org/integration/autoland/rev/13a4a0eb572e
Support mingw clang in configure scripts. r=glandium

Comment 8

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/13a4a0eb572e
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63

Comment 9

11 months ago
Comment on attachment 8988136 [details]
Bug 1471556 - Support mingw clang in configure scripts.

[Approval Request Comment]

This is one of several MinGW Build patches I'd like to land in esr60 for Tor. It will prevent them from carrying their own patches for the lifetime of esr60 and will enable us to keep the MinGW build functioning and know if/when/how it was broken by new commits into esr60.

This commit affects only MinGW builds, so it is low risk.
Attachment #8988136 - Flags: approval-mozilla-esr60?
Comment on attachment 8988136 [details]
Bug 1471556 - Support mingw clang in configure scripts.

Makes downstream maintenance easier for Tor. Approved for ESR 60.2.
Attachment #8988136 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
You need to log in before you can comment on or make changes to this bug.