Last Comment Bug 333308 - (makeclean) make clean and make distclean miss various files
(makeclean)
: make clean and make distclean miss various files
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: jag (Peter Annema)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-04-09 03:12 PDT by jag (Peter Annema)
Modified: 2008-03-14 13:25 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Remove generated / copied files (1.11 KB, patch)
2006-04-09 03:59 PDT, jag (Peter Annema)
neil: review+
neil: superreview+
Details | Diff | Splinter Review
This is redundant (480 bytes, patch)
2006-04-09 04:36 PDT, jag (Peter Annema)
benjamin: review+
Details | Diff | Splinter Review
Clean up _img_list (546 bytes, patch)
2006-04-09 05:38 PDT, jag (Peter Annema)
benjamin: review+
Details | Diff | Splinter Review
Remove more (8.71 KB, patch)
2006-04-11 21:32 PDT, Mike Hommey [:glandium]
benjamin: review+
Details | Diff | Splinter Review
Remove more for trunk ; nsprpub excluded (12.91 KB, patch)
2006-04-17 15:14 PDT, Mike Hommey [:glandium]
benjamin: review+
Details | Diff | Splinter Review
Remove more for trunk ; nsprpub part (checked in on moz & nspr trunks) (1.60 KB, patch)
2006-04-17 15:15 PDT, Mike Hommey [:glandium]
cls: review+
benjamin: approval‑branch‑1.8.1+
Details | Diff | Splinter Review
Some more cleaning (617 bytes, patch)
2006-06-02 12:26 PDT, Mike Hommey [:glandium]
benjamin: review+
Details | Diff | Splinter Review
Even more cleanup (1.49 KB, patch)
2006-07-20 08:23 PDT, Mike Hommey [:glandium]
benjamin: review-
Details | Diff | Splinter Review
Even more cleanup... fixed (1.49 KB, patch)
2006-07-25 11:36 PDT, Mike Hommey [:glandium]
benjamin: review+
Details | Diff | Splinter Review
Yet some more for suite (1.66 KB, patch)
2007-02-05 13:39 PST, Mike Hommey [:glandium]
mark: review+
Details | Diff | Splinter Review
diff of post-checkout file list and post-make, distclean file list (153.40 KB, text/plain)
2007-09-17 09:32 PDT, Ted Mielczarek [:ted.mielczarek]
no flags Details
Clean up various things (18.93 KB, patch)
2008-02-05 07:55 PST, jag (Peter Annema)
benjamin: superreview+
Details | Diff | Splinter Review
Clean up various things minus c-sdk (13.98 KB, patch)
2008-02-08 08:51 PST, jag (Peter Annema)
jag-mozilla: superreview+
mtschrep: approval1.9+
Details | Diff | Splinter Review
Cleanup even more (3.60 KB, patch)
2008-02-10 11:39 PST, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
Cleanup even more, take 2 (3.62 KB, patch)
2008-02-10 22:52 PST, Mike Hommey [:glandium]
benjamin: review+
mtschrep: approval1.9+
Details | Diff | Splinter Review
Cleanup even more 2 (1.53 KB, patch)
2008-03-02 16:51 PST, Fabien Tassin
benjamin: review-
Details | Diff | Splinter Review
cleanup even more 2, v2 (1.16 KB, patch)
2008-03-12 14:29 PDT, Mike Hommey [:glandium]
benjamin: review+
dsicore: approval1.9+
Details | Diff | Splinter Review

Description jag (Peter Annema) 2006-04-09 03:12:19 PDT
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).
Comment 1 jag (Peter Annema) 2006-04-09 03:59:06 PDT
Created attachment 217729 [details] [diff] [review]
Remove generated / copied files
Comment 2 jag (Peter Annema) 2006-04-09 04:36:41 PDT
Created attachment 217733 [details] [diff] [review]
This is redundant

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
Comment 3 jag (Peter Annema) 2006-04-09 05:38:36 PDT
Created attachment 217738 [details] [diff] [review]
Clean up _img_list
Comment 4 Steve England [:stevee] 2006-04-10 07:28:41 PDT
Did this patch turn "WINNT 5.2 gaius Dep" orange? ("Trender:[FAILED]")

http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox
Comment 5 jag (Peter Annema) 2006-04-10 12:33:02 PDT
Nope.
Comment 6 jag (Peter Annema) 2006-04-10 15:36:31 PDT
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 Mike Hommey [:glandium] 2006-04-11 21:27:43 PDT
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 Mike Hommey [:glandium] 2006-04-11 21:32:39 PDT
Created attachment 218131 [details] [diff] [review]
Remove more

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.
Comment 9 Benjamin Smedberg [:bsmedberg] 2006-04-17 14:04:56 PDT
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?
Comment 10 Mike Hommey [:glandium] 2006-04-17 15:08:06 PDT
> >=== 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 Mike Hommey [:glandium] 2006-04-17 15:14:28 PDT
Created attachment 218726 [details] [diff] [review]
Remove more for trunk ; nsprpub excluded

New version against trunk, without nsprpub files
Comment 12 Mike Hommey [:glandium] 2006-04-17 15:15:16 PDT
Created attachment 218727 [details] [diff] [review]
Remove more for trunk ; nsprpub part (checked in on moz & nspr trunks)
Comment 13 Benjamin Smedberg [:bsmedberg] 2006-04-28 07:54:04 PDT
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?
Comment 14 Benjamin Smedberg [:bsmedberg] 2006-04-28 07:56:08 PDT
Fixed on trunk.
Comment 15 Mike Hommey [:glandium] 2006-06-02 12:26:36 PDT
Created attachment 224231 [details] [diff] [review]
Some more cleaning

Some leftovers in firefox build
Comment 16 Benjamin Smedberg [:bsmedberg] 2006-06-08 09:37:58 PDT
Landed attachment 224231 [details] [diff] [review] on trunk.
Comment 17 Mike Hommey [:glandium] 2006-07-20 08:23:27 PDT
Created attachment 229965 [details] [diff] [review]
Even more cleanup

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.
Comment 18 Mike Hommey [:glandium] 2006-07-20 08:24:07 PDT
Reopening for the new patch
Comment 19 Benjamin Smedberg [:bsmedberg] 2006-07-24 13:16:47 PDT
Comment on attachment 229965 [details] [diff] [review]
Even more cleanup

>=== config/Makefile.in

>+GARBAGE += system_wrappers

You mean GARBAGE_DIRS, right?
Comment 20 Mike Hommey [:glandium] 2006-07-25 11:36:37 PDT
Created attachment 230605 [details] [diff] [review]
Even more cleanup... fixed
Comment 21 O. Atsushi (Torisugari) 2007-01-17 01:17:51 PST
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 Mike Hommey [:glandium] 2007-02-05 13:39:39 PST
Created attachment 254064 [details] [diff] [review]
Yet some more for suite
Comment 23 Alex Vincent [:WeirdAl] 2007-02-05 13:51:12 PST
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.
Comment 24 Ted Mielczarek [:ted.mielczarek] 2007-09-17 09:32:46 PDT
Created attachment 281200 [details]
diff of post-checkout file list and post-make, distclean file list

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 Benjamin Smedberg [:bsmedberg] 2007-09-17 10:40:15 PDT
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 Ted Mielczarek [:ted.mielczarek] 2007-09-17 10:52:33 PDT
// 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 Reed Loden [:reed] (use needinfo?) 2007-10-11 23:57:08 PDT
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?
Comment 28 jag (Peter Annema) 2008-02-05 07:55:28 PST
Created attachment 301503 [details] [diff] [review]
Clean up various things

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.
Comment 29 jag (Peter Annema) 2008-02-05 08:09:50 PST
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).
Comment 30 jag (Peter Annema) 2008-02-05 08:19:17 PST
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 Benjamin Smedberg [:bsmedberg] 2008-02-08 06:24:47 PST
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).
Comment 32 jag (Peter Annema) 2008-02-08 08:51:37 PST
Created attachment 302138 [details] [diff] [review]
Clean up various things minus c-sdk
Comment 33 jag (Peter Annema) 2008-02-08 08:59:18 PST
Bug 416377 covers the directory/c-sdk changes.
Comment 34 jag (Peter Annema) 2008-02-09 02:38:53 PST
Attachment 302138 [details] [diff] ("Clean up various things minus c-sdk") has been checked in.
Comment 35 Mike Hommey [:glandium] 2008-02-10 11:39:10 PST
Created attachment 302444 [details] [diff] [review]
Cleanup even more

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...
Comment 36 jag (Peter Annema) 2008-02-10 17:03:36 PST
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.
Comment 37 jag (Peter Annema) 2008-02-10 17:20:53 PST
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 Mike Hommey [:glandium] 2008-02-10 22:52:44 PST
Created attachment 302529 [details] [diff] [review]
Cleanup even more, take 2

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
Comment 39 jag (Peter Annema) 2008-02-11 03:19:29 PST
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.
Comment 40 Mike Schroepfer 2008-02-12 11:48:47 PST
Comment on attachment 302529 [details] [diff] [review]
Cleanup even more, take 2

Much appreciated!
Comment 41 Reed Loden [:reed] (use needinfo?) 2008-02-13 03:33:53 PST
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
Comment 42 Fabien Tassin 2008-03-02 16:51:07 PST
Created attachment 306952 [details] [diff] [review]
Cleanup even more 2

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.
Comment 43 Benjamin Smedberg [:bsmedberg] 2008-03-04 18:01:57 PST
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.
Comment 44 Mike Hommey [:glandium] 2008-03-12 14:29:05 PDT
Created attachment 308959 [details] [diff] [review]
cleanup even more 2, v2

This should be what you want
Comment 45 Damon Sicore (:damons) 2008-03-13 11:26:59 PDT
Comment on attachment 308959 [details] [diff] [review]
cleanup even more 2, v2

a1.9+=damons
Comment 46 Reed Loden [:reed] (use needinfo?) 2008-03-14 08:18:39 PDT
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
Comment 47 Benjamin Smedberg [:bsmedberg] 2008-03-14 10:36:10 PDT
I'm going to call this bug FIXED because it's long and windy. If you discover more uncleanness, please file another bug.
Comment 48 Fabien Tassin 2008-03-14 13:25:18 PDT
I've created Bug 422991 with more uncleanness.

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