Closed Bug 296294 Opened 20 years ago Closed 20 years ago

Incorporate libbz2 (bzip2 library) into the source tree

Categories

(Firefox Build System :: General, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.8beta3

People

(Reporter: darin.moz, Assigned: darin.moz)

References

Details

Attachments

(1 file, 1 obsolete file)

Incorporate libbz2 (bzip2 library) into the source tree This is needed by the new update system. And, I suspect that it would be handy to expose as a stream converter in the future. Brendan agreed to modules/libbz2 for this over IRC. Patch coming up.
Attached patch v1 patch (obsolete) (deleted) — Splinter Review
Attachment #185075 - Flags: superreview?(brendan)
Attachment #185075 - Flags: review?(benjamin)
This is version 1.0.3 of bzip2 (which according to www.bzip.org is the latest version).
Blocks: 290390
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8alpha3
Target Milestone: mozilla1.8alpha3 → mozilla1.8beta3
Blocks: 292021
(also... NPL license?)
Comment on attachment 185075 [details] [diff] [review] v1 patch >Index: libbz2/Makefile.in >+# The contents of this file are subject to the Netscape Public MPL tri-license Why are we making a modules/libbz2/src directory when the headers and source files are intermingled? Why not just put it all in modules/libbz2 directly? >Index: libbz2/src/Makefile.in >+# The contents of this file are subject to the Netscape Public Ditto. >+# BIGFILES >+DEFINES += -D_FILE_OFFSET_BITS=64 Why? I don't see either of these being used anywhere. >+FORCE_STATIC_LIB = 1 Where is this code being linked? I don't think we want LIBXUL_LIBRARY for code that is not linked into libxul.so (I'm pretty sure that LIBXUL_LIBRARY sets FORCE_USE_PIC, and that this code is only linked into standalone binaries). I did not review any of the bzip code, but I assume it's copied verbatim?
> Why are we making a modules/libbz2/src directory when the headers and source > files are intermingled? Why not just put it all in modules/libbz2 directly? following convention of modules/zlib. not a good idea? > >+# BIGFILES > >+DEFINES += -D_FILE_OFFSET_BITS=64 > > Why? I don't see either of these being used anywhere. I just copied the compile line that I observed while building libbz2 from the bzip2 sources. I should have checked to see which source files reference this define. Thanks for checking! > >+FORCE_STATIC_LIB = 1 > > Where is this code being linked? I don't think we want LIBXUL_LIBRARY for code > that is not linked into libxul.so (I'm pretty sure that LIBXUL_LIBRARY sets > FORCE_USE_PIC, and that this code is only linked into standalone binaries). I only want this to be built as a static library. It will eventually be something we link into libxul once we implement a nsIStreamConverter for this. But, we can definitely punt on supporting that. I probably just copied this Makefile from the zlib one and hacked it. What should I have here in order to get a simple static library output into dist/lib? > I did not review any of the bzip code, but I assume it's copied verbatim? Yes.
err, -D_FILE_OFFSET_BITS=64 is checked by libc, at least by gnu's...
Attachment #185075 - Flags: review?(benjamin) → review+
Attached patch v1.1 patch (deleted) — Splinter Review
Final patch with bsmedberg's nits applied. Brendan: I'm just looking for directory structure approval from you. a= would be appreciated too.
Attachment #185075 - Attachment is obsolete: true
Attachment #185209 - Flags: superreview?(brendan)
Attachment #185209 - Flags: approval1.8b3?
Attachment #185209 - Flags: approval-aviary1.1a2?
Attachment #185075 - Flags: superreview?(brendan)
Comment on attachment 185209 [details] [diff] [review] v1.1 patch sr+a=me. /be
Attachment #185209 - Flags: superreview?(brendan)
Attachment #185209 - Flags: superreview+
Attachment #185209 - Flags: approval1.8b3?
Attachment #185209 - Flags: approval1.8b3+
Attachment #185209 - Flags: approval-aviary1.1a2?
Attachment #185209 - Flags: approval-aviary1.1a2+
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: