546642 - Cleanup during make distclean

Status: RESOLVED FIXED

(MailNews Core :: Build Config, defect)

Description

[Mike Hommey [:glandium]]

Attachment: Patch

+++ This bug was initially created as a clone of Bug #541770 +++

A bunch of files are not cleaned up from the main directory when doing make distclean.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Patch v2

Some more cleanup.

Comment 2

[Serge Gautherie (:sgautherie)]

Comment on attachment 427338 [details] [diff] [review]
Patch v2

>diff --git a/Makefile.in b/Makefile.in
>+
>+distclean::
>+	cat unallmakefiles | $(XARGS) rm -f
>+	rm -f unallmakefiles $(DIST_GARBAGE)
>+
>+DIST_GARBAGE = config.cache config.log config.status \
>+   config/autoconf.mk \
>+   unallmakefiles comm-config.h \
>+   $(topsrcdir)/.mozconfig.mk $(topsrcdir)/.mozconfig.out

This diff can't be right: similar code already exist since bug 446690.
(Please fully fix bug 541767 first.)

Comment 3

[Mike Hommey [:glandium]]

The diff is right, except it is against 1.9.1

Comment 4

[Serge Gautherie (:sgautherie)]

(In reply to comment #3)
> The diff is right, except it is against 1.9.1

Wouahou! So you (silently) cloned a m-c /js/src patch to create a c-1.9.1 patch?
What's wrong with porting from c-c (i.e. the bug that fixed it there) and being explicit about your target?
As in: I'm happy you care, but why create a difference between c-c and c-1.9.1?

Comment 5

[Mike Hommey [:glandium]]

I'm sending patches against what I have and at the moment it is 1.9.1. It turns out most of the time, they apply as is on trunk, but not in the current case. (and the m-c js/src patch was also against 1.9.1 originally).

Also note that the change in Makefile.in on c-c occurred less than a month ago...

Anyways, do you realize that instead of arguing both of us could have spent this time updating the patch ?

Comment 6

[Mike Hommey [:glandium]]

Attachment: Patch for c-c

Here you are

Comment 7

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)

> I'm sending patches against what I have and at the moment it is 1.9.1. It turns
> out most of the time, they apply as is on trunk, but not in the current case.

Good for you.
But you're explicitly supposed *not* to do that, especially without _warning_ people.

> Also note that the change in Makefile.in on c-c occurred less than a month
> ago...

... which is much older than your 2-days old patch!

> Anyways, do you realize that instead of arguing both of us could have spent
> this time updating the patch ?

You, probably.
But not me, as I'm yet trying to figure out what you're doing:
for example, it looks like your new patch (context) here applies on top of your additional patch in bug 541767, though (it's fine but) you didn't say it.

Again, I appreciate you're doing patches, but please either do it by the rules or explain what you're doing differently: it really helps the other people involved.

Comment 8

[Mike Hommey [:glandium]]

> But you're explicitly supposed *not* to do that, especially without _warning_
> people.

I usually do. Sorry to have been sloppy once in a while.

> for example, it looks like your new patch (context) here applies on top of your
> additional patch in bug 541767, though (it's fine but) you didn't say it.

But you asked for it. cf. comment 2.

Comment 9

[Serge Gautherie (:sgautherie)]

(In reply to comment #8)

> I usually do. Sorry to have been sloppy once in a while.

np. (Just don't react as if all was as expected when I wonder what is going on.)

> But you asked for it. cf. comment 2.

Yes and it's perfectly fine to have done it (that way):
I was simply saying that you are expected to tell the (context) dependency (especially from different bugs) when you attach a patch.

***

I was going (again) to check this in, but I still wonder:
for example, I see $(MOZ_APP_NAME).1 in no other app.

Ftr, could you write (in the bug) a (1+ line) explanation about why you're adding/removing each file? Thanks.

Comment 10

[Justin Wood (:Callek)]

(In reply to comment #9)
> Ftr, could you write (in the bug) a (1+ line) explanation about why you're
> adding/removing each file? Thanks.

Not necessary, I checked what all file changes are for. SeaMonkey has that .1 used.

Comment 11

[Mike Hommey [:glandium]]

http://hg.mozilla.org/comm-central/rev/48a4baab97ba