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)

enhancement
Not set
normal

Tracking

(firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

Details

Attachments

(3 files)

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

Created:
Updated:
Size: