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

RESOLVED FIXED in mozilla35

Status

defect
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

unspecified
mozilla35
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

No description provided.
Blocks: 1059113
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.
Attachment #8479638 - Flags: review?(mshal) → feedback+
(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.
Flags: needinfo?(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.
Flags: needinfo?(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.
Attachment #8479638 - Flags: feedback+ → review+
https://hg.mozilla.org/mozilla-central/rev/17bee965226a
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.