Closed Bug 899840 Opened 7 years ago Closed 7 years ago

Use sort or equivalent tool to provide consistent file ordering in makefiles created by fennec-copy-code.sh

Categories

(Android Background Services Graveyard :: Build & Test, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mcomella, Assigned: mcomella)

Details

Attachments

(1 file, 1 obsolete file)

Attached file Reorder example
Currently, running ./fennec-copy-code.sh may reorder the files it dumps into makefiles (see the attached example patch for an example). This is likely due to different versions of the find command.

Sort (or another consistency tool) should be used to ensure the found files are in the correct order (i.e. "find . | sort").
Attachment #783443 - Attachment filename: lol → reorder_example
I should mention that this reordering is bad because it creates patches with large changes to makefiles when all that really changed was the (unimportant) ordering of the lines.
Summary: Use sort or equivalent tool to provide consistent file ordering in fennec-copy-code.sh → Use sort or equivalent tool to provide consistent file ordering in makefiles created by fennec-copy-code.sh
r? - https://github.com/mozilla-services/android-sync/pull/336
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
Flags: needinfo?(nalexander)
This patch also has associated makefile changes on m-c which I will post after the above patch is r+.
Attached patch m-c patch (obsolete) — Splinter Review
Waiting for discussion on Github before review.
Comment on attachment 783842 [details] [diff] [review]
m-c patch

This isn't a bad idea, but 65k of churn for a small win isn't worth it right now.  |find| already orders (just in a slightly different way -- "E." sorts before "Ea.").

I expect to touch this code when we transition mobile/android to moz.build entirely (See Bug 854530 and friends).  Let's do this type of change then.
Attachment #783842 - Flags: review-
Pushed to github, doesn't need to land on m-c:

https://github.com/mozilla-services/android-sync/commit/9ed44571a0b76a8b3b8185bdd4ece73d8144010a

Great work, mcomella!
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: needinfo?(nalexander)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.