Closed Bug 1355671 Opened 7 years ago Closed 7 years ago

Import and build the brotli command line utility

Tracking

(firefox55 fixed)

Status:

RESOLVED FIXED

Milestone:

mozilla55

Tracking Flags:

Tracking

Status

firefox55

---

fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(3 files, 1 obsolete file)

Bug 1355671 - Import brotli encoder and command line tool code. 7 years ago Mike Hommey [:glandium] 59 bytes, text/x-review-board-request	gps : review+	Details
Bug 1355671 - Build the brotli command line tool as a host program. 7 years ago Mike Hommey [:glandium] 59 bytes, text/x-review-board-request	gps : review+	Details
Bug 1355671 - Add optimization flags to HOST_CXXFLAGS similarly to HOST_CFLAGS. 7 years ago Mike Hommey [:glandium] 59 bytes, text/x-review-board-request	gps : review+	Details
Bug 1355671 - Add optimization flags to HOST_CXXFLAGS similarly to HOST_CFLAGS. 7 years ago Mike Hommey [:glandium] 59 bytes, text/x-review-board-request	gps : review+	Details

Mike Hommey [:glandium]

Assignee

Description

•

7 years ago

      No description provided.

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/129218/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129218/

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/129220/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129220/

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/129222/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129222/

Gregory Szorc [:gps]

Comment 4

•

7 years ago

mozreview-review

Comment on attachment 8857279 [details]
Bug 1355671 - Import brotli encoder and command line tool code.

https://reviewboard.mozilla.org/r/129218/#review131748

I'll rubber stamp this.

My only comment would be considering moving this to third_party/ before adding the encoder and tools. But we don't really have precedent for C libraries in third_party yet. So it's probably not worth the hassle.

Attachment #8857279 - Flags: review?(gps) → review+

Gregory Szorc [:gps]

Comment 5

•

7 years ago

mozreview-review

Comment on attachment 8857280 [details]
Bug 1355671 - Build the brotli command line tool as a host program.

https://reviewboard.mozilla.org/r/129220/#review131750

Attachment #8857280 - Flags: review?(gps) → review+

Gregory Szorc [:gps]

Comment 6

•

7 years ago

mozreview-review

Comment on attachment 8857281 [details]
Bug 1355671 - Add optimization flags to HOST_CXXFLAGS similarly to HOST_CFLAGS.

https://reviewboard.mozilla.org/r/129222/#review131752

I don't want to know how the old code for HOST_CFLAGS got into the state it was in.

Attachment #8857281 - Flags: review?(gps) → review+

Mike Hommey [:glandium]

Assignee

Comment 7

•

7 years ago

(In reply to Gregory Szorc [:gps] from comment #6)
> I don't want to know how the old code for HOST_CFLAGS got into the state it
> was in.

You mean the two branches with the same content? Funny isn't it?

Pulsebot

Comment 8

•

7 years ago

Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/820a15b7a5dd
Import brotli encoder and command line tool code. r=gps
https://hg.mozilla.org/integration/autoland/rev/5dd97d010a17
Build the brotli command line tool as a host program. r=gps
https://hg.mozilla.org/integration/autoland/rev/af627820b970
Add optimization flags to HOST_CXXFLAGS similarly to HOST_CFLAGS. r=gps

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 9

•

7 years ago

Backed out for Windows static bustage:

https://hg.mozilla.org/integration/autoland/rev/4bad7cb2a80dab851b35440184a5abaa44225858
https://hg.mozilla.org/integration/autoland/rev/c3cbe4e99c63452d1094119e1c9c79b329197d31
https://hg.mozilla.org/integration/autoland/rev/96b444ee590c9d76f2ff09148ab5c056733d8860

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=af627820b9706d8c6feae9b27323b07a48ca7c61&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=92467886&repo=autoland

21:55:21     INFO -  z:/task_1492549345/build/src/modules/brotli/tools/bro.cc(290,7):  error: cannot use 'try' with exceptions disabled
21:55:21     INFO -        try {
21:55:21     INFO -        ^
21:55:21     INFO -  1 error generated.
21:55:21     INFO -  z:/task_1492549345/build/src/config/rules.mk:844: recipe for target 'host_bro.obj' failed
21:55:21     INFO -  mozmake.EXE[5]: *** [host_bro.obj] Error 1
21:55:21     INFO -  mozmake.EXE[5]: Leaving directory 'z:/task_1492549345/build/src/obj-firefox/modules/brotli'
21:55:21     INFO -  z:/task_1492549345/build/src/config/recurse.mk:73: recipe for target 'modules/brotli/host' failed
21:55:21     INFO -  mozmake.EXE[4]: *** [modules/brotli/host] Error 2

Flags: needinfo?(mh+mozilla)

Comment hidden (mozreview-request)

Comment on attachment 8857279 [details]
Bug 1355671 - Import brotli encoder and command line tool code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129218/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8857280 [details]
Bug 1355671 - Build the brotli command line tool as a host program.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129220/diff/1-2/

Mike Hommey [:glandium]

Assignee

Updated

•

7 years ago

Attachment #8857281 - Attachment is obsolete: true

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/131430/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131430/

Comment hidden (mozreview-request)

Comment on attachment 8857279 [details]
Bug 1355671 - Import brotli encoder and command line tool code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129218/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8857280 [details]
Bug 1355671 - Build the brotli command line tool as a host program.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129220/diff/2-3/

Gregory Szorc [:gps]

Comment 15

•

7 years ago

mozreview-review

Comment on attachment 8857280 [details]
Bug 1355671 - Build the brotli command line tool as a host program.

https://reviewboard.mozilla.org/r/129220/#review134028

I don't have review powers over Gecko C/C++. But since this code doesn't ship, I'm comfortable giving it my approval.

Gregory Szorc [:gps]

Comment 16

•

7 years ago

mozreview-review

Comment on attachment 8859402 [details]
Bug 1355671 - Add optimization flags to HOST_CXXFLAGS similarly to HOST_CFLAGS.

https://reviewboard.mozilla.org/r/131430/#review134030

Attachment #8859402 - Flags: review?(gps) → review+

Pulsebot

Comment 17

•

7 years ago

Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/666f87b2d58f
Import brotli encoder and command line tool code. r=gps
https://hg.mozilla.org/integration/autoland/rev/55cb8ab2f846
Build the brotli command line tool as a host program. r=gps
https://hg.mozilla.org/integration/autoland/rev/6b410448e079
Add optimization flags to HOST_CXXFLAGS similarly to HOST_CFLAGS. r=gps

Mike Hommey [:glandium]

Assignee

Updated

•

7 years ago

Flags: needinfo?(mh+mozilla)

Carsten Book [:Tomcat]

Comment 18

•

7 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/666f87b2d58f
https://hg.mozilla.org/mozilla-central/rev/55cb8ab2f846
https://hg.mozilla.org/mozilla-central/rev/6b410448e079

Status: NEW → RESOLVED

Closed: 7 years ago

status-firefox55: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla55

Ionuț Goldan [:igoldan]

Updated

•

7 years ago

Depends on: 1357718

BMO Automation

Updated

•

6 years ago

Product: Core → Firefox Build System

You need to log in before you can comment on or make changes to this bug.