Rename 'all' to 'most', split out debugger build.

Rename 'all' to 'most', split out debugger build.

[bungeman, 2012-10-08 21:53:27]

This is an alternative to https://codereview.appspot.com/6598054/ plus splitting
out debugger so that those without QT can still build everything else without
any headache.

This should not affect any present uses of 'make all' or 'ninja all', but will
affect uses of 'make' or 'ninja' which will no longer build the debugger (which
matches the behavior of our 'make' scripts for Mac and Windows). On Windows,
Visual Studio will have separate 'most' and 'debugger' projects, so it will be
possible to build the 'most' project without needing to build the debugger.

[EricB, 2012-10-09 15:28:55]

On 2012/10/08 21:53:27, bungeman wrote:
> This is an alternative to https://codereview.appspot.com/6598054/ plus
splitting
> out debugger so that those without QT can still build everything else without
> any headache.
> 
> This should not affect any present uses of 'make all' or 'ninja all', but will
> affect uses of 'make' or 'ninja' which will no longer build the debugger
(which
> matches the behavior of our 'make' scripts for Mac and Windows). On Windows,
> Visual Studio will have separate 'most' and 'debugger' projects, so it will be
> possible to build the 'most' project without needing to build the debugger.

I have this dream where our Makefile just hooks into the one created by gyp.  If
that were the case, then this *would* affect the behavior of "make all". 
Something to consider, I guess.

[epoger, 2012-10-09 15:49:55]

https://codereview.appspot.com/6637045/diff/1/gyp_skia
File gyp_skia (right):

https://codereview.appspot.com/6637045/diff/1/gyp_skia#newcode91
gyp_skia:91: # Make and ninja reserve the 'all' target. Use 'most' instead.
If make automatically generates the "all" target, can we remove these lines from
https://code.google.com/p/skia/source/browse/trunk/Makefile ?

.PHONY: all
all: $(ALL_TARGETS)

Or maybe those are needed for some compilers (e.g. Xcode, which is the default
builder for Mac) that do NOT automatically create the "all" target?

Maybe the answer is to change the default builder on Mac to be "make" instead of
Xcode, in which case we can rip out most of the toplevel makefile and just make
it pass control to the gyp-generated makefile?

(EDIT: Ah, I see that this is Eric's dream...)

https://codereview.appspot.com/6637045/diff/1/gyp_skia#newcode92
gyp_skia:92: args.extend(['-Gdefault_target=most'])
P.S. Please be aware of any adverse reactions this CL may have to Eric's pending
https://codereview.appspot.com/6619049/ ('Build Skia as a static library')

https://codereview.appspot.com/6637045/diff/1/skia.gyp
File skia.gyp (right):

https://codereview.appspot.com/6637045/diff/1/skia.gyp#newcode39
skia.gyp:39: 'variables' : {
Why set the debugger variable and then use it in exactly one place (setting
dependencies)?  Wouldn't it be simpler to just set dependencies within this
conditional?

https://codereview.appspot.com/6637045/diff/1/skia.gyp#newcode40
skia.gyp:40: 'debugger' : [ 'Cannot build debugger with skia_os == <(skia_os)
and skia_arch_width == <(skia_arch_width).' ],
So, does it actually output this error message in this circumstance?  Cool.

[EricB, 2012-10-09 15:54:14]

https://codereview.appspot.com/6637045/diff/1/skia.gyp
File skia.gyp (right):

https://codereview.appspot.com/6637045/diff/1/skia.gyp#newcode34
skia.gyp:34: 'target_name': 'debugger',
Actually, what if we added a sk_want_debugger gyp define, and conditionally
include it in most/all/whatever based on that?

[bungeman, 2012-10-09 16:00:19]

So yes, the eventual goal of this is to also get rid of (at least most of) the
top-level Makefile. And hopefully restore some sanity.

https://codereview.appspot.com/6637045/diff/1/skia.gyp
File skia.gyp (right):

https://codereview.appspot.com/6637045/diff/1/skia.gyp#newcode34
skia.gyp:34: 'target_name': 'debugger',
On 2012/10/09 15:54:15, EricB wrote:
> Actually, what if we added a sk_want_debugger gyp define, and conditionally
> include it in most/all/whatever based on that?

Well, this just makes debugger a top level target. It will be in 'all', it just
won't be in 'most'. Instead of a gyp define, it would probably make more sense
in that case for debugger to just be in it's own gyp and gyp_skia create a whole
other project for it.

https://codereview.appspot.com/6637045/diff/1/skia.gyp#newcode39
skia.gyp:39: 'variables' : {
On 2012/10/09 15:49:56, epoger wrote:
> Why set the debugger variable and then use it in exactly one place (setting
> dependencies)?  Wouldn't it be simpler to just set dependencies within this
> conditional?

I tried that, but ran into trouble. I think 'dependencies' has to be top level
in the target.

https://codereview.appspot.com/6637045/diff/1/skia.gyp#newcode40
skia.gyp:40: 'debugger' : [ 'Cannot build debugger with skia_os == <(skia_os)
and skia_arch_width == <(skia_arch_width).' ],
On 2012/10/09 15:49:56, epoger wrote:
> So, does it actually output this error message in this circumstance?  Cool.

Obviously this target won't exist, so there will be an error when the
'dependencies' are evaluated. It was the best I could come up with to get an
error only in this case and which is semi-readable.

[tfarina1, 2012-10-09 16:00:57]

On Tue, Oct 9, 2012 at 12:49 PM,  <epoger@google.com> wrote:
>
> https://codereview.appspot.com/6637045/diff/1/gyp_skia
> File gyp_skia (right):
>
> https://codereview.appspot.com/6637045/diff/1/gyp_skia#newcode91
> gyp_skia:91: # Make and ninja reserve the 'all' target. Use 'most'
> instead.
> If make automatically generates the "all" target, can we remove these
> lines from https://code.google.com/p/skia/source/browse/trunk/Makefile ?
>
> .PHONY: all
> all: $(ALL_TARGETS)
>
> Or maybe those are needed for some compilers (e.g. Xcode, which is the
Xcode is not a compiler as far as I know :) It's the IDE, the compiler is
clang.

There is some confusion here.
-- 
Thiago

[epoger, 2012-10-09 17:30:42]

On 2012/10/09 16:00:57, tfarina1 wrote:
> On Tue, Oct 9, 2012 at 12:49 PM,  <mailto:epoger@google.com> wrote:
> >
> > https://codereview.appspot.com/6637045/diff/1/gyp_skia
> > File gyp_skia (right):
> >
> > https://codereview.appspot.com/6637045/diff/1/gyp_skia#newcode91
> > gyp_skia:91: # Make and ninja reserve the 'all' target. Use 'most'
> > instead.
> > If make automatically generates the "all" target, can we remove these
> > lines from https://code.google.com/p/skia/source/browse/trunk/Makefile ?
> >
> > .PHONY: all
> > all: $(ALL_TARGETS)
> >
> > Or maybe those are needed for some compilers (e.g. Xcode, which is the
> Xcode is not a compiler as far as I know :) It's the IDE, the compiler is
> clang.
> 
> There is some confusion here.
> -- 
> Thiago

Line 106 of https://code.google.com/p/skia/source/browse/trunk/Makefile?r=4653
calls "xcodebuild" to build on Mac.  Is that "clang" under the covers?

At any rate, targets are needed in IDEs, because that's what the IDE user uses
to trigger a build.

[epoger, 2012-10-09 17:30:52]

https://codereview.appspot.com/6637045/diff/1/gyp_skia
File gyp_skia (right):

https://codereview.appspot.com/6637045/diff/1/gyp_skia#newcode91
gyp_skia:91: # Make and ninja reserve the 'all' target. Use 'most' instead.
On 2012/10/09 15:49:56, epoger wrote:
> Maybe the answer is to change the default builder on Mac to be "make" instead
of
> Xcode, in which case we can rip out most of the toplevel makefile and just
make
> it pass control to the gyp-generated makefile?

BTW, if we *do* change the default builder on Mac to be "make", we should
probably make the gypfile generate BOTH "make" and "xcode" build files, so that
developers can still use the Xcode debugger IDE.

But I guess that would present its own problem, because (if we get rid of the
explicit "all" target in the gypfiles) the "make" build would have its automatic
"all" target, but the "xcode" build would not.

Bottom line, IMHO: if it is true that gyp automatically creates an "all" target
for SOME but NOT ALL build systems, then THAT is the problem.  And if that is
indeed the case, we probably need an "everything" target that will reliably
build everything on all platforms.

https://codereview.appspot.com/6637045/diff/1/skia.gyp
File skia.gyp (right):

https://codereview.appspot.com/6637045/diff/1/skia.gyp#newcode34
skia.gyp:34: 'target_name': 'debugger',
On 2012/10/09 16:00:19, bungeman wrote:
> On 2012/10/09 15:54:15, EricB wrote:
> > Actually, what if we added a sk_want_debugger gyp define, and conditionally
> > include it in most/all/whatever based on that?
> 
> Well, this just makes debugger a top level target.

Just to make sure we're on the same page: "debugger" is already a top-level
target for the platforms that it works on (because it is conditionally included
in the "all" target).

$ cd .../trunk

$ svn info . | grep ^Revision
Revision: 5861

$ make clean && GYP_DEFINES="skia_os=mac skia_arch_width=32" make debugger
rm -rf out xcodebuild
/Users/epoger/src/skia/blue/trunk/gyp_skia
Updating projects from gyp files...
rm -f out/Debug || if test -d out/Debug; then echo "run 'make clean' or
otherwise delete out/Debug"; exit 1; fi
xcodebuild -project out/gyp/debugger.xcodeproj -configuration Debug
xcodebuild: error: 'out/gyp/debugger.xcodeproj' does not exist.
make: *** [debugger] Error 66

$ make clean && GYP_DEFINES="skia_os=mac skia_arch_width=64" make debugger
rm -rf out xcodebuild
/Users/epoger/src/skia/blue/trunk/gyp_skia
Updating projects from gyp files...
rm -f out/Debug || if test -d out/Debug; then echo "run 'make clean' or
otherwise delete out/Debug"; exit 1; fi
xcodebuild -project out/gyp/debugger.xcodeproj -configuration Debug
=== BUILD NATIVE TARGET core OF PROJECT core WITH CONFIGURATION Debug ===
[it builds...]

> It will be in 'all', it just
> won't be in 'most'. Instead of a gyp define, it would probably make more sense
> in that case for debugger to just be in it's own gyp and gyp_skia create a
whole
> other project for it.

I like Eric's idea of a gyp variable (maybe sk_include_debugger, set to 1 or 0).
 If the developer does not set the variable explicitly, common_variables.gypi
can set it to a default value based on skia_os.  Then this file can pretty much
stay as it was, just changing the conditional check from OS type to the new gyp
variable.

I prefer that to the developer choosing between "make all" and "make most",
because it is clearer exactly what the developer is choosing to include or not
include.

But this is just my opinion, and I will happily defer to Ben's decision.

[tfarina1, 2012-10-09 17:57:39]

On Tue, Oct 9, 2012 at 2:30 PM,  <epoger@google.com> wrote:
> Line 106 of
> https://code.google.com/p/skia/source/browse/trunk/Makefile?r=4653 calls
> "xcodebuild" to build on Mac.  Is that "clang" under the covers?
>
Xcode is like Visual Studio for Windows. So in this sense I'd say
xcodebuild is what msbuild is for vs. xcodebuild is also in pair of
ninja, both are build tools, they call the compiler/linker etc.

-- 
Thiago

[bungeman, 2012-10-12 16:13:43]

http://code.google.com/p/skia/issues/detail?id=932 was opened about gyp ninja
and make generators creating 'all' targets automatically. I have opened
https://codereview.chromium.org/11109016/ to see about getting a change into gyp
so that these can be suppressed.

[epoger, 2012-10-12 18:55:34]

I think we can deprecate this CL in favor of
https://codereview.appspot.com/6651064/ ('gyp: generate "everything" and "most"
targets instead of "all"')

[bungeman, 2012-10-12 18:57:38]

On 2012/10/12 18:55:34, epoger wrote:
> I think we can deprecate this CL in favor of
> https://codereview.appspot.com/6651064/ ('gyp: generate "everything" and
"most"
> targets instead of "all"')

Since https://codereview.chromium.org/11109016/ doesn't seem to be of interest
to gyp (and they seem fine with stomping on 'all', sometimes), I agree.