38061 - mozilla/include obsolete? If so, cvs remove it

Status: VERIFIED FIXED

(SeaMonkey :: Build Config, defect, P3)

Description

[Norris Boyd]

Is the mozilla/include directory obsolete? I believe it is a remnant from 
Mozilla Classic. Certainly a lxr search on allxpstr indicates that it is not 
used.

Comment 1

[cls]

You just don't know how much I would enjoy that....but there is still some code
that uses at least nscore.h.  We're probably going to have to cvs remove a file
at a time to keep people from backsliding.

Comment 2

[cls]

Attachment: Preliminary pruning of mozilla/include header usage

Comment 3

[cls]

The attached patch shows that we only use about 18 of the 101 header files in
mozilla/include .  In my previous comment, I meant xp_core.h, not nscore.h. 
xp_core.h contains many of the standard defines (XP_WIN, XP_MAC,
XP_BEGIN_PROTOS) so I'm not sure if we'll ever be able to get rid of it.  Of the
remaining headers, a few of them were trimmed (via #ifdef 0) to remove unused
functions and/or to stop them from dragging in other header dependencies.
mailnews/mime also suffered some damage as it still had functions that took
MWContext as an argument.

The patch works under linux.  Still needs to be tested under windows, mac & on
the commercial side. 

Comment 4

[Mike Kaply [:mkaply]]

I would just like to say for the record is that the reason I see a lot of people 
including xp_core.h is because XP_WIN is NOT defined by the windows build.

People include xp_core.h so they can have XP_WIN.

Getting rid of xp_core.h and making people do stuff the right way would go a 
long way toward making the build more cross platform.

Comment 5

[Brendan Eich [:brendan]]

I say we should move the minimum amount of junk from xp_core.h to 
xpcom/base/nscore.h or some such place.  How hard can that be?  Then back off 
and nuke mozilla/include from orbit (it's the only way to be sure).

/be

Comment 6

[cls]

With only 1st degree burns to the tree, an updated version of the patch has been
checked in.  On win32 & linux, the current mozilla/include files which are still
being used:

                csid.h 
                intl_csi.h 
                libi18n.h 
                merrors.h 
                minicom.h 
                msgcom.h 
                msgtypes.h 
                net.h 
                npapi.h 
                npupp.h 
                ntypes.h 
                platform.h 
                xlate.h 
                xp_core.h 
                xp_file.h 
                xp_mem.h 
                xp_path.h 
                xp_regexp.h 
                xp_str.h 

include/MANIFEST contains around 100 files.  The only major difference in the
mac include set are the ifdef XP_MAC chunks in include/net.h.  Like mkaply
pointed out, a good chunk of xp_core.h ifdefs can probably be removed just by
adding -DXP_WIN to config/config.mak.

Comment 7

[Brendan Eich [:brendan]]

If someone has done the legwork to find out what other files include these old 
decrepit headers, please summarize here.  Otherwise I'll go crazy with lxr.

/be

Comment 8

[cls]

*** Bug 45531 has been marked as a duplicate of this bug. ***

Comment 9

[Axel Hecht]

Hi,
I just had a look at include/intl_csi.h.
It is only used for three functions, all implemented in
mailnews/mime/src/oldi18n.c
Should the functions INTL_GetCSIDocCSID, INTL_SetCSIDocCSID and
LO_GetDocumentCharacterSetInfo get moved to mailnews/mime/src?
Those functions are only accessed from mailnews/mime/src as well.

Axel

Comment 10

[Axel Hecht]

Oh, darn spammers.
me got confused, I had this done, and didn't realize.
Patch to oldi18n.c get's attached, please test on other platforms, this was 
tested on solaris.
This should make intl_csi.h go ^.

Axel

Comment 11

[Axel Hecht]

Attachment: Patch to mozilla/mailnews/mime/src/oldi18n.c for intl_csi.h

Comment 12

[Axel Hecht]

Attachment: getting rid of intl_csi.h

Comment 13

[cls]

*** Bug 14004 has been marked as a duplicate of this bug. ***

Comment 14

[cls]

Can I get a mac buddy to verify that the mac can build if you remove all *.h
from mozilla/include except:

csid.h 
intl_csi.h 
libi18n.h 
merrors.h 
minicom.h 
msgcom.h 
msgtypes.h 
net.h 
npapi.h 
npupp.h 
ntypes.h 
platform.h 
shistele.h
xlate.h 
xp_core.h 
xp_file.h 
xp_list.h
xp_mem.h 
xp_path.h 
xp_regexp.h 
xp_str.h 

The test may be as simple as removing the other headers from
mozilla/include/MANIFEST and doing a clobber build.

Comment 15

[cls]

Thanks to bryner, we're down to 61 headers.  Definitely not going to make it for
M18, nominating for mozilla 0.9.

Comment 16

[Brian Ryner (not reading)]

Attachment: eliminate mailnews dependencies on msgcom.h and merrors.h

Comment 17

[Scott Collins]

looks good, bryner.  sr=scc

Comment 18

[Brendan Eich [:brendan]]

Cc'ing mailnews reviewers for r= and sr= on bryner's patch.

Who should review Axel's patch for intl_csi.h?

/be

Comment 19

[Brian Ryner (not reading)]

brendan: This patch was actually already checked in, sorry for not getting sr=
marked in the bug.  Also, the intl_csi.h patch is no longer relevant because
jgmyers has removed the file entirely.

Comment 20

[(not reading, please use [e-mail] instead)]

note to brendan, I've been reviewing bryner's mailnews patches.

Comment 21

[Brian Ryner (not reading)]

Attachment: Removing dependencies on obsolete files [Part 1]

Comment 22

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Mmmm, cleanup.  sr=shaver.

Comment 23

[Brendan Eich [:brendan]]

sr=[e-mail], go bryner!

/be

Comment 24

[Brian Ryner (not reading)]

Attachment: get rid of reference to xp_file.h from nspr

Comment 25

[cls]

Comment on attachment 50352 [details] [diff] [review]
get rid of reference to xp_file.h from nspr

r=cls

Comment 26

[Simon Fraser [no longer active]]

Comment on attachment 50352 [details] [diff] [review]
get rid of reference to xp_file.h from nspr

sr=sfraser

Comment 27

[cls]

Attachment: Remove some xpfile.h references from mailnews & layout

Comment 28

[Alec Flett]

Comment on attachment 50357 [details] [diff] [review]
Remove some xpfile.h references from mailnews & layout

sr=alecf

Comment 29

[Brian Ryner (not reading)]

Comment on attachment 50357 [details] [diff] [review]
Remove some xpfile.h references from mailnews & layout

r=bryner

Comment 30

[Brian Ryner (not reading)]

attachment 50357 [details] [diff] [review] is now checked in.

Comment 31

[Alec Flett]

Comment on attachment 50357 [details] [diff] [review]
Remove some xpfile.h references from mailnews & layout

I've got another patch for this which I'll also attach.

Comment 32

[Alec Flett]

Attachment: get rid of XP_File

Comment 33

[David :Bienvenu]

rs=bienvenu on the msg send part of the xp_file patch.

Comment 34

[Brian Ryner (not reading)]

We probably don't really need to edit xp_file.h if we're going to remove it soon...

Comment 35

[Alec Flett]

eh, can't hurt, right? :)

Comment 36

[Brian Ryner (not reading)]

Comment on attachment 52535 [details] [diff] [review]
get rid of XP_File

r=bryner on the nsMsgSendPart part of this patch

Comment 37

[hacker formerly known as [e-mail]]

Attachment: Remove XP_File usage from gfx/src/ps

Comment 38

[Brian Ryner (not reading)]

Comment on attachment 53719 [details] [diff] [review]
Remove XP_File usage from gfx/src/ps

r=bryner

Comment 39

[Alec Flett]

Comment on attachment 53719 [details] [diff] [review]
Remove XP_File usage from gfx/src/ps

good god!
sr=alecf

Comment 40

[Brian Ryner (not reading)]

Attachment: get rid of xp_file references from mork

Comment 41

[Brian Ryner (not reading)]

Attachment: buh bye

Comment 42

[Brian Ryner (not reading)]

I'm pretty sure the mork changes are safe unless we plan to start defining 
MORK_OBSOLETE sometime soon.  The actual removal of the header needs to first 
be tested on the commercial build on all three platforms. (I can report that 
it's fine on Win32).

Comment 43

[hacker formerly known as [e-mail]]

Comment on attachment 54032 [details] [diff] [review]
buh bye

r=cls

Comment 44

[hacker formerly known as [e-mail]]

If we're going to break MORK_OBSOLETE like that, then we should just remove it
entirely.  I think it may be better to just replace the XP_File wrapper defines
with their underlying C counterparts (why they were wrapped to begin with is
still a mystery to me).

Comment 45

[Alec Flett]

Comment on attachment 54032 [details] [diff] [review]
buh bye

yay. sr=alecf

Comment 46

[Alec Flett]

Comment on attachment 54030 [details] [diff] [review]
get rid of xp_file references from mork

let's make sure MORK_OBSOLETE doesn't refer to anything else besides xp_file. I would guess we just want to kill it. I doubt it even works.

Comment 47

[David :Bienvenu]

MORK_OBSOLETE won't be resurrected. It was probably left in there from when mork
was developed standalone with its own test harness. That's my guess, anyway.

Comment 48

[Brian Ryner (not reading)]

Attachment: completely remove MORK_OBSOLETE and MORK_ALONE

Comment 49

[Alec Flett]

Comment on attachment 54382 [details] [diff] [review]
completely remove MORK_OBSOLETE and MORK_ALONE

sr=alecf

Comment 50

[John G. Myers]

*** Bug 63834 has been marked as a duplicate of this bug. ***

Comment 51

[John G. Myers]

The files

mozilla/netwerk/dns/src/unix-dns.c
mozilla/include/unix-dns.h

appear to be unreferenced.

Comment 52

[David :Bienvenu]

Comment on attachment 54382 [details] [diff] [review]
completely remove MORK_OBSOLETE and MORK_ALONE

r=bienvenu

Comment 53

[Brian Ryner (not reading)]

latest patch checked in.

Comment 54

[Brian Ryner (not reading)]

It appears we can remove xp_file.h with no problems.  Successfully built Linux
and Win32 (moz and ns) and mach-o mozilla with the file removed.

Comment 55

[Alec Flett]

sr=alecf
good luck and godspeed.

Comment 56

[Simon Fraser [no longer active]]

You need to test building Mac commercial before removing this file.

Comment 57

[Brian Ryner (not reading)]

jag reports that mac commercial builds fine without this file

Comment 58

[Brendan Eich [:brendan]]

a=[e-mail] for the 0.9.9-frozen trunk -- free at last!  Thanks,

/be

Comment 59

[Christopher Blizzard (:blizzard)]

cls, are you about ready to check this in?

Comment 60

[hacker formerly known as [e-mail]]

bryner already removed xp_file.h .

Comment 61

[John G. Myers]

Can I get r= to remove the two files mentioned in comment #51?

Comment 62

[Alec Flett]

r=alecf on the removal of unix-dns.h and unix-dns.c

Comment 63

[Darin Fisher]

sr=darin on the removal of unix-dns.h and unix-dns.c

Comment 64

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

a=dbaron for trunk checkin of the removal of unix-dns.h and unix-dns.c

Comment 65

[Asa Dotzler [:asa]]

Chris, we're nearing the end of 0.9.9 and want to push out the Milestone ASAP.
Can you make these changes ASAP and pull this bug off of the 122050 dependency
list if it needs to stay open after the 0.9.9 checkins. Thanks.

Comment 66

[hacker formerly known as [e-mail]]

Attachment: Remove xp_core.h & xp_mem.h. Trim xp_str.h & xp_mcom.h

The majority of this patch is replacing the xp_core.h include with prtypes.h &
prlog.h and search-n-replace on the xp_mem.h & xp_str.h macros.  There are a
few of questionable bits.

1)  XP_BEGIN_PROTOS/XP_END_PROTOS were replaced by
PR_BEGIN_EXTERN_C/PR_END_EXTERN_C.  This moved some deps from xp_core.h to
prtypes.h.

2) The XP_Bool type detection was completely thrown out.  Added a 'typedef int
XP_Bool' to nsPostscriptObj.h in the same fasion as libreg & movemail.	

3) NULL & nil are no longer defined.

4) Removed all of the PA_LOCK/PA_UNLOCK stuff along with the XP_Block stuff.

5) Copied the _INT16,_INT32 defines into npapi.h since it's the only place that
requires it.

6) Moved the PUBLIC, PRIVATE & MODULE_PRIVATE defines into nsComObsolete.h

One question: How does NOT_NULL work in DEBUG builds?

Comment 67

[hacker formerly known as [e-mail]]

I need a mac buddy.  I already verified that the patch works on linux & win32 on
both moz & ns trees.
 

Comment 68

[hacker formerly known as [e-mail]]

bugscape 12706

Comment 69

[hacker formerly known as [e-mail]]

Attachment: Same with mac updates

I verified that this patch works on mac, linux & win32. Can I get some r/sr
love? Anyone? Anyone?

Comment 70

[Alec Flett]

Comment on attachment 76139 [details] [diff] [review]
Same with mac updates

rs=alecf

Comment 71

[Brendan Eich [:brendan]]

Comment on attachment 76139 [details] [diff] [review]
Same with mac updates

Looks good to me, but can we reomve xp_mcom.h's include here:

+++ dbm/include/mcom_db.h	2002/03/26 02:53:26
@@ -240,9 +240,9 @@
 #include "xp_mcom.h"
 #define O_ACCMODE	 3	 /* Mask for file access modes */
 #define EFTYPE 2000
-XP_BEGIN_PROTOS
+PR_BEGIN_EXTERN_C
 int mkstemp(const char *path);
-XP_END_PROTOS
+PR_END_EXTERN_C
 #endif /* MACINTOSH */

In for a penny...

/be

Comment 72

[hacker formerly known as [e-mail]]

That include is wrapped in a macintosh ifdef and the mac needs xp_mcom.h to get
the prototype for strdup.  Anyone know in which codewarrior header strdup is
actually declared?

Comment 73

[hacker formerly known as [e-mail]]

Attachment: Purge platform.h

Comment 74

[hacker formerly known as [e-mail]]

Attachment: Previous + get rid of xpassert.h, xp_debug.h & xp_trace.h

It turns out that we weren't using any of the defines from xpassert.h,
xp_debug.h or xp_trace.h in any active code.  I translated a few of the ifdef'd
out XP_ASSERTs into NS_ASSERTIONs. Minicom.h isn't used either but it's still
referenced by some of the stub-java code so I'm not sure if it can be cvs
removed just yet.

Comment 75

[Brian Ryner (not reading)]

Comment on attachment 76983 [details] [diff] [review]
Previous + get rid of xpassert.h, xp_debug.h & xp_trace.h

r=bryner, but please make sure the ns tree builds as well.

Comment 76

[Alec Flett]

Comment on attachment 76983 [details] [diff] [review]
Previous + get rid of xpassert.h, xp_debug.h & xp_trace.h

sr=alecf

Comment 77

[tor]

Comment on attachment 76983 [details] [diff] [review]
Previous + get rid of xpassert.h, xp_debug.h & xp_trace.h

a=tor

Comment 78

[hacker formerly known as [e-mail]]

*sigh*

Comment 79

[Jaime Rodriguez, Jr.]

Removing adt1.0.0. you can check in with Drivers approval.

Comment 80

[scottputterman]

Putting back adt1.0.0 keyword and asking for info.  Will the user notice any of
these changes?  For example, I see there are changes to mailnews.  Is there any
possibility that the code will behave differently than it does now?  What's the
risk associated with this and is there any benefit to users?

Comment 81

[hacker formerly known as [e-mail]]

There are no end-user visible changes in these patches.  The purpose of the
changes are to remove obsolete code that we no longer support and have replaced
wholesale.

The mailnews code that is changed is either 
a) commented out
b) wrapped in a debug ifdef
or
c) wrapped in some other ifdef that we don't compile in (nothing broke when the
5 files in question were replaced wholesale with an #error clause)

I made the macro changes anyway so that the code doesn't mysteriously break when
someone tries to enable one of those disabled ifdefs.

Comment 82

[scottputterman]

adding ADT1.0.0+

Comment 83

[hacker formerly known as [e-mail]]

Oops.  Restoring ADT signature and opening new bug (maybe) for the last 3
remaining headers per Jud.

Comment 84

[scottputterman]

I'm not sure if this was checked in.  If it wasn't, removing the adt1.0.0+.  If
it was, please add the fixed1.0.0 keyword.

Comment 85

[Jon Granrose]

v fixed.

is there a new bug number for those last few header files?