Closed Bug 881446 Opened 11 years ago Closed 3 years ago

Move MIDL_GENERATED_FILES to moz.build

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1620133

People

(Reporter: joey, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [leave open])

Attachments

(1 file)

      No description provided.
No longer blocks: 880254
No longer depends on: 880246
Assignee: nobody → joey
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 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-
(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.
Because we can't remove the Makefile.in until those rules are gone!
What Ms2ger said.
(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.
(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.
Assignee: joey → nobody
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
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.
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.
Product: Core → Firefox Build System
Assignee: ted → nobody

MIDL_GENERATED_FILES no longer exists.

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
Resolution: WORKSFORME → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: