Closed Bug 1218612 Opened 4 years ago Closed 4 years ago

`./mach build devtools/shared/heapsnapshot` doesn't propogate cpp changes

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(firefox44 fixed)

RESOLVED FIXED
Firefox 44
Tracking Status
firefox44 --- fixed

People

(Reporter: fitzgen, Assigned: jryans)

References

Details

(Keywords: regression)

Attachments

(1 file)

STR:

* Checkout fx-team
* Build
* Follow STR in https://bugzilla.mozilla.org/show_bug.cgi?id=1218597#c0
  * Note that STR lead to parseMessage failure described in https://bugzilla.mozilla.org/show_bug.cgi?id=1218597#c1
* Edit devtools/shared/heapsnapshot/HeapSnapshot.cpp, add `asm("int3");` before the `return false;` on this line: https://dxr.mozilla.org/mozilla-central/rev/28068d907290d1f5138a0b9e59ae2233a1c1b7a3/devtools/shared/heapsnapshot/HeapSnapshot.cpp#120
* Run `./mach build devtools/shared/heapsnapshot`

ER:

Re-running the STR under gdb will pause on the int3's SIGTRAP

AR:

It does not. However, if you do `./mach build` and then re-run the STR it does.
Flags: needinfo?(jryans)
I feel like it is not linking the new object file, since I do see "HeapSnapshot.o" in the log when running `./mach build devtools/shared/heapsnapshot`.
(In reply to Nick Fitzgerald [:fitzgen][:nf] from comment #1)
> I feel like it is not linking the new object file, since I do see
> "HeapSnapshot.o" in the log when running `./mach build
> devtools/shared/heapsnapshot`.

For example:

> 0:00.15 /usr/bin/make -C /Users/fitzgen/src/mozilla-central/obj.noindex -j8 -s backend.RecursiveMakeBackend
> 0:00.29 /usr/bin/make -C devtools/shared/heapsnapshot -j8 -s
> 0:00.33 HeapSnapshot.o
> 0:02.80 libdevtools_shared_heapsnapshot.a.desc
> 0:03.11 /usr/bin/make -C browser/app -j8 -s
> 0:03.83 ccache (direct) hit rate: 0.0%; (preprocessed) hit rate: 0.0%; miss rate: 100.0%
> 0:03.83 Your build was successful!
I believe this is specific to C++ linking, which you also seem to be guessing.

Does `./mach build devtools/shared/heapsnapshot toolkit/library` work?
Flags: needinfo?(jryans) → needinfo?(nfitzgerald)
And when I do `./mach build binaries` I see logs which I think are XUL being linked, that do not show up in doing `./mach build devtools/shared/heapsnapshot`:

> 0:03.52 libxul_s.a.desc
> 0:03.53 XUL
> 0:10.83 ld: warning: could not create compact unwind for _ffi_call_unix64: does not use RBP or RSP based frame
I believe the issue is that our dependency tree in dumbmake (which is used to auto-run toolkit/library) is not correct.

We can likely update this list to resolve it.  There was recent talk[1] of removing this feature, but there was no decision made.  It may be best to become comfortable with ./mach build binaries in any case.

[1]: https://groups.google.com/d/topic/mozilla.dev.platform/NxjF8Q4geGE/discussion
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #3)
> I believe this is specific to C++ linking, which you also seem to be
> guessing.
> 
> Does `./mach build devtools/shared/heapsnapshot toolkit/library` work?

This does work.
Flags: needinfo?(nfitzgerald)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #5)
> I believe the issue is that our dependency tree in dumbmake (which is used
> to auto-run toolkit/library) is not correct.
> 
> We can likely update this list to resolve it.  There was recent talk[1] of
> removing this feature, but there was no decision made.  It may be best to
> become comfortable with ./mach build binaries in any case.
> 
> [1]:
> https://groups.google.com/d/topic/mozilla.dev.platform/NxjF8Q4geGE/discussion

Why can't we just link if we re-built any object files? The other options seem like hacks...
(In reply to Nick Fitzgerald [:fitzgen][:nf] from comment #7)
> Why can't we just link if we re-built any object files? The other options
> seem like hacks...

I refer you to your nearest build peer for questions of this nature. :P I'm sure it would work once we've set the current system on fire sufficiently.
Blocks: 912121
Keywords: regression
Does this cause `./mach build devtools/shared/heapsnapshot` to do what you mean?
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Attachment #8679592 - Flags: review?(nfitzgerald)
Comment on attachment 8679592 [details] [diff] [review]
devtools-dumbmake

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

Yup, this fixes the issue!
Attachment #8679592 - Flags: review?(nfitzgerald) → review+
https://hg.mozilla.org/mozilla-central/rev/bdbacb254896
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.