Build failure when trying to use Tup with "--enable-js-shell"

RESOLVED FIXED in Firefox 64

Status

defect
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: bzbarsky, Assigned: cmanchester)

Tracking

(Blocks 1 bug)

Trunk
mozilla64
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(2 attachments)

When trying to use Tup with a clean (well, nonexistent until I mach build) objdir, I get:

 0:17.27  ~99%   145) [0.002s] obj-firefox/js/src/gdb
 0:17.27 * ~99%   144) [0.001s] obj-firefox/js/src/shell
 0:17.27 tup error: Explicitly named file 'js' not found in subdir 'obj-firefox/dist/bin'
 0:17.27 tup error: Error parsing Tupfile line 9
 0:17.27   Line was: ': /home/bzbarsky/mozilla/debug/obj-firefox/dist/bin/js |> !tup_ln |> $(MOZ_OBJ_ROOT)/js/src/js $(MOZ_OBJ_ROOT)/<default>'
 0:17.27  *** tup: 1 job failed.

(those first two lines are Tup scanning directories, I think).

There is no js shell in the objdir's dist/bin because this is just starting a toplevel build, so the shell hasn't been built yet...
Can you post your mozconfig?

The error message indicates that the rule is trying to use obj-firefox/dist/bin/js as an input, but there is no rule to create that file. Sounds like we're either missing a mozbuild object, or generating the rules incorrectly for your config.
  mk_add_options MOZ_CO_PROJECT=suite,browser,mail,xulrunner,calendar

  mk_add_options MOZ_CO_MODULE="mozilla/other-licenses/libart_lgpl mozilla/tools"

  mk_add_options CCACHE_CPP2=1
  export CCACHE_CPP2=1

  export PKG_CONFIG_PATH=/usr/lib/x86_64-linux-gnu/pkgconfig
  mk_add_options PKG_CONFIG_PATH=/usr/lib/x86_64-linux-gnu/pkgconfig

  ac_add_options --disable-libjpeg-turbo
  ac_add_options --disable-crashreporter

  ac_add_options --enable-js-shell

  mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../obj-firefox

  ac_add_options --enable-application=browser
  ac_add_options --enable-debug
  ac_add_options --disable-optimize
  ac_add_options --enable-extensions=default

  export TUP=~/.mozbuild/tup/tup
  ac_add_options --enable-build-backends=Tup

There's some old cruft in there.  I expect --enable-js-shell is the relevant bit...
Hmm, we have a fix for the js shell, but that must have only worked for standalone js builds. This is the rule-ordering thing. I'll come up with a patch.
Assignee: nobody → cmanchester
Actually this is a little more complicated than that. Once I re-order the commands I get:

 0:20.07 * ~ 0% 3851) obj-tup/js/src/shell: CXX Unified_cpp_js_src_shell0.cpp
 0:20.07  *** tup messages ***
 0:20.07 tup error: Missing input dependency - a file was read from, and was not specified as an input link for the command. This is an issue because the file was created from another command, and without the input link the commands may execute out of order. You should add this file as an input, since it is possible this could randomly break in the future.
 0:20.07  - [560869] obj-tup/js/src/js

which I recall from getting js standalone builds to work means we're trying to read from "$objdir/js/src/js" when reading #includes, which tup thinks is the same thing as the symlink from "$objdir/js/src/js" to "$objdir/dist/bin/js". Mike, do you have any advice for a workaround here?

Maybe skipping the additional convenience symlink for now would be acceptable as well.
Flags: needinfo?(mshal)
(In reply to Chris Manchester (:chmanchester) from comment #4)
> which I recall from getting js standalone builds to work means we're trying
> to read from "$objdir/js/src/js" when reading #includes, which tup thinks is
> the same thing as the symlink from "$objdir/js/src/js" to
> "$objdir/dist/bin/js". Mike, do you have any advice for a workaround here?

Yeah, that's the problem. Since objdir/js/src/js is a file generated from a rule, other rules aren't allowed to read from it unless they declare it as a dependency. And we pick it up as a file access because when the preprocessor handles #include "js/Foo.h", with -Iobjdir/js/src, it accesses the generated js file. The includedir here is added via config.mk, so it's probably not straightforward to remove here even if js compiles don't rely on it.

We could try to use the feature I added for incremental rust compilation here, and ignore accesses to this file for js compilation commands. It's kinda cheaty, because if the objdir/js/src/js was no longer the js binary, but now a directory for header files, tup wouldn't recompile the commands that need to be recompiled. We can probably live with that though.

@@ -191,11 +191,14 @@ class BackendTupfile(object):
                 cmd.extend(shell_quote(f) for f in self.local_flags[flags])
                 cmd.extend(shell_quote(f) for f in self.per_source_flags[src])
                 cmd.extend([dash_c, '%f', '-o', '%o'])
+                outputs = [prefix + '%B.o']
+                if 'js/src' in self.relobjdir:
+                    outputs.append('^/js/src/js$')
                 self.rule(
                     cmd=cmd,
                     inputs=[src],
                     extra_inputs=extra_inputs,
-                    outputs=[prefix + '%B.o'],
+                    outputs=outputs,
                     display='%s %%f' % compiler,
                 )

> Maybe skipping the additional convenience symlink for now would be
> acceptable as well.

I thought we already did that?

https://dxr.mozilla.org/mozilla-central/rev/024521c589d28744b5c4a6c01127fa35edef2c53/python/mozbuild/mozbuild/backend/tup.py#1090

Though apparently it isn't getting triggered in this case.
Flags: needinfo?(mshal)
(In reply to Michael Shal [:mshal] from comment #5)
> (In reply to Chris Manchester (:chmanchester) from comment #4)
> > which I recall from getting js standalone builds to work means we're trying
> > to read from "$objdir/js/src/js" when reading #includes, which tup thinks is
> > the same thing as the symlink from "$objdir/js/src/js" to
> > "$objdir/dist/bin/js". Mike, do you have any advice for a workaround here?
> 
> Yeah, that's the problem. Since objdir/js/src/js is a file generated from a
> rule, other rules aren't allowed to read from it unless they declare it as a
> dependency. And we pick it up as a file access because when the preprocessor
> handles #include "js/Foo.h", with -Iobjdir/js/src, it accesses the generated
> js file. The includedir here is added via config.mk, so it's probably not
> straightforward to remove here even if js compiles don't rely on it.
> 
> We could try to use the feature I added for incremental rust compilation
> here, and ignore accesses to this file for js compilation commands. It's
> kinda cheaty, because if the objdir/js/src/js was no longer the js binary,
> but now a directory for header files, tup wouldn't recompile the commands
> that need to be recompiled. We can probably live with that though.
> 
> @@ -191,11 +191,14 @@ class BackendTupfile(object):
>                  cmd.extend(shell_quote(f) for f in self.local_flags[flags])
>                  cmd.extend(shell_quote(f) for f in
> self.per_source_flags[src])
>                  cmd.extend([dash_c, '%f', '-o', '%o'])
> +                outputs = [prefix + '%B.o']
> +                if 'js/src' in self.relobjdir:
> +                    outputs.append('^/js/src/js$')
>                  self.rule(
>                      cmd=cmd,
>                      inputs=[src],
>                      extra_inputs=extra_inputs,
> -                    outputs=[prefix + '%B.o'],
> +                    outputs=outputs,
>                      display='%s %%f' % compiler,
>                  )
> 
> > Maybe skipping the additional convenience symlink for now would be
> > acceptable as well.
> 
> I thought we already did that?
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 024521c589d28744b5c4a6c01127fa35edef2c53/python/mozbuild/mozbuild/backend/
> tup.py#1090
> 
> Though apparently it isn't getting triggered in this case.

Right, apparently JS_SHELL_NAME is only set under js/src. So the question is whether the symlink from 'dist/bin/js' to 'js/src/js' is useful enough to warrant this hack.

bz, is it important for your workflow that the shell can be accessed from $objdir/js/src/js as well as $objdir/dist/bin/js ?
Flags: needinfo?(bzbarsky)
For my purposes, I think objdir/dist/bin/js is the way I access the shell; I did not realize the other even worked.

I guess we should check which one "mach jsapi-tests" and "mach jstests" use.
Flags: needinfo?(bzbarsky)
Ok, I'm inclined to say we should just skip the symlink for now rather than add more hacks for it. I've confirmed those mach commands (and the others that seem to use a js shell) will find it under $objdir/dist/bin/js.
Summary: Build failure when trying to use Tup → Build failure when trying to use Tup with "--enable-js-shell"
Comment on attachment 9013446 [details]
Bug 1495108 - Skip symlinking $objdir/dist/bin/js to $objdir/js/src/js in the tup backend in browser builds.

Michael Shal [:mshal] has approved the revision.
Attachment #9013446 - Flags: review+
Comment on attachment 9013448 [details]
Bug 1495108 - Build the js shell during tup builds in automation.

Michael Shal [:mshal] has approved the revision.
Attachment #9013448 - Flags: review+
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8ee841ba4103
Skip symlinking $objdir/dist/bin/js to $objdir/js/src/js in the tup backend in browser builds. r=mshal
https://hg.mozilla.org/integration/autoland/rev/167c811dfccc
Build the js shell during tup builds in automation. r=mshal
https://hg.mozilla.org/mozilla-central/rev/8ee841ba4103
https://hg.mozilla.org/mozilla-central/rev/167c811dfccc
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.