1047823 - mach build-backend -b CppEclipse fails

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Félix B. (:NinjaTrappeur)]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140722064054

Steps to reproduce:

Mach crash when I try to generate the eclipse project using the mach build-backend -b CppEclipse command.


Actual results:

254 ninjatrappeur@Bobby ~/Code/C++/firefox (hg)-[default] % python2 mach build-backend -b CppEclipse                        :(
 0:00.49 /home/ninjatrappeur/Code/C++/firefox/obj-x86_64-unknown-linux-gnu/_virtualenv/bin/python /home/ninjatrappeur/Code/C++/firefox/obj-x86_64-unknown-linux-gnu/config.status --backend=CppEclipse
Traceback (most recent call last):
  File "/home/ninjatrappeur/Code/C++/firefox/obj-x86_64-unknown-linux-gnu/config.status", line 982, in <module>
    config_status(**args)
  File "/home/ninjatrappeur/Code/C++/firefox/python/mozbuild/mozbuild/config_status.py", line 131, in config_status
    the_backend = backend_cls(env)
  File "/home/ninjatrappeur/Code/C++/firefox/python/mozbuild/mozbuild/backend/base.py", line 164, in __init__
    self._init()
  File "/home/ninjatrappeur/Code/C++/firefox/python/mozbuild/mozbuild/backend/cpp_eclipse.py", line 37, in _init
    self._cppflags = self.environment.substs['CPPFLAGS']
KeyError: 'CPPFLAGS'

Comment 1

[Arnaud Bienner]

Attachment: CPP_eclipse_backend.patch

Got the same problem.

Probably we just don't have CPPFLAGS set in our project? I believe this might be possible under some circumstances. 

In my case, simply ignoring this attribute if it doesn't exist fixed the problem, and my generated project looks great (everything indexed correctly when opening with Eclipse Luna)

Here is the patch.

Jonathan, as your name is mentionned in [1], I believe you might help be able to give some feedback about this?

[1]: https://developer.mozilla.org/en-US/docs/Eclipse_CDT

Comment 2

[Arnaud Bienner]

Comment on attachment 8467278 [details] [diff] [review]
CPP_eclipse_backend.patch

Actually, it seems that you, Benoit, worked on this file.
What do you think about this, and about the patch I proposed?

Comment 3

[Benoit Girard (:BenWa)]

Comment on attachment 8467278 [details] [diff] [review]
CPP_eclipse_backend.patch

gps do you know what happened that causes the CPPFLAGS to no longer be found?

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8467278 [details] [diff] [review]
CPP_eclipse_backend.patch

There have been a flurry of build system patches in the last few weeks. I was on PTO and my perspective of the world is out of date. Reassigning review to glandium.

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8467278 [details] [diff] [review]
CPP_eclipse_backend.patch

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

::: python/mozbuild/mozbuild/backend/cpp_eclipse.py
@@ +36,5 @@
>          # Note: We need the C Pre Processor (CPP) flags, not the CXX flags
> +        if 'CPPFLAGS' in self.environment.substs:
> +            self._cppflags = self.environment.substs['CPPFLAGS']
> +        else:
> +            self._cppflags = "";

You want
  self.environment.substs.get('CPPFLAGS')
or
  self.environment.substs.get('CPPFLAGS', '')

I see no reason why not do the same for other variables.

Comment 6

[Arnaud Bienner]

(In reply to Mike Hommey [:glandium] from comment #5)
> You want
>   self.environment.substs.get('CPPFLAGS')
> or
>   self.environment.substs.get('CPPFLAGS', '')

Yes indeed (didn't write python since long), but more likely the second one, because getting "None" will cause trouble when we will try to concatenate strings later.

> I see no reason why not do the same for other variables.

Just that the python script is failing otherwise, and having a default, empty value doesn't prevent the workspace to be generated and to be fully functional.
I'm not sure it would be the case for the other variables, so I would prefer to modify only this one for now.

Comment 7

[Arnaud Bienner]

Attachment: CPP_eclipse_backend_v2.patch

Patch cleaned.
Still wondering why CPPFLAGS aren't found, and if this can have any negative impact.

Comment 8

[Arnaud Bienner]

While running mach build, at the end, I have some warnings like this:

32:11.48 /home/arno/Firefox/central/src/netwerk/base/public/security-prefs.js: WARNING: no preprocessor directives found

So I believe CPPFLAGS are not set (it is not the script not finding them).
Not sure if it's normal to not have CPPFLAGS defined.
I believe this might be possible (depending on the plaform, they aren't always needed, probably): am I right?

Comment 9

[Gregory Szorc [:gps]]

The "WARNING: no preprocessor directives found" message is due to the preprocessor running on a file that doesn't have any preprocessor syntax. This is a nit-level bug and should be fixed. Most causes are due to cargo culting and people not realizing what does and doesn't need preprocessed.

Comment 10

[Yuan Xulei [:yxl] (Leave from May 18)]

I encountered the same issue with my Ubuntu 13.10 64bit system when building browser.

Comment 11

[Arnaud Bienner]

(In reply to Yuan Xulei [:yxl] from comment #10)
> I encountered the same issue with my Ubuntu 13.10 64bit system when building
> browser.

My patch got a r+
I will clean it (add a proper commit message, etc.) and ask it to be checked in into the tree.
So hopefully this should be fixed very soon.
If you're in a hurry, you can apply my patch on your local sources, waiting for it to be integrated: attachment 8467927 [details] [diff] [review].

Comment 12

[Yuan Xulei [:yxl] (Leave from May 18)]

(In reply to Arnaud Bienner from comment #11)
> (In reply to Yuan Xulei [:yxl] from comment #10)
> > I encountered the same issue with my Ubuntu 13.10 64bit system when building
> > browser.
> 
> My patch got a r+
> I will clean it (add a proper commit message, etc.) and ask it to be checked
> in into the tree.
> So hopefully this should be fixed very soon.
> If you're in a hurry, you can apply my patch on your local sources, waiting
> for it to be integrated: attachment 8467927 [details] [diff] [review].

Thanks. I manually applied your patch and it works perfectly.

Comment 13

[Arnaud Bienner]

Attachment: CPP_eclipse_backend_v2.patch

Same patch but with commit message.

Comment 14

[Benoit Girard (:BenWa)]

Thanks for looking into this, I noticed it but didn't have time to fix it.

There's several improvements we have pending on generating a better eclipse project for the mozilla code base. If you're interested in helping out let me know.

Comment 15

[Arnaud Bienner]

(In reply to Benoit Girard (:BenWa) from comment #14) 
> There's several improvements we have pending on generating a better eclipse
> project for the mozilla code base. If you're interested in helping out let
> me know.

There several other things I would like to work on first (and I don't have all the time I want to work on this, as I write code for Firefox on my spare time).
But if one day I have more time, or if I notice something while using Eclipse project, I will get in touch with you, thanks.

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/da07872dde7d

Comment 17

[Ed Morley [:emorley]]

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