454120 - Problems Compiling SECURITY module for WinMobile

Status: VERIFIED FIXED

(NSS :: Build, defect)

Description

[John Wolfe (:wolfe)]

Attachment: Collection of changes needed to make WinMobile compile

Security module no longer compiles for WinMobile.  The last time SECURITY was compiled for WinCE was WinCE version 4.2, a number of years ago.

I have a patch with creates a new security/coreconf/WINCE.MK file, as well as modifies various files to facilitate compilation of the security modules.

The patches here are all changes within the security module, while WinCE shunt and building tools changes are located in bug # 454119.

NOTE: These are not the total sum of all changes needed to compile the security module -- this is only as far as I have gotten to date.  I have fixed: 
(a) Proper compiler calling with appropriate pathname conversions
(b) New WINCE.mk file, patterned after the Win954.0.mk file
(c) Moved NSS version defines into their own header file, for RC compilation
(d) Proper resource compiler calling with appropriate pathname conversions
(e) Proper linker calling with appropriate pathname conversions

First bug you will hit when trying to conpile security module for WinMobile is a link error because "main" is not defined for linking a DLL. WinMobile entry points should be DLLMain().

Comment 1

[Doug Turner (:dougt)]

Comment on attachment 337368 [details]
Collection of changes needed to make WinMobile compile

the patch is binary, making it text.

Comment 2

[Doug Turner (:dougt)]

wolfe, wtc and/or bs should review this.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Please attach a proper cvs diff. 
It should be a unified diff that bears enough information about the revision
numbers of the files being patched that Bugzilla's patch diff tool can display
the patch in context.

Please separate NSS and PSM components into separate patches, since they 
are separate modules and have different reviewers.

Comment 4

[Kai Engert [:KaiE:]]

Comment on attachment 337368 [details]
Collection of changes needed to make WinMobile compile

moa=kaie for change to security/manager/Makefile.in

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Attachment: above patch for NSS as a cvs unified diff

This is the NSS portion of the above patch, converted to a cvs diff 
for review purposes.  I haven't even built it, so I don't know if it 
compiles.  I'm not trying to take ownership of this bug.  Just showing
what's desired for proper review.  

If this bug is predominately a set of requested changes to NSS, then it
should be changed to product NSS, component build.  Otherwise it will not
show up on NSS reviewers' radar.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 338671 [details] [diff] [review]
above patch for NSS as a cvs unified diff

Some initial review observations:

1. in the patch to coreconf/WINCE.mk we see:

>+	DLLFLAGS   += -OUT:"$@"
>+echo break build here
>+	#
>+	# Add symbolic information for a profiler
>+	#
>+	ifdef MOZ_PROFILE

break build here?  
Maybe this patch is not ready for prime time?

2. The version string copied into nssversion.h does not match the
version string presently on the trunk.

>--- security/nss/lib/nss/nss.h	11 Aug 2008 20:48:30 -0000	1.58
>+++ security/nss/lib/nss/nss.h	15 Sep 2008 16:47:15 -0000

>-#define NSS_VERSION  "3.12.1.0" _NSS_ECC_STRING _NSS_CUSTOMIZED
>-#define NSS_VMAJOR   3
>-#define NSS_VMINOR   12
>-#define NSS_VPATCH   1
>-#define NSS_BETA     PR_FALSE
>+#include "nssversion.h"

>+++ security/nss/lib/nss/nssversion.h	15 Sep 2008 16:47:15 -0000

>+#define NSS_VERSION  "3.12.1.1" _NSS_ECC_STRING _NSS_CUSTOMIZED
>+#define NSS_VMAJOR   3
>+#define NSS_VMINOR   12
>+#define NSS_VPATCH   1
>+#define NSS_BETA     PR_FALSE

this makes me suspect that the changes recently done on the "mini branch"
for NSS 3.12.1.1 were not also applied to the trunk.

Kai, is that correct?  If so, we've got some preliminary work to do on
the trunk, ASAP.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 338671 [details] [diff] [review]
above patch for NSS as a cvs unified diff

Wan-Teh, please review.

Comment 8

[John Wolfe (:wolfe)]

Comment on attachment 337368 [details]
Collection of changes needed to make WinMobile compile

Removed Review flag in favor of a CVS version of this diff file.

Comment 9

[John Wolfe (:wolfe)]

For attachment 338671 [details] [diff] [review], the file coreconf/WinCE.mk was created by taking the file coreconf/Win954.0.mk, modifying the contents and replacing the previous WinCE.mk

In order to isolate my changes as much as possible, I replaced the line:

include $(CORE_DEPTH)/coreconf/WIN32.mk

with the current contents of coreconf/WIN32.MK

The line within WinCE.mk which said "echo break build here" did not stop my compilation, and was a mistake which was left over from my trying to figure out how the security module build procedure actually works.


I moved the version information from nss.h to its own file because the WinCE resource compiler is not very smart, and could not handle the various includes within the nss.h header file.

When I had created the original patch, I had not realized how quickly the version information in nss.h is changed.

Unfortunately, since the version information which I moved to nssversion.h changes frequently, that version information change is almost guaranteed to not apply cleanly.  I am not sure what the best way is to get around this version information limitation.  

Should I create a patch that is JUST the version information moving to its own header file?  That way, the version information migration could have a patch created, reviewed, and landed very, very quickly.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

John, when I did a very cursory review of the patch, I found two issues,
one of which was an issue with the patch, and one of which is an issue with 
the present state of the trunk, an issue which was revealed (not caused) by 
the patch.  

Don't worry about the version number mismatch.  If and when the patch is 
committed, the committer can tweak the version number.

Comment 11

[John Wolfe (:wolfe)]

Attachment: HG Patch for WinCE collection of changes to security module for compilation to succeed

Same as attachment 337368 [details], except the "echo build breaks here" was removed.

Comment 12

[John Wolfe (:wolfe)]

Attachment: CVS version of attachment 338791 above - patch for NSS as a cvs unified diff

Same as attachment 338671 [details] [diff] [review], except the line "echo break build here" was removed from my new coreconf/WinCE.mk file.

Comment 13

[Nelson Bolyard (seldom reads bugmail)]

John, thanks for producing the new patch.  
FYI, it isn't necessary to produce both CVS and Hg patches for NSS & NSPR
because the CVS repository is the authoritative repository, and the Hg 
repository should just be a copy of the CVS repository contents as of the 
most recent CVS release tag.  The process of copying the "snapshot" of the 
CVS repository to the Hg repository is automated, as I understand it.

Comment 14

[John Wolfe (:wolfe)]

Nelson, this is mostly me being paranoid, and figuring that it might take a while for these changes to land.  

Since we are trying to get more developers working with mozilla-central for WinMobile, I wanted to be able to point to a HG diff that these new developers can apply themselves, while the struggle continues to get these changes checked into the authoritative repository.

Comment 16

[John Wolfe (:wolfe)]

Attachment: HG Smaller patch with JUST NEW FILE NSSVERSION.H

When Doug landed these changes today, he forgot to add the file nssversion.h

This new file as derived from nss.h, and is a new file made by taking the contents of nss.h needed for compiling the resources for NSS.

Hopefully, we can get this file landed in time to save the tinderbox from blowing up on Wed 25-Sep-08.

Comment 17

[John Wolfe (:wolfe)]

Comment on attachment 340289 [details] [diff] [review]
HG Smaller patch with JUST NEW FILE NSSVERSION.H

Remember, local hg patch queue is NOT mozilla-central main repository.

My mistake as I was double-checking today's work tonight.

Please ignore the totally wrong comment I previously made about tinderbox breaking.

Comment 18

[Doug Turner (:dougt)]

Attachment: patch v.1 (wip)

Comment 19

[Doug Turner (:dougt)]

Attachment: patch v.2

Comment 20

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 340956 [details] [diff] [review]
patch v.2

Please provide CVS diffs for NSS reviews.
Don't use c++ style comments (//) in c code.

I suggest you resist the urge to combine separate independent patches
into one megapatch.  The nssversion.h patches, for example, appear to 
be independent of the rest of the patches here.

In the patch to secport.h, we see:

>+// is there a better place for this
>+#define _WIN32_WCE

Yes, there must be.  It CANNOT go in that file.  
Probably belongs in the wince file in coreconf.

Please explain this comment:
// This is because we redefine ERROR to anything via windows.h/shunt

redefine ERROR to anything?  

Do WinCE developers have no choice but to use MSYS? 
I never used MSYS when I worked on NSS for WinCE.

I'll do a more thorough review when I get the cvs diffs.

Comment 21

[Doug Turner (:dougt)]

Attachment: nssversion patch

this patch moves the nss version information out of nss.h and into nssversion.h.

this patch is required because the windows mobile rc.exe is dumb and doesn't understand some of the things that are in nss.h.

nssversion.h will be included in the next attachment.

Comment 22

[Doug Turner (:dougt)]

Attachment: nssversion_h needed by the above attachment.

Comment 23

[Doug Turner (:dougt)]

Attachment: coreconf changes

Comment 24

[Doug Turner (:dougt)]

i think the coreconf stuff needs to either define _WIN32_WCE, or WINCE, or both.  Right now we are a bit back and forth on our usage in NSS.  Wolfe can you look at this?

the other two parts of the original patch (to dbm and PSM) will be either resolved in other bugs.  Hopefully we will not need them at all.

Building without msys isn't supported now.  Furthermore building NSS independent of the mozilla windows ce shunt (http://mxr.mozilla.org/mozilla-central/source/build/wince/shunt/) is not supported.

lastly, ERROR is defined by WINCE SHUNT.  It is also used as a macro in pkix.  To avoid this, we need to redefine ERROR back to just ERROR.  Sounds wierd, don't it.

Comment 25

[Doug Turner (:dougt)]

Attachment: remaining hg diff that we have to figure out

Comment 26

[Nelson Bolyard (seldom reads bugmail)]

It may be that building the browser requires MSYS, but today, NSS can be
built with MSYS or MKS or even cygwin (I think), and I'd rather not 
impose a limitation on the shell packages with which NSS can be built
just because mozilla browsers have settled on one.  

The technique used by NSS for such things is to let environment variables 
or values obtained from the uname command control them.  There's already
code that detects MinGW and sets MSYS for that environment.  Also, the 
browser makefiles are free to define environment variables when building 
NSS as part of the browser.  Please use one of those techniques rather than
forcing all who build NSS for WinCE to adopt MSYS.

Comment 27

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 341008 [details] [diff] [review]
nssversion patch

Doug, 
Aside from the small problem that the version information that 
it would put into the new nssversion.h is not up to date, 
(the version has changed on the trunk), I agree in principle 
with this patch.  

But I have some questions.  This patch changes 4 .rc files,
corresponding to 4 of NSS's shared libs.  But NSS now has 8 or 9 
shared libs (one is optional) and 9 .rc files, listed below.

I wonder why your patch doesn't touch all of them.

lib/ckfw/builtins/nssckbi.rc
lib/ckfw/capi/nsscapi.rc
lib/freebl/freebl.rc
lib/nss/nss.rc
lib/smime/smime.rc
lib/softoken/softokn.rc
lib/softoken/legacydb/nssdbm.rc
lib/ssl/ssl.rc
lib/util/nssutil.rc

Comment 28

[Doug Turner (:dougt)]

http://mxr.mozilla.org/mozilla-central/search?string=%23include+%22nss.h%22&find=\.rc&findi=&filter=&hitlimit=&tree=mozilla-central

Only .rc files that #include nss.h were changed.

Comment 29

[Nelson Bolyard (seldom reads bugmail)]

Attachment: nssversion patch brought forward to tip

I updated the previous nssversion patch by making it apply cleanly to the 
trunk, and preserving the current trunk version numbers.  
Since this is primarily someone else's patch, I give it r+.

Doug, please verify that I've accurately reproduced the patch.

Julien, please review.

Comment 30

[Wan-Teh Chang]

Comment on attachment 341041 [details] [diff] [review]
nssversion patch brought forward to tip

In keeping with the 8.3 naming convention that our public header
files have faithfully followed, perhaps nssversion.h should be
renamed nssver.h.

Adding a new header just for the definition of the NSS version
macros is fine if there is no other way to support WinCE's rc.exe.
But the Win32 rc.exe has a predefined macro RC_INVOKED to solve
exactly this problem:
http://msdn.microsoft.com/en-us/library/aa381032(VS.85).aspx

Does WinCE's rc.exe not define RC_INVOKED?  An MXR search showed
that the wince shunt also tests RC_INVOKED:
http://mxr.mozilla.org/mozilla-central/search?string=RC_INVOKED

So we should try the RC_INVOKED solution.

Comment 31

[Julien Pierre]

Wan-Teh,

I don't think our public headers follow the 8.3 convention anymore. There are at least 2 public header files that don't : cmsreclist.h and utilrename.h . I am not aware of any platform where there is an 8.3 limit for header files.

Comment 32

[Julien Pierre]

Comment on attachment 341041 [details] [diff] [review]
nssversion patch brought forward to tip

This patch seems correct, though I'm not sure if it's required (see Wan-Teh's comments).

Comment 33

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 341010 [details] [diff] [review]
coreconf changes

Having more thoroughly reviewed this patch, my overall impression
is that most of this patch is fine, but some parts of it are not. 
These review comments are an expansion on my previous comment 20.

The changes to PREFIXes, SUFFIXes, IMPORT_LIBRARY, SHARED_LIBRARY, 
and TARGET_OSES are all good.  

It appears that the Fennec project has made some project-specific 
choices for itself, such as 
- using MSYS and
- using these curious alternative arm-wince-*.exe tools 
This patch would impose these choices on all who would build NSS 
for WinCE. 

It appears to me that this patch
- eliminates the ability to build with any shell except MSYS
- eliminates the ability use use WinCE with any CPU other than ARM, and
- eliminates the ability to use Microsoft's free tools for building for 
WinCE, giving the user only the choice between gcc and arm-wince-gcc.  
The choices appear to be gcc or gcc.  :(

These choices may be entirely appropriate for the Fennec project,
but IMO should not be imposed on all who would build NSS for WinCE.
I won't want to see the fennec project's way of building NSS for WinCE
become the ONLY way of building NSS for WinCE.

IMO, Fennec should regard itself as a "downstream" project with respect
to NSS, and in pushing changes "upstream" should do so in a way that 
doesn't reduce the choices for all other (potential) downstream projects.  

Please use another (new) make variable to enable the use of these new 
arm-wince-*.exe tools.  The default should continue to be to use 
Microsoft's tools, with the use of GCC and the use of these 
arm-wince-* tools being alternatives enabled through make (environment) 
variables.

Comment 34

[Doug Turner (:dougt)]

Comment on attachment 341041 [details] [diff] [review]
nssversion patch brought forward to tip

wtc's suggestion about RC_INVOKED works.  patch coming up.

Comment 35

[Doug Turner (:dougt)]

Attachment: Use of RC_INVOKED in nss.h

this patch, as suggested by wan-teh, works fine using the windows mobile resource compiler.

It simply surrounds the prototypes with an #ifndef RC_INVOKED block.

Comment 36

[Doug Turner (:dougt)]

Attachment: nss_build_changes v.1

Nelson, how is this?  I still need to figure out a way to pass the correct AR from the security/manager/Makefile.in.  (Ted, any ideas?)

Comment 37

[Doug Turner (:dougt)]

Attachment: nss_build_changes v.2

figured out a way to pass in the right AR

Comment 38

[(not currently active) Ted Mielczarek]

Nelson's review is sufficient for me on the security/manager/Makefile.in bits.

Comment 39

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 344175 [details] [diff] [review]
nss_build_changes v.2

1. Doesn't this define a symbol named LL ?
Is that really the intended result?

>+OS_DLLFLAGS  += -DLL

2. this file is outside of NSS and has different review rules than NSS 
does.  Please ask a PSM developer or BDS to review the change to this file.

>diff --git a/security/manager/Makefile.in b/security/manager/Makefile.in
>--- a/security/manager/Makefile.in
>+++ b/security/manager/Makefile.in

3. Does WinCE include CAPI?  
Does it include the UI for managing Windows' cert store?
If so, I'd think this would be desirable.

>+ifneq ($(OS_ARCH),WINCE)
> ifdef MOZILLA_CLIENT
> NSS_BUILD_CAPI = 1
>+endif
> endif

4. The patch makes the following change in several source files:

>+// This is because we redefine ERROR to anything via windows.h/shunt
>+#ifdef WINCE
>+#undef ERROR
>+#define ERROR ERROR
>+#endif

multiple comments about that:

a) it uses //, a C++ comment in c code.  
Some (most) of the c compilers that build NSS will barf on it.

b) I still don't know what "redefine ERROR to anything" means.
#define ERROR anything 
?  At a minimum, a better comment is needed.  

c) I really hope there is a better solution to this problem,
one that doesn't require peppering NSS with that #define.
I'd try to offer a better solution, but I still don't entirely
understand the problem.  

5. This seems to say "if wince or not wince".
To be or not to be? 

>diff --git a/security/nss/lib/ssl/win32err.c b/security/nss/lib/ssl/win32err.c
>--- a/security/nss/lib/ssl/win32err.c
>+++ b/security/nss/lib/ssl/win32err.c
>@@ -44,7 +44,7 @@
>  * ***** END LICENSE BLOCK ***** */
> /* $Id: win32err.c,v 1.4 2004/04/27 23:04:39 gerv%gerv.net Exp $ */
> 
>-#if !defined(_WIN32_WCE)
>+#if !defined(_WIN32_WCE) || defined(WINCE)

Comment 40

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #39)
 
> 4. The patch makes the following change in several source files:
> 
> >+// This is because we redefine ERROR to anything via windows.h/shunt
> >+#ifdef WINCE
> >+#undef ERROR
> >+#define ERROR ERROR
> >+#endif

Doug, we really need to keep the shunt from causing us to add clutter to so many files. This is just sloppy. Use a different shunt for these files, one that doesn't pull in windows.h

Do these files need the shunt at all? or is the "shotgun" approach just adding the shunt unnecessarily?

Comment 41

[Doug Turner (:dougt)]

@mfinkle is it a work around until we fix 456788.

@nelson  I will fix up the patch.

Comment 42

[Doug Turner (:dougt)]

1) Yes.  OS_DLLFLAGS  += -DLL  is required because we need to pass "-DLL" to any link that is intented to be a dll.  This was also in the WINCE3.0.mk coreconf file.

2) Ted reviewed it (said it was okay, if you were okay with it).  I am cc'ing Kaie as well.

3) I have enabled capi.  thanks for spotting that.

4) Yes, as I mentioned above, it is a work around until 456788 is fixed.  But I think mfinkle is right.  we should address this problem now.

Comment 43

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 344027 [details] [diff] [review]
Use of RC_INVOKED in nss.h

looks like this will be OK.

Comment 44

[Doug Turner (:dougt)]

nelson/ted, how do we get this checked in?

Comment 45

[Nelson Bolyard (seldom reads bugmail)]

Please note: henceforth, I'm only going to review cvs diffs.  
I can't apply these git diffs to my cvs tree without a bunch of work 
that I shouldn't need to do.

Comment 46

[Doug Turner (:dougt)]

@nelson  fwiw, given a patch that has:

b/security/nss/lib/nss/nss.h

you could cd into the directory where nss.h is, and run

patch -p5 < diff.txt

Comment 47

[Doug Turner (:dougt)]

Attachment: patch v.3

this is a comprehensive patch that allows us to build on windows mobile.

GetProcAddress is now explicitly GetProcAddressA

Comment 48

[(not currently active) Ted Mielczarek]

Comment on attachment 344175 [details] [diff] [review]
nss_build_changes v.2

Hopefully you weren't blocked on me here, but I can't review NSS changes anyway.

Comment 49

[Doug Turner (:dougt)]

Comment on attachment 346493 [details] [diff] [review]
patch v.3

ignore the change to GetProcAddressA.

Comment 50

[Nelson Bolyard (seldom reads bugmail)]

This bug got to the head of my patch review queue yesterday, and POOF, the 
patch review request disappeared.  My queue is very short right now, so ...

Comment 51

[Doug Turner (:dougt)]

Attachment: patch v.3

thanks!

Comment 52

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 346934 [details] [diff] [review]
patch v.3

This review applies to all parts of this patch except one.
I did not review the diffs for security/manager/Makefile.in
My review applies to the parts that are in NSS.

r+, with one requested change and a comment.

1. In sslmutex.c and sslsnce.c, you removed "&& !defined(_WIN32_WCE)" from a #if.
Evidently, that code, which formerly did not build for wince, now does.
But in win32err.c, instead of removing that same #if, you changed it as:

>-#if !defined(_WIN32_WCE)
>+#if !defined(_WIN32_WCE) || defined(WINCE)

That still reads to me like "if (A || !A)".  Am I right that it means that?
I think you probably just want to eliminate that #if and its matching #endif.

2. In comment 42 you wrote:
> 3) I have enabled capi.  thanks for spotting that.

but this patch still disables the CAPI stuff.  
Well, if you want to build CAPI or not, that's your business. 
I just want to be sure that the part of this patch change changes
security/nss/lib/ckfw/Makefile is not accidental.

Comment 53

[Doug Turner (:dougt)]

win32err.c:

We can safely remove the ifdef completely.  There is NO need to ifdef for the wince platform.


regarding the CAPI stuff -- there were some link errors:

cobject.obj : error LNK2019: unresolved external symbol RpcStringFree referenced in function ckcapi_getContai
cobject.obj : error LNK2019: unresolved external symbol UuidToString referenced in function ckcapi_getContain
cobject.obj : error LNK2019: unresolved external symbol UuidCreate referenced in function ckcapi_getContainer

I avoided including it as I am not sure there is even a need yet.  It is easy enough to reinclude at some later point.


Thanks for the reviews, Nelson!

Comment 55

[Doug Turner (:dougt)]

Attachment: patch v.4

same as v.3, but i have removed the #ifdefs from win32err.c relating to wince.

Comment 56

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 347451 [details] [diff] [review]
patch v.4

r=nelson for all parts except the patch to security/manager/Makefile.in
which I did not review because it's outside of NSS.  

I have checked this in (except for security/manager/Makefile.in)
I will shortly attach the CVS diff for the patch I committed.

Bug 454120: Problems Compiling SECURITY module for WinMobile
Patch by Doug Turner <[e-mail]>, r=nelson

coreconf/WINCE.mk;      new revision: 1.4;  previous revision: 1.3
coreconf/config.mk;     new revision: 1.25; previous revision: 1.24
nss/lib/ckfw/Makefile;  new revision: 1.15; previous revision: 1.14
nss/lib/nss/nss.h;      new revision: 1.63; previous revision: 1.62
nss/lib/ssl/sslmutex.c; new revision: 1.21; previous revision: 1.20
nss/lib/ssl/sslsnce.c;  new revision: 1.47; previous revision: 1.46
nss/lib/ssl/win32err.c; new revision: 1.5;  previous revision: 1.4

Comment 57

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v.4 as checked in (a cvs diff)

Comment 58

[Wan-Teh Chang]

Attachment: Minor tweaks

WINCE.mk now has "OS_DLLFLAGS  += -DLL", so we can remove that
from WINCE3.0.mk, which includes WINCE.mk.

Comment 59

[Nelson Bolyard (seldom reads bugmail)]

Wan-Teh, It's not clear to me whether Dougt does or does not want 
WinCE builds to include the bridge-to-CAPI pkcs11 module.  

Also, what all versions of WinCE exist now?  
Should the +DLL line be in a new WINCE4.0.mk?  or a WINCE5.0.mk? 
I can't answer these questions because I don't work on WinCE any more.
This is a case where we need someone who is more up to date on WinCE 
than I am, and who understands the coreconf files structure to advise
on where best to put that +DLL line.

Comment 60

[Wan-Teh Chang]

Comment on attachment 349344 [details] [diff] [review]
Minor tweaks

Doug, could you review and test this patch?  Thanks.

WINCE3.0.mk: remove a line that's now redundant (was just added
to WINCE.mk, which WINCE3.0 includes).

lib/ckfw/Makefile: prevent the building of the 'capi' directory
at the right place.

Comment 61

[John Wolfe (:wolfe)]

As for versions of Windows CE / Windows Mobile, there is a lot of potential confusion.  Right now, all WinCE efforts are actually to get the code base running within Windows Mobile 6 Professional devices (touch screen capable Windows Mobile 6 handsets).

To further confuse things, WinMobile 6 is based upon WinCE 5.x - different WinCE minor versions apply to WinMobile 6.0 and WinMobile 6.1.

Finally, I do not believe anyone has an interest in making Windows Mobile 5 (or earlier) work with the code base.  Likewise, I believe that no one would want to make the code base work with a Windows CE version earlier than 5 (i.e., 4.2, 4.1, 4.0, 3.x, etc.)

This means, for accuracy, that we should probably rename the WinCE3.0.mk to be WinCE5.0.mk.

At some later date, if there is enough interest in making a distinction between Windows Mobile builds and Windows CE builds, then a lot of work will need to be done with any WINCE and _WIN32_WCE defines.

For right now, all our efforts are focused on getting a WinMobile 6 build that works at all.

Comment 62

[Wan-Teh Chang]

Comment on attachment 349344 [details] [diff] [review]
Minor tweaks

Nelson's comment 59 and John's comment 61 on WINCE5.0.mk made
me realize that you are now using just WINCE.mk for all versions
of WINCE, without using the version-specific WINCEx.y.mk files.
(We do this for the platforms listed in TARGET_OSES.)

So I will attach a new patch that cvs removes WINCE3.0.mk.

Comment 63

[Wan-Teh Chang]

Attachment: Minor tweaks for makefiles (checked in)

Comment 64

[Wan-Teh Chang]

Attachment: Minor tweaks for lib/ssl (checked in)

Remove parentheses that are no longer necessary after "&& !defined(_WIN32_WCE)"
was removed.

Comment 65

[Wan-Teh Chang]

Comment on attachment 349443 [details] [diff] [review]
Minor tweaks for makefiles (checked in)

Doug, John, you may want to see if there's anything else you can salvage from
WINCE3.0.mk.  (Probably not, because you aren't using it.)

Comment 66

[Wan-Teh Chang]

Comment on attachment 349445 [details] [diff] [review]
Minor tweaks for lib/ssl (checked in)

Checked in on the NSS trunk (NSS 3.12.3).

Checking in sslmutex.c;
/cvsroot/mozilla/security/nss/lib/ssl/sslmutex.c,v  <--  sslmutex.c
new revision: 1.22; previous revision: 1.21
done
Checking in sslsnce.c;
/cvsroot/mozilla/security/nss/lib/ssl/sslsnce.c,v  <--  sslsnce.c
new revision: 1.48; previous revision: 1.47
done

Comment 67

[Doug Turner (:dougt)]

CAPI on windows mobile does not compile.  See comment #53.  I filed bug 466215 to track this issue.

Comment 68

[Nelson Bolyard (seldom reads bugmail)]

Doug, there is a patch attached to this bug, awaiting your review, for the
issue about which you just filed bug 466215

Comment 69

[(not currently active) Ted Mielczarek]

Comment on attachment 347451 [details] [diff] [review]
patch v.4

+	CC="arm-wince-gcc.exe" \
+	CCC="arm-wince-gcc.exe" \

Doesn't our build system set CC/CXX properly and you could just persist those values? Gonna make life harder for crowder if you hardcode values here.

r=me on the security/manager bits.

Comment 70

[Brian Crowder]

Ted:  configure.in at the top level doesn't set them right, but it should, I'll roll this part back when my stuff is done (and already have locally).

Comment 71

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 344027 [details] [diff] [review]
Use of RC_INVOKED in nss.h

This patch was superseded, and was never committed.

Comment 72

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 347451 [details] [diff] [review]
patch v.4

This is not the patch that was committed.  The patch that was committed is the next one below this one.

Comment 73

[Doug Turner (:dougt)]

Comment on attachment 349443 [details] [diff] [review]
Minor tweaks for makefiles (checked in)

this patch, when applied to the windows mobile build, does not cause any problems.

r=dougt

Comment 74

[Wan-Teh Chang]

Comment on attachment 349443 [details] [diff] [review]
Minor tweaks for makefiles (checked in)

Doug, all the patches in this bug have been checked in.
I think you can mark the bug fixed now.

I checked in this patch on the NSS trunk (NSS 3.12.3).

Removing coreconf/WINCE3.0.mk;
/cvsroot/mozilla/security/coreconf/WINCE3.0.mk,v  <--  WINCE3.0.mk
new revision: delete; previous revision: 1.5
done
Checking in nss/lib/ckfw/Makefile;
/cvsroot/mozilla/security/nss/lib/ckfw/Makefile,v  <--  Makefile
new revision: 1.16; previous revision: 1.15
done

Comment 75

[Doug Turner (:dougt)]

Attachment: patch v.5 -- mozilla/security makefile change

Comment 76

[(not currently active) Ted Mielczarek]

Comment on attachment 351511 [details] [diff] [review]
patch v.5 -- mozilla/security makefile change

Is this any different from the previous patch that I already gave r+? Also, note that only security/manager/Makefile.in is part of the mozilla build system, the rest is still NSS.

Comment 77

[Doug Turner (:dougt)]

Comment on attachment 351511 [details] [diff] [review]
patch v.5 -- mozilla/security makefile change

this is exactly the same as the previous patch that you already reviewed.  I teased it out of the previous patch so that we could point to what we are going to check in.

but i goof'ed.  ignore that change to security/nss/lib/ckfw/Makefile

thanks for the r+

Comment 78

[Doug Turner (:dougt)]

Comment on attachment 351511 [details] [diff] [review]
patch v.5 -- mozilla/security makefile change

double goof.  i promise not to attach patches while drinking.

The only thing that ted reviewed that matters for the check in is the change to :

security/manager/Makefile.in

Comment 79

[Doug Turner (:dougt)]

patch 351511 has been checked in: 555e338a03ca

everything has been checked in now.  Marking fixed.

Comment 80

[Joel Maher ( :jmaher ) (UTC -8)]

verified with alpha 3...we can build;)