1043692 - move DIST_INSTALL into moz.build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Nathan Froyd [:froydnj]]

This is pretty easy; the most complex part is describing exactly what DIST_INSTALL does, which I think I understand now.

Comment 1

[Mike Hommey [:glandium]]

Actually, there's something even easier for, I guess, quite a few DIST_INSTALLs: remove them. (when looking at the patch in bug 1042878, i saw those in intl/unicharutil/util/*, and I bet they are useless nowadays ; I'm sure there are many others)

Comment 2

[Mike Hommey [:glandium]]

(and many others, like, I suspect, the ones under xpcom/*, could be replaced with NO_EXPAND_LIBS)

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Mike Hommey [:glandium] from comment #1)
> Actually, there's something even easier for, I guess, quite a few
> DIST_INSTALLs: remove them. (when looking at the patch in bug 1042878, i saw
> those in intl/unicharutil/util/*, and I bet they are useless nowadays ; I'm
> sure there are many others)

I'm not sure I'm prepared to have the conversation about what libraries we should and shouldn't export.  I guess Benjamin might have an opinion about ceasing to export librdfutil_external, libunicharutil_external, and maybe a few others?

(In reply to Mike Hommey [:glandium] from comment #2)
> (and many others, like, I suspect, the ones under xpcom/*, could be replaced
> with NO_EXPAND_LIBS)

NO_EXPAND_LIBS is somewhat less obvious in its effects in this case, IMHO, compared to DIST_INSTALL.  NO_EXPAND_LIBS seems like it should just be renamed, since the build/ cases of NO_EXPAND_LIBS should be replaced with DIST_INSTALL, and the js/src/ usage of NO_EXPAND_LIBS can be something more sensible.  WDYT?

Comment 4

[Mike Hommey [:glandium]]

(In reply to Nathan Froyd (:froydnj) from comment #3)
> (In reply to Mike Hommey [:glandium] from comment #1)
> > Actually, there's something even easier for, I guess, quite a few
> > DIST_INSTALLs: remove them. (when looking at the patch in bug 1042878, i saw
> > those in intl/unicharutil/util/*, and I bet they are useless nowadays ; I'm
> > sure there are many others)
> 
> I'm not sure I'm prepared to have the conversation about what libraries we
> should and shouldn't export.  I guess Benjamin might have an opinion about
> ceasing to export librdfutil_external, libunicharutil_external, and maybe a
> few others?

Anything that's not in the SDK doesn't need to be in dist/bin|dist/lib anymore unless it ends up in the final product. The only reason we needed things in dist/lib was to simplify *LIBS things in the Make land.

> (In reply to Mike Hommey [:glandium] from comment #2)
> > (and many others, like, I suspect, the ones under xpcom/*, could be replaced
> > with NO_EXPAND_LIBS)
> 
> NO_EXPAND_LIBS is somewhat less obvious in its effects in this case, IMHO,
> compared to DIST_INSTALL.  NO_EXPAND_LIBS seems like it should just be
> renamed, since the build/ cases of NO_EXPAND_LIBS should be replaced with
> DIST_INSTALL, and the js/src/ usage of NO_EXPAND_LIBS can be something more
> sensible.  WDYT?

There is no case of NO_EXPAND_LIBS that should be replaced with DIST_INSTALL. They do different things. DIST_INSTALL tells to copy stuff in dist/bin or dist/lib. It has the side effect of kind of implying NO_EXPAND_LIBS. NO_EXPAND_LIBS tells to create a real library file instead of a fake lib used by expandlib (a .desc file ; although in that case, the .desc file is still created). Yes, it could be renamed.

Comment 5

[Mike Hommey [:glandium]]

Attachment: Add a DIST_INSTALL variable to moz.build, and replace NO_DIST_INSTALL with it

As I thought I needed this knowledge for some other reason, I just went ahead. Turns out I don't need it, but now that it's done...

Comment 6

[Mike Hommey [:glandium]]

Attachment: Move DIST_INSTALL to moz.build

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8603635 [details] [diff] [review]
Add a DIST_INSTALL variable to moz.build, and replace NO_DIST_INSTALL with it

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

Mostly a semantic refactor. LGTM.

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8603636 [details] [diff] [review]
Move DIST_INSTALL to moz.build

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

I love reviewing patches that remove Makefile.in files!

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/59e335c30dfd
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bc266372d00

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/59e335c30dfd
https://hg.mozilla.org/mozilla-central/rev/9bc266372d00