Last Comment Bug 769976 - pymake API for converting back to make files
: pymake API for converting back to make files
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Gregory Szorc [:gps]
:
Mentors:
Depends on:
Blocks: 778495
  Show dependency treegraph
 
Reported: 2012-06-30 18:32 PDT by Gregory Szorc [:gps]
Modified: 2012-08-07 07:36 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Implement make file source writing (40.68 KB, patch)
2012-06-30 18:32 PDT, Gregory Szorc [:gps]
no flags Details | Diff | Review

Description Gregory Szorc [:gps] 2012-06-30 18:32:54 PDT
Created attachment 638161 [details] [diff] [review]
Implement make file source writing

The attached patch adds functionality to pymake so it can take its own parser output and write it back to make file syntax.

I also implemented __eq__ and __ne__ on Expansion, Function, Statement, and derived classes to facilitate comparing instances. I'm using this functionality in the test code to ensure that the parsed output from the reformatting is equivalent to the original parsed statement list. Surprisingly, all but 3 make files are lossless!

The reformatting logic isn't perfect and there are some known failures and likely some unknown ones as well.

For the known failures, I currently have trouble figuring out how to escape backslashes and comments ('#') when they appear in variable assignments. Specifically, I'm not sure WTF make does when backslashes appear at the end of the line. I know the single trailing '\' is a line continuation. But, things are really funky in the case where you have lines like:

  VAR4 = lit4\\#backslash
  VAR5 = lit5\\\\#backslash
  VAR7 = lit7\\
  VAR8 = lit8\\\\
  VAR9 = lit9\\\\extra

These apparently translate to literals:

  lit4\
  lit5\\
  lit7\\
  lit8\\\\
  lit9\\\\extra

I'm trying to wrap my ahead around the algorithm here. Why is VAR5 not "lit5\\\\"? I guess the extra comment at the end of the line actually has significance here?! I scoured the GNU make manual and couldn't find a clear answer.

Anyway, the end of line backslashes are really the only known failures. Files from the test/ corpus that exercise this are disabled.

I'm content with checking things in as-is because functionality is at least 95% correct. I'm thinking the bugs can be addressed with a follow-ups.

FWIW, the empty-rule.mk test is failing on tip. This patch does not regress any existing tests.
Comment 1 Benjamin Smedberg [:bsmedberg] 2012-07-10 12:58:32 PDT
As far as I know, the details of backslash evaluation are pure implementation details, which happen to be relied-upon in real life now so nobody can change them. Something about \\# is super-special.
Comment 2 Benjamin Smedberg [:bsmedberg] 2012-07-10 13:14:09 PDT
Comment on attachment 638161 [details] [diff] [review]
Implement make file source writing

I really don't want to review this: any other takers?
Comment 3 Benjamin Smedberg [:bsmedberg] 2012-07-26 11:46:23 PDT
Comment on attachment 638161 [details] [diff] [review]
Implement make file source writing

I really don't have the time to review this.
Comment 4 Gregory Szorc [:gps] 2012-07-27 11:22:18 PDT
Comment on attachment 638161 [details] [diff] [review]
Implement make file source writing

Magic 8 ball says: khuey

Since I originally posted this patch, I've fed all Makefile.in's through this "reformatter" and have successfully built mozilla-central with the reformatted Makefile's. In other words, the conversion appears to be perfect!
Comment 5 Kyle Huey [:khuey] (khuey@mozilla.com) (Away until 6/13) 2012-08-06 10:18:09 PDT
Comment on attachment 638161 [details] [diff] [review]
Implement make file source writing

Review of attachment 638161 [details] [diff] [review]:
-----------------------------------------------------------------

Unfortunately I don't really have time to review this.  Lets just land it.
Comment 7 Ed Morley [:emorley] 2012-08-07 07:36:37 PDT
https://hg.mozilla.org/mozilla-central/rev/e97779ff2263

Note You need to log in before you can comment on or make changes to this bug.