Backport #7830: Ruby packages should not build with -Werror when distributed

• Status: Closed
• Assignee: usa (Usaku NAKAMURA)

[kremenek (Ted Kremenek)]

I represent the Clang compiler team at Apple, and I've heard some complaints that Clang does not work well for building Ruby. Clearly we care that Clang can compile the Ruby sources.

The complaints seemed to have varied over time, but the most recent one that was reported to me seems captured by the following discussion:

https://github.com/plamoni/SiriProxy/issues/436

Here the problem is that a warning is being promoted to an error via -Werror. While -Werror is a great development tool, it is not a great mechanism to enable for distributing packages as sources. Different compilers, and different versions of the same compiler, may issue different warnings, and using -Werror in source packages makes those packages very brittle to new compiler changes. With Xcode releases, we frequently enable new (or existing) warnings by default that have shown to be quite useful to finding problems. Those new warnings typically are great for development, but when coupled with -Werror and packaged sources (that aren't really changing) they can be disastrous.

I do not actively build Ruby myself, but I request that Ruby software packages should not be distributed with -Werror enabled. This will greatly help users installing Ruby via homebrew or macports, and this likely will be an issue on other platforms besides OS X as well.

[kosaki (Motohiro KOSAKI)]

Seems very reasonable request to me.

[luislavena (Luis Lavena)]

kosaki (Motohiro KOSAKI) wrote:

Seems very reasonable request to me.

Kosaki, who should be changing this?

Shall we backport this to 2_0_0 and 1_9_3 too?

[kosaki (Motohiro KOSAKI)]

• File disable-werror.patch disable-werror.patch added

Attached RFC patch.

Usa-san, Nagachika-san, I'd like propose to add --disable-werror new configure option. The intention is, trunk developer continue to use -Werror
for finding a mistake as far as earlier and branch maintainers manually disable -Werror when making tar ball. Is this acceptable?

[nobu (Nobuyoshi Nakada)]

Your patch includes an unintentional change, -Werror-implicit-function-declaration to -Werror=implicit-function-declaration.

[kosaki (Motohiro KOSAKI)]

Your patch includes an unintentional change, -Werror-implicit-function-declaration to -Werror=implicit-function-declaration.

This is intentional. -Werror=implicit-function-declaration and
-Werror-implicit-function-declaration are same meaning and there is no
reason only -Wimplicit-function-declaration keeps exceptional.
Apple made a lot of incompatibility changes in past. We can't believe
they never make the same mistake again.

[nobu (Nobuyoshi Nakada)]

• File disable-werror.patch disable-werror.patch added

It's a different story, isn't it?

[kosaki (Motohiro KOSAKI)]

File disable-werror.patch added

It's a different story, isn't it?

No. -Werror-implicit-function-declaration also make compilation error
when compiler make new code analysis. Why do you think so?

[luislavena (Luis Lavena)]

kosaki (Motohiro KOSAKI) wrote:

Attached RFC patch.

Usa-san, Nagachika-san, I'd like propose to add --disable-werror new configure option. The intention is, trunk developer continue to use -Werror
for finding a mistake as far as earlier and branch maintainers manually disable -Werror when making tar ball. Is this acceptable?

Kosaki, I think the option should be the opposite.

By default, -Werror should be disabled and only enabled when --enable-werror is supplied to configure script, your patch just makes this configurable, it doesn't change the default.

[mpapis (Michal Papis)]

The problem is only with clang, maybe we could make the flag default only for it?

[jonforums (Jon Forums)]

It might not be isolated to clang. IIRC, ~1-2 years ago the curb gem failed to build/install with mingw on windows due to -werror settings embedded in RbConfig::CONFIG['warnflags'].

I share Luis' perspective.

[nobu (Nobuyoshi Nakada)]

kosaki (Motohiro KOSAKI) wrote:

No. -Werror-implicit-function-declaration also make compilation error
when compiler make new code analysis. Why do you think so?

Of course it is same. Unfortunately apple-gcc still doesn't make an
implicit function declaration error by -Werror= option, even if sole
-Werror.

[nobu (Nobuyoshi Nakada)]

I found that -Werror-implicit-function-declaration has never been used.

[kremenek (Ted Kremenek)]

-Werror-implicit-function-declaration isn't really an issue. It is a standard warning with very clear semantics. If Ruby always builds with that warning promoted to an error that's not likely to be an issue in practice.

The problem is when a new compiler (gcc, clang, or whatever) comes out with a new warning that is enabled by default, or changes some existing warning in a way that causes it to trigger in a way it did not before. That will cause the build to break with -Werror.

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r39221.
Ted, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

configure.in: no Werror

• configure.in (warnflags): disable -Werror by default unless
  development. [ruby-core:52131] [Bug #7830]

[usa (Usaku NAKAMURA)]

• Status changed from Closed to Assigned
• Assignee set to nagachika (Tomoyuki Chikanaga)

I can accept current state (r39221) to backport 1.9.3.
It seems to match with kosaki-san's proposal at [ruby-core:52141].
nagachika-san, how do you think about this?

[Hanmac (Hans Mackowiak)]

i get this with some of my gems:
cc1plus: warning: command line option '-Wdeclaration-after-statement' is valid for C/ObjC but not for C++ [enabled by default]
cc1plus: warning: command line option '-Wimplicit-function-declaration' is valid for C/ObjC but not for C++ [enabled by default]

so do i need to take care about that or can ruby do that for me?

[nobu (Nobuyoshi Nakada)]

Please file new ticket.

[nagachika (Tomoyuki Chikanaga)]

r39214 and r39221 seems good to me, and I think personally it could be included in 2.0.0 release.
But off course mame-san has final approval for backporting to ruby_2_0_0 before release 2.0.0.

mame-san, how do you think about this? If you decide not to backport them before release 2.0.0, I'll take care about this after release 2.0.0.

[kosaki (Motohiro KOSAKI)]

-Werror-implicit-function-declaration isn't really an issue. It is a standard warning
with very clear semantics. If Ruby always builds with that warning promoted to an error
that's not likely to be an issue in practice.

The problem is when a new compiler (gcc, clang, or whatever) comes out with a new warning
that is enabled by default, or changes some existing warning in a way that causes it to
trigger in a way it did not before. That will cause the build to break with -Werror.

You don't understand current situation correctly. We didn't and never use -Werror. The problem is,
we used -Werror=shorten-64-to-32 and other some specific -Werror= options and Apple changed the
behavior of -Werror=shorten-64-to-32.

So, your comment completely make no sense.

[kremenek (Ted Kremenek)]

kosaki (Motohiro KOSAKI) wrote:

-Werror-implicit-function-declaration isn't really an issue. It is a standard warning
with very clear semantics. If Ruby always builds with that warning promoted to an error
that's not likely to be an issue in practice.

The problem is when a new compiler (gcc, clang, or whatever) comes out with a new warning
that is enabled by default, or changes some existing warning in a way that causes it to
trigger in a way it did not before. That will cause the build to break with -Werror.

You don't understand current situation correctly. We didn't and never use -Werror. The problem is,
we used -Werror=shorten-64-to-32 and other some specific -Werror= options and Apple changed the
behavior of -Werror=shorten-64-to-32.

So, your comment completely make no sense.

I was simply responding to the comment regarding -Werror-implicit-function-declaration, which was posted before my comment.

My main point was that Ruby building with any flag that promotes a warning to an error for source package distribution is a bad idea. No compiler warning should be treated as being stable output between compiler versions and compiler vendors. There are legitimate reasons for this:

(1) Between compiler versions, the behavior of a warning may change. There may be a case that warning that was accidentally not being emitted before (a compiler bug) now gets emitted in a new version. Moreover, some new warnings may get added under an existing -W flag because that's where they belong.

(2) Different compiler vendors support different warnings, or the same warnings with slightly different behavior. What may warn with one compiler may not with another, etc.

For this reason, I believe that Ruby should not build with any of the following flags for source package distribution:

-Werror
-Werror-*
-Werror=*

Hopefully this comment makes more sense, in the broader context in which it was intended.

[nagachika (Tomoyuki Chikanaga)]

• Tracker changed from Bug to Backport
• Project changed from Ruby master to Backport200

[nagachika (Tomoyuki Chikanaga)]

• Status changed from Assigned to Closed

This issue was solved with changeset r39564.
Ted, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

merge revision(s) 39214,39221: [Backport #7830]

configure.in: Werror-implicit-function-declaration

* configure.in (warnflags): -Werror-implicit-function-declaration

haven't been used as-is, but always replaced with -Werror= or -W.
* configure.in (warnflags): disable -Werror by default unless
development. [ruby-core:52131] [Bug #7830]

[nagachika (Tomoyuki Chikanaga)]

• Project changed from Backport200 to Backport193
• Assignee changed from nagachika (Tomoyuki Chikanaga) to usa (Usaku NAKAMURA)

Is it needed in 1.9.3 too?

[usa (Usaku NAKAMURA)]

• Status changed from Closed to Assigned

[usa (Usaku NAKAMURA)]

• Status changed from Assigned to Closed

This issue was solved with changeset r40742.
Ted, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

merge revision(s) 39214,39221: [Backport #7830]

configure.in: Werror-implicit-function-declaration

* configure.in (warnflags): -Werror-implicit-function-declaration

haven't been used as-is, but always replaced with -Werror= or -W.
* configure.in (warnflags): disable -Werror by default unless
development. [ruby-core:52131] [Bug #7830]