Closed
Bug 912293
Opened 11 years ago
Closed 11 years ago
Add a generic header and footer to generated Makefiles
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla26
People
(Reporter: glandium, Assigned: glandium)
References
Details
Attachments
(2 files, 7 obsolete files)
(deleted),
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
For bug 912292, I needed to reliably add something to all makefiles, either at the beginning or the end. Since I was doing this, I figured I might as well just add the usual boilerplate, and remove it from Makefile.in.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #799180 -
Flags: review?(gps)
Assignee | ||
Comment 2•11 years ago
|
||
I want to remove the blank lines this removes before putting this up for review.
Assignee | ||
Comment 3•11 years ago
|
||
This also takes into account the makefiles we have in CONFIGURE_SUBST_FILES. (without this, make package fails, for instance)
Attachment #799198 -
Flags: review?(gps)
Assignee | ||
Updated•11 years ago
|
Attachment #799180 -
Attachment is obsolete: true
Attachment #799180 -
Flags: review?(gps)
Comment 4•11 years ago
|
||
Comment on attachment 799198 [details] [diff] [review]
Add a generic header and footer to generated Makefiles
Review of attachment 799198 [details] [diff] [review]:
-----------------------------------------------------------------
\o/ would review again.
::: js/src/configure.in
@@ -4327,5 @@
>
> dnl Spit out some output
> dnl ========================================================
>
> -AC_OUTPUT([js-confdefs.h Makefile config/autoconf.mk config/emptyvars.mk])
I'm surprised this lasted as long as it did!
::: python/mozbuild/mozbuild/backend/configenvironment.py
@@ +227,5 @@
> + pp.handleLine('include $(DEPTH)/config/autoconf.mk\n')
> + if not stub:
> + pp.do_include(self.get_input(path))
> + pp.handleLine('\n') # Empty line to avoid failures when last line
> + # in Makefile.in ends with a backslash.
Nit: Move comment to own line(s)
::: python/mozbuild/mozbuild/backend/recursivemake.py
@@ +175,5 @@
>
> if isinstance(obj, DirectoryTraversal):
> self._process_directory_traversal(obj, backend_file)
> elif isinstance(obj, ConfigFileSubstitution):
> + if obj.output_path.endswith('/Makefile'):
Is relying on / safe here? Should we throw in a .replace('/', os.sep) just to be sure? Maybe we should do that in emitter.py if we're not already?
@@ +179,5 @@
> + if obj.output_path.endswith('/Makefile'):
> + f = backend_file.environment.create_makefile(obj.output_path)
> + else:
> + f = backend_file.environment.create_config_file(obj.output_path)
> + self._update_from_avoid_write(f)
This originally confused me because I thought f was a file and not a FileAvoidWrite result. Change variable to "result" or "r"?
Attachment #799198 -
Flags: review?(gps) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #799223 -
Flags: review?(gps)
Assignee | ||
Updated•11 years ago
|
Attachment #799182 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #799223 -
Flags: review?(gps)
Assignee | ||
Comment 6•11 years ago
|
||
Attachment #799231 -
Flags: review?(gps)
Assignee | ||
Updated•11 years ago
|
Attachment #799223 -
Attachment is obsolete: true
Assignee | ||
Comment 7•11 years ago
|
||
Removed a spurious DEPTH in js/xpconnect/loader/Makefile.in
Attachment #799233 -
Flags: review?(gps)
Assignee | ||
Updated•11 years ago
|
Attachment #799231 -
Attachment is obsolete: true
Attachment #799231 -
Flags: review?(gps)
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 799233 [details] [diff] [review]
Remove now redundant boilerplate from Makefile.in
sigh. it's now broken for some reason :(
Attachment #799233 -
Flags: review?(gps)
Assignee | ||
Comment 9•11 years ago
|
||
I /think/ this is the one. Giving it a try spin.
Assignee | ||
Updated•11 years ago
|
Attachment #799233 -
Attachment is obsolete: true
Assignee | ||
Comment 10•11 years ago
|
||
A couple changes to make config.status --file do kind of the right thing.
Attachment #799326 -
Flags: review?(gps)
Assignee | ||
Updated•11 years ago
|
Attachment #799198 -
Attachment is obsolete: true
Assignee | ||
Comment 11•11 years ago
|
||
Hopefully this one is good, although it's not against current m-i and will need some merge conflict resolution.
Assignee | ||
Updated•11 years ago
|
Attachment #799246 -
Attachment is obsolete: true
Comment 12•11 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #11)
> Created attachment 799327 [details] [diff] [review]
> Remove now redundant boilerplate from Makefile.in
>
> Hopefully this one is good, although it's not against current m-i and will
> need some merge conflict resolution.
Makefiles under dom/imptests/*/ are automatically generated - http://mxr.mozilla.org/mozilla-central/source/dom/imptests/writeBuildFiles.py will need to be updated.
Assignee | ||
Comment 13•11 years ago
|
||
(In reply to Ed Morley [:edmorley UTC+1] from comment #12)
> Makefiles under dom/imptests/*/ are automatically generated -
> http://mxr.mozilla.org/mozilla-central/source/dom/imptests/writeBuildFiles.
> py will need to be updated.
https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=912293&attachment=799327 ;)
Comment 14•11 years ago
|
||
Ah dammit, case sensitive sort order in https://bugzilla.mozilla.org/attachment.cgi?id=799327&action=diff after using "collapse all"; saw dom/imptests/Makefile.in and didn't look below the directories under that. Sorry! :-)
Assignee | ||
Comment 15•11 years ago
|
||
Comment on attachment 799327 [details] [diff] [review]
Remove now redundant boilerplate from Makefile.in
This is the one.
Attachment #799327 -
Flags: review?(gps)
Comment 16•11 years ago
|
||
Thanks for the consideration, both :)
Updated•11 years ago
|
Attachment #799326 -
Flags: review?(gps) → review+
Comment 17•11 years ago
|
||
Comment on attachment 799327 [details] [diff] [review]
Remove now redundant boilerplate from Makefile.in
Review of attachment 799327 [details] [diff] [review]:
-----------------------------------------------------------------
I only glanced at large parts of this patch because OMG IT MAKES MY EYES BLEED.
But, I did have a look at a few paths that interest me, notably dom/imptests and config/makefiles.
Attachment #799327 -
Flags: review?(gps) → review+
Assignee | ||
Comment 18•11 years ago
|
||
Comment 19•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/92ee006ec0a5
https://hg.mozilla.org/mozilla-central/rev/45097bc3a578
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•