Closed Bug 1542352 Opened 5 years ago Closed 5 years ago

Restore zero-argument constructor of nsBaseCommandController

Categories

(Core :: XUL, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

References

Details

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #1540965 +++

Attached patch 1542352.patchSplinter Review

Sadly neither I nor the sheriffs can't use Lando since I simply uploaded a patch. So I'm attaching it again here with Boris' r+

Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #9056231 - Flags: review+
Keywords: checkin-needed

Jorg, I looked in phabricator and you have https://irccloud.mozilla.com/file/NvY5rWT6/blocked.png as a blocking reason.

You can fix that by following these steps: https://wiki.mozilla.org/Phabricator/FAQ#Lando_says_.22This_diff_does_not_have_the_proper_author_information_uploaded_to_Phabricator.22.2C_but_I_see_an_author_on_Phabricator._What.27s_wrong.3F

The Lando says "This diff does not have the proper author information uploaded to Phabricator", but I see an author on Phabricator. What's wrong? section

Do you want to do that or should I just land the patch in comment 2 via console on inbound?

Also, as a side note, even if lando is blocked, the initial patch could have been landed with arc patch D26365 --nobranch.

Aryx, please correct the above if I am wrong.

Flags: needinfo?(jorgk)
Flags: needinfo?(aryx.bugmail)

As I said in comment #2, Lando doesn't work neither for me (with Level 3 access rights) nor the sheriffs. That's due to bug 1529506. Note that this bug was originally filed as "Make Phabricator Web UI work to upload patches" and was later hijacked for other purposes :-( - In summary, there is missing integration between BMO and LDAP, see bug 1529506 comment #3 and below.

The alternative would be for me to set up arcanist or moz-phab to submit patches. Sadly on Windows, that's a twelve step process documented here: https://moz-conduit.readthedocs.io/en/latest/arcanist-windows.html. Since I have three development machines to maintain, that effort is prohibitive.

In short, please just land the patch "manually". You can easily see that it removes one line, nothing else.

Flags: needinfo?(jorgk)
Flags: needinfo?(aryx.bugmail)

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0be4787336d0
Restore zero-argument constructor of nsBaseCommandController. r=bz

Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

I had the same problem building Thunderbird on Debian Stable as of 14Apr2019
using the .mozbuild version of clang (8.0, I think).

The default constructor nsBaseCommandController() = delete; causes the build
to fail with the error messages:

32:48.61 thunderbird/comm/common/src/nsCommonModule.cpp:19:32: error: call to constructor of 'nsBaseCommandController' is ambiguous
32:48.61 NS_GENERIC_FACTORY_CONSTRUCTOR(nsBaseCommandController)
32:48.61 ^
32:48.61 thunderbird/obj-x86_64-pc-linux-gnu/dist/include/nsBaseCommandController.h:28:3: note: candidate constructor has been explicitly deleted
32:48.61 nsBaseCommandController() = delete;
32:48.61 ^
32:48.61 thunderbird/obj-x86_64-pc-linux-gnu/dist/include/nsBaseCommandController.h:29:12: note: candidate constructor
32:48.61 explicit nsBaseCommandController(
32:48.61 ^
32:48.69 1 error generated.
32:48.70 /thunderbird/config/rules.mk:805: recipe for target 'nsCommonModule.o' failed

Reading about C++, it appears that the explicit constructor on the line below the

nsBaseCommandController() = delete;

line causes clang not to create the default constructor, which, I guess, means that
clang can't delete it. Removing the line allows the compile to complete & the resultant
thunderbird seems to work correctly.

I'd like to submit a patch to either remove or comment out that line, but don't know how.
I'd appreciate it if someone would manually remove that line from the hg pull -u versions
that I'm using today.

(In reply to bugzilla from comment #7)

I'd like to submit a patch to either remove or comment out that line, but don't know how.

The line was already removed, see here:
https://hg.mozilla.org/mozilla-central/rev/0be4787336d0#l1.12

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: