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

Status: RESOLVED FIXED

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

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: 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").

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

r? - https://github.com/mozilla-services/android-sync/pull/336

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

This patch also has associated makefile changes on m-c which I will post after the above patch is r+.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: m-c patch

Waiting for discussion on Github before review.

Comment 5

[Nick Alexander :nalexander [he/him]]

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.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 783842 [details] [diff] [review]
m-c patch

The latest commit (https://github.com/mcomella/android-sync/commit/c1fbd8204730e70f40cfa319ca344e3995cf17f5) causes zero churn on Arch Linux.

Comment 7

[Nick Alexander :nalexander [he/him]]

Pushed to github, doesn't need to land on m-c:

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

Great work, mcomella!