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.)

CommentFileSizeAuthor
#6 devel.xhprof_and_sitename_with_spaces.1345240.6.patch1.18 KBsalvis
devel.patch1.18 KBAnonymous (not verified)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented
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

salvis
he/his
CreditAttribution: salvis commented

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

salvis
he/his
CreditAttribution: salvis commented
Status: Needs review » Active

Hmm, still "Test request sent"...

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented
Status: Active » Needs review

Playing with the testbot...

Status: Needs review » Needs work

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

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented
Status: Needs work » Needs review
FileSize
devel.xhprof_and_sitename_with_spaces.1345240.6.patch1.18 KB

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

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented

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

salvis
he/his
CreditAttribution: salvis commented
Status: Needs review » Fixed

Pushed to the -dev version.

Status: Fixed » Closed (fixed)

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