Sync up MFBT's Decimal class with the upstream code

RESOLVED FIXED in Firefox 47

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

Trunk
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(11 attachments)

53.66 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
2.06 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
26.56 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
898 bytes, patch
Waldo
: review+
Details | Diff | Splinter Review
1.98 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
3.45 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
7.93 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
3.95 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
5.27 KB, patch
Details | Diff | Splinter Review
6.98 KB, patch
Details | Diff | Splinter Review
12.42 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
No description provided.
Our tests seem to all pass on Try, despite a decent amount of unrelated orange:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c77d6560428c
Attachment #8715191 - Flags: review?(jwalden+bmo) → review+
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.
Attachment #8715193 - Flags: review?(jwalden+bmo) → review+
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!
Attachment #8715195 - Flags: review?(jwalden+bmo) → review+
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.  ;-)
Attachment #8715197 - Flags: review?(jwalden+bmo) → review+
Attachment #8715199 - Flags: review?(jwalden+bmo) → review+
Attachment #8715200 - Flags: review?(jwalden+bmo) → review+
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!
Attachment #8715201 - Flags: review?(jwalden+bmo) → review+
Attachment #8715202 - Flags: review?(jwalden+bmo) → review+
Attachment #8715294 - Flags: review?(jwalden+bmo) → review+
(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.
You need to log in before you can comment on or make changes to this bug.