Closed
Bug 333308
(makeclean)
Opened 18 years ago
Closed 16 years ago
make clean and make distclean miss various files
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jag+mozilla, Assigned: jag+mozilla)
Details
Attachments
(10 files, 7 obsolete files)
1.11 KB,
patch
|
neil
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
480 bytes,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
546 bytes,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
12.91 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
1.60 KB,
patch
|
cls
:
review+
benjamin
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
617 bytes,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
153.40 KB,
text/plain
|
Details | |
13.98 KB,
patch
|
jag+mozilla
:
superreview+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
3.62 KB,
patch
|
benjamin
:
review+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
1.16 KB,
patch
|
benjamin
:
review+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
To reproduce, just do a build of your favourite product and do a make distclean and see what's left behind (easier if you do this in a separate OBJDIR).
Assignee | ||
Comment 1•18 years ago
|
||
Attachment #217729 -
Flags: superreview?(neil)
Attachment #217729 -
Flags: review?(neil)
Assignee | ||
Comment 2•18 years ago
|
||
Initially I was gonna add SDK_XPIDLSRCS to that check, but I found that XPIDL_GEN_DIR is already appended to GARBAGE_DIRS for both those cases around line 1407
Attachment #217733 -
Flags: review?
Assignee | ||
Updated•18 years ago
|
Attachment #217733 -
Flags: review? → review?(benjamin)
Updated•18 years ago
|
Attachment #217729 -
Flags: superreview?(neil)
Attachment #217729 -
Flags: superreview+
Attachment #217729 -
Flags: review?(neil)
Attachment #217729 -
Flags: review+
Assignee | ||
Comment 3•18 years ago
|
||
Attachment #217738 -
Flags: review?(benjamin)
Updated•18 years ago
|
Attachment #217733 -
Flags: review?(benjamin) → review+
Updated•18 years ago
|
Attachment #217738 -
Flags: review?(benjamin) → review+
Comment 4•18 years ago
|
||
Did this patch turn "WINNT 5.2 gaius Dep" orange? ("Trender:[FAILED]") http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox
Assignee | ||
Comment 5•18 years ago
|
||
Nope.
Assignee | ||
Comment 6•18 years ago
|
||
Here's a list of files that are currently left behind after make distclean in a SeaMonkey debug OBJDIR: ./gfx/gfx-config.h ./xpcom/xpcom-private.h ./xpcom/xpcom-config.h ./netwerk/necko-config.h ./build/unix/seamonkey-config ./extensions/xmlextras/base/src/nsRect.cpp ./extensions/p3p/p3p200010.xsl ./extensions/p3p/p3p200005.xsl ./extensions/p3p/p3p200201.xsl ./extensions/p3p/p3p200109.xsl ./extensions/p3p/p3p200012.xsl ./directory/c-sdk/config/autoconf.mk ./directory/c-sdk/ldap/libraries/libutil/Makefile ./directory/c-sdk/ldap/libraries/libssldap/Makefile ./directory/c-sdk/ldap/clients/tools/Makefile ./nsprpub/lib/tests/Makefile ./nsprpub/pkg/solaris/SUNWpr/Makefile ./nsprpub/pkg/solaris/Makefile ./nsprpub/pkg/solaris/SUNWprd/Makefile ./nsprpub/pkg/Makefile ./nsprpub/pkg/linux/Makefile ./security/manager/.nss.cleaned
Comment 7•18 years ago
|
||
The following files are left over after a make distclean on xulrunner 1.8: build/unix/xulrunner-config extensions/java/xpcom/interfaces/.done extensions/java/xpcom/src/.done extensions/pref/system-pref/src/gconf/nsSystemPrefFactory.cpp extensions/xmlextras/base/src/nsRect.cpp gfx/gfx-config.h modules/libpr0n/build/_img_list netwerk/necko-config.h nsprpub/lib/tests/Makefile nsprpub/pkg/linux/Makefile nsprpub/pkg/solaris/SUNWpr/Makefile nsprpub/pkg/solaris/SUNWprd/Makefile nsprpub/pkg/solaris/Makefile nsprpub/pkg/Makefile profile/dirserviceprovider/standalone/nsProfileDirServiceProvider.cpp profile/dirserviceprovider/standalone/nsProfileLock.cpp security/manager/.nss.cleaned toolkit/library/dlldeps-javaxpcom.cpp toolkit/library/dlldeps-obs.cpp toolkit/library/dlldeps.cpp toolkit/mozapps/downloads/src/nsHelperAppDlg.js toolkit/mozapps/extensions/src/nsExtensionManager.js toolkit/mozapps/update/src/nsUpdateService.js toolkit/xre/nsSigHandlers.cpp toolkit/xre/nsStackFrameUnix.cpp toolkit/xre/nsStackFrameUnix.h toolkit/xre/showOSAlert.cpp xpcom/build/nsCOMPtr.cpp xpcom/build/nsComponentManagerUtils.cpp xpcom/build/nsDebug.cpp xpcom/build/nsGREGlue.cpp xpcom/build/nsGenericFactory.cpp xpcom/build/nsID.cpp xpcom/build/nsIInterfaceRequestorUtils.cpp xpcom/build/nsINIParser.cpp xpcom/build/nsMemory.cpp xpcom/build/nsTHashtable.cpp xpcom/build/nsTraceRefcnt.cpp xpcom/build/nsVersionComparator.cpp xpcom/build/nsWeakReference.cpp xpcom/glue/standalone/nsCOMPtr.cpp xpcom/glue/standalone/nsComponentManagerUtils.cpp xpcom/glue/standalone/nsDebug.cpp xpcom/glue/standalone/nsGREGlue.cpp xpcom/glue/standalone/nsID.cpp xpcom/glue/standalone/nsIInterfaceRequestorUtils.cpp xpcom/glue/standalone/nsINIParser.cpp xpcom/glue/standalone/nsMemory.cpp xpcom/glue/standalone/nsTHashtable.cpp xpcom/glue/standalone/nsTraceRefcnt.cpp xpcom/glue/standalone/nsVersionComparator.cpp xpcom/glue/standalone/nsWeakReference.cpp xpcom/xpcom-config.h xpcom/xpcom-private.h xpfe/components/filepicker/src/nsFilePicker.js The following file is *removed* by the distclean, while it is in the source tarball. security/coreconf/nsinstall/nfspwd I'm going to attach a patch for all these.
Comment 8•18 years ago
|
||
I still need to check what still needs to be applied after the patches already attached here, and against the trunk. The nfspwd fix is not required any more, for instance. It has been fixed in bug #311074. There are some changes that could be done otherwise: - The leftover .js files could be installed as EXTRA_PP_COMPONENTS instead of EXTRA_COMPONENTS, but that would mean renaming the .js.in files to .js. - The _img_list seems to be useless, it could be plain removed (i.e. not generated at all) - Maybe nsprpub/pkg could be removed, but I'm not sure if it's any useful for anyone.
Attachment #218131 -
Flags: review?(benjamin)
Comment 9•18 years ago
|
||
Comment on attachment 218131 [details] [diff] [review] Remove more >=== nsprpub/Makefile.in NSPR patches need to be attached separately and reviewed by wtchang. >=== security/coreconf/nsinstall/Makefile NSS patches need to be attached separately and reviewed by nelsonb or wtchang. But I'm pretty sure this was already done on trunk, and shouldn't be in this patch. r=me without those files. Could you please provide an updated patch which doesn't have those files, and which will apply to trunk?
Attachment #218131 -
Flags: review?(benjamin) → review+
Comment 10•18 years ago
|
||
> >=== security/coreconf/nsinstall/Makefile > > NSS patches need to be attached separately and reviewed by nelsonb or wtchang. > But I'm pretty sure this was already done on trunk, and shouldn't be in this > patch. This is indeed the case > r=me without those files. Could you please provide an updated patch which > doesn't have those files, and which will apply to trunk? Patches coming.
Comment 11•18 years ago
|
||
New version against trunk, without nsprpub files
Attachment #218131 -
Attachment is obsolete: true
Attachment #218726 -
Flags: review?(benjamin)
Comment 12•18 years ago
|
||
Attachment #218727 -
Flags: review?(wtchang)
Attachment #218727 -
Flags: review?(wtchang) → review+
Attachment #218727 -
Attachment description: Remove more for trunk ; nsprpub part → Remove more for trunk ; nsprpub part (checked in on moz & nspr trunks)
Attachment #218727 -
Flags: approval-branch-1.8.1?
Comment 13•18 years ago
|
||
Comment on attachment 218727 [details] [diff] [review] Remove more for trunk ; nsprpub part (checked in on moz & nspr trunks) cls, if you meant to ask me for approval-branch1.8.1, a=me... or maybe you meant to ask wtchang?
Attachment #218727 -
Flags: approval-branch-1.8.1? → approval-branch-1.8.1+
Comment 14•18 years ago
|
||
Fixed on trunk.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Updated•18 years ago
|
Attachment #218726 -
Flags: review?(benjamin) → review+
Comment 15•18 years ago
|
||
Some leftovers in firefox build
Attachment #224231 -
Flags: review?(benjamin)
Updated•18 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•18 years ago
|
Attachment #224231 -
Flags: review?(benjamin) → review+
Comment 16•18 years ago
|
||
Landed attachment 224231 [details] [diff] [review] on trunk.
Status: REOPENED → RESOLVED
Closed: 18 years ago → 18 years ago
Resolution: --- → FIXED
Comment 17•18 years ago
|
||
Note the allmakefiles.sh part applies to the 1.8 branch only. The xmlextras/base reference don't exist on trunk, and the files exist in 1.8.0 branch, so they need to be created. They got removed in 1.8 branch, though, as it seems. The rest is against firefox 2.0b1, it may apply to trunk as well.
Attachment #229965 -
Flags: review?(benjamin)
Comment 18•18 years ago
|
||
Reopening for the new patch
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 19•18 years ago
|
||
Comment on attachment 229965 [details] [diff] [review] Even more cleanup >=== config/Makefile.in >+GARBAGE += system_wrappers You mean GARBAGE_DIRS, right?
Attachment #229965 -
Flags: review?(benjamin) → review-
Comment 20•18 years ago
|
||
Attachment #229965 -
Attachment is obsolete: true
Attachment #230605 -
Flags: review?(benjamin)
Updated•18 years ago
|
Attachment #230605 -
Flags: review?(benjamin) → review+
Comment 21•17 years ago
|
||
1. The last patch (attachment 230605 [details] [diff] [review]) haven't been checked in, though the last comment (comment #20) is a bit confusing. 2. bug 336244 removed extensions/xmlextras/base so there's no need to delete those files, on trunk 3. nsMicrosummaryService.js.in was renamed to nsMicrosummaryService.js, (see bug 354449) so there's no need to delete it on trunk. i.e. now the useful part of the patch is only >=== config/Makefile.in >================================================================== >--- config/Makefile.in (revision 13) >+++ config/Makefile.in (local) >@@ -136,6 +136,8 @@ > if test ! -d system_wrappers; then mkdir system_wrappers; fi > $(PERL) $(topsrcdir)/nsprpub/config/make-system-wrappers.pl >system_wrappers < $(srcdir)/system-headers > $(INSTALL) system_wrappers $(DIST)/include >+ >+GARBAGE_DIRS += system_wrappers > endif > > # we don't use an explicit dependency here because then we would
Comment 22•17 years ago
|
||
Attachment #254064 -
Flags: review?(ajvincent)
Comment 23•17 years ago
|
||
Comment on attachment 254064 [details] [diff] [review] Yet some more for suite I'm not a reviewer for makefile changes; per biesi's advice, requesting review from mentovai.
Attachment #254064 -
Flags: review?(ajvincent) → review?(mark)
Updated•17 years ago
|
Attachment #254064 -
Flags: review?(mark) → review+
Comment 24•17 years ago
|
||
This is a diff of running "find . -type f" in a srcdir right after checkout, and right after running "make" and then "make distclean". Note that there are some files that actually got removed, notably the .manifest files are bug 375671.
Comment 25•17 years ago
|
||
instgen, staticlib and _tests are easy to remove with GARBAGE_DIRS I suspect we're just not cleaning nspr at all, which is probably a regression from toolkit-tiers.mk and whatnot... shouldn't be hard to fix. Do you know what the pstorec.tlh files are? We should probably add a $(wildcard *.pyc) global garbage rule.
Comment 26•17 years ago
|
||
// d:\build\testmoz\clean\mozilla\browser\components\build\pstorec.tlh // // C++ source equivalent of Win32 type library C:\WINDOWS\system32\pstorec.dll It's a typelib header, apparently. Sounds like we could just add it to GARBAGE. It looks like there are other scattered files that could be special cased, but not too bad.
Comment 27•17 years ago
|
||
Hmm, I'm lost as to which patches are still valid and need to be checked-in... could somebody help? Also, are we sure the patches are still valid after this long of time?
Assignee | ||
Updated•16 years ago
|
Alias: makeclean
Assignee | ||
Comment 28•16 years ago
|
||
Remove _tests, _javagen, config/system_wrappers, config/buildid, generated *QI.cpp files, run distclean for nspr, clean up libical better, use the unallmakefiles approach for directories/c-sdk (what are the check-in rules here?), clean up copied and linked dirs and files for various mac specific things, and a few other small clean-ups.
Assignee: nobody → jag
Attachment #230605 -
Attachment is obsolete: true
Attachment #254064 -
Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #301503 -
Flags: superreview?(benjamin)
Assignee | ||
Comment 29•16 years ago
|
||
Hrm, I forgot to add something for "__.SYMDEF SORTED" (which appears to be Mac only). Maybe in config/rules.mk near where we handle similar stuff for SunOS and OpenVMS? Hmm, and I just noticed that $(JAVA_GEN_DIR) is added to GARBAGE_DIRS, so that should already have been cleaned. Let me go figure out what's going on there. On a related note, I'm removing the leading "./" from the XPIDLSRCS line in extensions/typeaheadfind/public/Makefile.in since that causes the .pp file to be created in the wrong place (it combines with "$(JAVA_GEN_DIR)/." to go up a dir).
Assignee | ||
Comment 30•16 years ago
|
||
Ah, we only add $(JAVA_GEN_DIR) to GARBAGE_DIRS when one of XPIDLSRCS or SDK_XPIDLSRCS isn't empty. Since they're not set at the top level, $objdir/_javagen won't get removed. I guess we could move the GARBAGE_DIRS += $(JAVA_GEN_DIR) line outside the ifneq (but still inside ifdef MOZ_JAVAXPCOM). Or we just special case this like I did in my patch.
Comment 31•16 years ago
|
||
Comment on attachment 301503 [details] [diff] [review] Clean up various things r=me on the parts that aren't directory/c-sdk. That is an independent project and will need separate review (and branches/tags separately also).
Attachment #301503 -
Flags: superreview?(benjamin) → superreview+
Assignee | ||
Comment 32•16 years ago
|
||
Attachment #301503 -
Attachment is obsolete: true
Attachment #302138 -
Flags: superreview+
Attachment #302138 -
Flags: approval1.9?
Assignee | ||
Comment 33•16 years ago
|
||
Bug 416377 covers the directory/c-sdk changes.
Updated•16 years ago
|
Attachment #302138 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 34•16 years ago
|
||
Attachment 302138 [details] [diff] ("Clean up various things minus c-sdk") has been checked in.
Comment 35•16 years ago
|
||
These are some others that are not removed in beta3. Note that a lot of them come from exporting the files from some other directory, with rules running $(INSTALL). A new variable could be created (for example EXPORT_FILES) that would trigger new rules that would $(INSTALL) the files listed there and remove them at clean time... Well, people already don't use GARBAGE, why would they use EXPORT_FILES...
Attachment #302444 -
Flags: review?(benjamin)
Assignee | ||
Comment 36•16 years ago
|
||
Looks good to me. I would move the GARBAGE line in xpcom/glue/standalone/Makefile.in above the DEFINES line so it's right next to the $(INSTALL). I like the idea of EXPORT_FILES, but it'd need a better name.
Assignee | ||
Comment 37•16 years ago
|
||
Note that .pyc files get created in $(srcdir) (yuck!), so you might want to make that $(srcdir)/Expression.pyc. I also found configobj.pyc in the same place.
Comment 38•16 years ago
|
||
Updated patch following Peter's comments. > Note that .pyc files get created in $(srcdir) (yuck!), so you might want to > make that $(srcdir)/Expression.pyc. I also found configobj.pyc in the same > place. That makes sense, that they are in $(srcdir), it's python bytecode, and it's put there by python when Expression.py is imported from Preprocessor.py. Unfortunately, there's no way to prevent bytecode to get generated. Only in 2.6. http://bugs.python.org/issue602345
Attachment #302444 -
Attachment is obsolete: true
Attachment #302529 -
Flags: review?(benjamin)
Attachment #302444 -
Flags: review?(benjamin)
Assignee | ||
Comment 39•16 years ago
|
||
I understand why they're in $(srcdir) but it's still yucky. It'd be nice if python could be told to put those files in whatever PYTHON_CACHE is set to, or ~/.python/cache/, or something. Anyway... I think I'd name the two .pyc files explicitly instead of doing *.pyc, but I'll leave that one up to Benjamin.
Updated•16 years ago
|
Attachment #302529 -
Flags: review?(benjamin) → review+
Updated•16 years ago
|
Attachment #302529 -
Flags: approval1.9?
Comment 40•16 years ago
|
||
Comment on attachment 302529 [details] [diff] [review] Cleanup even more, take 2 Much appreciated!
Attachment #302529 -
Flags: approval1.9? → approval1.9+
Updated•16 years ago
|
Keywords: checkin-needed
Comment 41•16 years ago
|
||
Checking in Makefile.in; /cvsroot/mozilla/Makefile.in,v <-- Makefile.in new revision: 1.398; previous revision: 1.397 done Checking in config/Makefile.in; /cvsroot/mozilla/config/Makefile.in,v <-- Makefile.in new revision: 3.144; previous revision: 3.143 done Checking in db/morkreader/external/Makefile.in; /cvsroot/mozilla/db/morkreader/external/Makefile.in,v <-- Makefile.in new revision: 1.2; previous revision: 1.1 done Checking in embedding/base/standalone/Makefile.in; /cvsroot/mozilla/embedding/base/standalone/Makefile.in,v <-- Makefile.in new revision: 1.3; previous revision: 1.2 done Checking in extensions/java/xpcom/interfaces/Makefile.in; /cvsroot/mozilla/extensions/java/xpcom/interfaces/Makefile.in,v <-- Makefile.in new revision: 1.19; previous revision: 1.18 done Checking in intl/unicharutil/util/internal/Makefile.in; /cvsroot/mozilla/intl/unicharutil/util/internal/Makefile.in,v <-- Makefile.in new revision: 1.7; previous revision: 1.6 done Checking in rdf/util/src/internal/Makefile.in; /cvsroot/mozilla/rdf/util/src/internal/Makefile.in,v <-- Makefile.in new revision: 1.2; previous revision: 1.1 done Checking in xpcom/glue/standalone/Makefile.in; /cvsroot/mozilla/xpcom/glue/standalone/Makefile.in,v <-- Makefile.in new revision: 1.39; previous revision: 1.38 done
Keywords: checkin-needed
Comment 42•16 years ago
|
||
Some more leftovers (_profile/* and netwerk/dns/src/etld_data.inc). I've also fixed the xulrunner/installer part (we create some .pc files in there) which has the proper GARBAGE but was never cleaned up.
Attachment #306952 -
Flags: review?
Updated•16 years ago
|
Attachment #306952 -
Flags: review? → review?(benjamin)
Comment 43•16 years ago
|
||
Comment on attachment 306952 [details] [diff] [review] Cleanup even more 2 >Index: mozilla/Makefile.in >=================================================================== >--- mozilla.orig/Makefile.in >+++ mozilla/Makefile.in >@@ -49,7 +49,7 @@ > $(RM) -rf $(DIST)/include > $(RM) -rf $(DIST)/private > $(RM) -rf $(DIST)/public >- $(RM) -rf _tests >+ $(RM) -rf _javagen _profile _tests I don't think we want to blow this away on rebuild. The other changes look correct.
Attachment #306952 -
Flags: review?(benjamin) → review-
Comment 44•16 years ago
|
||
This should be what you want
Attachment #306952 -
Attachment is obsolete: true
Attachment #308959 -
Flags: review?(benjamin)
Updated•16 years ago
|
Attachment #308959 -
Flags: review?(benjamin) → review+
Updated•16 years ago
|
Attachment #308959 -
Flags: approval1.9?
Comment 45•16 years ago
|
||
Comment on attachment 308959 [details] [diff] [review] cleanup even more 2, v2 a1.9+=damons
Attachment #308959 -
Flags: approval1.9? → approval1.9+
Updated•16 years ago
|
Keywords: checkin-needed
Comment 46•16 years ago
|
||
Checking in Makefile.in; /cvsroot/mozilla/Makefile.in,v <-- Makefile.in new revision: 1.403; previous revision: 1.402 done Checking in netwerk/dns/src/Makefile.in; /cvsroot/mozilla/netwerk/dns/src/Makefile.in,v <-- Makefile.in new revision: 1.30; previous revision: 1.29 done Checking in xulrunner/build.mk; /cvsroot/mozilla/xulrunner/build.mk,v <-- build.mk new revision: 1.8; previous revision: 1.7 done
Keywords: checkin-needed
Comment 47•16 years ago
|
||
I'm going to call this bug FIXED because it's long and windy. If you discover more uncleanness, please file another bug.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago → 16 years ago
Resolution: --- → FIXED
Comment 48•16 years ago
|
||
I've created Bug 422991 with more uncleanness.
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•