Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 2015/02/03 15:24, Mike Hommey wrote: On Tue, Feb 03, 2015 at 02:27:52PM +0900, ISHIKAWA,chiaki wrote: I did a non-unified build and saw the expected failure. This is a summary of what I saw. Background: I may need to modify and debug basic I/O routines on local PC, and so want to avoid unnecessary compilation. I use ccache locally to make sure I can avoid re-compilation of touched but not modified C++ source files (files get touched and remain unmodified when I execute hg qpop and hg qpush in successions to work on different patches. Without ccache, I have to compile many files. ccache helps a lot.) There is a different perspective on unified compilation. Compiler farm users: One time fast compilation is very important. So unified compilation is a win. (I suspect precompiled headers, -pch, would be a good win, too.) Developers who repeats edit a small set of files, compile and link many times on local PC: He/she may modify only a few files and want quick turn around of the compile of a few files and link time. Unified compilation actually compiles more lines than he/she wants (because of the extra source lines included in unified source files in which his/her modified files are also included. (Correct? Am I missing something here?) So he/she may not like unified compilation in such scenario. Here's my take on this: yes, we should optimize for build times when code is modified. But here's the thing: in most directories, unified compilation shouldn't be making a huge difference. That is, compiling one unified source vs. compiling one source shouldn't make a big difference. If it does (and it does in some directories like js/src), then the number of unified sources in the directory where it's a problem should be adjusted. Mike Mike, thank you for the comment. I suspect this is indeed the case in many directories. (I mean unless a change of a single file caused 20 or 30 files to be included into a unified source, then it is an overhead certainly. But so far, the upper-bound of single change of a file is less than a couple of minutes including the link with -gsplit-dwarf.) I will report if I find a file, when touched, causes an extraordinarily long compilation time (by including many of the source files during unified compilation). By the way, I saw Unified_binding_*.cpp files during compilation, and I suspect they are different types of unified compilation since this unified_binding compilation seems to occur no matter what the setting of FILES_PER_UNIFIED_FILE. TIA CI ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On Tue, Feb 03, 2015 at 02:27:52PM +0900, ISHIKAWA,chiaki wrote: I did a non-unified build and saw the expected failure. This is a summary of what I saw. Background: I may need to modify and debug basic I/O routines on local PC, and so want to avoid unnecessary compilation. I use ccache locally to make sure I can avoid re-compilation of touched but not modified C++ source files (files get touched and remain unmodified when I execute hg qpop and hg qpush in successions to work on different patches. Without ccache, I have to compile many files. ccache helps a lot.) There is a different perspective on unified compilation. Compiler farm users: One time fast compilation is very important. So unified compilation is a win. (I suspect precompiled headers, -pch, would be a good win, too.) Developers who repeats edit a small set of files, compile and link many times on local PC: He/she may modify only a few files and want quick turn around of the compile of a few files and link time. Unified compilation actually compiles more lines than he/she wants (because of the extra source lines included in unified source files in which his/her modified files are also included. (Correct? Am I missing something here?) So he/she may not like unified compilation in such scenario. Here's my take on this: yes, we should optimize for build times when code is modified. But here's the thing: in most directories, unified compilation shouldn't be making a huge difference. That is, compiling one unified source vs. compiling one source shouldn't make a big difference. If it does (and it does in some directories like js/src), then the number of unified sources in the directory where it's a problem should be adjusted. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 2015-01-17 9:58 AM, ISHIKAWA,chiaki wrote: On 2015/01/16 1:08, Jonathan Kew wrote: On 15/1/15 15:56, ISHIKAWA,chiaki wrote: Debugging using gdb will be very difficult when the unified build creates a source file on the fly (but it is removed, correct?). No sane compiler/debugger combination can help me do the source level debugging if the source code that the compiler compiled is gone by the time debugger tries to find it... Maybe I am missing something... This shouldn't be a problem. The unified source file simply #includes the real sources, and so the debugger will know the paths to those. JK I see, thank you. If things don't work out with -gsplit-dwarf [there still can be some rough edges every now and then], I will simply apply the trick mentioned in this thread to virtually disable uniform build on my local build. BTW, I recall that Joshua Cranmer's coverage report system may not work well with uniform build the last time I checked. Please file bugs about the specific issues that you encounter in the future. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On Thu, Jan 15, 2015 at 06:39:56PM +0100, Sylvestre Ledru wrote: On 15/01/2015 16:56, ISHIKAWA,chiaki wrote: On 2015/01/15 10:37, 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? I understand always building unified in automation, but not having a straightforward way at all to see if your code is buggy seems... suboptimal. If someone wants to go through occasionally and make our codebase valid C++, how should they do it after this change? Perhaps the previous dev-platform thread would provide enlightenment and I ought to go back and reread it :( , but I do not offhand recall it deciding to remove the configure option altogether. I have an issue here, too. Debugging using gdb will be very difficult when the unified build creates a source file on the fly (but it is removed, correct?). No sane compiler/debugger combination can help me do the source level debugging if the source code that the compiler compiled is gone by the time debugger tries to find it... Maybe I am missing something... I have a similar question about static analysis tools (clang analyzer, coverity, etc). Doing the SA on the whole unified file is going to produce some useless results if all the files content gets merged. why? In general I would expect it to work better with unified compilation since it can see more code at once. Trev But maybe I am just missing the point. Cheers, Sylvestre ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 2015-01-16 2:10 AM, Steve Fink wrote:
On 01/15/2015 04:52 PM, Ehsan Akhgari wrote:
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))
This is a little cryptic, but something similar to this does seem
preferable to keeping all of the code for --disable-unified-compilation.
It just needs to be discoverable by the people who would usefully
discover it.
Can you think of something better/more obvious?
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.
No, because I agree with that decision.
OK, that's good to know. :-)
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).
I don't think that's really fair. I don't give a rip about sticking to
only c++0x or whatever. Our code conforms to an even stricter standard,
namely the common subset supported by a collection of different compilers.
What I'm concerned about is code that doesn't compile on any compiler if
you do it the normal way, one file at a time. Code that uses symbols
that are totally unknown or not in any using'd namespace. Code referring
to mysterious ALL_CAPS_SYMBOLS (that happen to be defined as macros in
headers that are not #included.) Code that depends on template
specializations from a non-included file for correctness. Unified
compilation creates dummy .cpp files that #include dogs and cats and
force them to live together.
Here is the issue that I have with what you're saying. It has
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On Fri, Jan 16, 2015 at 3:17 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: Ben Turner pointed out to me on IRC yesterday that Visual Studio will have a hard time because of this, and he's right, but that's because the Visual Studio project generator is broken. It should add the UnifiedXXX.cpp files to the project, not FileIncludedInUnified.cpp. So that's solvable. This has already been fixed! (Well, not for bindings files or ipdl files, but those were pre-existing problems.) -Nathan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 1/16/15 12:17 PM, Ehsan Akhgari wrote:
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))
This is a little cryptic, but something similar to this does seem
preferable to keeping all of the code for --disable-unified-compilation.
It just needs to be discoverable by the people who would usefully
discover it.
Can you think of something better/more obvious?
recursivemake.py could allow the user to override files_per_unification
with an environment variable like MOZ_FILES_PER_UNIFICATION or
MOZ_DISABLE_UNIFIED_COMPILATION (files_per_unification = 1). This would
not depend on mozconfig and configure support for
--disable-unified-compilation.
chris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On Fri, Jan 16, 2015 at 03:17:38PM -0500, Ehsan Akhgari wrote:
On 2015-01-16 2:10 AM, Steve Fink wrote:
On 01/15/2015 04:52 PM, Ehsan Akhgari wrote:
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))
This is a little cryptic, but something similar to this does seem
preferable to keeping all of the code for --disable-unified-compilation.
It just needs to be discoverable by the people who would usefully
discover it.
Can you think of something better/more obvious?
Add this to toplevel moz.build:
FILES_PER_UNIFIED_FILE = 1
export('FILES_PER_UNIFIED_FILE')
Mike
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 2015-01-16 5:06 PM, Chris Peterson wrote:
On 1/16/15 12:17 PM, Ehsan Akhgari wrote:
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))
This is a little cryptic, but something similar to this does seem
preferable to keeping all of the code for --disable-unified-compilation.
It just needs to be discoverable by the people who would usefully
discover it.
Can you think of something better/more obvious?
recursivemake.py could allow the user to override files_per_unification
with an environment variable like MOZ_FILES_PER_UNIFICATION or
MOZ_DISABLE_UNIFIED_COMPILATION (files_per_unification = 1). This would
not depend on mozconfig and configure support for
--disable-unified-compilation.
I prefer the environment variable.
But I urge everyone to please wait to see if we start getting fixes for
non-unified builds from now on before adding this.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On Wed, Jan 14, 2015 at 5:37 PM, Steve Fink sf...@mozilla.com 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? I understand always building unified in automation, but not having a straightforward way at all to see if your code is buggy seems... suboptimal. If someone wants to go through occasionally and make our codebase valid C++, how should they do it after this change? IIUC, in the absence of somebody committed to doing this on a daily basis, we will quickly slide towards hundreds of include errors, and the configure option will quickly become useless. bholley ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 15/01/2015 16:56, ISHIKAWA,chiaki wrote: On 2015/01/15 10:37, 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? I understand always building unified in automation, but not having a straightforward way at all to see if your code is buggy seems... suboptimal. If someone wants to go through occasionally and make our codebase valid C++, how should they do it after this change? Perhaps the previous dev-platform thread would provide enlightenment and I ought to go back and reread it :( , but I do not offhand recall it deciding to remove the configure option altogether. I have an issue here, too. Debugging using gdb will be very difficult when the unified build creates a source file on the fly (but it is removed, correct?). No sane compiler/debugger combination can help me do the source level debugging if the source code that the compiler compiled is gone by the time debugger tries to find it... Maybe I am missing something... I have a similar question about static analysis tools (clang analyzer, coverity, etc). Doing the SA on the whole unified file is going to produce some useless results if all the files content gets merged. But maybe I am just missing the point. Cheers, Sylvestre ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 01/15/2015 09:31 AM, Bobby Holley wrote: On Wed, Jan 14, 2015 at 5:37 PM, Steve Fink sf...@mozilla.com mailto:sf...@mozilla.com 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? I understand always building unified in automation, but not having a straightforward way at all to see if your code is buggy seems... suboptimal. If someone wants to go through occasionally and make our codebase valid C++, how should they do it after this change? IIUC, in the absence of somebody committed to doing this on a daily basis, we will quickly slide towards hundreds of include errors, and the configure option will quickly become useless. If that is the case, then adding a file has the potential of exposing a bunch of those include errors, if it perturbs the chunking boundaries. But we've already had that argument, and decided in favor of always building unified in automation. I think the question of removing the configure option is separate. It's ok if we gradually accumulate these errors. I just don't want to make life unnecessarily hard for the occasional angelic figure with supernatural papercut fixing abilities (hi cpeterson!). And if this is actually useful to a subset of people (eg IDE users), then perhaps the debt won't build up too much. Personally, I do occasionally run a build, grab the compile command, change the *unified*.cpp file to just the file I'm interested in, and rerun. But that's a very niche use. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 1/14/15 5:37 PM, Steve Fink wrote: Why is the configure option being removed? I understand always building unified in automation, but not having a straightforward way at all to see if your code is buggy seems... suboptimal. If someone wants to go through occasionally and make our codebase valid C++, how should they do it after this change? Another small benefit of optional non-unified builds is that clang does a better job of reporting -Wunused-variable warnings with smaller translation units. I assume that the number of identifiers in the unified files exceed some clang analysis limit. chris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 2015/01/15 10:37, 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? I understand always building unified in automation, but not having a straightforward way at all to see if your code is buggy seems... suboptimal. If someone wants to go through occasionally and make our codebase valid C++, how should they do it after this change? Perhaps the previous dev-platform thread would provide enlightenment and I ought to go back and reread it :( , but I do not offhand recall it deciding to remove the configure option altogether. I have an issue here, too. Debugging using gdb will be very difficult when the unified build creates a source file on the fly (but it is removed, correct?). No sane compiler/debugger combination can help me do the source level debugging if the source code that the compiler compiled is gone by the time debugger tries to find it... Maybe I am missing something... TIA ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 15/1/15 15:56, ISHIKAWA,chiaki wrote: Debugging using gdb will be very difficult when the unified build creates a source file on the fly (but it is removed, correct?). No sane compiler/debugger combination can help me do the source level debugging if the source code that the compiler compiled is gone by the time debugger tries to find it... Maybe I am missing something... This shouldn't be a problem. The unified source file simply #includes the real sources, and so the debugger will know the paths to those. JK ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On Wed, Jan 14, 2015 at 8:37 PM, Steve Fink sf...@mozilla.com wrote: Why is the configure option being removed? I understand always building unified in automation, but not having a straightforward way at all to see if your code is buggy seems... suboptimal. If someone wants to go through occasionally and make our codebase valid C++, how should they do it after this change? For what its worth, you can still verify individual directories by changing moz.build to use SOURCES instead of UNIFIED_SOURCES. Ben ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 2015-01-15 1:50 PM, Steve Fink wrote: On 01/15/2015 09:39 AM, Sylvestre Ledru wrote: On 15/01/2015 16:56, ISHIKAWA,chiaki wrote: On 2015/01/15 10:37, 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? I understand always building unified in automation, but not having a straightforward way at all to see if your code is buggy seems... suboptimal. If someone wants to go through occasionally and make our codebase valid C++, how should they do it after this change? Perhaps the previous dev-platform thread would provide enlightenment and I ought to go back and reread it :( , but I do not offhand recall it deciding to remove the configure option altogether. I have an issue here, too. Debugging using gdb will be very difficult when the unified build creates a source file on the fly (but it is removed, correct?). No sane compiler/debugger combination can help me do the source level debugging if the source code that the compiler compiled is gone by the time debugger tries to find it... Maybe I am missing something... I have a similar question about static analysis tools (clang analyzer, coverity, etc). Doing the SA on the whole unified file is going to produce some useless results if all the files content gets merged. But maybe I am just missing the point. So far, that's worked out for the things I've looked at. gdb and the hazard analysis both end up obeying the #line directives that give the original source filename. There are occasionally problems using emacs to jump to error messages, I can't remember why, but it hasn't been too bad. But I don't know about the other static analyses. Yes, clang can handle all of this very well. So can every other compiler on the planet. :-) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 2015-01-15 1:49 PM, Daniel Holbert wrote: On 01/15/2015 09:46 AM, Chris Peterson wrote: Another small benefit of optional non-unified builds is that clang does a better job of reporting -Wunused-variable warnings with smaller translation units. I assume that the number of identifiers in the unified files exceed some clang analysis limit. (I don't think it's a number-of-identifiers thing. In the cases I've seen, e.g. bug 1008083, clang doesn't report some variables as unused in #included files because it assumes the #included thing is a header, and the variable might be used in some *other* translation unit that includes this header.) Yeah, that is definitely one of the implications of unified builds. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
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. :-) I understand always building unified in automation, but not having a straightforward way at all to see if your code is buggy seems... suboptimal. If someone wants to go through occasionally and make our codebase valid C++, how should they do it after this change? Switch the UNIFIED_SOURCES variable to SOURCES in the moz.build file. That still works. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 2015-01-15 6:46 PM, Seth Fowler wrote: It wouldn’t be true of, say, a mach argument specifying directories that should be built non-unified. Not that it matters nearly as much now that we’ve made the decision not to support non-unified builds. Yeah that's true, but that mach command would break people's builds constantly. I don't see any value in supporting that, *unless* we actually see a flurry of patches coming in to fix non-unified builds. When and if that happens, I would happily change my mind. :-) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On Thu, Jan 15, 2015 at 02:11:16PM -0500, Benjamin Kelly wrote: For what its worth, you can still verify individual directories by changing moz.build to use SOURCES instead of UNIFIED_SOURCES. On Thu, Jan 15, 2015 at 02:21:25PM -0500, Ehsan Akhgari wrote: Switch the UNIFIED_SOURCES variable to SOURCES in the moz.build file. That still works. Instead of that, set FILES_PER_UNIFIED_FILE to 1. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 2015-01-15 5:57 PM, Seth Fowler wrote: What’s a bit unfortunate about these approaches is that you can accidentally qref them into your patch. I’ve managed to do so repeatedly. That's true of any local change, right? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
It wouldn’t be true of, say, a mach argument specifying directories that should be built non-unified. Not that it matters nearly as much now that we’ve made the decision not to support non-unified builds. On Jan 15, 2015, at 3:36 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: On 2015-01-15 5:57 PM, Seth Fowler wrote: What’s a bit unfortunate about these approaches is that you can accidentally qref them into your patch. I’ve managed to do so repeatedly. That's true of any local change, right? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
What’s a bit unfortunate about these approaches is that you can accidentally qref them into your patch. I’ve managed to do so repeatedly. - Seth On Jan 15, 2015, at 2:52 PM, Mike Hommey m...@glandium.org wrote: On Thu, Jan 15, 2015 at 02:11:16PM -0500, Benjamin Kelly wrote: For what its worth, you can still verify individual directories by changing moz.build to use SOURCES instead of UNIFIED_SOURCES. On Thu, Jan 15, 2015 at 02:21:25PM -0500, Ehsan Akhgari wrote: Switch the UNIFIED_SOURCES variable to SOURCES in the moz.build file. That still works. Instead of that, set FILES_PER_UNIFIED_FILE to 1. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
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)
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On Thu, Jan 15, 2015 at 4:30 PM, Steve Fink sf...@mozilla.com wrote: 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. Why would that even be a goal? If the project builds, is that not sufficient? I don't see any value in some platonic correctness. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
On 01/15/2015 04:52 PM, Ehsan Akhgari wrote:
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))
This is a little cryptic, but something similar to this does seem
preferable to keeping all of the code for --disable-unified-compilation.
It just needs to be discoverable by the people who would usefully
discover it.
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.
No, because I agree with that decision.
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).
I don't think that's really fair. I don't give a rip about sticking to
only c++0x or whatever. Our code conforms to an even stricter standard,
namely the common subset supported by a collection of different compilers.
What I'm concerned about is code that doesn't compile on any compiler if
you do it the normal way, one file at a time. Code that uses symbols
that are totally unknown or not in any using'd namespace. Code referring
to mysterious ALL_CAPS_SYMBOLS (that happen to be defined as macros in
headers that are not #included.) Code that depends on template
specializations from a non-included file for correctness. Unified
compilation creates dummy .cpp files that #include dogs and cats and
force them to live together.
It's bad for IDEs, it's bad for people reading the code (what is this
Value thing? Oh... some other .cpp file must be using JS::Value...),
PSA: Non-unified builds no longer occurring on central/inbound and friends
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. This puts an end to the issue of burning non-unified builders when pushing. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: Non-unified builds no longer occurring on central/inbound and friends
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? I understand always building unified in automation, but not having a straightforward way at all to see if your code is buggy seems... suboptimal. If someone wants to go through occasionally and make our codebase valid C++, how should they do it after this change? Perhaps the previous dev-platform thread would provide enlightenment and I ought to go back and reread it :( , but I do not offhand recall it deciding to remove the configure option altogether. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
