bring pyxpcom up to speed

RESOLVED FIXED

Status

()

Core
XPCOM
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: Christian Persch (GNOME) (away; not receiving bug mail), Assigned: markh)

Tracking

({fixed1.8})

Trunk
x86
Linux
fixed1.8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

split from 267040 comment 33:

PyXPCOM doesn't compile anymore since the removal of NS_GetGlobalComponentManager.
Created attachment 187714 [details] [diff] [review]
port to non-obsolete component manager/registrar, and various cleanups

I ported all uses of nsIComponentManagerObsolete over to nsICompnentManager
resp. nsIComponentRegistrar (except in one place where I couldn't find the
equivalent functionality). For backward compatibility, the
NS_GetGlobalComponentManager python function is retained, wrapping
nsIComponentManagerObsolete. The not backward-compatible changes are:
* _xpcom.IID_nsIComponentManager previously was nsICMObsolete'd IID, but now
really is nsICM's IID, and
* xpcom.components.manager now is nsICM, not nsICMObsolete
I hope that's not going to be a problem?

Because it would have been hard to separate it out, the patch also includes
various gcc4 fixes (mostly ";" removed in inappropriate places), and Shane
Hathaway's patch from bug 281156 for python using UTF-32 on linux.

There is one FIXME added for a pre-existing problem in
Py_nsISupports::PyObjectFromInterface, which I want to take in another round
since this patch is already getting too big.
Assignee: dougt → christian.persch
Status: NEW → ASSIGNED
Attachment #187714 - Flags: review?(mhammond)

Comment 2

13 years ago
With patch id=187714 actually I can compile pyxpcom but when I try to do 
import xpcom.components I get the following error. 
>>> import xpcom.components
Failed to get info for IID '{a88e5a60-205a-4bb1-94e1-2628daf51eae}'
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File "/usr/local/lib/python2.3/site-packages/xpcom/components.py", line 55, in ?
    manager = xpcom.client.Component(_xpcom.GetComponentManager(),
_xpcom.IID_nsIComponentManager)
  File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
229, in __init__
    self.QueryInterface(iid)
  File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
299, in QueryInterface
    self._remember_interface_info(iid)
  File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
272, in _remember_interface_info
    method_infos, getters, setters, constants = BuildInterfaceInfo(iid)
  File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
147, in BuildInterfaceInfo
    interface = xpt.Interface(iid)
  File "/usr/local/lib/python2.3/site-packages/xpcom/xpt.py", line 83, in __init__
    item = iim.GetInfoForIID(iid)
xpcom.Exception: 0x80004005 (NS_ERROR_FAILURE)

Comment 3

13 years ago
(In reply to comment #2)
> With patch id=187714 actually I can compile pyxpcom but when I try to do 
> import xpcom.components I get the following error. 
> >>> import xpcom.components
> Failed to get info for IID '{a88e5a60-205a-4bb1-94e1-2628daf51eae}'
> Traceback (most recent call last):
>   File "<stdin>", line 1, in ?
>   File "/usr/local/lib/python2.3/site-packages/xpcom/components.py", line 55, in ?
>     manager = xpcom.client.Component(_xpcom.GetComponentManager(),
> _xpcom.IID_nsIComponentManager)
>   File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
> 229, in __init__
>     self.QueryInterface(iid)
>   File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
> 299, in QueryInterface
>     self._remember_interface_info(iid)
>   File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
> 272, in _remember_interface_info
>     method_infos, getters, setters, constants = BuildInterfaceInfo(iid)
>   File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
> 147, in BuildInterfaceInfo
>     interface = xpt.Interface(iid)
>   File "/usr/local/lib/python2.3/site-packages/xpcom/xpt.py", line 83, in __init__
>     item = iim.GetInfoForIID(iid)
> xpcom.Exception: 0x80004005 (NS_ERROR_FAILURE)

I tested the patch with Firefox trunk (Jul 25, 2005) and python-2.3.5 but
couldn't reproduce the problem. I followed the steps described in
http://live.gnome.org/Epiphany/EphyPython/PyXPCOM (replacing the patch with the
updated version, of course) and everything worked just fine.
Also this message on the PyXPCOM mailing list suggests that the patch works fine
on win and macosx too: http://aspn.activestate.com/ASPN/Mail/Message/pyxpcom/2753221
(Assignee)

Comment 5

13 years ago
Created attachment 191153 [details] [diff] [review]
Updated to trunk and test fix

Updating the patch to the trunk, plus:
* It appears Windows now uses "xpcom_trunk.dll" rather than 'xpcom.dll'.  I am
building the 'suite' so I have a slight concern that different builds use
different names for this DLL - can anyone confirm this?

* The test_streams test was not being run via regrtest.  The test needed
reworking, so at the same time I converted it to use unittest.

My CVS is all working again so I'm able to steer this through.
Assignee: chpe → mhammond
Attachment #187714 - Attachment is obsolete: true
> * It appears Windows now uses "xpcom_trunk.dll" rather than 'xpcom.dll'.

xpcom.dll exists. it contains only frozen functions, though. depending on
whether libxul is enabled, the rest of the functions live in
xul.dll/xpcom_core.dll. I'm not sure what happens in static builds.

Comment 7

13 years ago
Comment on attachment 191153 [details] [diff] [review]
Updated to trunk and test fix

ok, I have a few questions before I stamp this r=alecf

1) in PyCreateInstanceByContractID, why must the 2nd parameter be None? There
should at least be comments documenting why this is the case (i.e. what future
API feature are we expecting)

2)
+#if defined(NSCAP_FEATURE_TEST_DONTQUERY_CASES)
+	nsISupports *query_result = nsnull;
+	pis->QueryInterface (riid, (void**)&query_result);
+	NS_ASSERTION(query_result == pis, "QueryInterface needed");
+	NS_IF_RELEASE(query_result);
+#endif

This should be in #ifdef NS_DEBUG, since the only point of the code is to test
the assertion

(I'm curious why pyxpcom doesn't use nsCOMPtr<> but that is probably a much
bigger question!)


If you want to just give me the explanation in this bug for 1) or how to fix
it, I will fix both locally before checking in.
(Assignee)

Comment 8

13 years ago
Created attachment 191674 [details] [diff] [review]
Updated patch

Thanks Alec,

1) The missing arg is an "outer" interface.  pyxpcom has never supported this
param - the patch has just reorganized the old code.  I've updated a new patch
with a brief comment about why we ignore it (basically as I'm not sure of the
semantics yet and I've never seen anyone actually use it.

2) I've come to the conclusion that the new debug test is actually invalid. 
Some cases where the assertion fails are valid - it is the responsibility of
the caller to ensure things are sane.  All such callers are in pyxpcom, and I
believe they are sane.	This may help trap errors in other components, but as
it has false-positives, it should go.  My new patch has this removed.

Regarding nsCOMPtr<> - there are probably some places where it should be used
(and that new block was an example).  However, in most cases a pyxpcom C++
function is called which steals the reference from the caller - ie, most places
that get a new reference pass ownership on instead of releasing it.  Although
nsCOMPtr does have facilities for this, its primary use (ensuring references
are released) is not needed, so it is not used.

This patch also changes "xpcom_core.dll" to "nspr4.dll" as we discussed in
mail, including a new comment.
(Assignee)

Updated

13 years ago
Attachment #191153 - Attachment is obsolete: true
Attachment #187714 - Flags: review?(mhammond)
What is the status here?  Is this ready for review?

Comment 10

12 years ago
Comment on attachment 191674 [details] [diff] [review]
Updated patch

oh yeah, heh :)

I'm giving this r=alecf, and asking for a super-reviewer now.

Shaver, do you mind doing the review here? This brings PyXPCOM back from the
dead (again) and does some significant work to get Unicode working correctly
between XPCOM and Python...
Attachment #191674 - Flags: superreview?(shaver)
Attachment #191674 - Flags: review+
Comment on attachment 191674 [details] [diff] [review]
Updated patch

rs=shaver on the strength of it being confined to extensions/python, and that
stuff already being savagely busted on the trunk
Attachment #191674 - Flags: superreview?(shaver) → superreview+
(Assignee)

Comment 12

12 years ago
Checked in the trunk - thanks everyone.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(In reply to comment #12)
> Checked in the trunk - thanks everyone.

IMHO this should be merged to 1.8 branch too. Since it's no part of the default
build, there's no risk associated. What do you think?

Comment 14

12 years ago
(In reply to comment #13)
> IMHO this should be merged to 1.8 branch too. 

FYI I'm using cvs trunk of extension/python with the 1.8 branch, it's working
fine.  It would be nice to have pyxpcom buildable in the 1.8 branch.

Comment on attachment 191674 [details] [diff] [review]
Updated patch

Not part of the build, no risk. This patch fixes pyxpcom to work at all on 1.8
branch.
Attachment #191674 - Flags: approval1.8b4?

Updated

12 years ago
Attachment #191674 - Flags: approval1.8b4? → approval1.8b4+
(Assignee)

Comment 16

12 years ago
FYI, checked in on the 1.8 branch.

Updated

12 years ago
Keywords: fixed1.8
It looks like the new file
mozilla/extensions/python/xpcom/src/PyIComponentManagerObsolete.cpp wasn't added
on 1.8 branch, making the build fail on a fresh checkout.
(Assignee)

Comment 18

12 years ago
thanks - added that missing file to the branch.
You need to log in before you can comment on or make changes to this bug.