1381955 - Respect RUSTFLAGS after Bug 1380381

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Tom Prince [:tomprince]]

Bug 1380381 inadvertently overrides RUSTFLAGS everywhere to just specify the debuginfo level.

Since Thunderbird needs supply a debuginfo for the same reason as that bug for native OSX builds, we need support passing RUSTFLAGS from mozconfig again.

Comment 1

[Tom Prince [:tomprince]]

Attachment: Bug 1381955 - Don't override RUSTFLAGS to set rust's debuginfo;

Review commit: https://reviewboard.mozilla.org/r/158500/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/158500/

Comment 2

[Tom Prince [:tomprince]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=55597d0cb0bc7d00a27bfd887d5d34363e7b3d34

Comment 3

[Mike Hommey [:glandium]]

In fact, this is worse than that, bug 1380381 also overrides the flags set in config/rules.mk itself, for e.g. non-optimized builds.

Mike, can you fix this?

Comment 4

[Tom Prince [:tomprince]]

> Mike, can you fix this?

There is already a fix, it just needs someone to review and land it.

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8887623 [details]
Bug 1381955 - Don't override RUSTFLAGS to set rust's debuginfo;

https://reviewboard.mozilla.org/r/158500/#review163954

::: config/rules.mk:911
(Diff revision 1)
>  sccache_wrap := RUSTC_WRAPPER='$(CCACHE)'
>  endif
>  
>  # XXX hack to work around dsymutil failing on cross-OSX builds (bug 1380381)
>  ifeq ($(HOST_OS_ARCH)-$(OS_ARCH),Linux-Darwin)
> -rust_debug_info=1
> +default_rustflags += -C debuginfo=1

We still want the -C debuginfo=2 for the `else` case.

Comment 6

[Tom Prince [:tomprince]]

Comment on attachment 8887623 [details]
Bug 1381955 - Don't override RUSTFLAGS to set rust's debuginfo;

https://reviewboard.mozilla.org/r/158500/#review163954

> We still want the -C debuginfo=2 for the `else` case.

Is that actually needed? Looking at [the previous patch](https://reviewboard.mozilla.org/r/157208/diff/2#index_header), neither was set before. I had assumed that the `debuginfo=2` was added because `debuginfo` was being passed unconditionally.

Comment 7

[Mike Hommey [:glandium]]

(In reply to Tom Prince [:tomprince] from comment #6)
> Comment on attachment 8887623 [details]
> Bug 1381955 - Don't override RUSTFLAGS to set rust's debuginfo;
> 
> https://reviewboard.mozilla.org/r/158500/#review163954
> 
> > We still want the -C debuginfo=2 for the `else` case.
> 
> Is that actually needed? Looking at [the previous
> patch](https://reviewboard.mozilla.org/r/157208/diff/2#index_header),
> neither was set before. I had assumed that the `debuginfo=2` was added
> because `debuginfo` was being passed unconditionally.

That patch also removed debug=true from the cargo build profile, which means cargo won't set it itself. Which is why it's needed.

Comment 8

[Tom Prince [:tomprince]]

Comment on attachment 8887623 [details]
Bug 1381955 - Don't override RUSTFLAGS to set rust's debuginfo;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158500/diff/1-2/

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8887623 [details]
Bug 1381955 - Don't override RUSTFLAGS to set rust's debuginfo;

https://reviewboard.mozilla.org/r/158500/#review163958

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/405a90abd28a
Don't override RUSTFLAGS to set rust's debuginfo; r=glandium

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/405a90abd28a