Closed Bug 1169769 Opened 5 years ago Closed 5 years ago

js shell is not rebuilt when rebuilding by running make inside OBJDIR/js/src.

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: arai, Assigned: glandium)

References

Details

Attachments

(1 file)

Steps to reproduce:
  1. Run following command to build spidermonkey
    (cd js/src; autoconf213)
    mkdir obj-tmp; cd obj-tmp
    ../js/src/configure && make
  2. add empty line to the end of js/src/builtin/RegExp.cpp
  3. build spidermonkey again with following command (in obj-tmp)
    cd js/src
    make

Actual result:
  js shell (obj-tmp/js/src/shell/js) is not rebuilt on step 3

Expected result:
  js shell is rebuilt on step 3


Similar issue happens when I do incremental build with mach.

Steps to reproduce (2):
  1. Run following command to build firefox
    ./mach build
  2. add empty line to the end of js/src/builtin/RegExp.cpp
  3. build firefox again with following command
    ./mach build js/src

Actual result (2):
  js shell is not rebuilt on step 3

Expected result (2):
  js shell is rebuilt on step 3


The behavior is changed by bug 1168251:
  http://hg.mozilla.org/mozilla-central/rev/a19fd434d22a
bug 1168251 comment 14:

> I bet this breaks someone's workflow. Let's land it.

Indeed it does.
OS: Mac OS X → All
Hardware: x86_64 → All
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1168251
Wait, what?  This bug is _caused_ by the patch for bug 1168251, no?  How is it a duplicate?

This totally bit me today; took forever to figure out why shell was not being rebuilt when it should have been...
Status: RESOLVED → REOPENED
Flags: needinfo?(mh+mozilla)
Resolution: DUPLICATE → ---
Something's weird here. When I saw this RESOLVED as a dupe of a FIXED bug, I updated and tried again, and sure enough it recursed properly. But a little later, I noticed it happening again (though I think it was with jsapi-tests this time.) So it looks like both bz and I are still seeing breakage here, though it may be a little more subtle than it was?

I haven't tried coming up with exact STR, and my tree is not compilable at the moment.
My str is trivial:

1)  Modify jsfun.cpp
2)  mach build js/src

shell is not rebuilt.

If instead of step 2 I do "mach build js/src/shell" then shell _is_ rebuilt if I'd changed js.cpp, but jsfun.cpp is not, so the shell executable doesn't get the updated jsfun object bits.

The only way I can get a properly rebuilt shell after changing jsfun.cpp is "mach build js/src js/src/shell".  Which is rather annoying.
Come to think of it, I might be wrong. Sometimes I compile from the toplevel ($objdir instead of $objdir/js/src), even though I've configured via js/src/configure. I bet that works, but compiling from $objdir/js/src still does not.

Yes, I tried that out, and it is definitely compiling neither the shell/ nor jsapi-tests/ subdirs if I run make from $objdir/js/src, while it does if I run it from $objdir/.
(In reply to Not doing reviews right now from comment #3)
> Wait, what?  This bug is _caused_ by the patch for bug 1168251, no?  How is
> it a duplicate?

The root cause of that bug was the lack of recursion for the compile target when not doing top-level builds. That's exactly what this bug is too... but see below

(In reply to Not doing reviews right now from comment #5)
> My str is trivial:
> 
> 1)  Modify jsfun.cpp
> 2)  mach build js/src
> 
> shell is not rebuilt.

Well, that would be a problem, and I can reproduce. The crux of the issue is that the build system believes js/src is a top-level directory, and what has been done in bug 1168251 doesn't apply in this particular case. That is, if that were about any other directory in the tree, it would have worked, but js/src is special. Let's change this.

> If instead of step 2 I do "mach build js/src/shell" then shell _is_ rebuilt
> if I'd changed js.cpp, but jsfun.cpp is not, so the shell executable doesn't
> get the updated jsfun object bits.
> 
> The only way I can get a properly rebuilt shell after changing jsfun.cpp is
> "mach build js/src js/src/shell".  Which is rather annoying.

You should just use mach build binaries.
Flags: needinfo?(mh+mozilla)
The dependency is opposite, because without bug 1168251, the fix I'm about to attach wouldn't do anything.
No longer blocks: 1168251
Depends on: 1168251
Attachment #8617679 - Flags: review?(gps) → review+
Mike, thanks for fixing this!

> You should just use mach build binaries.

Yeah, I probably should.  It's noticeably slower than just building the relevant dir for builds that are no-ops or hit ccache, which is a lot of the builds I do.  :(
https://hg.mozilla.org/mozilla-central/rev/89a60988b587
Assignee: nobody → mh+mozilla
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.