852869 - Remove some redundant Makefile.in files in comm-central

Status: RESOLVED FIXED

(MailNews Core :: Build Config, defect)

Description

[Mark Banner (:standard8)]

Now that we've started moving makefile variables to moz.build, and that m-c has started removing empty Makefile.in we're able to do the same as well.

Attaching some patches that start us doing this. They've been tested on try and passed to the same extent as the tree state at the time I pushed:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=ce7c10122be8

Comment 1

[Mark Banner (:standard8)]

Attachment: [checked in] Port rules.mk part of bug 757637

This ports the rules.mk part of bug 757637 from m-c and is the part that lets us actually not need to have Makefile.in files.

Comment 2

[Mark Banner (:standard8)]

Attachment: [checked in] Simple removals

These are all empty Makefile.ins

Comment 3

[Mark Banner (:standard8)]

Attachment: [checked in] More complicated removals

This consists of:

1) Removing Makefiles.in which have a MODULE definition, but don't actually do anything (nothing else in dir)

2) For some of those, I've also removed the entering in the empty directory.

Comment 4

[Mark Banner (:standard8)]

Attachment: [checked in] Move mime/emitters/src to mime/emitters Part 1

mime/emitters only has one directory - src. We can just collapse that into mime/emitters.

I've done this as two parts so we can preserve history better.

Comment 5

[Mark Banner (:standard8)]

Attachment: [checked in] Move mime/emitters/src to mime/emitters Part 2

Comment 6

[[e-mail]]

Comment on attachment 727105 [details] [diff] [review]
[checked in] Port rules.mk part of bug 757637

I was just about to file a bug on this because I noticed that my external API build got stuck on a Makefile.in file "missing" from LDAP...

Comment 7

[[e-mail]]

Although, rather annoyingly, all my mailnews/.../backend.mk files seem to turn off Makefile generation, which makes testing patches with Makefile changes interesting to say the least...

Comment 8

[Mark Banner (:standard8)]

(In reply to [e-mail] from comment #7)
> Although, rather annoyingly, all my mailnews/.../backend.mk files seem to
> turn off Makefile generation, which makes testing patches with Makefile
> changes interesting to say the least...

I think I know where the issue is here. I'll look at it in a hour or two.

Comment 9

[Mark Banner (:standard8)]

Attachment: [checked in] Port part 16 of bug 784841

This ports part 16 of bug 784841. This should fix the dependency issues - I think we'll need attachment 727105 [details] [diff] [review] and this one applied, then a clobber, then pushing the other makefile patches shouldn't need a clobber for each patch.

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 727105 [details] [diff] [review]
[checked in] Port rules.mk part of bug 757637

apparently bitrotted

Comment 11

[Magnus Melin [:mkmelin]]

Comment on attachment 727106 [details] [diff] [review]
[checked in] Simple removals

 hg qpush -v
applying makefile_remove_mime_emitter
patching file mailnews/build/Makefile.in
Hunk #1 succeeded at 48 (offset -1 lines).
Hunk #2 succeeded at 99 (offset -1 lines).
cannot create mailnews/mime/emitters/Makefile.in: destination already exists
mailnews/build/Makefile.in

Comment 12

[Mark Banner (:standard8)]

(In reply to Magnus Melin from comment #10)
> Comment on attachment 727105 [details] [diff] [review]
> Port rules.mk part of bug 757637
> 
> apparently bitrotted

Works fine here... Maybe you're applying the patches in the wrong order?

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 734499 [details] [diff] [review]
[checked in] Port part 16 of bug 784841

Seeing if Joshua wants to take these on

Comment 14

[Mark Banner (:standard8)]

Pushed all these patches to try server here:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=f990f0a95dc1

Comment 15

[Joshua Cranmer [:jcranmer]]

Comment on attachment 727106 [details] [diff] [review]
[checked in] Simple removals

Review of attachment 727106 [details] [diff] [review]:
-----------------------------------------------------------------

I've built with this locally on the try stack; given the Makefile.in-is-unnecessary patch landing first, I have no problems with this.

Comment 16

[Joshua Cranmer [:jcranmer]]

Comment on attachment 727109 [details] [diff] [review]
[checked in] Move mime/emitters/src to mime/emitters Part 1

It took me a while to realize that this patch isn't safe by itself, but needs the next patch.

Comment 17

[Joshua Cranmer [:jcranmer]]

Comment on attachment 727110 [details] [diff] [review]
[checked in] Move mime/emitters/src to mime/emitters Part 2

This is one of the directories that always struck me as needlessly deep.

Comment 18

[Mark Banner (:standard8)]

(In reply to Joshua Cranmer [:jcranmer] from comment #15)
> I've built with this locally on the try stack; given the
> Makefile.in-is-unnecessary patch landing first, I have no problems with this.

We need that patch, but also we need attachment 734499 [details] [diff] [review] landing before as well, as otherwise dep builds may get messed up.

Comment 19

[Joshua Cranmer [:jcranmer]]

Comment on attachment 734499 [details] [diff] [review]
[checked in] Port part 16 of bug 784841

Review of attachment 734499 [details] [diff] [review]:
-----------------------------------------------------------------

::: config/config.mk
@@ -686,5 @@
>  
> -ifdef TIERS
> -DIRS += $(foreach tier,$(TIERS),$(tier_$(tier)_dirs))
> -STATIC_DIRS += $(foreach tier,$(TIERS),$(tier_$(tier)_staticdirs))
> -endif

What's the purpose of moving this from config/config.mk to config/rules.mk?

Comment 20

[Joshua Cranmer [:jcranmer]]

Comment on attachment 727108 [details] [diff] [review]
[checked in] More complicated removals

Review of attachment 727108 [details] [diff] [review]:
-----------------------------------------------------------------

I've never been a really big fan of overuse of the public/src/build directory separation, particularly in cases like db/mork where the public directory contains a single header to export (perhaps I should file a bug on getting rid of excessive directory usage).

That said, I'm not clear why some of the other directories (like news) weren't inlined as well.

Comment 21

[Mark Banner (:standard8)]

(In reply to Joshua Cranmer [:jcranmer] from comment #19)
> ::: config/config.mk
> @@ -686,5 @@
> >  
> > -ifdef TIERS
> > -DIRS += $(foreach tier,$(TIERS),$(tier_$(tier)_dirs))
> > -STATIC_DIRS += $(foreach tier,$(TIERS),$(tier_$(tier)_staticdirs))
> > -endif
> 
> What's the purpose of moving this from config/config.mk to config/rules.mk?

I think just organisation - it is actually a rule, rather than a configuration, I was also following parts of this patch (https://bug784841.bugzilla.mozilla.org/attachment.cgi?id=717574) so gps might know more, but I really suspect its more of a rule option than a config one.

Comment 22

[Joshua Cranmer [:jcranmer]]

Comment on attachment 734499 [details] [diff] [review]
[checked in] Port part 16 of bug 784841

Review of attachment 734499 [details] [diff] [review]:
-----------------------------------------------------------------

According to gps, the original reason for the move is that the DIRS variable might not be defined at this point in time for config/config.mk. Since we've lasted this long without it, it's not a necessary move, but moving it to prevent any surprises is fine by me.

Comment 23

[Mark Banner (:standard8)]

Comment on attachment 727105 [details] [diff] [review]
[checked in] Port rules.mk part of bug 757637

https://hg.mozilla.org/comm-central/rev/3281b0e5ed4d

Comment 24

[Mark Banner (:standard8)]

Comment on attachment 734499 [details] [diff] [review]
[checked in] Port part 16 of bug 784841

https://hg.mozilla.org/comm-central/rev/fe96d228cc6a

Comment 25

[Mark Banner (:standard8)]

Comment on attachment 727106 [details] [diff] [review]
[checked in] Simple removals

https://hg.mozilla.org/comm-central/rev/6dca35019107

Comment 26

[Mark Banner (:standard8)]

Comment on attachment 727109 [details] [diff] [review]
[checked in] Move mime/emitters/src to mime/emitters Part 1

https://hg.mozilla.org/comm-central/rev/aeb3e1a89e9b

Comment 27

[Mark Banner (:standard8)]

Comment on attachment 727110 [details] [diff] [review]
[checked in] Move mime/emitters/src to mime/emitters Part 2

https://hg.mozilla.org/comm-central/rev/2f747ef4292e

Comment 28

[Mark Banner (:standard8)]

(In reply to Joshua Cranmer [:jcranmer] from comment #20)
> Comment on attachment 727108 [details] [diff] [review]
> More complicated removals
> 
> Review of attachment 727108 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I've never been a really big fan of overuse of the public/src/build
> directory separation, particularly in cases like db/mork where the public
> directory contains a single header to export (perhaps I should file a bug on
> getting rid of excessive directory usage).

For things like public/src/build we can get rid of those in follow-up bugs. This patch just gets rid of some of the empty redundant directories that we're going into at the moment. It'll be easy to re-instantiate those.

> That said, I'm not clear why some of the other directories (like news)
> weren't inlined as well.

See comment 3 - I was focusing on the ones which had a redundant MODULE definition in them, not just ones that were empty directories that are just forwarding on to others. I can file a follow-up to look for those if you want.

Comment 29

[Mark Banner (:standard8)]

Comment on attachment 727108 [details] [diff] [review]
[checked in] More complicated removals

https://hg.mozilla.org/comm-central/rev/e80ecf705881