The default bug view has changed. See this FAQ.

pymake API for converting back to make files

RESOLVED FIXED in mozilla17

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gps, Assigned: gps)

Tracking

Trunk
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
Attachment #638161 - Flags: review?(benjamin)
(Assignee)

Updated

5 years ago
Attachment #638161 - Attachment is patch: true
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 on attachment 638161 [details] [diff] [review]
Implement make file source writing

I really don't want to review this: any other takers?
Comment on attachment 638161 [details] [diff] [review]
Implement make file source writing

I really don't have the time to review this.
Attachment #638161 - Flags: review?(benjamin)
(Assignee)

Comment 4

5 years ago
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!
Attachment #638161 - Flags: review?(khuey)
(Assignee)

Updated

5 years ago
Blocks: 778495
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.
Attachment #638161 - Flags: review?(khuey)
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e97779ff2263
https://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/cf7d1d604b8f
Assignee: nobody → gps
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla17
https://hg.mozilla.org/mozilla-central/rev/e97779ff2263
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.