A mojom .d generator.

A mojom .d generator.

- A new generator that write .d files.
- The generators/common module can be used to help in writing generators in go.

This is part of the project to start supporting generators written in go. (Preliminary design doc: https://docs.google.com/a/chromium.org/document/d/1IR0TtyQv2UU7gDUbNx51kzr0dNRzV7souPP7Bk5Ut9g/edit?usp=sharing )

This should also make it easier to build mojo apps outside of the mojo repo. (which uses ninja)

R=rudominer@chromium.org, rudominer
BUG= #686

Committed: https://chromium.googlesource.com/external/mojo/+/fb22509d5062b365d1621ae011d4b2b55c47dcce

[rudominer, 2016-02-26 18:08:36]

You have added some context in the associated but but I would like more context
added to the CL description please:

- What larger project is this a part of? It seems like it is related to vtl's
work on a stand-alone SDK. Is that right?

- We do not currently have any code generators in Go--all of our code generators
are Jinja2 templates. Can you mention sometning in the CL description about the
fact that you are initiating the project of supporting code generators in Go?

- Is there a discussion or a design doc or a bug about a new way of driving the
code generators besides our current python scripts?

[azani, 2016-02-26 23:04:11]

Description was changed from

==========
A mojom .d generator.

- A new generator that write .d files.
- The generators/common module can be used to help in writing generators in go.
R=rudominer
BUG= #686
==========

to

==========
A mojom .d generator.

- A new generator that write .d files.
- The generators/common module can be used to help in writing generators in go.

This is part of the project to start supporting generators written in go.
(Preliminary design doc:
https://docs.google.com/a/chromium.org/document/d/1IR0TtyQv2UU7gDUbNx51kzr0dN...
)

This should also make it easier to build mojo apps outside of the mojo repo.
(which uses ninja)

R=rudominer
BUG= #686
==========

[azani, 2016-02-26 23:06:04]

I think between the discussion we had today on the topic and my clarifications
in the description of the issue I have addressed your comments. Let me know if
this is not the case. :-)

[rudominer, 2016-02-27 01:28:10]

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
File mojom/mojom_parser/generators/common/cli.go (right):

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/common/cli.go:5: // common groups together
functions which make it easier for generators
This is the same comment from common.go

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
File mojom/mojom_parser/generators/common/common.go (right):

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/common/common.go:31: GenImports() bool
My suggestion for a more complete description. Do with it as you will:

GenImports returns true if the generator should generate output files for all of
the files in the provided file graph, including those that were not specified on
the command line but are only present because they were referenced in an import
statement. It returns false if the generator should generate output files only
for the files which were explicitly specified (as indicated by the fact that
they have a non-empty |SpecifiedFileName|).

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/common/common.go:50: // GenerateOutput iterates
through the files in the file graph in config and
config -> |config|

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/common/common.go:57: writeOutput(fileName, config,
writer)
getOutputWriter isn't really helping here and instead is mandating a separation
that not all implementers might want. How about:

func GenerateOutput(generateFile generateFile, config GeneratorConfig) {
     for fileName, file := range config.FileGraph().Files {
         if config.GenImports() || *file.SpecifiedFileName != "" {
             generateFile(fileName, config)
}

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
File mojom/mojom_parser/generators/deps/deps_generator.go (right):

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/deps/deps_generator.go:9: // specify dependencies
in makefiles.
Can you give a link to something that defines a .d file?

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/deps/deps_generator.go:55: func
GetOutputFile(fileName string, config common.GeneratorConfig) (file
common.Writer) {
Seems like this or something like it should go in a common place: either
common.go or utils.go or something.

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/deps/deps_generator.go:84: func
GetTransitiveClosure(rootFile string, config common.GeneratorConfig) (result
[]string) {
Should this also be a common utility?

[azani, 2016-02-29 21:28:32]

ptal

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
File mojom/mojom_parser/generators/common/cli.go (right):

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/common/cli.go:5: // common groups together
functions which make it easier for generators
On 2016/02/27 01:28:10, rudominer wrote:
> This is the same comment from common.go

Done.

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
File mojom/mojom_parser/generators/common/common.go (right):

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/common/common.go:31: GenImports() bool
On 2016/02/27 01:28:10, rudominer wrote:
> My suggestion for a more complete description. Do with it as you will:
> 
> GenImports returns true if the generator should generate output files for all
of
> the files in the provided file graph, including those that were not specified
on
> the command line but are only present because they were referenced in an
import
> statement. It returns false if the generator should generate output files only
> for the files which were explicitly specified (as indicated by the fact that
> they have a non-empty |SpecifiedFileName|).

Done.

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/common/common.go:50: // GenerateOutput iterates
through the files in the file graph in config and
On 2016/02/27 01:28:10, rudominer wrote:
> config -> |config|

Done.

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/common/common.go:57: writeOutput(fileName, config,
writer)
On 2016/02/27 01:28:10, rudominer wrote:
> getOutputWriter isn't really helping here and instead is mandating a
separation
> that not all implementers might want. How about:
> 
> func GenerateOutput(generateFile generateFile, config GeneratorConfig) {
>      for fileName, file := range config.FileGraph().Files {
>          if config.GenImports() || *file.SpecifiedFileName != "" {
>              generateFile(fileName, config)
> }
> 
> 

I prefer this decoupling because it means that it's always possible to pass in a
different writer to writeOutput for testing purposes.

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
File mojom/mojom_parser/generators/deps/deps_generator.go (right):

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/deps/deps_generator.go:9: // specify dependencies
in makefiles.
On 2016/02/27 01:28:10, rudominer wrote:
> Can you give a link to something that defines a .d file?

Sorry, not really. The format seems to be indirectly specified via reference to
Make. For instance, look at the -M flag in man gcc. This is the closest:
http://www.microhowto.info/howto/automatically_generate_makefile_dependencies...

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/deps/deps_generator.go:55: func
GetOutputFile(fileName string, config common.GeneratorConfig) (file
common.Writer) {
On 2016/02/27 01:28:10, rudominer wrote:
> Seems like this or something like it should go in a common place: either
> common.go or utils.go or something.

Done.

https://codereview.chromium.org/1737143003/diff/40001/mojom/mojom_parser/gene...
mojom/mojom_parser/generators/deps/deps_generator.go:84: func
GetTransitiveClosure(rootFile string, config common.GeneratorConfig) (result
[]string) {
On 2016/02/27 01:28:10, rudominer wrote:
> Should this also be a common utility?

Looking at the other current generators, I don't think anyone else needs this.
I'm happy to move this to common in the future if it becomes useful to do so
though.

[rudominer, 2016-03-01 23:37:52]

lgtm

[azani, 2016-03-01 23:42:23]

Description was changed from

==========
A mojom .d generator.

- A new generator that write .d files.
- The generators/common module can be used to help in writing generators in go.

This is part of the project to start supporting generators written in go.
(Preliminary design doc:
https://docs.google.com/a/chromium.org/document/d/1IR0TtyQv2UU7gDUbNx51kzr0dN...
)

This should also make it easier to build mojo apps outside of the mojo repo.
(which uses ninja)

R=rudominer
BUG= #686
==========

to

==========
A mojom .d generator.

- A new generator that write .d files.
- The generators/common module can be used to help in writing generators in go.

This is part of the project to start supporting generators written in go.
(Preliminary design doc:
https://docs.google.com/a/chromium.org/document/d/1IR0TtyQv2UU7gDUbNx51kzr0dN...
)

This should also make it easier to build mojo apps outside of the mojo repo.
(which uses ninja)

R=rudominer@chromium.org, rudominer
BUG= #686

Committed:
https://chromium.googlesource.com/external/mojo/+/fb22509d5062b365d1621ae011d...
==========

[azani, 2016-03-01 23:42:25]

Committed patchset #7 (id:120001) manually as
fb22509d5062b365d1621ae011d4b2b55c47dcce (presubmit successful).