2065815 - 155.0b3: toolkit/components/remote/nsXRemoteServer.cpp did not catch up with Success -> X11Success redefinition

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[gabriele balducci]

During mach build I got:

59:09.68 E /home/balducci/tmp/install-us-d/firefox-155.0b3/firefox-155.0b3/toolkit/components/remote/nsXRemoteServer.cpp:114:19: error: use of undeclared identifier 'Success'
59:09.68 E   114 |     if (result != Success || bytes_after != 0) {
59:09.68 E       |                   ^~~~~~~
59:10.11 E 1 error generated.

Reason is:

Commit 27def1ef41e52d649470661dd2e1f35631a2c53e redefined Success to X11Success in gfx/src/X11UndefineNone.h.
However, toolkit/components/remote/nsXRemoteServer.cpp did not catch up with the change and still uses Success

Comment 1

[Rong Bao [:csmantle]]

Hmm, since Bug 2065007 is a macro renaming task, I was really worried about anything leaking past. This seems to be such a case. Unfortunately, even Try auto pushes didn't catch it.

Besides fixing this, I'd be happy to know what we could do to further ensure nothing more gets left behind.

Comment 2

[Dave Townsend [:mossop]]

The build failure here is for a non-default build config (--disable-dbus) so really the only way to catch it would be to add a build check with this config. It's not completely clear to me how important this is though.

Comment 3

[Rong Bao [:csmantle]]

Attachment: Bug 2065815 - Rename usage of macro Success to X11Success in nsXRemoteServer.cpp. r=mossop!

This is a fixup to Bug 2065007.

Comment 4

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 2065007

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c327de0a238d
https://hg.mozilla.org/integration/autoland/rev/35379c055599
Rename usage of macro Success to X11Success in nsXRemoteServer.cpp. r=mossop

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/35379c055599

Comment 7

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:csmantle, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox155 to wontfix.

For more information, please visit BugBot documentation.

Comment 8

[Rong Bao [:csmantle]]

I'm not really sure if this is significant enough for an uplift. Dave and Ryan, what do you think?

Comment 9

[Dave Townsend [:mossop]]

(In reply to Rong Bao [:csmantle] from comment #8)

I'm not really sure if this is significant enough for an uplift. Dave and Ryan, what do you think?

It only really impacts downstream linux distros who might build with dbus disabled, I don't know if anyone does that. That said that also makes is zero risk to uplift it.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

What Mossop said. I'd be open to taking it as a ride-along if I'm uplifting other fixes anyway but wouldn't go out of my way to take it.

Comment 11

[Rong Bao [:csmantle]]

Thanks. I'll submit an uplift request, but please feel free to review it at your convenience and accept or reject it at your discretion.

Comment 12

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: Downstream distros with D-Bus disabled would fail to build; However, the existence of such distros is uncertain.
• Code covered by automated testing?: no
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing: N/A.
• Risk associated with taking this patch: low
• Explanation of risk level: This is a change in non-default config, and will be caught early in compilation if anything goes wrong.
• String changes made/needed?: None.
• Is Android affected?: no

Comment 13

[Rong Bao [:csmantle]]

Attachment: Bug 2065815 - Rename usage of macro Success to X11Success in nsXRemoteServer.cpp.

This is a fixup to Bug 2065007.

Original Revision: https://phabricator.services.mozilla.com/D321140

Comment 14

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/3b55c0da6149
https://hg.mozilla.org/releases/mozilla-beta/rev/6ffb10534e59