source server support for mercurial

VERIFIED FIXED in mozilla1.9.2a1



Crash Reporting
9 years ago
8 years ago


(Reporter: ted, Assigned: Jesse Valianes)


({dev-doc-complete, verified1.9.1, verified1.9.2})

Windows XP
dev-doc-complete, verified1.9.1, verified1.9.2
Dependency tree / graph

Firefox Tracking Flags

(status1.9.1 .4-fixed)



(1 attachment, 2 obsolete attachments)



9 years ago
Split off from bug 428303, we need source server support for builds from mercurial.

"" % (rev, file)
Assignee: nobody → jvaliane

Comment 1

9 years ago
Created attachment 348287 [details] [diff] [review]
Add Source Server support to for Mercurial builds

This patch adds source server support for mercurial builds.
Attachment #348287 - Flags: review?
Attachment #348287 - Flags: review? → review?(ted.mielczarek)

Comment 2

9 years ago
Just a note: the SRCSRV_ROOT env variable needs to be set to "" if it isn't already
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

9 years ago
Comment on attachment 348287 [details] [diff] [review]
Add Source Server support to 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!
Attachment #348287 - Flags: review?(ted.mielczarek) → review-

Comment 5

9 years ago
Created attachment 350852 [details] [diff] [review]
*UPDATED* Add Source Server support to 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
Attachment #348287 - Attachment is obsolete: true
Attachment #350852 - Flags: review?(ted.mielczarek)

Comment 6

9 years ago
Comment on attachment 350852 [details] [diff] [review]
*UPDATED* Add Source Server support to 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/
@@ -351,8 +351,6 @@ def GetVCSFilename(file, srcdir):
         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:

%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: My proposed version would take a line like:
and give you:

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:

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:

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: , but for your symbol path, instead of, use the HTTP server where you unzipped your symbols. You'll also need to follow some of the directions here: although you can skip the parts about CVS.
Attachment #350852 - Flags: review?(ted.mielczarek) → review-

Comment 7

9 years ago
Created attachment 353494 [details] [diff] [review]
*UPDATED* Add Source Server Support for Mercurial to

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.
Attachment #350852 - Attachment is obsolete: true
Attachment #353494 - Flags: review?(ted.mielczarek)

Comment 8

9 years ago
Comment on attachment 353494 [details] [diff] [review]
*UPDATED* Add Source Server Support for Mercurial to

Looks good, thanks for the patch! I'll push this for you in a bit.
Attachment #353494 - Flags: review?(ted.mielczarek) → review+

Comment 9

9 years ago

We should land this on 1.9.1 as well.
Last Resolved: 9 years ago
Resolution: --- → FIXED
(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...
Target Milestone: --- → mozilla1.9.2a1

Comment 11

9 years ago
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.
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:

Ted or Josh, how can I get WinDBG to help me in fetching the source code?

Comment 13

9 years ago
dunno, i've never actually used it. maybe next month.
Could someone update now that this bug is fixed?
Keywords: dev-doc-needed
Oh wait, what about 1.9.1? It's not checked in yet.

Comment 16

8 years ago
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.
As given in comment 12 I cannot help in testing it. Doesn't work or I do the wrong steps. Removing keyword for now.
Keywords: dev-doc-needed
SRCSRV: The module 'C:\Program Files (x86)\Minefield\xul.dll' does not contain source server information. from yesterday/today's Minefield.
Resolution: FIXED → ---

Comment 19

8 years ago
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.)
$ /c/Program\ Files/Debugging\ Tools\ for\ Windows\ \(x64\)/srcsrv/pdbstr.exe -
r -p:29F852B2A3964A36B7EF0FAACE13B6EC2\\xul.pdb -s:srcsrv
(no output)

Comment 21

8 years ago
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

8 years ago
It's set here for the 1.9.0 Tinderbox builds:


8 years ago
Depends on: 506702

Comment 23

8 years ago
I'm just going to resolve this bug again. We'll finish this out in bug 506702.
Last Resolved: 9 years ago8 years ago
Resolution: --- → FIXED


8 years ago
Depends on: 520141

Comment 24

8 years ago
Actually, I found the root cause and split it off as bug 520141.

Comment 25

8 years ago
This finally works in today's trunk nightly!
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.
Keywords: dev-doc-needed
Ted, do we want to have this on 1.9.2 and 1.9.1 too?

Comment 28

8 years ago
Comment on attachment 353494 [details] [diff] [review]
*UPDATED* Add Source Server Support for Mercurial to

Crap, I landed that other patch on 1.9.1, but not this!
Attachment #353494 - Flags: approval1.9.1.4?

Comment 29

8 years ago
(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.
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
Keywords: verified1.9.2
Comment on attachment 353494 [details] [diff] [review]
*UPDATED* Add Source Server Support for Mercurial to

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

Approved for, a=dveditz for release-drivers
Attachment #353494 - Flags: approval1.9.1.4? → approval1.9.1.4+

Comment 32

8 years ago
Pushed to 1.9.1:
status1.9.1: --- → .4-fixed
This has been backed-out due to a bustage:

Removing fixed flag.
status1.9.1: .4-fixed → ---
status1.9.1: --- → ?

Comment 34

8 years ago

I think I cleaned up my mess this time.
status1.9.1: ? → .4-fixed
Looks good this time. Thanks Ted. Verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20091007 Firefox/3.5.4 (.NET CLR 3.5.30729) ID:20091007001339
Keywords: verified1.9.1
anyone want to tackle updating the docs for this?

Comment 37

8 years ago
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.

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.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.