Yuan Pengfei Reporter
	Description • 10 years ago

Due to unified compilation being enabled by default, there are code quality problems in various components.

	(no longer active)
	Comment 1 • 10 years ago

Yuan, what are the "code quality problems" specifically?  Please include as much detail as you can.  The list of blocking bugs in this bug seems like a random list of bugs that has nothing to do with unified builds.

	Steve Fink [:sfink] [:s:]
	Comment 2 • 10 years ago

My interpretation is that Yuan is finding nonunified compilation errors, tracking them back to the bug that introduced them, attaching fixes to those bugs, and marking those bugs as blocking this one. (In other words, patching the problems in the original bugs and recording everything encountered in this metabug.)

I don't know if that's the right way to go about it or not, but if it is, we should probably rename this bug to "[meta] non-unified compilation errors" or "[meta] namespacing errors" or something.

I'm happy that Yuan is fixing them, though!

	Boris Zbarsky [:bzbarsky]
	Comment 3 • 10 years ago

We probably need separate new bugs for the fixes, on the one-bug-per-problem basis.

	(no longer active)
	Comment 4 • 10 years ago

Yes, we need separate bugs for the fixes.  Thanks, Yuan!

	Yuan Pengfei Reporter
	Comment 5 • 10 years ago

(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #1)
> Yuan, what are the "code quality problems" specifically?  Please include as
> much detail as you can.  The list of blocking bugs in this bug seems like a
> random list of bugs that has nothing to do with unified builds.

Here is the logic:
unified build enabled by default
.cpp/.c files merged together during compilation
#include and using namespace in a.cpp take effect in b.cpp
people forget adding #include and using namespace when modifying b.cpp since the compiler does not complain
the correctness of source code depends on the somewhat unstable unified build system

	Yuan Pengfei Reporter
	Comment 6 • 10 years ago

(In reply to Yuan Pengfei from comment #5)
> the correctness of source code depends on the somewhat unstable unified
> build system

Proof of unstableness:
Bug 1043701 Comment 7
Bug 1043701 Comment 8
Bug 1112945

	Yuan Pengfei Reporter
	Comment 7 • 10 years ago

(In reply to Steve Fink [:sfink, :s:] from comment #2)
> My interpretation is that Yuan is finding nonunified compilation errors,
> tracking them back to the bug that introduced them, attaching fixes to those
> bugs, and marking those bugs as blocking this one. (In other words, patching
> the problems in the original bugs and recording everything encountered in
> this metabug.)
> 
> I don't know if that's the right way to go about it or not, but if it is, we
> should probably rename this bug to "[meta] non-unified compilation errors"
> or "[meta] namespacing errors" or something.
> 
> I'm happy that Yuan is fixing them, though!

It seems that unified builds are also affected...

	Yuan Pengfei Reporter
	Comment 8 • 10 years ago

(In reply to Boris Zbarsky [:bz] from comment #3)
> We probably need separate new bugs for the fixes, on the one-bug-per-problem
> basis.

Yeah.
I think the proper way is to file new bugs blocking the original ones as well as this one.
However, it is a little complicated for me to do that...

Bug 650161  -- blocked by Bug xxx \
Bug 1096089 -- blocked by Bug yyy -- blocking Bug 1125588
Bug 949703  -- blocked by Bug zzz /

	(no longer active)
	Comment 9 • 10 years ago

(In reply to Yuan Pengfei from comment #8)
> (In reply to Boris Zbarsky [:bz] from comment #3)
> > We probably need separate new bugs for the fixes, on the one-bug-per-problem
> > basis.
> 
> Yeah.
> I think the proper way is to file new bugs blocking the original ones as
> well as this one.
> However, it is a little complicated for me to do that...
> 
> Bug 650161  -- blocked by Bug xxx \
> Bug 1096089 -- blocked by Bug yyy -- blocking Bug 1125588
> Bug 949703  -- blocked by Bug zzz /

No, you just need to make bug xxx for example block both bug 650161 and bug 1125588.

	Steve Fink [:sfink] [:s:]
	Comment 10 • 10 years ago

(In reply to Boris Zbarsky [:bz] from comment #3)
> We probably need separate new bugs for the fixes, on the one-bug-per-problem
> basis.

Alternatively, how about we just give these fixes blanket r+, and land them without bugs?

  "No bug. Add missing header. r=unified"

or if you prefer a name, feel free to use

  "No bug. Add missing header. r=sfink"

as long as it's just a header addition or namespace qualification.

	Mike Hommey [:glandium]
	Comment 11 • 10 years ago

(In reply to Steve Fink [:sfink, :s:] from comment #10)
> (In reply to Boris Zbarsky [:bz] from comment #3)
> > We probably need separate new bugs for the fixes, on the one-bug-per-problem
> > basis.
> 
> Alternatively, how about we just give these fixes blanket r+, and land them
> without bugs?
> 
>   "No bug. Add missing header. r=unified"
> 
> or if you prefer a name, feel free to use
> 
>   "No bug. Add missing header. r=sfink"
> 
> as long as it's just a header addition or namespace qualification.

Might as well mention the bug that broke it: "Fixup for bug xxx. Add missing header. r=yyy"

	Gregory Szorc [:gps]
	Comment 12 • 7 years ago

All the code quality problems in Firefox have been fixed!

But seriously, this looks like a tracking bug and all dependencies are closed. So resolving.