Last Comment Bug 643817 - Replace xpt_link/dump with pyxpt
: Replace xpt_link/dump with pyxpt
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla6
Assigned To: Mike Hommey [:glandium]
:
:
Mentors:
http://hg.mozilla.org/users/tmielczar...
Depends on: 650476 651975 654448
Blocks: 602245
  Show dependency treegraph
 
Reported: 2011-03-22 11:07 PDT by Mike Hommey [:glandium]
Modified: 2011-05-03 08:15 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (156.85 KB, patch)
2011-03-22 17:53 PDT, Mike Hommey [:glandium]
ted: review+
benjamin: review+
Details | Diff | Splinter Review
Test patch (3.37 KB, patch)
2011-03-25 02:12 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2011-03-22 11:07:49 PDT
Everything is in the summary
Comment 1 Mike Hommey [:glandium] 2011-03-22 17:53:40 PDT
Created attachment 521078 [details] [diff] [review]
Patch

python code is current trunk on your pyxpt repo. As for the config.mk/rules.mk changes, I didn't think there was much interest in complicating things to keep the dependency for xpt files on XPIDL_LINK, the error message in case XPIDL_LINK doesn't work should be as explicit as a dependency miss.
Comment 2 Ted Mielczarek [:ted.mielczarek] 2011-03-24 12:04:23 PDT
Comment on attachment 521078 [details] [diff] [review]
Patch

The build bits look fine, but since I wrote all that Python code, someone else needs to give this a once-over. I don't know if bsmedberg has the time or inclination to review it, but if not, perhaps catlee would?
Comment 3 Mike Hommey [:glandium] 2011-03-25 02:12:22 PDT
Created attachment 521759 [details] [diff] [review]
Test patch

I used this patch to try to see how things differ, and I hit a breakage pretty early:
cmp _xpidlgen/xpcom_base.xpt _xpidlgen/xpcom_base-new.xpt
_xpidlgen/xpcom_base.xpt _xpidlgen/xpcom_base-new.xpt differ: char 24, line 3
make[5]: *** [_xpidlgen/xpcom_base.xpt] Error 1

However, if I xpt_dump both, the output is exactly the same.
Comment 4 Mike Hommey [:glandium] 2011-03-25 02:36:55 PDT
If I don't compare the files directly but compare dumps instead, I can go further, but stop again later:

--- obj-i686-pc-linux-gnu/xpcom/components/_xpidlgen/xpcom_components.xpt.dump	2011-03-25 09:34:00.702062243 +0000
+++ obj-i686-pc-linux-gnu/xpcom/components/_xpidlgen/xpcom_components-new.xpt.dump	2011-03-25 09:34:00.702062243 +0000
@@ -8,8 +8,6 @@
 Interface Directory:
    - ::nsIFile (00000000-0000-0000-0000-000000000000):
       [Unresolved]
-   - ::nsISimpleEnumerator (00000000-0000-0000-0000-000000000000):
-      [Unresolved]
    - ::nsISupports (00000000-0000-0000-c000-000000000046):
       [Unresolved]
    - ::nsIFactory (00000001-0000-0000-c000-000000000046):
@@ -114,3 +112,5 @@
            uint32 createInstanceByContractID(in string, in nsISupports, in nsIID &, out retval InterfaceIs *);
       Constants:
          No Constants
+   - ::nsISimpleEnumerator (d1899240-f9d2-11d2-bdd6-000064657374):
+      [Unresolved]
Comment 5 Ted Mielczarek [:ted.mielczarek] 2011-03-29 09:45:24 PDT
I investigated this, and it looks harmless. My patch from bug 455512 ( http://hg.mozilla.org/mozilla-central/rev/a78f9bb9006e ) attempted to make xpt_link prefer unresolved interface descriptors with a valid IID, but apparently that's broken and it prefers the ones with zero IIDs. xpt.py gets it right, so that's the only difference, This difference disappears in the final linked xpt, since all interfaces are resolved at that point. I think we simply declare the xpt_link binary buggy and move on in life.
Comment 6 Mike Hommey [:glandium] 2011-04-14 02:25:25 PDT
http://hg.mozilla.org/mozilla-central/rev/daf83138f3e2
Comment 7 Serge Gautherie (:sgautherie) 2011-04-15 05:09:09 PDT
Ftr,
http://hg.mozilla.org/comm-central/rev/e11c23c9a55b
"Fix xpt_link bustage from bug 643817 as it got replaced with xpt.py."

Note You need to log in before you can comment on or make changes to this bug.