Closed Bug 1416062 Opened 7 years ago Closed 7 years ago

Compile sources under xpcom/ in the tup backend

Categories

(Firefox Build System :: General, enhancement)

Product:
Firefox Build System
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox59 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

Details

Attachments

(3 files)

Bug 1416062 - xpidl/ipdl/webidl outputs should be in the installed-files group for tup;
59 bytes, text/x-review-board-request
chmanchester
: review+
Details
Bug 1416062 - Generated header files and wrappers should be in the installed-files group;
59 bytes, text/x-review-board-request
chmanchester
: review+
Details
Bug 1416062 - Start compiling things in the tup backend, limited to xpcom/*;
59 bytes, text/x-review-board-request
chmanchester
: review+
Details 
The xpcom directory seems like a reasonable place to start, since it has unified + non-unified cpp files, .c files, and a .S file.
Comment on attachment 8928673 [details]
Bug 1416062 - xpidl/ipdl/webidl outputs should be in the installed-files group for tup;

https://reviewboard.mozilla.org/r/199916/#review205046
Attachment #8928673 - Flags: review+
Comment on attachment 8928674 [details]
Bug 1416062 - Generated header files and wrappers should be in the installed-files group;

https://reviewboard.mozilla.org/r/199918/#review205050

::: python/mozbuild/mozbuild/backend/tup.py:278
(Diff revision 1)
> +        install_exts = (
> +            '.h',
> +            '.inc',
> +            'new', # 'new' is an output from make-stl-wrappers.py
> +        )

Should this just be the full list at: https://searchfox.org/mozilla-central/rev/550148ab69b2879bfb82ffa698720ede1fa626f2/python/mozbuild/mozbuild/backend/recursivemake.py#521 ?

I know this is for just one directory, we can revisit this later as well.
Attachment #8928674 - Flags: review+
Comment on attachment 8928675 [details]
Bug 1416062 - Start compiling things in the tup backend, limited to xpcom/*;

https://reviewboard.mozilla.org/r/199920/#review205052

::: python/mozbuild/mozbuild/backend/tup.py:120
(Diff revision 1)
> +        compilers = [
> +            ('.S', 'AS', 'ASFLAGS'),
> +            ('.cpp', 'CXX', 'CXXFLAGS'),
> +            ('.c', 'CC', 'CFLAGS'),

This should probably use the `canonical_suffix` stuff to pave over differences between .s, .S, .asm,  etc.

::: python/mozbuild/mozbuild/backend/tup.py:304
(Diff revision 1)
>              fh.write('PYTHON = $(MOZ_OBJ_ROOT)/_virtualenv/bin/python -B\n')
>              fh.write('PYTHON_PATH = $(PYTHON) $(topsrcdir)/config/pythonpath.py\n')
>              fh.write('PLY_INCLUDE = -I$(topsrcdir)/other-licenses/ply\n')
>              fh.write('IDL_PARSER_DIR = $(topsrcdir)/xpcom/idl-parser\n')
>              fh.write('IDL_PARSER_CACHE_DIR = $(MOZ_OBJ_ROOT)/xpcom/idl-parser/xpidl\n')
> +            fh.write('CC = %s' % self.environment.substs.get('CC'))  # Needed for AS = $(CC)

If we're not going to fix this right away, we could use `expand_variables` somewhere earlier in the process rather than relying on tup's variable syntax being the same as Make's.
Attachment #8928675 - Flags: review+
Attachment #8928673 - Flags: review?(core-build-config-reviews)
Attachment #8928674 - Flags: review?(core-build-config-reviews)
Attachment #8928675 - Flags: review?(core-build-config-reviews)
Comment on attachment 8928674 [details]
Bug 1416062 - Generated header files and wrappers should be in the installed-files group;

https://reviewboard.mozilla.org/r/199918/#review205072

::: python/mozbuild/mozbuild/backend/tup.py:278
(Diff revision 1)
> +        install_exts = (
> +            '.h',
> +            '.inc',
> +            'new', # 'new' is an output from make-stl-wrappers.py
> +        )

Likely it will be the same, but we can add them as they become necessary. I filed bug 1417658 as a followup.
Comment on attachment 8928675 [details]
Bug 1416062 - Start compiling things in the tup backend, limited to xpcom/*;

https://reviewboard.mozilla.org/r/199920/#review205080

::: python/mozbuild/mozbuild/backend/tup.py:120
(Diff revision 1)
> +        compilers = [
> +            ('.S', 'AS', 'ASFLAGS'),
> +            ('.cpp', 'CXX', 'CXXFLAGS'),
> +            ('.c', 'CC', 'CFLAGS'),

Oh, thanks for catching that! I'll ask for re-review to make sure this looks correct now.

::: python/mozbuild/mozbuild/backend/tup.py:304
(Diff revision 1)
>              fh.write('PYTHON = $(MOZ_OBJ_ROOT)/_virtualenv/bin/python -B\n')
>              fh.write('PYTHON_PATH = $(PYTHON) $(topsrcdir)/config/pythonpath.py\n')
>              fh.write('PLY_INCLUDE = -I$(topsrcdir)/other-licenses/ply\n')
>              fh.write('IDL_PARSER_DIR = $(topsrcdir)/xpcom/idl-parser\n')
>              fh.write('IDL_PARSER_CACHE_DIR = $(MOZ_OBJ_ROOT)/xpcom/idl-parser/xpidl\n')
> +            fh.write('CC = %s' % self.environment.substs.get('CC'))  # Needed for AS = $(CC)

Good idea - I removed this and use expand_variables instead.
Attachment #8928675 - Flags: review?(core-build-config-reviews)
Comment on attachment 8928675 [details]
Bug 1416062 - Start compiling things in the tup backend, limited to xpcom/*;

https://reviewboard.mozilla.org/r/199920/#review205142

Nice!
Attachment #8928675 - Flags: review+
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c0a9377392b4
xpidl/ipdl/webidl outputs should be in the installed-files group for tup; r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/0806fc45779f
Generated header files and wrappers should be in the installed-files group; r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/32467b9838ef
Start compiling things in the tup backend, limited to xpcom/*; r=chmanchester
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: