1755807 - XPIDL HTML output not properly semantically linking source records, possibly due to source records having "source" at the end

Status: RESOLVED FIXED

(Webtools :: Searchfox, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

We have analysis data for, for example nsIDocShell::SetCurrentURI but the token in the HTML view isn't getting linkified correctly. This likely has to do with the source records looking weird:

{"loc": "109:7-20", "sym": "_ZN11nsIDocShell13SetCurrentURIEP6nsIURI,#setCurrentURI", "pretty": "IDL method setCurrentURI", "source": 1}

I almost certainly broke this when updating us to use serde_json.

We didn't detect this because we have no checks in mozilla-central right now, and we have zero coverage in the mozsearch tests repo for IDL-related things at all. I'll try and add coverage for both, although I can only guarantee for the former.

We don't have analysis data at all for nsISupports and other interfaces may experience some churn, so I'm thinking https://searchfox.org/mozilla-central/source/js/xpconnect/tests/idl/xpctest_params.idl for the mozilla-central check and probably if I can import it to mozsearch too.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

The problem was the idl-analyze.py-generated "source" records were missing, and have always been missing, "syntax" keys. The weird ordering is a function of python 2.x dictionaries not being ordered by default. We're still using python2 for idl-analyze.py because the underlying xpidl.py is still using python 2.x, I think.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

Attachment: mozsearch test and fixes

This adds fixes and test coverage in the mozsearch repo for xpctest_params. This will fix things for mozilla-central too (unless everything explodes), but I still will need to do a mozsearch-mozilla PR after that to add production checks to detect regressions.

Note that "go to definition" will be sorta not useful like I think it always has not been, in that it will go to the C++ definition which is never what anyone wants. Bug 1727789 will probably address that most meaningfully.

Comment 3

[Andrew McCreight [:mccr8]]

I'm not familiar with the history of this, but looking at bug 1636006, it sounds like xpidl.py is Py2 compatible because SearchFox needs it. So maybe it can be run as Python 3, if all of the related SearchFox stuff that uses it is Python 3?

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Andrew McCreight [:mccr8] from comment #3)

So maybe it can be run as Python 3, if all of the related SearchFox stuff that uses it is Python 3?

I changed the #!/usr/bin/env python2 to #!/usr/bin/env python3 and it just worked in the vagrant VM, so I guess so[1]! Thanks for raising this! Also, many thanks to :kats who did all of the previous python3 changes that likely made this so easy!

I expect the concerns in https://bugzilla.mozilla.org/show_bug.cgi?id=1636006#c1 about idl-analyze.sh trying to use the in-tree version of "${FILES_ROOT}/xpcom/idl-parser/xpidl/xpidl.py" if it exist is presumably mooted by that bug having happened during Firefox 78 and so xpidl compat should be stable across all tier 1 searchfox builds.

I'll land this change shortly and initiate the config1 indexer and after that completes I'll do the mozsearch-mozilla XPIDL checks (as I need the JSON to have stabilized).

1: Actually the the test "snapshot" changed slightly as the JSON reordered to the source code key/value oprdering, but this is an exciting and reassuring change to approve!

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

Attachment: PR including python3 change

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

Attachment: mozsearch-mozilla production XPIDL checks

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

XPIDL is working again in mozilla-central and now has a targeted production check that's the same as the (non-xref) XPIDL testOctet checks in the searchfox "tests" tests corpus.