Closed Bug 440001 Opened 16 years ago Closed 15 years ago

source server support for mercurial


(Toolkit :: Crash Reporting, defect)

Windows XP
Not set



Tracking Status
status1.9.1 --- .4-fixed


(Reporter: ted, Assigned: jvaliane)



(Keywords: dev-doc-complete, verified1.9.1, verified1.9.2)


(1 file, 2 obsolete files)

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

"" % (rev, file)
Assignee: nobody → jvaliane
This patch adds source server support for mercurial builds.
Attachment #348287 - Flags: review?
Attachment #348287 - Flags: review? → review?(ted.mielczarek)
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 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-
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 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-
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 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+

We should land this on 1.9.1 as well.
Closed: 16 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
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?
dunno, i've never actually used it. maybe next month.
Oh wait, what about 1.9.1? It's not checked in yet.
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 → ---
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)
Ok then, it's completely missing the source server info. We might be missing the PDBSTR_PATH environment variable on the build machines.
Depends on: 506702
I'm just going to resolve this bug again. We'll finish this out in bug 506702.
Closed: 16 years ago15 years ago
Resolution: --- → FIXED
Depends on: 520141
Actually, I found the root cause and split it off as bug 520141.
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 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?
(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+
This has been backed-out due to a bustage:

Removing fixed flag.

I think I cleaned up my mess this time.
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?
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.
You need to log in before you can comment on or make changes to this bug.