Closed Bug 469083 Opened 12 years ago Closed 12 years ago

WinCE NSPRPUB PR Tests Do Not Run On WinCE/WinMobile6

Categories

(NSPR :: NSPR, enhancement, P1)

ARM
Windows Mobile 6 Professional
enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wolfe, Assigned: wolfe)

Details

(Keywords: mobile)

Attachments

(3 files, 7 obsolete files)

Attached patch v1.0 patch (obsolete) — Splinter Review
WinCE and WinMobile6 do not have a built-in command shell with pipes, so the configuration of NSPRPUB's PR Tests has no chance of working in an automated fashion under WinCE/WinMobile6.

In order to make WinMobile6 Tests function at all, work has to be done to massage the tests into compiling and running under WinCE/WinMobile6.

Finally, WinCE/WinMobile6 swallows crashes that happen in isolated executables, returning a zero even when the individual test applications completely crashed on the device.

Also, in order to have the tests have a chance of being automated, the structure of the tests has to change from lots of independent executables.

Finally, once the tests are running, we can file bugs against the WinCE/WinMobile6 implementation of NSPRPUB.
john,what is the current status of this and the status of nspr tests on windows ce?
Attached patch v2.0 patch (obsolete) — Splinter Review
Updated version to test new ASM changes to XPCOM.  

Started to revise the tests to NOT printf when no error.  

Need to finish revising tests and have tests return 0 for no error, or some error code.  Then need to have error code propogate out as an exit code - so scripts can identify when these tests fail.  

Once finished, these tests can become part of the daily unit testing, if desired.

For now, these tests are WinCE specific because of printf arguments for outputting PR_uint64 values (i.e., "%I64d" instead of "%lld" that everyone else uses!
Attachment #352491 - Attachment is obsolete: true
Attached patch v3.0 patch (obsolete) — Splinter Review
Oops - I worked on the wrong bug for the v2.0 patch on this bug.  Sorry for the confusion.

Here is the latest patch for updating the NSPRPUB tests to run under WinMobile as a single test with multiple sub-tests, each sub-test is its own DLL, loaded by the main testing application.

This is nasty manipulation of the files in nsprpub/pr/tests - but the manipulation keeps WinCE changes limited to WinCE builds, and should not affect other tests being run for any other platforms.

This patch applies cleanly to mozilla-central repository as of 12-Feb-2009, and takes a listing of tests to run from the nsprpub/pr/tests/runtests.sh shell script.
Attachment #362063 - Attachment is obsolete: true
Comment on attachment 362076 [details] [diff] [review]
v3.0 patch

John, 

For what bug was that other patch intended?
I have some comments about that patch, and I would rather attach 
them to the appropriate bug.

This most recent patch adds a #include for a file nst_wince.h 
which is not already present in NSPR source code, and is not 
added by this patch.  Where is that file?
Comment on attachment 362076 [details] [diff] [review]
v3.0 patch

John, thanks for the patch.  We should try very hard to come
up with a patch that doesn't require decorating the main
function of every test program with WINCE_EXPORT.  I wonder
if we can add something like -Dmain="WINCE_EXPORT main" on the
compiler command line.

Also, if I have to standardize on the return type of the main
function, I'd choose int over PRIntn.  PRIntn is just a
synonym of int.  I never understood why we need the PRIntn
typedef.  (Brendan Eich explained it to me before, but I
didn't get it.)  So it's better to just use the standard C
int type.
Attached patch v4.0 patch (obsolete) — Splinter Review
Nelson, the bogus patch here was intended for Bug 478268 - XPCOM Proxy Tests need to be expanded and improved, which had not been created by the time I replaced the bogus v2.0 patch with the correct v3.0 patch.

Sorry for the confusion.

Wan-Teh, this is a better patch which standardizes on a main function with this prototype:

int main(int argc, char **argv);

Some fo the file changes are simple formatting changes - all to bring the main fnction declaration into that form on one line.  

I also do your requested change by defining "main" inside the nst_wince.h header file.
Attachment #362076 - Attachment is obsolete: true
Attachment #362265 - Flags: review?(wtc)
Comment on attachment 362265 [details] [diff] [review]
v4.0 patch

In nsprpub/pr/tests/Makefile.in, you have

>+  CFLAGS += -Dmain="WINCE_EXPORT main"

I think this can be removed now because of your handling of
"main" in nst_wince.h.
Let me try restating my second question from comment 4:

I tried to review that patch, but didn't get far, because the patch adds
a #include of a file that I could not find anywhere, not in the patch,
and not already checked in to NSPR on the CVS trunk.  So, I view the patch
as incomplete, and worthy of r-, until someone provides that file.
Also, please provide NSS and NSPR patches as CVS diffs against the CVS trunk,
not as Hg patches.  That applies to all NSS and all NSPR patches, all the time.
Attached patch v5.0 patch - cvs diff this time (obsolete) — Splinter Review
Nelson and Wan-Teh, I apologize for not putting this in CVS diff 
format originally.  I had thought the CVS diff requirement was only 
for security, not also for nsprpub.  

I will remember a CVS format diff in the future.

This modified patch is in CVS format against tag NSPR_4_7_3_RTM 

This patch also removes the commented out -Dmain="..." from Makefile.in

This patch definitely includes new "nst_wince.h" and "wince_tester.cpp" 
files.  These files had to be added "by hand" into my local CVS copy, 
and so may have some strange-ness about them when you apply this patch.

Finally, please take a close look at the changes to the y2ktmo.c test, 
as this was the ugliest patch of the whole lot.  Any feedback or 
suggestions of how to do this better would be greatly appreciated.
Attachment #362265 - Attachment is obsolete: true
Attachment #362441 - Flags: review?(wtc)
Attachment #362265 - Flags: review?(wtc)
Thanks very much for the CVS diff!  
The patch v5.0 did not apply cleanly to the NSS trunk.  
This is the same patch, after applying it to the trunk.  
I used cvs diff -Npu which generated a smaller patch.
Bonsai's patch reader can add context as desired.

I will add some review comments shortly.
Comment on attachment 362441 [details] [diff] [review]
v5.0 patch - cvs diff this time

obsoleted by newer patch.
Attachment #362441 - Attachment is obsolete: true
Attachment #362441 - Flags: review?(wtc)
Attachment #362459 - Attachment description: v 5.1 patch - CVS diff applies cleanly to CVS trunk tip → v 5.1 patch - John's CVS diff applied cleanly to CVS trunk tip
Attachment #362459 - Flags: superreview?(wtc)
Attachment #362459 - Flags: review?(nelson)
Comment on attachment 362459 [details] [diff] [review]
v 5.1 patch - John's CVS diff applied cleanly to CVS trunk tip

This patch looks very good on the whole.  
I have reviewed all of it except these files: 
  Makefile.in, wince_test.cpp & testfile.c
I'd like Wan-Teh to also review the changes to those 3 files,
and to nst_wince.h

I'd give r+ to nearly all of what I have reviewed so far. 
I have a few comments/observations.

1. #including nspr.h.  
In quite a few files, you added a #include "nspr.h".  I gather thatn the new
header file nst_wince.h needs something defined by nspr.h, but I'm not sure
what.  I noticed PRIntN in nst_wince.h, and I suspect it can (and should) 
just use int in those places instead.  I think that if nst_wince.h can 
trivially be made not to require any new nspr headers, it should.  If not, 
then it should #include the headers it needs, rather than adding #includes 
for those headers to each file that #includes it. It would be best, IMO,
to includes just the minimum set of nspr headers needed, rather than nspr.h
which pretty much includes every nspr header file.

2. file selintr.c #includes nst_wince.h twice, 3 lines apart.  
I think one of these must have been a mistake.  

3. At least two files (tmocon.c & thrpool_server.c) contain their own 
implementation of getcwd, implemented in line, rather than as a function.  
I'd prefer that this be implemented as a function or macro named getcwd in 
nst_wince.h, so that every individual call to getcwd in the test programs 
need not be conditionally replaced.  That's trivial to do.  

4. the patch to y2ktmo.c conains this variable definition:

static start_time_tick;

that's implicitly an int declaration, I believe.  Please make it explicitly 
be a PRInt32.  There is a block of code that is replicated in numerous places
in that file, and your patch modifies each of those replications.  
That modification seems reasonable.  If we really wanted to clean this up and
make it more maintainable, I suppose we'd move that replicated code into a 
macro or subroutine function.  That would be nice but is not required.

5. Please remove new commented-out lines, such as lines 55-59 and 63-66 of 
nst_wince.h

6. Seems like this new code in nst_wince.h:

+int WINAPI nspr_test_runme(PRIntn argc, char **argv)
+{
+  return main(argc, argv);
+}
+/* NOW, take care of all the other main() functions out there */
+#define main __declspec(dllexport) WINAPI main

can be collapsed into this:

+#define main __declspec(dllexport) WINAPI nspr_test_runme 

Doing so may also obviate the prototype on line 80. (?)
Attached patch v6.0 patch - cvs diff (obsolete) — Splinter Review
Updated v5.1 patch - fixed several comments from Nelson, applies cleanly against HEAD as of 16-Feb-09, 9pm East Coast Time.  At least, I think I checked out the HEAD this time ("cvs co -r HEAD *" from nsprpub/pr/tests directory).



(In reply to comment #13)
> (From update of attachment 362459 [details] [diff] [review])
> 1. #including nspr.h.  
> In quite a few files, you added a #include "nspr.h".  I gather thatn the new
> header file nst_wince.h needs something defined by nspr.h, but I'm not sure
> what.  I noticed PRIntN in nst_wince.h, and I suspect it can (and should) 
> just use int in those places instead.  I think that if nst_wince.h can 
> trivially be made not to require any new nspr headers, it should.  If not, 
> then it should #include the headers it needs, rather than adding #includes 
> for those headers to each file that #includes it. It would be best, IMO,
> to includes just the minimum set of nspr headers needed, rather than nspr.h
> which pretty much includes every nspr header file.

DONE - replaced PRIntN with int inside nst_wince.h, removed extra '#include "nspr.h"' lines I had added.  I left the existing #includes of nspr.h alone - figuring that if it is not broken, then don't fix it. 
 
> 2. file selintr.c #includes nst_wince.h twice, 3 lines apart.  
> I think one of these must have been a mistake.  

I will Not Have Fat Fingers.  I Will Not Have Fat Fingers.  
Removed extra #include of nst_wince.h

> 3. At least two files (tmocon.c & thrpool_server.c) contain their own 
> implementation of getcwd, implemented in line, rather than as a function.  
> I'd prefer that this be implemented as a function or macro named getcwd in 
> nst_wince.h, so that every individual call to getcwd in the test programs 
> need not be conditionally replaced.  That's trivial to do.  

Implementation of WINCE getcwd moved to macro within nst_wince.h.  Removed cast to VOID of return value from getcwd() function call - it was messing up the macro Nelson suggested.  Otherwise, that was a great optimization!

> 4. the patch to y2ktmo.c conains this variable definition:
> 
> static start_time_tick;
> 
> that's implicitly an int declaration, I believe.  Please make it explicitly 
> be a PRInt32.  There is a block of code that is replicated in numerous places
> in that file, and your patch modifies each of those replications.  
> That modification seems reasonable.  If we really wanted to clean this up and
> make it more maintainable, I suppose we'd move that replicated code into a 
> macro or subroutine function.  That would be nice but is not required.

Changed to 'static DWORD start_time_tick;' - OK because that code is ONLY hit for WINCE, and nst_wince.h includes <windows.h> for WINCE compiles, which defines DWORD type.

I had not moved this nasty code before now, in order to modify as little as possible in each file that I touched.  If you like, I can move this nasty code out to a macro at the top of the file.  Just let me know.

> 5. Please remove new commented-out lines, such as lines 55-59 and 63-66 of 
> nst_wince.h

Oops - sorry about that.  Commented out lines removed in nst_wince.h AND wince_tester.cpp.


> 6. Seems like this new code in nst_wince.h:
> can be collapsed into this:
> 
> +#define main __declspec(dllexport) WINAPI nspr_test_runme 

Done - thanks for the great suggestion.  It cleans up the header code quite a bit.  Also had to do a small modification to the wince_tester.cpp to look for nspr_test_runme() exported function instead of main() exported function.
Attachment #362459 - Attachment is obsolete: true
Attachment #362642 - Flags: superreview?(wtc)
Attachment #362642 - Flags: review?(nelson)
Attachment #362459 - Flags: superreview?(wtc)
Attachment #362459 - Flags: review?(nelson)
Comment on attachment 362642 [details] [diff] [review]
v6.0 patch - cvs diff

John, your patch v6 does not patch the file selintr.c.  Your patch v5 did patch that file.  It its omission from patch v6 intentional?  If not, please submit a new patch that also includes the changes to that file.
Comment on attachment 362642 [details] [diff] [review]
v6.0 patch - cvs diff

Please make these additional cleanup changes.

1. Please remove new lines of commented-out code, in Makefiles 
and in source files, e.g. 

>+#CSRCS +=            \
>+#    wince_tester.c  \
>+#    $(NULL)

>+#  EXE_EXTRA_LDOPTS = -SUBSYSTEM:CONSOLE -ENTRY:mainCRTStartup

>+/* #include "nspr.h" */

and the two new large blocks of code inside #if 0.

2. There's something wrong with one of the lines in the patch, 
as shown here;

>+  ifdef PROFILE
>+    LDOPTS += -PROFILE -MAP                                                                  
>+  endif # profile

I suspect that line is supposed to read:

>+    LDOPS += -PROFILE -MAP 

but for some reason, the line is broken between -PROFILE and -MAP, and there 
is a LARGE number of spaces after the -MAP.  Please fix that.

I think that when these changes are made and the missing patch to selintr.c is 
present, that version of the patch will be ready for r+.
Interesting.  When I quoted that odd line in the bug comment, it did not
appear broken.  I guess the whole problem is the large number of trailing
spaces.
I cleaned up this patch and committed it on the CVS trunk.

sproc_p.c; 	new revision: 3.6; previous revision: 3.5
stack.c;	new revision: 3.7; previous revision: 3.6
stat.c;		new revision: 3.7; previous revision: 3.6
stdio.c;	new revision: 3.6; previous revision: 3.5
str2addr.c;	new revision: 3.6; previous revision: 3.5
strod.c;	new revision: 3.8; previous revision: 3.7
suspend.c;	new revision: 3.8; previous revision: 3.7
switch.c;	new revision: 3.7; previous revision: 3.6
system.c;	new revision: 3.9; previous revision: 3.8
testbit.c;	new revision: 1.6; previous revision: 1.5
testfile.c;	new revision: 3.18; previous revision: 3.17
threads.c;	new revision: 3.6; previous revision: 3.5
thrpool_client.c; new revision: 3.7; previous revision: 3.6
thrpool_server.c; new revision: 3.9; previous revision: 3.8
thruput.c;	new revision: 3.9; previous revision: 3.8
time.c;		new revision: 3.8; previous revision: 3.7
timemac.c;	new revision: 3.6; previous revision: 3.5
timetest.c;	new revision: 3.10; previous revision: 3.9
tmoacc.c;	new revision: 3.9; previous revision: 3.8
tmocon.c;	new revision: 3.13; previous revision: 3.12
tpd.c;		new revision: 3.9; previous revision: 3.8
udpsrv.c;	new revision: 3.8; previous revision: 3.7
vercheck.c;	new revision: 1.26; previous revision: 1.25
version.c;	new revision: 3.8; previous revision: 3.7
wince_tester.cpp; initial revision: 3.1
writev.c;	new revision: 3.6; previous revision: 3.5
xnotify.c;	new revision: 3.6; previous revision: 3.5
y2k.c;		new revision: 3.8; previous revision: 3.7
y2ktmo.c;	new revision: 3.6; previous revision: 3.5
yield.c;	new revision: 3.8; previous revision: 3.7
zerolen.c;	new revision: 3.6; previous revision: 3.5
The checkin is done, and the security tinderbox stayed green, suggesting
no regressions on other platforms.  Of course, we don't have a tinderbox 
for WinMobile, so I will leave it up to the Fennec guys to decide if this
bug is now resolved.
Assignee: wtc → wolfe
Severity: normal → enhancement
Priority: -- → P1
Target Milestone: --- → 4.8
Attachment #362642 - Flags: superreview?(wtc)
Attachment #362642 - Flags: review?(nelson)
Attachment #362642 - Flags: review+
Attachment #362642 - Attachment is obsolete: true
John, could you use the /FI (Name Forced Include File) compiler
option to include "nst_wince.h"?  This way we don't need to have
every test program's .c file include "nst_wince.h".  It could be
as simple as adding

    INCLUDES += -FI nst_wince.h

or

    INCLUDES += -FInst_wince.h

for WINCE in nsprpub/pr/tests/Makefile.in.  See
http://msdn.microsoft.com/en-us/library/8c5ztk84.aspx

What does "nst" stand for?

The block comment at the beginning of nst_wince.h mentions
"WinMain" three times.  Should they be changed to "DllMain"?
Wan-Teh, does gcc have something equivalent to -FI ?  

This patch implements the changes you suggested, but I think it has the 
side effect of making it no longer possible to build with gcc for WinCE.
Attachment #363622 - Flags: review?(wtc)
By the way, I tested the above patch on WinXP, to see that it builds and 
successfully includes the desired file, with the following commands:
  
gmake OS_ARCH=WINCE OPTIMIZER="-DWINCE -DUNICODE" socket.i
gmake OS_ARCH=WINCE OPTIMIZER="-DWINCE -DUNICODE" -k
Attachment #363622 - Flags: review?(wtc) → review+
Comment on attachment 363622 [details] [diff] [review]
patch - implement WTC's suggestions (checked in)

r=wtc.  Thanks, Nelson.

>@@ -251,13 +251,13 @@ ifeq ($(OS_ARCH),WINCE)
> ifdef NS_USE_GCC
>   EXTRA_LIBS += -lwsock32
> else

I believe the NS_USE_GCC code for WINCE was incorrectly
copied from the WINNT code right above it.  We should
just remove the ifdef NS_USE_GCC code for WINCE.  The
ifdef NS_USE_GCC code for WINCE cannot possibly be
correct because it's still using -lwsock32 (as opposed
to -lws2) and missing -lcoredll.

>   # NOTE: Need this to have a proper WinCE entry into main(argc, argv)
>-#  EXE_EXTRA_LDOPTS = -SUBSYSTEM:CONSOLE -ENTRY:mainCRTStartup

The NOTE comment should also be removed.

In prpoll.c

>@@ -62,7 +62,6 @@
> #include <string.h>
> #include <stdlib.h>
> 
>-#include "nst_wince.h"
> 
> #ifdef WINCE

Nit: please also delete one blank line.  Please do the same
to the following files:

socket.c, sockopt.c, suspend.c, testfile.c, thrpool_client.c,
thrpool_server.c, thruput.c.
Comment on attachment 363622 [details] [diff] [review]
patch - implement WTC's suggestions (checked in)

Makefile.in;      new revision: 1.57; previous revision: 1.56
accept.c;         new revision: 3.13; previous revision: 3.12
acceptread.c;     new revision: 1.9; previous revision: 1.8
acceptreademu.c;  new revision: 3.6; previous revision: 3.5
addrstr.c;        new revision: 3.7; previous revision: 3.6
affinity.c;       new revision: 3.8; previous revision: 3.7
alarm.c;          new revision: 3.9; previous revision: 3.8
anonfm.c;         new revision: 3.9; previous revision: 3.8
append.c;         new revision: 1.4; previous revision: 1.3
atomic.c;         new revision: 3.12; previous revision: 3.11
attach.c;         new revision: 3.15; previous revision: 3.14
bigfile.c;        new revision: 3.10; previous revision: 3.9
bigfile2.c;       new revision: 3.8; previous revision: 3.7
bigfile3.c;       new revision: 3.8; previous revision: 3.7
bug1test.c;       new revision: 3.7; previous revision: 3.6
cleanup.c;        new revision: 3.7; previous revision: 3.6
cltsrv.c;         new revision: 3.17; previous revision: 3.16
concur.c;         new revision: 3.9; previous revision: 3.8
cvar.c;           new revision: 3.7; previous revision: 3.6
cvar2.c;          new revision: 3.10; previous revision: 3.9
dbmalloc.c;       new revision: 3.7; previous revision: 3.6
dbmalloc1.c;      new revision: 3.7; previous revision: 3.6
dceemu.c;         new revision: 3.8; previous revision: 3.7
depend.c;         new revision: 3.7; previous revision: 3.6
dlltest.c;        new revision: 3.9; previous revision: 3.8
dtoa.c;           new revision: 1.7; previous revision: 1.6
env.c;            new revision: 1.6; previous revision: 1.5
errcodes.c;       new revision: 3.7; previous revision: 3.6
errset.c;         new revision: 1.4; previous revision: 1.3
exit.c;           new revision: 3.7; previous revision: 3.6
fdcach.c;         new revision: 1.7; previous revision: 1.6
fileio.c;         new revision: 3.7; previous revision: 3.6
foreign.c;        new revision: 3.15; previous revision: 3.14
forktest.c;       new revision: 3.8; previous revision: 3.7
formattm.c;       new revision: 1.7; previous revision: 1.6
freeif.c;         new revision: 3.8; previous revision: 3.7
fsync.c;          new revision: 3.8; previous revision: 3.7
getai.c;          new revision: 1.5; previous revision: 1.4
gethost.c;        new revision: 3.10; previous revision: 3.9
getproto.c;       new revision: 3.7; previous revision: 3.6
i2l.c;            new revision: 3.7; previous revision: 3.6
initclk.c;        new revision: 3.9; previous revision: 3.8
inrval.c;         new revision: 3.8; previous revision: 3.7
instrumt.c;       new revision: 3.10; previous revision: 3.9
intrio.c;         new revision: 3.7; previous revision: 3.6
intrupt.c;        new revision: 3.10; previous revision: 3.9
io_timeout.c;     new revision: 3.11; previous revision: 3.10
io_timeoutk.c;    new revision: 3.7; previous revision: 3.6
io_timeoutu.c;    new revision: 3.7; previous revision: 3.6
ioconthr.c;       new revision: 3.8; previous revision: 3.7
ipv6.c;           new revision: 3.14; previous revision: 3.13
join.c;           new revision: 3.9; previous revision: 3.8
joinkk.c;         new revision: 3.8; previous revision: 3.7
joinku.c;         new revision: 3.7; previous revision: 3.6
joinuk.c;         new revision: 3.7; previous revision: 3.6
joinuu.c;         new revision: 3.7; previous revision: 3.6
layer.c;          new revision: 3.13; previous revision: 3.12
lazyinit.c;       new revision: 3.7; previous revision: 3.6
libfilename.c;    new revision: 1.4; previous revision: 1.3
lltest.c;         new revision: 3.8; previous revision: 3.7
lock.c;           new revision: 3.8; previous revision: 3.7
lockfile.c;       new revision: 3.8; previous revision: 3.7
logger.c;         new revision: 3.9; previous revision: 3.8
makedir.c;        new revision: 3.7; previous revision: 3.6
many_cv.c;        new revision: 3.10; previous revision: 3.9
mbcs.c;           new revision: 3.6; previous revision: 3.5
multiacc.c;       new revision: 3.9; previous revision: 3.8
multiwait.c;      new revision: 3.10; previous revision: 3.9
nameshm1.c;       new revision: 3.6; previous revision: 3.5
nbconn.c;         new revision: 3.10; previous revision: 3.9
nblayer.c;        new revision: 1.12; previous revision: 1.11
nonblock.c;       new revision: 3.10; previous revision: 3.9
nst_wince.h;      new revision: 3.2; previous revision: 3.1
ntioto.c;         new revision: 1.9; previous revision: 1.8
ntoh.c;           new revision: 3.7; previous revision: 3.6
obsints.c;        new revision: 3.7; previous revision: 3.6
op_2long.c;       new revision: 3.10; previous revision: 3.9
op_excl.c;        new revision: 1.7; previous revision: 1.6
op_filnf.c;       new revision: 3.8; previous revision: 3.7
op_filok.c;       new revision: 3.12; previous revision: 3.11
op_noacc.c;       new revision: 3.8; previous revision: 3.7
op_nofil.c;       new revision: 3.8; previous revision: 3.7
openfile.c;       new revision: 3.7; previous revision: 3.6
parent.c;         new revision: 3.12; previous revision: 3.11
peek.c;           new revision: 3.8; previous revision: 3.7
perf.c;           new revision: 3.9; previous revision: 3.8
pipeping.c;       new revision: 3.11; previous revision: 3.10
pipeping2.c;      new revision: 3.7; previous revision: 3.6
pipepong.c;       new revision: 3.9; previous revision: 3.8
pipepong2.c;      new revision: 3.7; previous revision: 3.6
pipeself.c;       new revision: 3.10; previous revision: 3.9
poll_er.c;        new revision: 3.8; previous revision: 3.7
poll_nm.c;        new revision: 3.11; previous revision: 3.10
poll_to.c;        new revision: 3.10; previous revision: 3.9
pollable.c;       new revision: 3.8; previous revision: 3.7
prftest.c;        new revision: 3.8; previous revision: 3.7
prftest1.c;       new revision: 3.7; previous revision: 3.6
prftest2.c;       new revision: 3.8; previous revision: 3.7
primblok.c;       new revision: 3.7; previous revision: 3.6
priotest.c;       new revision: 3.9; previous revision: 3.8
provider.c;       new revision: 3.18; previous revision: 3.17
prpoll.c;         new revision: 3.14; previous revision: 3.13
prpollml.c;       new revision: 3.9; previous revision: 3.8
prselect.c;       new revision: 3.7; previous revision: 3.6
randseed.c;       new revision: 1.7; previous revision: 1.6
ranfile.c;        new revision: 3.9; previous revision: 3.8
rmdir.c;          new revision: 1.4; previous revision: 1.3
rwlocktest.c;     new revision: 1.7; previous revision: 1.6
sel_spd.c;        new revision: 3.16; previous revision: 3.15
selct_er.c;       new revision: 3.8; previous revision: 3.7
selct_nm.c;       new revision: 3.9; previous revision: 3.8
selct_to.c;       new revision: 3.9; previous revision: 3.8
select2.c;        new revision: 3.11; previous revision: 3.10
selintr.c;        new revision: 3.7; previous revision: 3.6
sem.c;            new revision: 3.7; previous revision: 3.6
sema.c;           new revision: 3.8; previous revision: 3.7
semaerr.c;        new revision: 3.7; previous revision: 3.6
semaerr1.c;       new revision: 3.7; previous revision: 3.6
semaping.c;       new revision: 3.9; previous revision: 3.8
semapong.c;       new revision: 3.9; previous revision: 3.8
sendzlf.c;        new revision: 3.7; previous revision: 3.6
server_test.c;    new revision: 3.12; previous revision: 3.11
servr_kk.c;       new revision: 3.14; previous revision: 3.13
servr_ku.c;       new revision: 3.13; previous revision: 3.12
servr_uk.c;       new revision: 3.13; previous revision: 3.12
servr_uu.c;       new revision: 3.13; previous revision: 3.12
short_thread.c;   new revision: 1.7; previous revision: 1.6
sigpipe.c;        new revision: 3.11; previous revision: 3.10
sleep.c;          new revision: 3.10; previous revision: 3.9
socket.c;         new revision: 3.20; previous revision: 3.19
sockopt.c;        new revision: 3.14; previous revision: 3.13
sockping.c;       new revision: 3.11; previous revision: 3.10
sockpong.c;       new revision: 3.9; previous revision: 3.8
sprintf.c;        new revision: 3.7; previous revision: 3.6
sproc_ch.c;       new revision: 3.7; previous revision: 3.6
sproc_p.c;        new revision: 3.7; previous revision: 3.6
stack.c;          new revision: 3.8; previous revision: 3.7
stat.c;           new revision: 3.8; previous revision: 3.7
stdio.c;          new revision: 3.7; previous revision: 3.6
str2addr.c;       new revision: 3.7; previous revision: 3.6
strod.c;          new revision: 3.9; previous revision: 3.8
suspend.c;        new revision: 3.9; previous revision: 3.8
switch.c;         new revision: 3.8; previous revision: 3.7
system.c;         new revision: 3.10; previous revision: 3.9
testbit.c;        new revision: 1.7; previous revision: 1.6
testfile.c;       new revision: 3.19; previous revision: 3.18
threads.c;        new revision: 3.7; previous revision: 3.6
thrpool_client.c; new revision: 3.8; previous revision: 3.7
thrpool_server.c; new revision: 3.10; previous revision: 3.9
thruput.c;        new revision: 3.10; previous revision: 3.9
time.c;           new revision: 3.9; previous revision: 3.8
timemac.c;        new revision: 3.7; previous revision: 3.6
timetest.c;       new revision: 3.11; previous revision: 3.10
tmoacc.c;         new revision: 3.10; previous revision: 3.9
tmocon.c;         new revision: 3.14; previous revision: 3.13
tpd.c;            new revision: 3.10; previous revision: 3.9
udpsrv.c;         new revision: 3.9; previous revision: 3.8
vercheck.c;       new revision: 1.27; previous revision: 1.26
version.c;        new revision: 3.9; previous revision: 3.8
writev.c;         new revision: 3.7; previous revision: 3.6
xnotify.c;        new revision: 3.7; previous revision: 3.6
y2k.c;            new revision: 3.9; previous revision: 3.8
y2ktmo.c;         new revision: 3.7; previous revision: 3.6
yield.c;          new revision: 3.9; previous revision: 3.8
zerolen.c;        new revision: 3.7; previous revision: 3.6

OK, so, once again, I leave it up to the Fennec guys to tell us
if this bug is finally resolved.
Attachment #363622 - Attachment description: patch - implement WTC's suggestions → patch - implement WTC's suggestions (checked in)
I thought of one minor disadvantage of using the /FI option to include 
nst_wince.h rather than using #include in each file.  That is, the mkdepend
program will not record that dependency.  The workaround/solution is to 
put an explicit dependency into the Makefile for all .obj files, to make 
them depend on nst_wince.h.
Nelson, do you feel strongly enough about this for me to create a new patch to remove the #include "nst_wince.h" lines from all the source files and modify the Makefile.in appropriately?

I will do it - I just want to make sure that you are really serious about wanting this change.
John,  I already did that, and committed it.  See comment 25 for the checkin.
I am marking this bug resolved fixed.  
Fennec folks are welcome to reopen it if they have a specific issue.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Implement Nelson's suggestion in comment 26:
Add an explicit dependency into Makefile.in for all .obj files,
to make them depend on nst_wince.h.
Attachment #364475 - Flags: review?(nelson)
Attachment #364475 - Flags: review?(nelson) → review+
Comment on attachment 364475 [details] [diff] [review]
Make all .obj files in pr/tests depend on nst_wince.h

r=nelson
I'm reopening this bug because it has an r+ patch waiting to be committed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I checked in that patch (attachment 364475 [details] [diff] [review]) on the NSPR trunk
(NSPR 4.8).

Checking in Makefile.in;
/cvsroot/mozilla/nsprpub/pr/tests/Makefile.in,v  <--  Makefile.in
new revision: 1.59; previous revision: 1.58
done
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.