Closed
Bug 881446
Opened 11 years ago
Closed 3 years ago
Move MIDL_GENERATED_FILES to moz.build
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1620133
People
(Reporter: joey, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [leave open])
Attachments
(1 file)
5.50 KB,
patch
|
gps
:
review-
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Updated•11 years ago
|
Reporter | ||
Comment 1•11 years ago
|
||
Reporter | ||
Updated•11 years ago
|
Assignee: nobody → joey
Reporter | ||
Comment 2•11 years ago
|
||
Comment on attachment 760937 [details] [diff] [review] move MIDL_GENERATED_FILES to moz.build (logic). Add MIDL_GENERATED_FILES as a passthrough variable.
Attachment #760937 -
Flags: review?(gps)
Comment 3•11 years ago
|
||
Comment on attachment 760937 [details] [diff] [review] move MIDL_GENERATED_FILES to moz.build (logic). Review of attachment 760937 [details] [diff] [review]: ----------------------------------------------------------------- MIDL_GENERATED_FILES is defined in 3 Makefile.in and in all 3 the rules for $(MIDL) invocation are defined in the respective Makefile.in. The proper way to port MIDL_GENERATED_FILES is to define a shared rule for invocation of $(MIDL), not by using a pass-thru variable.
Attachment #760937 -
Flags: review?(gps) → review-
Reporter | ||
Comment 4•11 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #3) > Comment on attachment 760937 [details] [diff] [review] > move MIDL_GENERATED_FILES to moz.build (logic). > > Review of attachment 760937 [details] [diff] [review]: > ----------------------------------------------------------------- > > MIDL_GENERATED_FILES is defined in 3 Makefile.in and in all 3 the rules for > $(MIDL) invocation are defined in the respective Makefile.in. The proper way > to port MIDL_GENERATED_FILES is to define a shared rule for invocation of > $(MIDL), not by using a pass-thru variable. Why not finish bulk porting content over to moz.build to get that activity out of the way/finally deprecate Makefile.in -- then anyone can grab bugs for writing python routines for cleaner interaction with moz.build.
Comment 5•11 years ago
|
||
Because we can't remove the Makefile.in until those rules are gone!
Comment 6•11 years ago
|
||
What Ms2ger said.
Reporter | ||
Comment 7•11 years ago
|
||
(In reply to :Ms2ger from comment #5) > Because we can't remove the Makefile.in until those rules are gone! Sure they can be, it is easy. If the content is shared by multiple makefiles refactor the rules logic into a standalone makefile. Modify rules.mk to include the rules when MIDL_GENERATED_FILES is defined. Minimal content change, behavior is not altered and Makefile.in can be removed.
Comment 8•11 years ago
|
||
(In reply to Joey Armstrong [:joey] from comment #7) > (In reply to :Ms2ger from comment #5) > > Because we can't remove the Makefile.in until those rules are gone! > > Sure they can be, it is easy. If the content is shared by multiple > makefiles refactor the rules logic into a standalone makefile. Modify > rules.mk to include the rules when MIDL_GENERATED_FILES is defined. Minimal > content change, behavior is not altered and Makefile.in can be removed. Then do that - *before* things move to moz.build files. It just doesn't make sense to have data living in moz.build that is used by code still in Makefile.in. Conceptually, by moving something from Makefile.in to moz.build, we're saying "OK, now we are free to refactor how that data is used by the build backend." We kinda bend that with things like compilation, but only because the set of variables is so large that a full conversion would be unwieldy. I don't want rules in Makefile.in relying on content in moz.build files.
Reporter | ||
Updated•11 years ago
|
Assignee: joey → nobody
Comment 9•7 years ago
|
||
midl invocation is uniquely terrible, as it turns out, so I think I need to fix this to make the WSL build work.
Assignee: nobody → ted
Blocks: wsl-build
Comment 10•7 years ago
|
||
To clarify a bit: for Windows builds we currently set PATH via moz.configure to include the Visual C++ and Windows SDK bin directories, and then invoke all the tools by name, like 'midl'. That doesn't work when invoking them in WSL, because environment variables in the bash shell don't get preserved when executing native Windows binaries. This is a problem for midl because it wants to invoke the C preprocessor (cl.exe). Conveniently midl supports a `/cpp_cmd` argument for specifying the path to the preprocessor ( https://msdn.microsoft.com/en-us/library/windows/desktop/aa367317(v=vs.85).aspx ), but that doesn't actually work if the path to the compiler includes spaces. I ran process monitor and traced the execution: midl invokes cl.exe correctly, but then I think cl.exe chokes on argument parsing or something and it all fails.
Comment 12•6 years ago
|
||
After thinking about this a bit, I'm pretty sure I can do this without adding anything to the moz.build sandbox. It should be possible to simply define a template that uses GENERATED_FILES to run midl, and then adds the outputs to SOURCES etc.
Updated•6 years ago
|
Product: Core → Firefox Build System
Updated•5 years ago
|
Assignee: ted → nobody
Comment 13•3 years ago
|
||
MIDL_GENERATED_FILES
no longer exists.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
Updated•3 years ago
|
Resolution: WORKSFORME → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•