The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

Release Engineering
Tools
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: ffledgling, Assigned: ffledgling)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [kanban:engops:https://mozilla.kanbanize.com/ctrl_board/6/2585] )

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
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.
Another fix: set LANG=C prior to invoking sort

That produces the (II) order on linux
$ 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
(Assignee)

Comment 3

3 years ago
Created attachment 8443196 [details] [diff] [review]
Bug1027871.patch

Patch using :hwine's suggested method of LANG=C.
Assignee: nobody → ffledgling
Status: NEW → ASSIGNED
Attachment #8443196 - Flags: review?(nthomas)
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.
Attachment #8443196 - Flags: review?(nthomas) → review-
(Assignee)

Comment 5

3 years ago
Created attachment 8444531 [details] [diff] [review]
Bug1027871v2.patch

Updated patch with nits fixed.
Attachment #8443196 - Attachment is obsolete: true
Attachment #8444531 - Flags: review?(nthomas)
(Assignee)

Updated

3 years ago
Blocks: 770995
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
Attachment #8444531 - Flags: review?(nthomas) → feedback+
(Assignee)

Updated

3 years ago
Blocks: 1051617

Updated

2 years ago
Whiteboard: [kanban:engops:https://mozilla.kanbanize.com/ctrl_board/6/2585]
Funsize uses linux to generate partial MARs for all platforms. Additionally LANG=C is set.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.