Last Comment Bug 440001 - source server support for mercurial
: source server support for mercurial
Status: VERIFIED FIXED
: dev-doc-complete, verified1.9.1, verified1.9.2
Product: Toolkit
Classification: Components
Component: Breakpad Integration (show other bugs)
: Trunk
: x86 Windows XP
: -- normal with 1 vote (vote)
: mozilla1.9.2a1
Assigned To: Jesse Valianes
:
Mentors:
Depends on: 428303 506702 520141
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-18 08:09 PDT by Ted Mielczarek [:ted.mielczarek]
Modified: 2009-10-15 08:27 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.4-fixed


Attachments
Add Source Server support to symbolstore.py for Mercurial builds (8.93 KB, patch)
2008-11-14 17:09 PST, Jesse Valianes
ted: review-
Details | Diff | Splinter Review
*UPDATED* Add Source Server support to symbolstore.py for Mercurial builds (8.10 KB, patch)
2008-12-01 15:34 PST, Jesse Valianes
ted: review-
Details | Diff | Splinter Review
*UPDATED* Add Source Server Support for Mercurial to symbolstore.py (9.01 KB, patch)
2008-12-17 11:49 PST, Jesse Valianes
ted: review+
dveditz: approval1.9.1.4+
Details | Diff | Splinter Review

Description Ted Mielczarek [:ted.mielczarek] 2008-06-18 08:09:59 PDT
Split off from bug 428303, we need source server support for builds from mercurial.

raw:
"http://hg.mozilla.org/mozilla-central/index.cgi/raw-file/%s/%s" % (rev, file)
Comment 1 Jesse Valianes 2008-11-14 17:09:41 PST
Created attachment 348287 [details] [diff] [review]
Add Source Server support to symbolstore.py for Mercurial builds

This patch adds source server support for mercurial builds.
Comment 2 Jesse Valianes 2008-11-15 09:14:13 PST
Just a note: the SRCSRV_ROOT env variable needs to be set to "http://hg.mozilla.org/mozilla-central" if it isn't already
Comment 3 Justin Wood (:Callek) (Away until Aug 29) 2008-11-15 18:19:45 PST
Jesse, while this is much better than what we have so far; is there any way we can get this to work in cases where there is more than one hg repo?

Aka: SeaMonkey/TB.

Or even when we crash in CVS parts of SeaMonkey (ldapsdk), or the *other* hg repo in SM (domi)
Comment 4 Ted Mielczarek [:ted.mielczarek] 2008-11-24 09:38:19 PST
Comment on attachment 348287 [details] [diff] [review]
Add Source Server support to symbolstore.py for Mercurial builds

-            return "cvs:%s:%s:%s" % (self.clean_root, file, self.revision)
+            return "hg:%s:%s:%s" % (self.clean_root, file, self.revision)

This change doesn't make any sense. Why are you changing the CVSFileInfo class to return filenames starting with hg:?

-def SourceIndex(fileStream, outputPath, cvs_root):
+def SourceIndex(fileStream, outputPath, hg_root):

I feel like I don't want the CVS srcsrv bits removed entirely, but I guess given that the code lives in Hg, I can't really justify that.

+    pdbStreamFile.write('''\r\nSRCSRVVERCTRL=http\r\nHTTP_EXTRACT_TARGET=%hgserver%/index.cgi/raw-file/%var3%/%var2%\r\nSRCSRVTRG=%http_extract_target%\r\nSRCSRVCMD=\r\nSRCSRV: source files ---------------------------------------\r\n''')

You don't need the index.cgi in the URL anymore.

-        cvs_root = os.environ.get("SRCSRV_ROOT")
+        # tries to get hgroot from the .mozconfig first - if it's not set
+        # the tinderbox hg_path will be assigned further down
+        hg_root = os.environ.get("SRCSRV_ROOT")

I'm not really wild about all this s/cvs/hg/ renaming. In addition, the HgFileInfo class above already checks this environment variable on its own, so I don't think this change is really necessary. How about you just rename this variable to vcs_root instead, and make the language more generic? Just refer to it as a "Version Control System" instead of mentioning CVS or Hg specifically.

This looks like it's pretty close. If you can fix up those few things you'll be in good shape!
Comment 5 Jesse Valianes 2008-12-01 15:34:06 PST
Created attachment 350852 [details] [diff] [review]
*UPDATED* Add Source Server support to symbolstore.py for Mercurial builds

Changes made as per Ted's review.

-CVSFileInfo class reverted back to previous state
-index.cgi removed from HTTP_EXTRACT_TARGET var in SourceIndex
-hg_root renamed to vcs_root
Comment 6 Ted Mielczarek [:ted.mielczarek] 2008-12-08 12:10:44 PST
Comment on attachment 350852 [details] [diff] [review]
*UPDATED* Add Source Server support to symbolstore.py for Mercurial builds

I tried this out and it still has some issues. It's a bit tough to test, just due to the way we get the hg changeset (`hg id -i` will give you a result that you can't use elsewhere if you're using MQ, and will add a + if you have local changes, so it's tough to test with a patch applied.)

First, I hit a few problems when trying to run this without SRCSRV_ROOT set. We should be able to make this work in the HG case, since unlike with CVS, we can determine the root here and it will work. (The tinderboxes all pull via HTTP.) Can you add these changes to your patch?

+++ b/toolkit/crashreporter/tools/symbolstore.py
@@ -351,8 +351,6 @@ def GetVCSFilename(file, srcdir):
     else:
         if os.path.isdir(os.path.join(path, "CVS")):
             fileInfo = CVSFileInfo(file, srcdir)
-            if fileInfo:
-               root = fileInfo.root
         elif os.path.isdir(os.path.join(path, ".svn")) or \
              os.path.isdir(os.path.join(path, "_svn")):
             fileInfo = SVNFileInfo(file);
@@ -363,6 +361,7 @@ def GetVCSFilename(file, srcdir):

     if fileInfo:
         file = fileInfo.filename
+        root = fileInfo.root

     # we want forward slashes on win32 paths
     return (file.replace("\\", "/"), root)

-                    if self.srcsrv:
+                    if self.srcsrv and vcs_root:
                         # Call on SourceServerIndexing
-                        result = self.SourceServerIndexing(debug_file, guid, sourceFileStream, cvs_root)
+                        result = self.SourceServerIndexing(debug_file, guid, sourceFileStream, vcs_root)

Ok, those two changes aside, I have some other things that seem broken: (sorry for missing these on the first go-round)

+    pdbStreamFile.write('''SRCSRV: ini ------------------------------------------------\r\nVERSION=2\r\nINDEXVERSION=2\r\nVERCTRL=http\r\nSRCSRV: variables ------------------------------------------\r\nHGSERVER=''')
+    pdbStreamFile.write(vcs_root)
+    pdbStreamFile.write('''\r\nSRCSRVVERCTRL=http\r\nHTTP_EXTRACT_TARGET=%hgserver%/raw-file/%var3%/%var2%\r\nSRCSRVTRG=%http_extract_target%\r\nSRCSRVCMD=\r\nSRCSRV: source files ---------------------------------------\r\n''')

SRCSRVTRG=%http_extract_target% seems wrong. This variable should resolve to the full path on the client that the file is extracted to. I think it needs to be:
SRCSRVTRG=%targ%\%var3%\%fnbksl%(%var2%)

%targ% is the directory that the debugger will put source files in. The other entries are the parts of the file line, separated by asterisks, which is generated here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/tools/symbolstore.py#522 My proposed version would take a line like:
d:\build\mozilla-central\toolkit\xre\nsWindowsWMain.cpp*toolkit/xre/nsWindowsWMain.cpp*38e9fb492ae9
and give you:
c:\wherever-your-source-is\38e9fb492ae9\toolkit\xre\nsWindowsWMain.cpp

SRCSRVCMD=
I think we can ditch this too, since it's empty anyway. (And having an empty one might confuse srcsrv.)

Finally, it has occurred to me that you might not know how to fully test this. After you run "make buildsymbols", you'll get a symbol archive. Currently it winds up in a directory next to your objdir, like mine is:
../20081205072927/crashreporter-symbols-firefox-3.2a1pre-WINNT-20081205072927.zip

You'll need to unzip this somewhere that you can access via HTTP. I tested it on my web site, it should look something like this when done:
http://mavra.perilith.com/~luser/symbols/

You'll then need to package your build--"make package" in the objdir will do the trick, you'll get a zip file in objdir/dist. Then you'll need to unzip and run that build on a different machine, and debug it. You can follow the directions here:
https://developer.mozilla.org/en/Using_the_Mozilla_symbol_server , but for your symbol path, instead of  http://symbols.mozilla.org/firefox, use the HTTP server where you unzipped your symbols. You'll also need to follow some of the directions here:
https://developer.mozilla.org/en/Using_the_Mozilla_source_server although you can skip the parts about CVS.
Comment 7 Jesse Valianes 2008-12-17 11:49:23 PST
Created attachment 353494 [details] [diff] [review]
*UPDATED* Add Source Server Support for Mercurial to symbolstore.py

Changes made as per ted's comments, however SRCSRVTRG is unchanged as it seems it must be set up as the %HTTP_EXTRACT_TARGET% in order to successfully retrieve the source code.

One other note: This implementation works successfully in Visual Studio 2008 and WinDBG, however Visual Studio 2005 doesn't like it. According to ted this could be due to incorrect implementation of the source server in 2005.
Comment 8 Ted Mielczarek [:ted.mielczarek] 2008-12-19 05:44:07 PST
Comment on attachment 353494 [details] [diff] [review]
*UPDATED* Add Source Server Support for Mercurial to symbolstore.py

Looks good, thanks for the patch! I'll push this for you in a bit.
Comment 9 Ted Mielczarek [:ted.mielczarek] 2008-12-19 08:05:57 PST
Pushed:
http://hg.mozilla.org/mozilla-central/rev/9410ad10c6f7

We should land this on 1.9.1 as well.
Comment 10 Henrik Skupin (:whimboo) 2009-01-27 17:32:19 PST
(In reply to comment #9)
> We should land this on 1.9.1 as well.

Ted, any progress so far here? Looks like we have to ask for approval...
Comment 11 Ted Mielczarek [:ted.mielczarek] 2009-01-28 05:50:01 PST
I meant to test source server support on a nightly before asking for approval, but never got around to it. If anyone else wants to try it out, feel free to let me know if it's working right.
Comment 12 Henrik Skupin (:whimboo) 2009-02-14 16:55:40 PST
I tried to verify this bug by running a test on Windows 7 beta and the latest version of WinDBG installed. Setting up the symbols server wasn't a problem and I can see the source location in the call stack window. Sadly I don't get a dialog which prompts me to download the source. I was running the steps from this page: https://developer.mozilla.org/en/Using_the_Mozilla_source_server

Ted or Josh, how can I get WinDBG to help me in fetching the source code?
Comment 13 timeless 2009-04-05 06:07:24 PDT
dunno, i've never actually used it. maybe next month.
Comment 14 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-04-22 15:51:30 PDT
Could someone update https://developer.mozilla.org/en/Using_the_Mozilla_source_server now that this bug is fixed?
Comment 15 Henrik Skupin (:whimboo) 2009-04-23 05:54:13 PDT
Oh wait, what about 1.9.1? It's not checked in yet.
Comment 16 Ted Mielczarek [:ted.mielczarek] 2009-04-23 05:59:43 PDT
Needs verification on trunk before we land it on branch. It's possible the patch doesn't work perfectly as-is, or we're missing some configuration on the tinderboxes.
Comment 17 Henrik Skupin (:whimboo) 2009-04-23 06:07:56 PDT
As given in comment 12 I cannot help in testing it. Doesn't work or I do the wrong steps. Removing keyword for now.
Comment 18 Benjamin Smedberg [:bsmedberg] 2009-07-27 08:35:32 PDT
SRCSRV: The module 'C:\Program Files (x86)\Minefield\xul.dll' does not contain source server information. from yesterday/today's Minefield.
Comment 19 Ted Mielczarek [:ted.mielczarek] 2009-07-27 10:18:21 PDT
If someone has a .pdb file (any one will do) from a recent trunk build, and WinDBG installed, could you run:
pdbstr -r -p:file.pdb -s:srcsrv

and attach the output? (pdbstr lives under the WinDBG install dir somewhere.)
Comment 20 Benjamin Smedberg [:bsmedberg] 2009-07-27 10:25:40 PDT
$ /c/Program\ Files/Debugging\ Tools\ for\ Windows\ \(x64\)/srcsrv/pdbstr.exe -
r -p:29F852B2A3964A36B7EF0FAACE13B6EC2\\xul.pdb -s:srcsrv
$
(no output)
Comment 21 Ted Mielczarek [:ted.mielczarek] 2009-07-27 11:25:33 PDT
Ok then, it's completely missing the source server info. We might be missing the PDBSTR_PATH environment variable on the build machines.
Comment 22 Ted Mielczarek [:ted.mielczarek] 2009-07-27 11:27:55 PDT
It's set here for the 1.9.0 Tinderbox builds:
http://mxr.mozilla.org/mozilla/source/tools/tinderbox-configs/firefox/win32/tinder-config.pl#14
Comment 23 Ted Mielczarek [:ted.mielczarek] 2009-10-02 03:37:33 PDT
I'm just going to resolve this bug again. We'll finish this out in bug 506702.
Comment 24 Ted Mielczarek [:ted.mielczarek] 2009-10-02 03:58:53 PDT
Actually, I found the root cause and split it off as bug 520141.
Comment 25 Ted Mielczarek [:ted.mielczarek] 2009-10-05 07:26:27 PDT
This finally works in today's trunk nightly!
Comment 26 Henrik Skupin (:whimboo) 2009-10-06 02:46:29 PDT
Looks great with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091005 Minefield/3.7a1pre (.NET CLR 3.5.30729) ID:20091005045052

I have no other cvs.exe installed beside the one from mozilla build. Looks like it works fine now and we need a docs update.
Comment 27 Henrik Skupin (:whimboo) 2009-10-06 02:49:59 PDT
Ted, do we want to have this on 1.9.2 and 1.9.1 too?
Comment 28 Ted Mielczarek [:ted.mielczarek] 2009-10-06 03:41:40 PDT
Comment on attachment 353494 [details] [diff] [review]
*UPDATED* Add Source Server Support for Mercurial to symbolstore.py

Crap, I landed that other patch on 1.9.1, but not this!
Comment 29 Ted Mielczarek [:ted.mielczarek] 2009-10-06 03:42:54 PDT
(In reply to comment #26)
> I have no other cvs.exe installed beside the one from mozilla build. Looks like
> it works fine now and we need a docs update.

Yeah, the source server with builds from Mercurial obviously doesn't require cvs.exe. :) Just needs clarification in the docs.

FWIW, I landed bug 520141 on 1.9.2 and 1.9.1, so source server should work fine in the 1.9.2 nightlies.
Comment 30 Henrik Skupin (:whimboo) 2009-10-06 07:08:36 PDT
Verified fixed on 1.9.2 with the fix on bug 520141 and a todays nightly build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b1pre) Gecko/20091006 Namoroka/3.6b1pre (.NET CLR 3.5.30729) ID:20091006045110
Comment 31 Daniel Veditz [:dveditz] 2009-10-06 11:29:36 PDT
Comment on attachment 353494 [details] [diff] [review]
*UPDATED* Add Source Server Support for Mercurial to symbolstore.py

Please land ASAP, build team is standing by to make candidate builds.

Approved for 1.9.1.4, a=dveditz for release-drivers
Comment 32 Ted Mielczarek [:ted.mielczarek] 2009-10-06 11:54:37 PDT
Pushed to 1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/53e39a465d7b
Comment 33 Henrik Skupin (:whimboo) 2009-10-06 15:58:11 PDT
This has been backed-out due to a bustage:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5e3391896442

Removing fixed flag.
Comment 34 Ted Mielczarek [:ted.mielczarek] 2009-10-06 17:20:14 PDT
Re-landed:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0b981d1c3254

I think I cleaned up my mess this time.
Comment 35 Henrik Skupin (:whimboo) 2009-10-07 13:06:31 PDT
Looks good this time. Thanks Ted. Verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4) Gecko/20091007 Firefox/3.5.4 (.NET CLR 3.5.30729) ID:20091007001339
Comment 36 Eric Shepherd [:sheppy] 2009-10-14 13:03:34 PDT
anyone want to tackle updating the docs for this?
Comment 37 Ted Mielczarek [:ted.mielczarek] 2009-10-14 14:43:23 PDT
It actually just requires some clarification. Using the source server for Hg builds doesn't require cvs.exe (obviously), and won't prompt you with the "is it ok to run cvs.exe" dialog. It's actually even simpler than it previously was.
Comment 38 Eric Shepherd [:sheppy] 2009-10-15 08:27:49 PDT
Updated:

https://developer.mozilla.org/en/Using_the_Mozilla_source_server

Have a look, make any tweaks necessary if you see issues. This article still mentions cvs for now, but adds discussion of the Hg changes.

Note You need to log in before you can comment on or make changes to this bug.