553142 - Update symbol generation for hg

Status: VERIFIED FIXED

(Camino Graveyard :: General, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I haven't figured out yet *what* will need changing (the regexp in the Makefile *and* symbolstore.py?), but buildcaminosymbols is giving me local filepaths for files referenced in Camino (even when I comment out the regexp).  Incidentally, this is the same problem Sparkle symbols have (bug 535614).

Also incidentally, there's a comment that mentions "if we can't figure out the source root from the mozconfig, use the tinderbox path" http://mxr.mozilla.org/mozilla/source/camino/scripts/symbolstore.py#506 that appears to have come from adding Hg support to symbolstore.py :P (bug 440001).

Hoping Stuart will want to tackle this one ;)

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I wonder if this is related to the sourcestamp/sourcerepo stuff in platform.ini?  We used not to build in toolkit/xre and had to make our own; now we do build there (because we can't not, although we don't link or package or use any of that code), and it generates a platform.ini file for us with valid Gecko info.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

comm-* apps put their comm-* revision in application.ini, so I'll see if faking an application.ini with that info, like we used to fake a platform.ini, helps (though I still don't understand where symbolstore.py is deciding where to look for this info).

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

OK, that's not it.  I found the patch that added multiple-repo support to symbolstore.py (bug 452866) and that code is already in our fork.

So it looks like "-s $(shell pwd)" isn't doing the right thing anymore http://hg.mozilla.org/users/alqahira_ardisson.org/camino-1.9.2-test/file/3d797c134820/Makefile.in#l380

(That could also possibly explain the sparkle weirdness in bug 535614; dunno.)

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

If I switch $(shell pwd) to $(srcdir) (or to another variable that's equivalent to $(srcdir)), symbolstore.py becomes very unhappy:

Unexpected error:  <type 'exceptions.UnboundLocalError'>
Traceback (most recent call last):
  File "/Users/smokey/Camino/dev/192branch/mozilla/camino/scripts/symbolstore.py", line 824, in <module>
    main()
  File "/Users/smokey/Camino/dev/192branch/mozilla/camino/scripts/symbolstore.py", line 820, in main
    dumper.Process(arg)
  File "/Users/smokey/Camino/dev/192branch/mozilla/camino/scripts/symbolstore.py", line 482, in Process
    return self.ProcessFile(file_or_dir)
  File "/Users/smokey/Camino/dev/192branch/mozilla/camino/scripts/symbolstore.py", line 742, in ProcessFile
    res = Dumper.ProcessFile(self, file)
  File "/Users/smokey/Camino/dev/192branch/mozilla/camino/scripts/symbolstore.py", line 549, in ProcessFile
    (filename, rootname) = GetVCSFilename(filename, self.srcdirs)
  File "/Users/smokey/Camino/dev/192branch/mozilla/camino/scripts/symbolstore.py", line 378, in GetVCSFilename
    fileInfo = HGFileInfo(file, srcdir)
  File "/Users/smokey/Camino/dev/192branch/mozilla/camino/scripts/symbolstore.py", line 309, in __init__
    HGRepoInfo.repos[srcdir] = HGRepoInfo(path, rev, cleanroot)
UnboundLocalError: local variable 'cleanroot' referenced before assignment
make: *** [buildcaminosymbols] Error 1

(If I try that switch on cvs trunk, I end up with bogus paths in my symbol files, so $(shell pwd) is clearly the right thing there.)

Why does $(topsrcdir) work for Core files but $(srcdir) (which actually has a .hg/ folder) not work for Camino files?  Why did we use $(shell pwd) before (and how in the world does it make symbolstore.py do the right thing in an objdir that, by its very nature, has no VCS markers at all)?

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

More info: 

1) When I set $(srcdir), Appearance and Camino both get processed, meaning it dies on Downloads.  Appearance and Camino don't, however, end up with vcs-info-tuple paths.

2) My camino/ was a local clone of my clone of the remote repo (and so "default=" was set to the path to the clone of the remote repo); when I changed the "default=" in the [paths] section to be the url of the remote repo, then setting $(srcdir) doesn't cause symbolstore.py to bail (there must be some bad assumption in the script that the clone is always directly of a remote repo; it's a fine assumption for a tinderbox, but not at all useful for local work on the script).

However, even then, I still don't get vcs-info-tuple paths :(  I still have full local objdir paths.

I really don't know how the script does the "whatever path dump_syms outputs -> vcs-info-tuple" writing, and even if I did, I don't know python, so I don't know where or how I could put in some NSLog-type statements to see where things are going wrong :(

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

OK, I *think* I know what's going on now.  symbolstore.py is expecting a 1:1 correlation between |file/path/segment/to/strip| and |vcsroot|.

As far as I can tell, Core files are created with file paths for the srcdir (dump_syms without symbolstore.py seems to only export public functions and not FILE info from non-dSYM bundles?!?, so I can't see what is in the symbols "natively" before symbolstore.py touches them).

Our files are created with file paths for the objdir, but there's no vcs info there for Hg (I'm really not sure how we cheated for CVS, though?!), so "-s $(shell pwd)" causes symbolstore.py not to output any vcs-info, so it makes no attempt to modify the paths.  Passing "-s $(srcdir)" gets valid vcs-info, but then the paths in the files don't match, so symbolstore.py's s/path/vcs-info/g doesn't do anything, and the end result is the same, local file paths for the objdir for all FILEs.

I'm not sure how we fix this; either we pass in an extra (and yet optional) argument for one of the two things "-s foo" is supposed to represent, or we hard-code some assumptions into symbolstore.py about the relationship between the srcdir and objdir in camino/ and logic only to apply those assumptions when "camino" appears in the FILE path.

The former doesn't sound like a lot of fun, and the latter sounds like a nightmare. :-(

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

A third, and very evil and gross hack, option might be to ln -s camino/.hg/ into the objdir and just let the existing symbolstore code work.

This is very evil because suddenly hg operations will work on the camino objdir, which opens things up for all sorts of bad accidents.  We could #ifdef this symlink on MOZILLA_OFFICIAL and reduce the risk (assuming everyone who works on symbol generation remembers to remove the symlink they create after they're done hacking symbol generation) and assuming the tinderbox code will never escape its cage.

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Also, NSS symbols with DWARF don't have paths and revisions, just filenames.  This seems to be an artifact of NSS being crazy (rather than the fact NSS and NSPR are periodically imported from CVS), since NSPR has paths and revisions; I need to poke around Gecko and see if that's filed, since it affects everyone except maybe Windows.

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Ugh. I just realized that for Camino itself (and any of the auxiliary binaries that still pull Gecko headers, and so forth), we have another problem:

FILE 416 /Volumes/tinder/Cm2.1-M1.9.2/Darwin_9.8.0_Depend/build/ppc/camino/src/bookmarks/SafariBookmarkConverter.m
FILE 417 /Volumes/tinder/Cm2.1-M1.9.2/Darwin_9.8.0_Depend/build/ppc/camino/src/extensions/NSMenu+Gecko.mm
[…]
FILE 421 /Volumes/tinder/Cm2.1-M1.9.2/Darwin_9.8.0_Depend/mozilla/embedding/base/nsEmbedAPI.cpp
FILE 422 /Volumes/tinder/Cm2.1-M1.9.2/Darwin_9.8.0_Depend/mozilla/netwerk/build/nsNetModule.cpp

Compare those files in http://symbols.mozilla.org/symbols_camino/Camino/5699AB4023337429B8DD53461852B4060/Camino.sym (today's 1.9.2 PPC; above) to http://symbols.mozilla.org/symbols_camino/Camino/224A9724551F4BCF82ED3690511406540/Camino.sym (today's 1.9 PPC).

We've got to be able to resolve Gecko files from the static libs back to hg.mozilla.org/releases/mozilla-1.9.2 and the Camino files back to hg.mozilla.org/camino, in addition to the old problem of our files being relative to the objdir and theirs being relative to the srcdir.

The .hg symlink hack isn't going to fix the Core references (which is most of the Camino.sym, and where most of our crashes are from).

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Also, passing two values to -s doesn't seem to work; $(topsrcdir) (which works just fine for the Core dylibs) doesn't get the Core files in Camino.app symbolized, in my testing :(

Comment 11

[Stuart Morgan]

(In reply to comment #4)
> Why does $(topsrcdir) work for Core files but $(srcdir) (which actually has a
> .hg/ folder) not work for Camino files?  Why did we use $(shell pwd) before

As you discovered, it's because we need the paths that were actually used for building

> (and how in the world does it make symbolstore.py do the right thing in an
> objdir that, by its very nature, has no VCS markers at all)?

In the CVS days, it was because we symlinked in all of src, and CVS has folders in each directory, so it was still possible to perform CVS operations on files from within the objdir.


I think your extra argument version is probably the best way to get this working within our current build setup, but I'm still poking around.

Comment 12

[Stuart Morgan]

Attachment: fix part 1

This is Smokey's first suggestion in comment 6; it allows optionally passing in a builddir,vcsdir argument instead of just a single multi-purpose srcdir. I did it this way instead of with another argument since it's possible to have multiple -s arguments, and having two separate lists that had to line up in number and order seemed like an accident waiting to happen.

I haven't looked at the core-file-in-Camino part yet; that will be part two.

Comment 13

[Stuart Morgan]

Hm, I don't have any core files in the Camino symbol file in my static non-debug build. I also don't have anything but PUBLIC entries for the various core dylibs. Am I forgetting a step in making core give us symbol info?

Comment 14

[Stuart Morgan]

Actually, it looks like this wasn't really a static build. I'll try again with a totally fresh tree.

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 448293 [details] [diff] [review]
fix part 1

>diff --git a/Makefile.in b/Makefile.in
>--- a/Makefile.in
>+++ b/Makefile.in
>@@ -417,17 +417,17 @@ libs:: \
> 	  $(DIST)/$(APP_NAME).app/Contents/MacOS/components/camino.xpt
> 
> buildcaminosymbols:
> 	$(PBBUILD) -project google-breakpad/src/tools/mac/dump_syms/dump_syms.xcodeproj \
> 	  -configuration Release
> 	@echo Generating symbols
> 	mkdir -p $(SYMBOL_OUTPUT_DIR)
> 	$(PYTHON) $(srcdir)/scripts/symbolstore.py \
>-	  -a $(OS_TEST) -s $(shell pwd) --vcs-info \
>+	  -a $(OS_TEST) -s $(shell pwd),$(srcdir) --vcs-info \

Make this 

>+	  -a $(OS_TEST) -s $(shell pwd),$(srcdir) -s $(topsrcdir) --vcs-info \

with linebreaks where needed, and you'll fix the whole bug.  (Apparently I had tested -s "foo bar" like I saw used in comm-central, and that doesn't work for us.)

> 	  google-breakpad/src/tools/mac/dump_syms/build/Release/dump_syms \
> 	  $(SYMBOL_OUTPUT_DIR) \
> 	  $(CAMINO_SYMBOL_SOURCE_FILES) > \
> 	  $(SYMBOL_OUTPUT_DIR)/$(SYMBOL_ARCHIVE_BASENAME)-symbols.txt

> 	find $(SYMBOL_OUTPUT_DIR) -name '*.sym' -exec sed -e 's#cvsroot:camino#cvsroot:mozilla/camino#' -e "s#/camino$(topsrcdir)##" -i '' "{}" \;

This isn't still needed for Hg.

> 	$(PYTHON) $(srcdir)/scripts/symbolstore.py \
> 	  -a $(OS_TEST) -s $(topsrcdir) --vcs-info \
> 	  google-breakpad/src/tools/mac/dump_syms/build/Release/dump_syms \
> 	  $(SYMBOL_OUTPUT_DIR) \
> 	  $(CORE_SYMBOL_SOURCE_FILES) >> \

Do we want to keep generation in two steps now that we have logic internal to symbolstore.py to handle the Camino special-casing (and with us already needing to add -s $(topsrcdir) to fix the Core parts of Camino.sym)?  Or should we just collapse the list of symbol files and remove this hunk? (I haven't tested removing this, but I see no reason why it wouldn't work.)

>diff --git a/scripts/symbolstore.py b/scripts/symbolstore.py
>--- a/scripts/symbolstore.py
>+++ b/scripts/symbolstore.py

I didn't really review this, just verified that Core symbols still were generated OK.  I also ran OS symbols and sanity-checked the output is identical (though I don't think this should have touched that).

Also, you should add yourself as a contributor to both our symbolstore.py and the main Makefile.in; so much of both are your code now. 

With that, and at least the new -s and the find removal in the Makefile, r=ardissone.  Ship it!

Comment 16

[Stuart Morgan]

Attachment: fix

Version as I'm about to land it, with all the changes suggested above.

For some reason (something 10.6-y? PEBCAK?) I can't get my tree to give me any core symbols, so you'll want to sanity-check this. It's no worse than the previous version in my tree at least :)

Comment 17

[Stuart Morgan]

Landed as 3099:bee400615e19

Comment 18

[Stuart Morgan]

Note to self: attachment ID is not the same as bug ID. The checkin comment bug number is totally wrong :(

I wish bugzilla would use some totally different scheme for attachment IDs so they weren't so easily confusable.

Comment 19

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

We determined that Stuart didn't have Core symbols because his mozconfig did not export the required -gdwarf magic incantations for creating symbols.

Comment 20

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Verified with bp-7c80b060-452a-4363-a336-9807f2100531 (we have all of main: in there, but hey!  We even get start and _start with actual names instead of hex, too!)

Comment 21

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #8)
> Also, NSS symbols with DWARF don't have paths and revisions, just filenames. 
> This seems to be an artifact of NSS being crazy (rather than the fact NSS and
> NSPR are periodically imported from CVS), since NSPR has paths and revisions; I
> need to poke around Gecko and see if that's filed, since it affects everyone
> except maybe Windows.

Bug 570383; it only took me just shy of a month to file it ;)