Last Comment Bug 522220 - should generate NSS checksum files even if --disable-install-strip
: should generate NSS checksum files even if --disable-install-strip
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla1.9.3a1
Assigned To: Ted Mielczarek [:ted.mielczarek]
: Gregory Szorc [:gps]
Depends on:
Blocks: 503418 522041
  Show dependency treegraph
Reported: 2009-10-14 03:58 PDT by Ted Mielczarek [:ted.mielczarek]
Modified: 2009-11-12 07:57 PST (History)
7 users (show)
ted: blocking1.9.2+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

works, but ugly (1.01 KB, patch)
2009-10-16 09:02 PDT, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
ifdef to victory (1.01 KB, patch)
2009-10-16 11:31 PDT, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
ifdef to victory, for real (1001 bytes, patch)
2009-10-16 11:32 PDT, Ted Mielczarek [:ted.mielczarek]
benjamin: review+
dveditz: approval1.9.1.6+
Details | Diff | Splinter Review

Description Ted Mielczarek [:ted.mielczarek] 2009-10-14 03:58:46 PDT
Currently, if you build a Universal OS X build with --disable-install-strip, we remove the NSS .chk files from each architecture-specific side of the build, then fail to regenerate them when building the universal package:

The packager should regenerate them. We could either do it unconditionally (which would make us regenerate them when we don't need to, not sure if that's a problem) or we could test for their existence and regenerate if they don't exist.
Comment 1 Nick Thomas [:nthomas] 2009-10-14 04:04:07 PDT
As long as the chk files in the dmg and the complete.mar are the same I don't mind.
Comment 2 Ted Mielczarek [:ted.mielczarek] 2009-10-16 09:02:05 PDT
Created attachment 406693 [details] [diff] [review]
works, but ugly

This works, but it is ugly even by standards. I think I can make it suck a little less with $(wildcard) if I'm careful.
Comment 3 Ted Mielczarek [:ted.mielczarek] 2009-10-16 11:31:08 PDT
Created attachment 406728 [details] [diff] [review]
ifdef to victory

Better. Judicious use of ifdefs to get rid of the shell ugliness.
Comment 4 Ted Mielczarek [:ted.mielczarek] 2009-10-16 11:32:57 PDT
Created attachment 406729 [details] [diff] [review]
ifdef to victory, for real

Oops, qref'ed properly this time.
Comment 5 Ted Mielczarek [:ted.mielczarek] 2009-10-19 05:47:18 PDT
I built a clobber universal build on my Mac with this patch, and --disable-install-strip in the mozconfig, and verified that the checksum files are present in the generated DMG.
Comment 6 Ted Mielczarek [:ted.mielczarek] 2009-10-23 08:08:48 PDT
Pushed to m-c:

Did we --disable-install-strip on 1.9.{1,2}? Will we need to take this patch there? It's pretty harmless in general, since it only affects this exact case. (OS X Universal build with --disable-install-strip.)
Comment 7 Nick Thomas [:nthomas] 2009-10-23 10:55:23 PDT
It's used on on 1.9.1 and 1.9.2 too
and 1.9.2 is symlinked to the central nightly mozconfig. We should probably take the patch on those branches too.
Comment 8 Ted Mielczarek [:ted.mielczarek] 2009-10-25 13:43:09 PDT
Comment on attachment 406729 [details] [diff] [review]
ifdef to victory, for real

Simple patch, only affects this particular build configuration (OS X Universal builds with --disable-install-strip), which happen to be what we're shipping now. We either need to take this or turn this setting off, or we'll break FIPS mode.
Comment 9 Nick Thomas [:nthomas] 2009-10-25 17:01:16 PDT
We're only using --disable-install-strip for nightlies and shark builds right now, rather than "shipping" builds.
Comment 10 Ted Mielczarek [:ted.mielczarek] 2009-10-25 18:34:11 PDT
Ah, ok. Well, we should still either take this patch or turn that off, since we're shipping nightlies without FIPS support in the current configuration.
Comment 11 Daniel Veditz [:dveditz] 2009-11-04 16:26:40 PST
Comment on attachment 406729 [details] [diff] [review]
ifdef to victory, for real

Approved for, a=dveditz for release-drivers
Comment 12 Henrik Skupin (:whimboo) 2009-11-04 17:58:28 PST
FIPS mode works as expected with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091104 Minefield/3.7a1pre. Marking as verified.
Comment 13 Ted Mielczarek [:ted.mielczarek] 2009-11-10 13:02:27 PST
Pushed to 1.9.1:

Marked blocking1.9.2 since this is on 1.9.1 and trunk but not 1.9.2. Still would like explicit approval, though.
Comment 14 Mike Beltzner [:beltzner, not reading bugmail] 2009-11-12 07:39:16 PST
Comment on attachment 406729 [details] [diff] [review]
ifdef to victory, for real

(this is a blocker, doesn't need explicit approval, please land ASAP)
Comment 15 Ted Mielczarek [:ted.mielczarek] 2009-11-12 07:48:49 PST
To be fair, I marked it as a blocker myself, but felt squeamish then landing my own patch without approval. :)
Comment 16 Ted Mielczarek [:ted.mielczarek] 2009-11-12 07:57:28 PST
Pushed to 1.9.2:

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