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)

x86
Linux
defect
Not set
normal

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+
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+
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).
Attachment #217729 - Flags: superreview?(neil)
Attachment #217729 - Flags: review?(neil)
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?
Attachment #217733 - Flags: review? → review?(benjamin)
Attachment #217729 - Flags: superreview?(neil)
Attachment #217729 - Flags: superreview+
Attachment #217729 - Flags: review?(neil)
Attachment #217729 - Flags: review+
Attachment #217738 - Flags: review?(benjamin)
Attachment #217733 - Flags: review?(benjamin) → review+
Attachment #217738 - Flags: review?(benjamin) → review+
Did this patch turn "WINNT 5.2 gaius Dep" orange? ("Trender:[FAILED]")

http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox
Nope.
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
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.
Attached patch Remove more (obsolete) — Splinter Review
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 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+
> >=== 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.
New version against trunk, without nsprpub files
Attachment #218131 - Attachment is obsolete: true
Attachment #218726 - Flags: review?(benjamin)
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 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+
Fixed on trunk.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Attachment #218726 - Flags: review?(benjamin) → review+
Some leftovers in firefox build
Attachment #224231 - Flags: review?(benjamin)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #224231 - Flags: review?(benjamin) → review+
Landed attachment 224231 [details] [diff] [review] on trunk.
Status: REOPENED → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
Attached patch Even more cleanup (obsolete) — Splinter Review
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)
Reopening for the new patch
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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-
Attached patch Even more cleanup... fixed (obsolete) — Splinter Review
Attachment #229965 - Attachment is obsolete: true
Attachment #230605 - Flags: review?(benjamin)
Attachment #230605 - Flags: review?(benjamin) → review+
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
Attached patch Yet some more for suite (obsolete) — Splinter Review
Attachment #254064 - Flags: review?(ajvincent)
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)
Attachment #254064 - Flags: review?(mark) → review+
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.
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.
// 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.
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?
Alias: makeclean
Attached patch Clean up various things (obsolete) — Splinter Review
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)
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).
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 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+
Attachment #301503 - Attachment is obsolete: true
Attachment #302138 - Flags: superreview+
Attachment #302138 - Flags: approval1.9?
Bug 416377 covers the directory/c-sdk changes.
Attachment #302138 - Flags: approval1.9? → approval1.9+
Attachment 302138 [details] [diff] ("Clean up various things minus c-sdk") has been checked in.
Attached patch Cleanup even more (obsolete) — Splinter Review
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)
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.
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.
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)
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.
Attachment #302529 - Flags: review?(benjamin) → review+
Attachment #302529 - Flags: approval1.9?
Comment on attachment 302529 [details] [diff] [review]
Cleanup even more, take 2

Much appreciated!
Attachment #302529 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
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
Attached patch Cleanup even more 2 (obsolete) — Splinter Review
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?
Attachment #306952 - Flags: review? → review?(benjamin)
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-
This should be what you want
Attachment #306952 - Attachment is obsolete: true
Attachment #308959 - Flags: review?(benjamin)
Attachment #308959 - Flags: review?(benjamin) → review+
Attachment #308959 - Flags: approval1.9?
Comment on attachment 308959 [details] [diff] [review]
cleanup even more 2, v2

a1.9+=damons
Attachment #308959 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
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
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 ago16 years ago
Resolution: --- → FIXED
I've created Bug 422991 with more uncleanness.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.