If your site name is 'My Site', then the generated xhprof data file would look like '/tmp/xhprof/4ec53fd3bb4b2.My Site' but the xhprof reports look for '/tmp/xhprof/4ec53fd3bb4b2.My%20Site' due to URL encoding from the 'source' parameter. They're not the same, so the reporting breaks. I've attached a patch to strip spaces and apostrophes from the site name, which is used to generated the 'source' parameter used by xhprof reporting.

For the same reason I've changed the default in the variable_get() for the site name to be the string 'devel' instead of an empty string, so that reporting doesn't break for sites which for whatever reason don't have a site name variable defined.

(This is the first patch I've submitted so I'd appreciate knowing if I've done it the right way. I don't know how to use git so I generated this from my Eclipse IDE using SVN.)

#6 devel.xhprof_and_sitename_with_spaces.1345240.6.patch1.18 KBsalvis
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View
devel.patch1.18 KBno longer here 793948
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch devel_0.patch. See the log in the details link for more information. View
Members fund testing for the Drupal project. Drupal Association Learn more


salvis’s picture

Status: Active » Needs review

The patch looks good, thank you. Set the status to NR when you attach a patch.

Now we need an xhprof user who will verify it.

And please check the D7/D8 versions, whether they need fixing, too. We always fix the latest versions first, to ensure that the bug is gone for good.

salvis’s picture

Never mind D7/D8 — moshe wants to remove xhprof from them anyway.

See #1278858-9: xhprof outputted file has the wrong format for latest version.

salvis’s picture

Status: Needs review » Active

Hmm, still "Test request sent"...

salvis’s picture

Status: Active » Needs review

Playing with the testbot...

Status: Needs review » Needs work

The last submitted patch, devel.patch, failed testing.

salvis’s picture

Status: Needs work » Needs review
1.18 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View

Let's try this — I added "a/" and "b/" in front of "devel.module".

salvis’s picture

It was my fault that the testbot didn't kick in right away after setting "needs review", so ignore #3 and #4, but I can't explain why we insist on

--- a/devel.module	(revision 1)
+++ b/devel.module	(working copy)

and reject

--- devel.module	(revision 1)
+++ devel.module	(working copy)

I've opened #1345584: -p0 vs. -p1 and associated error messages to ask for an explanation.

salvis’s picture

Status: Needs review » Fixed

Pushed to the -dev version.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.