Mike Hommey [:glandium] Assignee
	Description • 11 years ago

First baby step in the direction of bug 950297.

configure will still leave in js/src, but running it will create a tree with at least mfbt, js/src and intl/icu in it.

	Mike Hommey [:glandium] Assignee
	Comment 1 • 11 years ago

This passed try on an older tree, except for the Hf job. I'll attach incremental patches for fixups.

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 2 • 11 years ago

Comment on attachment 8357709 [details] [diff] [review]
Make the js build system use top-level as its root objdir

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

::: moz.build
@@ +21,3 @@
>  
> +    if CONFIG['ENABLE_CLANG_PLUGIN']:
> +      add_tier_dir('base', 'build/clang-plugin', external=True)

Two more spaces while you're reindenting

	Mike Hommey [:glandium] Assignee
	Comment 3 • 11 years ago

This is the hg rm part.

	Mike Hommey [:glandium] Assignee
	Comment 4 • 11 years ago

Same as the previous, without the hg rm parts, and with a few fixes. It now doesn't break hazard analysis builds on try because bug 958295 has landed.

	Mike Hommey [:glandium] Assignee
	Comment 5 • 11 years ago

With indentation fixed as per ms2ger's comment.

	Gregory Szorc [:gps]
	Comment 6 • 11 years ago

Comment on attachment 8358090 [details] [diff] [review]
Remove js/src/config and js/src/build

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

/me pours gasoline and lights a match.

	Gregory Szorc [:gps]
	Comment 7 • 11 years ago

Comment on attachment 8358220 [details] [diff] [review]
Make the js build system use top-level as its root objdir

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

Crazy patch. It looks sane to me. Super excited about what this is setting us up for!

I think you should do a PGO try before landing if you haven't already - if history has taught me anything, it's than any changes to /Makefile.in end up breaking PGO on first landing.

::: Makefile.in
@@ +126,5 @@
> +# finishing to merge gecko and js build systems, removing the need for the
> +# latter.
> +ifdef BUILDING_JS
> +NO_REMOVE=1
> +endif

There's a followup bug lingering here.

::: config/rules.mk
@@ +1461,5 @@
>  
>  # If we're using binary nsinstall and it's not built yet, fallback to python nsinstall.
> +ifneq (,$(filter $(DIST)/bin/nsinstall$(HOST_BIN_SUFFIX),$(install_cmd)))
> +ifeq (,$(wildcard $(DIST)/bin/nsinstall$(HOST_BIN_SUFFIX)))
> +nsinstall_is_usable = $(if $(wildcard $(DIST)/bin/nsinstall$(HOST_BIN_SUFFIX)),yes)

Why are you using $(DIST)/bin/nsinstall$(HOST_BIN_SUFFIX) instead of $(NSINSTALL)?

Can we just move this check to config.mk? Should we just build nsinstall more specially than we do today? Although, uses of nsinstall will wane with everything moving to mozpack.

Followup bugs, I reckon.

::: config/static-checking-config.mk
@@ +47,4 @@
>  CLANG_PLUGIN := $(DEPTH)/build/clang-plugin/$(DLL_PREFIX)clang-plugin$(DLL_SUFFIX)
> +else
> +CLANG_PLUGIN := $(DEPTH)/../../build/clang-plugin/$(DLL_PREFIX)clang-plugin$(DLL_SUFFIX)
> +endif

Shouldn't CLANG_PLUGIN be defined in config.mk?

::: moz.build
@@ +5,5 @@
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  CONFIGURE_SUBST_FILES += [
> +    'config/autoconf.mk',
> +    'config/emptyvars.mk',

Ah, I was wondering where these went!

::: python/mozbuild/mozbuild/backend/recursivemake.py
@@ +552,5 @@
>                  if dirs:
>                      # For build systems without tiers (js/src), output a list
>                      # of directories for each tier.
> +                    all_dirs = list(self._traversal.traverse('', filter))
> +                    root_mk.add_statement('%s_dirs := %s' % (tier, ' '.join(all_dirs)))

You don't need to cast to a list since .join() will take a generator.

::: testing/testsuite-targets.mk
@@ +478,5 @@
>  stage-xpcshell: make-stage-dir
>  	$(MAKE) -C $(DEPTH)/testing/xpcshell stage-package
>  
>  stage-jstests: make-stage-dir
> +	$(MAKE) -C $(DEPTH)/js/src/js/src/tests stage-package

That's a funky path!

	Mike Hommey [:glandium] Assignee
	Comment 8 • 11 years ago

(In reply to Gregory Szorc [:gps] from comment #7)
> Comment on attachment 8358220 [details] [diff] [review]
> Make the js build system use top-level as its root objdir
> 
> Review of attachment 8358220 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Crazy patch. It looks sane to me. Super excited about what this is setting
> us up for!
> 
> I think you should do a PGO try before landing if you haven't already - if
> history has taught me anything, it's than any changes to /Makefile.in end up
> breaking PGO on first landing.
> 
> ::: Makefile.in
> @@ +126,5 @@
> > +# finishing to merge gecko and js build systems, removing the need for the
> > +# latter.
> > +ifdef BUILDING_JS
> > +NO_REMOVE=1
> > +endif
> 
> There's a followup bug lingering here.

?

> ::: config/rules.mk
> @@ +1461,5 @@
> >  
> >  # If we're using binary nsinstall and it's not built yet, fallback to python nsinstall.
> > +ifneq (,$(filter $(DIST)/bin/nsinstall$(HOST_BIN_SUFFIX),$(install_cmd)))
> > +ifeq (,$(wildcard $(DIST)/bin/nsinstall$(HOST_BIN_SUFFIX)))
> > +nsinstall_is_usable = $(if $(wildcard $(DIST)/bin/nsinstall$(HOST_BIN_SUFFIX)),yes)
> 
> Why are you using $(DIST)/bin/nsinstall$(HOST_BIN_SUFFIX) instead of
> $(NSINSTALL)?

Because NSINSTALL is either nsinstall or nsinstall.py, and we obviously don't want this block to work for nsinstall.py. Note I'm not changing any logic here.
 
> ::: config/static-checking-config.mk
> @@ +47,4 @@
> >  CLANG_PLUGIN := $(DEPTH)/build/clang-plugin/$(DLL_PREFIX)clang-plugin$(DLL_SUFFIX)
> > +else
> > +CLANG_PLUGIN := $(DEPTH)/../../build/clang-plugin/$(DLL_PREFIX)clang-plugin$(DLL_SUFFIX)
> > +endif
> 
> Shouldn't CLANG_PLUGIN be defined in config.mk?

This *is* config.mk (indirectly).
 
> ::: testing/testsuite-targets.mk
> @@ +478,5 @@
> >  stage-xpcshell: make-stage-dir
> >  	$(MAKE) -C $(DEPTH)/testing/xpcshell stage-package
> >  
> >  stage-jstests: make-stage-dir
> > +	$(MAKE) -C $(DEPTH)/js/src/js/src/tests stage-package
> 
> That's a funky path!

Temporary until things are actually merged.

	Mike Hommey [:glandium] Assignee
	Comment 9 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a1968da61b3

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 10 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/1a1968da61b3