1245414 - Sync up MFBT's Decimal class with the upstream code

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: part 1 - Delete the mfbt/decimal/LICENSE* files since upstream now just uses inline comments

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: part 2 - Update mfbt/decimal/update.sh to reflect Blink's switch from svn to git, and the different files we now pull

Comment 3

[Jonathan Watt [:jwatt]]

Attachment: part 3 - Overwrite mfbt/decimal/Decimal.* with vanilla upstream copies

Comment 4

[Jonathan Watt [:jwatt]]

Attachment: part 4 - Update mfbt/decimal/zero-serialization.patch

Comment 5

[Jonathan Watt [:jwatt]]

Attachment: part 5 - Update mfbt/decimal/comparison-with-nan.patch

Comment 6

[Jonathan Watt [:jwatt]]

Attachment: part 6 - Update mfbt/decimal/mfbt-abi-markers.patch

Comment 7

[Jonathan Watt [:jwatt]]

Attachment: part 7 - Update mfbt/decimal/to-moz-dependencies.patch

Comment 8

[Jonathan Watt [:jwatt]]

Attachment: part 8 - Remove mfbt/decimal/floor-ceiling.patch now that the issue is fixed upstream

Comment 9

[Jonathan Watt [:jwatt]]

Attachment: (new mfbt-abi-markers.patch for reference)

Comment 10

[Jonathan Watt [:jwatt]]

Attachment: (new to-moz-dependencies.patch for reference)

Comment 11

[Jonathan Watt [:jwatt]]

Attachment: part 9 - Apply the Mozilla patches via mfbt/decimal/update.sh

Comment 12

[Jonathan Watt [:jwatt]]

Our tests seem to all pass on Try, despite a decent amount of unrelated orange:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c77d6560428c

Comment 13

[Jeff Walden [:Waldo]]

Comment on attachment 8715193 [details] [diff] [review]
part 2 - Update mfbt/decimal/update.sh to reflect Blink's switch from svn to git, and the different files we now pull

Review of attachment 8715193 [details] [diff] [review]:
-----------------------------------------------------------------

::: mfbt/decimal/update.sh
@@ +40,4 @@
>    for F in "${FILES[@]}"
>    do
> +    printf "Downloading `basename $F`..."
> +    curl "$REPO_PATH/${F}?format=TEXT" | base64 -D > "$F"

We should think about dumping git hash in a sibling file or something, so we know the exact base rev we imported.  Just like intl/icu/SVN-INFO exists.  Separate patch?  Don't care about formatting or anything, just want *something* recording the info.

Comment 14

[Jeff Walden [:Waldo]]

Comment on attachment 8715195 [details] [diff] [review]
part 3 - Overwrite mfbt/decimal/Decimal.* with vanilla upstream copies

Review of attachment 8715195 [details] [diff] [review]:
-----------------------------------------------------------------

Psh, we don't need no stinkin' changes!

Comment 15

[Jeff Walden [:Waldo]]

Comment on attachment 8715197 [details] [diff] [review]
part 4 - Update mfbt/decimal/zero-serialization.patch

Review of attachment 8715197 [details] [diff] [review]:
-----------------------------------------------------------------

...or maybe we do.  ;-)

Comment 16

[Jeff Walden [:Waldo]]

Comment on attachment 8715201 [details] [diff] [review]
part 7 - Update mfbt/decimal/to-moz-dependencies.patch

Review of attachment 8715201 [details] [diff] [review]:
-----------------------------------------------------------------

::: mfbt/decimal/to-moz-dependencies.patch
@@ +126,5 @@
>   
>  +/**
>  + * Imported from:
> ++ * https://chromium.googlesource.com/chromium/src.git/+/master/third_party/WebKit/Source/platform/Decimal.h
> ++ * Check hg log for the date of the last update from Blink core.

Frowny-face!

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8d669d419453
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2b2fdf5b0b2
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1ca6010b131
https://hg.mozilla.org/integration/mozilla-inbound/rev/9da84cc6a236
https://hg.mozilla.org/integration/mozilla-inbound/rev/4929064f96e0
https://hg.mozilla.org/integration/mozilla-inbound/rev/678d101c5d60
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c730fc39712
https://hg.mozilla.org/integration/mozilla-inbound/rev/54a485b76667
https://hg.mozilla.org/integration/mozilla-inbound/rev/37a35e3c6648
https://hg.mozilla.org/integration/mozilla-inbound/rev/5626cc714fb1

Comment 18

[Jonathan Watt [:jwatt]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #13)
> Comment on attachment 8715193 [details] [diff] [review]
> part 2 - Update mfbt/decimal/update.sh to reflect Blink's switch from svn to
> git, and the different files we now pull
> 
> We should think about dumping git hash in a sibling file or something, so we
> know the exact base rev we imported.  Just like intl/icu/SVN-INFO exists. 
> Separate patch?  Don't care about formatting or anything, just want
> *something* recording the info.

I've made update.sh dump out the SHA to a new file called UPSTREAM-GIT-SH.

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #16)
> Comment on attachment 8715201 [details] [diff] [review]
> part 7 - Update mfbt/decimal/to-moz-dependencies.patch
>
> > ++ * Check hg log for the date of the last update from Blink core.
> 
> Frowny-face!

And updated this comment to reflect that.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8d669d419453
https://hg.mozilla.org/mozilla-central/rev/f2b2fdf5b0b2
https://hg.mozilla.org/mozilla-central/rev/d1ca6010b131
https://hg.mozilla.org/mozilla-central/rev/9da84cc6a236
https://hg.mozilla.org/mozilla-central/rev/4929064f96e0
https://hg.mozilla.org/mozilla-central/rev/678d101c5d60
https://hg.mozilla.org/mozilla-central/rev/3c730fc39712
https://hg.mozilla.org/mozilla-central/rev/54a485b76667
https://hg.mozilla.org/mozilla-central/rev/37a35e3c6648
https://hg.mozilla.org/mozilla-central/rev/5626cc714fb1