794162 - Add a script which generates a proper .clang_complete file based on the mozconfig in use

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(no longer active)]

I have written a script which helps people generate a proper .clang_complete file in the top of their source directory based on the mozconfig they have in use, which I believe is useful enough for us to have in the tree.

Comment 1

[(no longer active)]

Attachment: Patch (v1)

Comment 2

[Gregory Szorc [:gps]]

We should just refactor the mozconfig parsing code to emit JSON of the results. Data centric alleviates the need for alternate "execution contexts" like this script provides.

Comment 3

[Gregory Szorc [:gps]]

And by "mozconfig parsing code" I obviously mean the shell environment from which mozconfigs execute.

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 664573 [details] [diff] [review]
Patch (v1)

Review of attachment 664573 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/generate_clang_complete.sh
@@ +44,5 @@
> +  . "$FOUND_MOZCONFIG"
> +fi
> +
> +if [ -z "$MOZ_OBJDIR" ]; then
> +  echo 'Your .mozconfig does not define MOZ_OBJDIR'

A .mozconfig doesn't have to define MOZ_OBJDIR. And MOZ_OBJDIR may contain make variables (for instance, my .mozconfig defines it with $(CONFIG_GUESS)).

Comment 5

[Gregory Szorc [:gps]]

FWIW, I have code in bug 780329 nearing landing that "parses" mozconfig files and exposes the results to Python. I'm sensitive to wheel reinvention. My code in that bug did it. The patch here does it. There's obviously room for consolidation. Presumably the code in bug 780329 is the future (as client.mk dies). So, I'd prefer to land something there that would work for this bug. Also, this bug should probably be refactored as a mach sub-command (bug 751795). I'm *really* close to landing that. Just need to fix Mike Hommey's review feedback w.r.t. coincidentally mozconfig integration!

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 664573 [details] [diff] [review]
Patch (v1)

gps seems to want this one!

Comment 7

[(no longer active)]

(In reply to Mike Hommey [:glandium] from comment #4)
> ::: build/generate_clang_complete.sh
> @@ +44,5 @@
> > +  . "$FOUND_MOZCONFIG"
> > +fi
> > +
> > +if [ -z "$MOZ_OBJDIR" ]; then
> > +  echo 'Your .mozconfig does not define MOZ_OBJDIR'
> 
> A .mozconfig doesn't have to define MOZ_OBJDIR. And MOZ_OBJDIR may contain
> make variables (for instance, my .mozconfig defines it with $(CONFIG_GUESS)).

Shoot!

(In reply to Gregory Szorc [:gps] from comment #5)
> FWIW, I have code in bug 780329 nearing landing that "parses" mozconfig
> files and exposes the results to Python. I'm sensitive to wheel reinvention.
> My code in that bug did it. The patch here does it. There's obviously room
> for consolidation. Presumably the code in bug 780329 is the future (as
> client.mk dies). So, I'd prefer to land something there that would work for
> this bug.

That sounds fine to me, as long as the mozconfig parsing code is something that I can use through a simple API, and that it handles the case that glandium is talking about above.

> Also, this bug should probably be refactored as a mach sub-command
> (bug 751795). I'm *really* close to landing that. Just need to fix Mike
> Hommey's review feedback w.r.t. coincidentally mozconfig integration!

How does one create mach sub-commands?  I'm willing to do that, but I don't know much python so I need some docs to rescue.  :-)

Comment 8

[Gregory Szorc [:gps]]

(In reply to Ehsan Akhgari [:ehsan] from comment #7)
> How does one create mach sub-commands?  I'm willing to do that, but I don't
> know much python so I need some docs to rescue.  :-)

See https://hg.mozilla.org/mozilla-central/file/default/python/mach/README.rst and the various Python modules in python/mach/mach. e.g. settings.py or testing.py.

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 664573 [details] [diff] [review]
Patch (v1)

Ehsan: With your permission, I'd like to refactor this into a mach command.

Some of the work in bug 808357 will add a Python API to "parse" mozconfig files. And, since the output of |make showbuild| is redundant with the data already in config.status (a Python file!), this should turn into about a dozen lines of Python I think.

I think this approach will be simpler, easier to maintain, and more discoverable.

Comment 10

[(no longer active)]

Sounds good to me, specially if you're going to do the work.  ;-)

Comment 11

[Gregory Szorc [:gps]]

Attachment: Mach command to generate a .clang_complete file, v1

I think I ported everything from the shell script.

Do you think we should just write out .clang_complete or should we have the user do shell redirects?

Comment 12

[(no longer active)]

We should just write it down, I think.

Comment 13

[(no longer active)]

Comment on attachment 688631 [details] [diff] [review]
Mach command to generate a .clang_complete file, v1

Review of attachment 688631 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good overall, but can you please attach a .clang_complete file that is generated by this?

Thanks!

::: python/mozbuild/mozbuild/mach_commands.py
@@ +171,5 @@
> +
> +        print_from_variable('COMPILE_CXXFLAGS')
> +
> +        print('-I%s/ipc/chromium/src' % self.topsrcdir)
> +        print('-I%s/ipc/glu' % self.topsrcdir)

typo here.

Comment 14

[(no longer active)]

Comment on attachment 688631 [details] [diff] [review]
Mach command to generate a .clang_complete file, v1

Clearing the request for now.

Comment 15

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/b546bd987ed4

Comment 16

[Julien Wajsberg [:julienw]]

Is this script better than the instructions in https://wiki.mozilla.org/Clang_complete_in_mozilla (found with Google)? Should they be updated?

Comment 17

[Benoit Girard (:BenWa)]

The plan is to land and finish bug 892973 which gives the best results overall.