1059090 - Don't require SOURCES to be set for CPP_UNIT_TESTS and SIMPLE_PROGRAMS

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Don't require SOURCES to be set for CPP_UNIT_TESTS and SIMPLE_PROGRAMS

Comment 2

[Michael Shal [:mshal]]

Comment on attachment 8479638 [details] [diff] [review]
Don't require SOURCES to be set for CPP_UNIT_TESTS and SIMPLE_PROGRAMS

SimplePrograms([
    'leakstats',
    'tmstats',
], ext='.c')

HostSimplePrograms([
   'host_jskwgen',
])

I definitely like not having to list the files in two places, but these both end up being really odd constructions. If it's a source in a library, we can do SOURCES += ['foo.c'], but in a program, it becomes 'foo', ext='.c'? That's weird. And for HostSimplePrograms, you have to know about the "host_" magic prefix? That's also weird.

Why not just list the source file and calculate the program name in templates.mozbuild, rather than list the program and calculating the source file? Eg:

SimplePrograms([
    'leakstats.c',
    'tmstats.c',
])

HostSimplePrograms([
    'jskwgen.cpp',
])

def SimplePrograms(names):
    SIMPLE_PROGRAMS += [split extension from names]
    SOURCES += names

etc.

Comment 3

[Mike Hommey [:glandium]]

(In reply to Michael Shal [:mshal] (no reviews 8/29 - 9/8) from comment #2)
> Comment on attachment 8479638 [details] [diff] [review]
> Don't require SOURCES to be set for CPP_UNIT_TESTS and SIMPLE_PROGRAMS
> 
> SimplePrograms([
>     'leakstats',
>     'tmstats',
> ], ext='.c')
> 
> HostSimplePrograms([
>    'host_jskwgen',
> ])
> 
> I definitely like not having to list the files in two places, but these both
> end up being really odd constructions. If it's a source in a library, we can
> do SOURCES += ['foo.c'], but in a program, it becomes 'foo', ext='.c'?
> That's weird. And for HostSimplePrograms, you have to know about the "host_"
> magic prefix? That's also weird.
> 
> Why not just list the source file and calculate the program name in
> templates.mozbuild, rather than list the program and calculating the source
> file?

Because then Program and SimpleProgram are different. Also, we only have a few SimpleProgram/CppUnitTests that have C sources, and we're unlikely to add more.

As for HostSimplePrograms, the host_ thing is because some host programs use a host_ prefix for the executable and not the source.

Comment 4

[Michael Shal [:mshal]]

(In reply to Mike Hommey [:glandium] (out from Sep 6 to Sep 22) from comment #3)
> Because then Program and SimpleProgram are different. Also, we only have a
> few SimpleProgram/CppUnitTests that have C sources, and we're unlikely to
> add more.

I don't see why you need parity between Program & SimpleProgram. I think the alternative proposed is much simpler.

> 
> As for HostSimplePrograms, the host_ thing is because some host programs use
> a host_ prefix for the executable and not the source.

It looks like they all have the host_ prefix, so why not just add it in the backend? Every other variable lists the name of an input file that you can see in the directory just by typing 'ls'. IMO these should follow that for ease of reading & understanding a moz.build file.

Comment 5

[Michael Shal [:mshal]]

Comment on attachment 8479638 [details] [diff] [review]
Don't require SOURCES to be set for CPP_UNIT_TESTS and SIMPLE_PROGRAMS

r+ for now if you really want to go this route. I'd suggest at least trying out an alternative.

Comment 6

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/17bee965226a

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/17bee965226a