Last Comment Bug 228190 - Remove unnecessary NSS_ENABLE_ECC defines from manifest.mn
: Remove unnecessary NSS_ENABLE_ECC defines from manifest.mn
Status: RESOLVED FIXED
ECC
:
Product: NSS
Classification: Components
Component: Build (show other bugs)
: 3.9
: All All
: P4 trivial (vote)
: 3.12
Assigned To: Julien Pierre
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2003-12-11 10:38 PST by Wan-Teh Chang
Modified: 2007-07-11 21:39 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Remove unnecessary NSS_ENABLE_ECC defines from manifest.mn (7.16 KB, patch)
2007-07-10 16:33 PDT, Julien Pierre
nelson: review+
Details | Diff | Splinter Review

Description Wan-Teh Chang 2003-12-11 10:38:38 PST
In NSS 3.9 we added the following to the manifest.mn
files in several NSS directories as part of the ECC
checkin:

+ifdef NSS_ENABLE_ECC
+DEFINES += -DNSS_ENABLE_ECC
+endif

The manifest.mn files are the wrong place for this
ifdef.  In the coreconf build system, manifest.mn
files should only contain plain definitions of some
make variables.  Any ifdef should be in some
config.mk file.

The right solution is to add a new NSS config.mk file:
  
  mozilla/security/nss/config/config.mk

move the above ifdef to that file, and have the relevant
Makefile's include the NSS config.mk file in section 3
("component" configuration information).

The new NSS config.mk will be useful for other similar
ifdefs that apply to NSS but not to other coreconf-based
projects.
Comment 1 Wan-Teh Chang 2006-02-27 15:46:32 PST
While checking in a fix for bug 320583 on the NSS_3_11_BRANCH,
Bob added the following to mozilla/security/coreconf/config.mk:

+#######################################################################
+# [16.0] Global environ ment defines
+#######################################################################
+
+ifdef NSS_ENABLE_ECC
+DEFINES += -DNSS_ENABLE_ECC
+endif

(Note: the above change was not checked in on the NSS trunk.
I don't know why.)

Although this is not the "right solution" I proposed, this
solution is much simpler, so I agree with it.  The remaining
work is to remove

 ifdef NSS_ENABLE_ECC
 DEFINES += -DNSS_ENABLE_ECC
 endif

from the manifest.mn, config.mk, or Makefile files in the
various directories.
Comment 2 Julien Pierre 2007-07-10 16:33:03 PDT
Created attachment 271761 [details] [diff] [review]
Remove unnecessary NSS_ENABLE_ECC defines from manifest.mn
Comment 3 Nelson Bolyard (seldom reads bugmail) 2007-07-11 20:45:41 PDT
Changed bug summary to express the actual change being made here.
Comment 4 Nelson Bolyard (seldom reads bugmail) 2007-07-11 20:55:52 PDT
Comment on attachment 271761 [details] [diff] [review]
Remove unnecessary NSS_ENABLE_ECC defines from manifest.mn

I verified that coreconf/config.mk has Bob's addition, 
even on the trunk.  I verified that the Makefile in each 
of the directories modified by this patch does include
coreconf/config.mk.  So I agree that these lines being 
removed by this patch are superfluous.  r=nelson
Comment 5 Julien Pierre 2007-07-11 21:39:38 PDT
Thanks for the review, Nelson . I checked this in to the trunk .

Checking in cmd/bltest/manifest.mn;
/cvsroot/mozilla/security/nss/cmd/bltest/manifest.mn,v  <--  manifest.mn
new revision: 1.8; previous revision: 1.7
done
Checking in cmd/certutil/manifest.mn;
/cvsroot/mozilla/security/nss/cmd/certutil/manifest.mn,v  <--  manifest.mn
new revision: 1.9; previous revision: 1.8
done
Checking in cmd/fipstest/Makefile;
/cvsroot/mozilla/security/nss/cmd/fipstest/Makefile,v  <--  Makefile
new revision: 1.4; previous revision: 1.3
done
Checking in cmd/lib/manifest.mn;
/cvsroot/mozilla/security/nss/cmd/lib/manifest.mn,v  <--  manifest.mn
new revision: 1.11; previous revision: 1.10
done
Checking in cmd/pk12util/manifest.mn;
/cvsroot/mozilla/security/nss/cmd/pk12util/manifest.mn,v  <--  manifest.mn
new revision: 1.12; previous revision: 1.11
done
Checking in cmd/selfserv/manifest.mn;
/cvsroot/mozilla/security/nss/cmd/selfserv/manifest.mn,v  <--  manifest.mn
new revision: 1.5; previous revision: 1.4
done
Checking in cmd/tstclnt/manifest.mn;
/cvsroot/mozilla/security/nss/cmd/tstclnt/manifest.mn,v  <--  manifest.mn
new revision: 1.5; previous revision: 1.4
done
Checking in lib/freebl/manifest.mn;
/cvsroot/mozilla/security/nss/lib/freebl/manifest.mn,v  <--  manifest.mn
new revision: 1.51; previous revision: 1.50
done
Checking in lib/softoken/manifest.mn;
/cvsroot/mozilla/security/nss/lib/softoken/manifest.mn,v  <--  manifest.mn
new revision: 1.30; previous revision: 1.29
done
Checking in lib/softoken/legacydb/manifest.mn;
/cvsroot/mozilla/security/nss/lib/softoken/legacydb/manifest.mn,v  <--  manifest.mn
new revision: 1.4; previous revision: 1.3
done
Checking in lib/ssl/manifest.mn;
/cvsroot/mozilla/security/nss/lib/ssl/manifest.mn,v  <--  manifest.mn
new revision: 1.15; previous revision: 1.14
done

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