Since bug 467583 landed, I don't think there's any reason to call "make install" in js/src anymore. All the binaries and libs should wind up in the right spots by default since we instruct the spidermonkey build to use the top-level dist directory. We will have to make js/src/Makefile.in use EXPORTS to get its headers into dist/include/js, but I think that's the only real change needed.

I think this might fix the underlying cause of bug 471097.

Also see bug 471097 comment 6

I think this is right. My original idea had been to make js/src and self-contained GNU-style subdirectory, since those are rules that are easy to implement and use. But Mozilla has a lot of expectations about how dist behaves (rebuilds are visible immediately; PGO data) that GNU behavior won't support. So there's no way to avoid having js/src/Makefile.in have two modes. It seems to me Mozilla mode should be enabled by --with-dist-dir.

That is, NSDISTMODE in js/src/Makefile should be affected by --with-dist-dir: when that flag is specified, NSDISTMODE should be left unset, to get the symlink behavior (relative on Linux; absolute on Darwin). When it's omitted or given as --without-dist-dir, NSDISTMODE should be set to copy, to get GNU behavior. Then, config/js/Makefile.in can be simplified as appropriate. There shouldn't be any need to set JS_MOZ_INSTALL. Actually, if config/js/build.mk can list js/src simply as an ordinary directory for the js tier, not a staticdir, then could config/js/Makefile.in go away entirely?

This patch removes config/js/Makefile.in, has config/js/build.mk simply set |tier_js_dirs = js/src|, and then tweaks js/src/Makefile.in a bit such that a normal make export/libs there should get everything into the place that the mozilla build expects it. I've thrown it at the try server to verify that it doesn't break anything stupid.

Yeah, this fails on the try server. I added EXPORTS to js/src/Makefile.in, but nsinstall isn't actually built yet there (since it gets built in js/src/config).

For in-mozilla use, shouldn't js/src just use the top-level nsinstall the way it used to? By the way, we should rename --with-dist-dir to --with-mozilla-dist-dir or something like that, to better indicate that this is the flag that changes the mode of the makefile from 'stand-alone' to 'in-mozilla'.

This patch builds fine on Win32/Linux/Mac. I removed config/js/Makefile.in entirely, cleaned up the install target in js/src to use SYSINSTALL, and added a slight hack to ensure that nsinstall gets built before it's needed in the export phase.

Comment on attachment 355784 [details] [diff] [review] don't call the install target, remove unnecessary stuff This looks fine to me. I'm glad to see all that code go away. If the reason for changing the $(SDK_PUBLIC) $(PUBLIC) rule in rules.mk to a single-colon rule is something other than "This should never have been a double-colon rule to begin with", could we get a comment for that?

(In reply to comment #9) > (From update of attachment 355784 [details] [diff] [review]) > This looks fine to me. I'm glad to see all that code go away. > > If the reason for changing the $(SDK_PUBLIC) $(PUBLIC) rule in rules.mk to a > single-colon rule is something other than "This should never have been a > double-colon rule to begin with", could we get a comment for that? It was just to make the dependency on nsinstall in the Makefile work. I didn't see any reason that should have been a double colon rule to begin with. Do you really think that justifies a comment?

(In reply to comment #10) > It was just to make the dependency on nsinstall in the Makefile work. I didn't > see any reason that should have been a double colon rule to begin with. Do you > really think that justifies a comment? In that case, no. It's only if there was some obvious (just not to me) reason for it to be :: and a non-obvious reason to make it :. But I don't see why it should have been :: either.

Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/3da64152f578

Confirmed no more multiple attempts at linking js/src. Also, no more js3250.lib and js-config in dist/bin. -V.FIXED

Comment on attachment 355784 [details] [diff] [review] don't call the install target, remove unnecessary stuff This is needed to fix bug 467271, which is blocking1.9.1+

Comment on attachment 355784 [details] [diff] [review] don't call the install target, remove unnecessary stuff a191=beltzner

Pushed to 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/27a7287e4809

(In reply to comment #10) > (In reply to comment #9) > > (From update of attachment 355784 [details] [diff] [review] [details]) > > This looks fine to me. I'm glad to see all that code go away. > > > > If the reason for changing the $(SDK_PUBLIC) $(PUBLIC) rule in rules.mk to a > > single-colon rule is something other than "This should never have been a > > double-colon rule to begin with", could we get a comment for that? > > It was just to make the dependency on nsinstall in the Makefile work. I didn't > see any reason that should have been a double colon rule to begin with. Do you > really think that justifies a comment? Ironically this caused my comm-central build to fail. My directory structure is as follows: C:\build\comm - hg clone of comm-central C:\build\mozilla - srcdir build of the 1.8 branch (contains dist) C:\build\objdir - objdir for comm-central (contains mozilla/dist) The problem arises when trying to export headers in a comm-central subdirectory 4 levels deep. In this case, $(PUBLIC) equals ../../../../mozilla/dist/include/$(MODULE) which should be C:\build\objdir\mozilla\dist\include\$(MODULE) however gmake discovers that it can resolve it against the VPATH (from rules.mk) of ../../../../mozilla/dist/lib i.e. C:\build\objdir\mozilla\dist\lib to make ../../../../mozilla/dist/lib/../../../../mozilla/dist/include/$(MODULE) which points into my 1.8 branch build :-(

(In reply to comment #16) > Pushed to 1.9.1: > http://hg.mozilla.org/releases/mozilla-1.9.1/rev/27a7287e4809 This push didn't change the install:: targets like the push to m-c did. This conflicts with a later patch of brendan's I'm trying to merge: -install:: $(SCRIPTS) $(PROGRAM) +install:: $(SCRIPTS) $(SYSINSTALL) $^ $(bindir) on the branch, it looks like this: install:: $(SCRIPTS) $(PROGRAM) $(SYSINSTALL) $(IFLAGS2) $^ $(bindir) what should be done?

Probably some merge fail. I believe the latter version (with $(IFLAGS2)) is the more correct one. Just change trunk to match? (r=me if you want it)