Closed
Bug 440001
Opened 16 years ago
Closed 15 years ago
source server support for mercurial
Categories
(Toolkit :: Crash Reporting, defect)
Tracking
()
VERIFIED
FIXED
mozilla1.9.2a1
Tracking | Status | |
---|---|---|
status1.9.1 | --- | .4-fixed |
People
(Reporter: ted, Assigned: jvaliane)
References
Details
(Keywords: dev-doc-complete, verified1.9.1, verified1.9.2)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
ted
:
review+
dveditz
:
approval1.9.1.4+
|
Details | Diff | Splinter Review |
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)
Updated•16 years ago
|
Assignee: nobody → jvaliane
Assignee | ||
Comment 1•16 years ago
|
||
This patch adds source server support for mercurial builds.
Attachment #348287 -
Flags: review?
Updated•16 years ago
|
Attachment #348287 -
Flags: review? → review?(ted.mielczarek)
Assignee | ||
Comment 2•16 years ago
|
||
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•16 years ago
|
||
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)
Reporter | ||
Comment 4•16 years ago
|
||
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!
Attachment #348287 -
Flags: review?(ted.mielczarek) → review-
Assignee | ||
Comment 5•16 years ago
|
||
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)
Reporter | ||
Comment 6•16 years ago
|
||
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.
Attachment #350852 -
Flags: review?(ted.mielczarek) → review-
Assignee | ||
Comment 7•16 years ago
|
||
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)
Reporter | ||
Comment 8•16 years ago
|
||
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.
Attachment #353494 -
Flags: review?(ted.mielczarek) → review+
Reporter | ||
Comment 9•16 years ago
|
||
Pushed:
http://hg.mozilla.org/mozilla-central/rev/9410ad10c6f7
We should land this on 1.9.1 as well.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 10•16 years ago
|
||
(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
Reporter | ||
Comment 11•16 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.
Comment 12•16 years ago
|
||
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•16 years ago
|
||
dunno, i've never actually used it. maybe next month.
Could someone update https://developer.mozilla.org/en/Using_the_Mozilla_source_server now that this bug is fixed?
Updated•16 years ago
|
Keywords: dev-doc-needed
Comment 15•16 years ago
|
||
Oh wait, what about 1.9.1? It's not checked in yet.
Reporter | ||
Comment 16•16 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.
Comment 17•16 years ago
|
||
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
Comment 18•15 years ago
|
||
SRCSRV: The module 'C:\Program Files (x86)\Minefield\xul.dll' does not contain source server information. from yesterday/today's Minefield.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 19•15 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.)
Comment 20•15 years ago
|
||
$ /c/Program\ Files/Debugging\ Tools\ for\ Windows\ \(x64\)/srcsrv/pdbstr.exe -
r -p:29F852B2A3964A36B7EF0FAACE13B6EC2\\xul.pdb -s:srcsrv
$
(no output)
Reporter | ||
Comment 21•15 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.
Reporter | ||
Comment 22•15 years ago
|
||
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
Reporter | ||
Comment 23•15 years ago
|
||
I'm just going to resolve this bug again. We'll finish this out in bug 506702.
Status: REOPENED → RESOLVED
Closed: 16 years ago → 15 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 24•15 years ago
|
||
Actually, I found the root cause and split it off as bug 520141.
Reporter | ||
Comment 25•15 years ago
|
||
This finally works in today's trunk nightly!
Comment 26•15 years ago
|
||
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.
Status: RESOLVED → VERIFIED
Keywords: dev-doc-needed
Comment 27•15 years ago
|
||
Ted, do we want to have this on 1.9.2 and 1.9.1 too?
Reporter | ||
Comment 28•15 years ago
|
||
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!
Attachment #353494 -
Flags: approval1.9.1.4?
Reporter | ||
Comment 29•15 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.
Comment 30•15 years ago
|
||
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 31•15 years ago
|
||
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
Attachment #353494 -
Flags: approval1.9.1.4? → approval1.9.1.4+
Reporter | ||
Comment 32•15 years ago
|
||
Pushed to 1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/53e39a465d7b
status1.9.1:
--- → .4-fixed
Comment 33•15 years ago
|
||
This has been backed-out due to a bustage:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5e3391896442
Removing fixed flag.
status1.9.1:
.4-fixed → ---
Updated•15 years ago
|
status1.9.1:
--- → ?
Reporter | ||
Comment 34•15 years ago
|
||
Re-landed:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0b981d1c3254
I think I cleaned up my mess this time.
Comment 35•15 years ago
|
||
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
Keywords: verified1.9.1
Comment 36•15 years ago
|
||
anyone want to tackle updating the docs for this?
Reporter | ||
Comment 37•15 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.
Comment 38•15 years ago
|
||
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.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•