1027871 - Fix tools/update-packaging/common.sh for consistent behaviour across platforms

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Anhad Jai Singh (:ffledgling)]

Right now `http://hg.mozilla.org/mozilla-central/file/ea703db56bcf/tools/update-packaging/make_incremental_update.sh` uses `http://hg.mozilla.org/mozilla-central/file/ea703db56bcf/tools/update-packaging/common.sh` to for list_dirs and list_files.
These functions do not return identical responses on OSX and Linux because the GNU sort's behaviour varies by platform. OSX sorts by ASCII value, Linux sorts lexically.
This results in problems when trying to confirm that the MARs generated by make_incremental_update.sh are identical on both platforms because the checksums don't match because of the different ordering of instructions in the update manifests.

Steps to reproduce:
I) create a file test.txt
A
a
B
b
C
c

II)
$ sort -r test.txt # On OSX -- c b a C B A
III)
$ sort -r test.txt # -- on Linux C c B b A a

Alternatively:
unwrap two partials generated on OSX and Linux from the same source complete MARs, then diff OSX/updatev* Linux/updatev* 

Potential fix : swap out GNU sort with a platform agnostic sort; A Python/Perl one liner sounds like a reasonable replacement.

Comment 1

[Hal Wine [:hwine] use NI!]

Another fix: set LANG=C prior to invoking sort

That produces the (II) order on linux

Comment 2

[Hal Wine [:hwine] use NI!]

$ uname -a
Linux cruncher.srv.releng.scl3.mozilla.com 2.6.32-220.13.1.el6.x86_64 #1 SMP Tue Apr 17 23:56:34 BST 2012 x86_64 x86_64 x86_64 GNU/Linux
0 [hwine@cruncher ~]
$ LANG=C sort -r test
c
b
a
C
B
A
0 [hwine@cruncher ~]

$ uname -a
Darwin Hals-MacBook-Air.local 13.2.0 Darwin Kernel Version 13.2.0: Thu Apr 17 23:03:13 PDT 2014; root:xnu-2422.100.13~1/RELEASE_X86_64 x86_64
0 [Hal@Hals-MacBook-Air tmp]
$ LANG=C sort -r test
c
b
a
C
B
A

Comment 3

[Anhad Jai Singh (:ffledgling)]

Attachment: Bug1027871.patch

Patch using :hwine's suggested method of LANG=C.

Comment 4

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8443196 [details] [diff] [review]
Bug1027871.patch

I think this should be fine except for renaming sortrev() to sort_reverse(), and using 2 spaces indent, to match existing style. Also, the $@ seems unnecessary given we're piping to sort (it tests fine without it!).

After that's done I'll do some checks on Windows and Linux.

Comment 5

[Anhad Jai Singh (:ffledgling)]

Attachment: Bug1027871v2.patch

Updated patch with nits fixed.

Comment 6

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8444531 [details] [diff] [review]
Bug1027871v2.patch

Granting f+ instead of a review, because I have belatedly realized there is a another copy of common.sh at test/common.sh.

Please make the same changes to test/common.sh (beware the different handling of the redirect in list_files and list_dirs functions). I think you can also remove the two echos at the end of test/diffmars.sh, the ones which warn about sort order.

You run the tests (which are for make_incremental_updates.py) by doing
 cd test
 ./buildrefmars.sh
 ./runtests.sh

Comment 7

[Rail Aliiev [:rail]]

Funsize uses linux to generate partial MARs for all platforms. Additionally LANG=C is set.