On 2015-01-15 7:30 PM, Steve Fink wrote:
On 01/15/2015 11:21 AM, Ehsan Akhgari wrote:
On 2015-01-14 8:37 PM, Steve Fink wrote:
On 01/14/2015 11:26 AM, Ehsan Akhgari wrote:
From now on, the only supported build mode is unified compilation. I
am planning to follow-up with removing support for the
--disable-unified-compilation configure option altogether in bug
1121000.
I commented in the bug, but I guess this is probably a better forum.
Why is the configure option being removed?
Because unsupported build configurations that are *guaranteed* to not
be maintained are not worth keeping around. I am 90% sure that
--disable-unified-compilation would have been broken since yesterday.
:-)
But that's not the point. I totally agree that non-unified builds will
get broken immediately and will stay broken most of the time. That's not
in question. The point is how hard it is for someone who comes along and
wants to fix our codebase to be valid C++, even if that fix is not
permanent.
Specifically, what I imagine happening is this: we start out nowish with
nonunified builds working. Within a day, they're busted. After some
time, somebody who gains from them being correct wants to fix them, and
uses --disable-unified-builds to do so. Rinse, repeat. The amount of
breakage to fix each time is determined by the time between fixes.
If this hypothetical person has to go through every moz.build and change
UNIFIED_SOURCES to SOURCES, or FILES_PER_UNIFIED_FILE to 1, then they're
less likely to do it, and the interval between fixes will grow, and
eventually the code base will be rotten enough that nobody will want to
deal with it. Oh, except that people will occasionally add new files and
uncover breakage that wasn't their fault because they shift around the
boundaries. Which isn't awful, because you'll only have to fix a
localized area, but it is guaranteed to baffle many of the people who
hit it until they've been educated. More overhead to contributing, more
"specialness" in our codebase.
If you were saying that the --disable-unified-builds mechanism itself is
likely to break, then I couldn't really argue with removing it. Is there
a simpler way to accomplish the same thing? A
FORCE_MAX_FILES_PER_UNIFIED_FILE_EVERYWHERE=1 setting in the toplevel
moz.build or something? It doesn't need to be pretty, but it should be
easier than
for f in **/moz.build; do echo "FILES_PER_UNIFIED_FILE=1" >> $f; done
(assuming that even works, and note that you'll need zsh for the '**'
thing).
There are many ways to force UNIFIED_SOURCES to be built in non-unified
mode. Reverting my patch is one of them. Applying the following patch
is another:
diff --git a/python/mozbuild/mozbuild/backend/recursivemake.py
b/python/mozbuild/mozbuild/backend/recursivemake.py
index 608f6f5..de754b6 100644
--- a/python/mozbuild/mozbuild/backend/recursivemake.py
+++ b/python/mozbuild/mozbuild/backend/recursivemake.py
@@ -394,7 +394,7 @@ class RecursiveMakeBackend(CommonBackend):
non_unified_var = var[len('UNIFIED_'):]
files_per_unification = obj.files_per_unified_file
- do_unify = files_per_unification > 1
+ do_unify = False
# Sorted so output is consistent and we don't bump mtimes.
source_files = list(sorted(obj.files))
Part of what you're complaining about above is not about removing
support for the configure option, it's about dropping support for
non-unified builds on infrastructure. Yes, there are definitely
downsides to what we decided to do. Our code is no longer "valid C++"
(which honestly is a very arbitrary line -- our code has never been
valid C++ from a puristic standpoint any way because of all of the
compiler specific extensions that we use). And yes, there will be the
potential for breakage when you add or remove unified sources. And that
breakage would not be your fault, and you would need to be educated
about what's going on, unless you have experience with another project
that uses the same idea (which is unlikely.)
But there are upsides too. You won't get backed out because you broke a
build configuration that we do not ship. We don't pay the monetary and
human resource cost of keeping it working. And you will get *fast*
builds. I'd like to remind folks that unified builds improved our build
times by more than 2x.
So it is a trade-off. It's completely fine if you disagree on the call
that we made there, but I'd like to keep that conversation separate with
the one about how to build the tree locally in non-unified mode.
For the latter conversation, there are multiple ways. I don't think we
should accept such patches because 1) we already have too many
configure/etc options that break your build, and we keep problem reports
from people who have options in their mozconfigs that are unsupported
and cause broken builds (Please see the archives of
<https://groups.google.com/forum/#!forum/mozilla.dev.builds>.) and 2)
because I have yet to see any evidence to suggest that there will be
people who will maintain that build configuration to work. However, I
very well realize that both of these points are my opinion. This is
ultimately gps/glandium's call, and if they disagree with me, I won't
fight more on this hill.
However, I very strongly believe that the trade-offs for whether we
should do these builds *on the infrastructure* tip very heavily towards
not doing so. Not sure whose call that will ultimately be, but I would
be extremely sad to impose the cost of backouts because of breaking a
configuration that we don't ship to our developers.
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
