Switch extensions from IDL to HNI #1480

Closed
simonwelsh opened this Issue Jan 3, 2014 · 28 comments

Comments

Projects
None yet
@simonwelsh
Contributor

simonwelsh commented Jan 3, 2014

This is more a longish-term task list issue to help track migrating extensions from IDL to HNI. I'll try to keep it updated as the migration progresses and include hints/types/recommendations as I come across them.

As part of this I'm also tidying up some of the API where possible, trying to move away from Variant if only a single type's expected/returned and getting everything working with ZendParamMode.

I'm using 512ecad as an example of what to do.

Existing idl files:

My current observations: (please correct if these are wrong)

  • You can't use closures inside a systemlib.php
  • Type hint parameters and return types (see the aforementioned commit for examples)
  • Feel free to move C++ methods/functions into PHP if it makes sense and is easy to do so.
@staabm

This comment has been minimized.

Show comment
Hide comment
@staabm

staabm Jan 3, 2014

Contributor

doing all at once could make it really hard to review..?

Contributor

staabm commented Jan 3, 2014

doing all at once could make it really hard to review..?

@simonwelsh

This comment has been minimized.

Show comment
Hide comment
@simonwelsh

simonwelsh Jan 3, 2014

Contributor

Also, would be really boring to do ;)

I was thinking of using this more as organising/tracking and then doing one extension at a time (for instance, I'm currently doing ext/reflection) and keeping a record of things that I've discovered if others want to help.

Contributor

simonwelsh commented Jan 3, 2014

Also, would be really boring to do ;)

I was thinking of using this more as organising/tracking and then doing one extension at a time (for instance, I'm currently doing ext/reflection) and keeping a record of things that I've discovered if others want to help.

@staabm

This comment has been minimized.

Show comment
Hide comment
@staabm

staabm Jan 3, 2014

Contributor

ah got it now... ;-).

Contributor

staabm commented Jan 3, 2014

ah got it now... ;-).

@scannell

This comment has been minimized.

Show comment
Hide comment
@scannell

scannell Jan 3, 2014

Contributor

Thanks for working on this @simonwelsh!

Contributor

scannell commented Jan 3, 2014

Thanks for working on this @simonwelsh!

@JoelMarcey

This comment has been minimized.

Show comment
Hide comment
@JoelMarcey

JoelMarcey Jan 7, 2014

Contributor

This is awesome. Making sure @ptarjan and @sgolemon are a permanent part of this thread too...

Contributor

JoelMarcey commented Jan 7, 2014

This is awesome. Making sure @ptarjan and @sgolemon are a permanent part of this thread too...

@ptarjan

This comment has been minimized.

Show comment
Hide comment
@ptarjan

ptarjan Jan 7, 2014

Contributor

Wow, awesome! I'd love if the IDL machinery died.

@jdelong and I were chatting about this, and migrating them is a great time to clean them as they move over.

  1. Move anything that can be done in PHP, into PHP (constant declaration, wrapper functions, anything that doesn't call C++)
  2. Remove any hand-rolled type checking
  3. Check the zend tests for any low hanging incompatibility fruit
Contributor

ptarjan commented Jan 7, 2014

Wow, awesome! I'd love if the IDL machinery died.

@jdelong and I were chatting about this, and migrating them is a great time to clean them as they move over.

  1. Move anything that can be done in PHP, into PHP (constant declaration, wrapper functions, anything that doesn't call C++)
  2. Remove any hand-rolled type checking
  3. Check the zend tests for any low hanging incompatibility fruit

This was referenced Jan 10, 2014

@Pomyk

This comment has been minimized.

Show comment
Hide comment
@Pomyk

Pomyk Jan 14, 2014

Contributor

I started working on mongo, but it will take some time ;)

Contributor

Pomyk commented Jan 14, 2014

I started working on mongo, but it will take some time ;)

@awakmu

This comment has been minimized.

Show comment
Hide comment
@awakmu

awakmu Jan 16, 2014

Hi all,

What is an IDL? What it's differents with HNI?

And I know about HNI from the last release note: Hhvm Native Interface -> Possibility to call c++ function from PHP.
But I still not sure about these two and can't found any documentation about them.

-Aris

awakmu commented Jan 16, 2014

Hi all,

What is an IDL? What it's differents with HNI?

And I know about HNI from the last release note: Hhvm Native Interface -> Possibility to call c++ function from PHP.
But I still not sure about these two and can't found any documentation about them.

-Aris

@elgenie

This comment has been minimized.

Show comment
Hide comment
@elgenie

elgenie Jan 18, 2014

Contributor

Thanks for taking this on @simonwelsh . Are you using a script that reads in .idl.json and produces the .php declarations? If so, is that script checked in as part of these conversion commits?

Contributor

elgenie commented Jan 18, 2014

Thanks for taking this on @simonwelsh . Are you using a script that reads in .idl.json and produces the .php declarations? If so, is that script checked in as part of these conversion commits?

@simonwelsh

This comment has been minimized.

Show comment
Hide comment
@simonwelsh

simonwelsh Jan 19, 2014

Contributor

@elgenie No, I'm doing it all by hand, basing of the PHP docs, then what the function actually returns. I've found that the IDL files aren't accurate enough to convert directly, so just deciding to skip the segfaults from mismatching types.

Contributor

simonwelsh commented Jan 19, 2014

@elgenie No, I'm doing it all by hand, basing of the PHP docs, then what the function actually returns. I've found that the IDL files aren't accurate enough to convert directly, so just deciding to skip the segfaults from mismatching types.

@WizKid

This comment has been minimized.

Show comment
Hide comment
@WizKid

WizKid Jan 19, 2014

Contributor

tools/docskel/docskel.php uses the Zend docs to create a HNI skeleton. At least I think that was the script I used when I created that start of mysqli. But @sgolemon should know more.

Contributor

WizKid commented Jan 19, 2014

tools/docskel/docskel.php uses the Zend docs to create a HNI skeleton. At least I think that was the script I used when I created that start of mysqli. But @sgolemon should know more.

@simonwelsh

This comment has been minimized.

Show comment
Hide comment
@simonwelsh

simonwelsh Jan 20, 2014

Contributor

@WizKid Sweet, that'll save me copy+pasting the descriptions. Still have to manually do the signatures, given how many edge cases the signature in the docs doesn't cover. Or things like bzopen with completely undocumented (but tested) features (it can take an open file resource instead of a filename).

Contributor

simonwelsh commented Jan 20, 2014

@WizKid Sweet, that'll save me copy+pasting the descriptions. Still have to manually do the signatures, given how many edge cases the signature in the docs doesn't cover. Or things like bzopen with completely undocumented (but tested) features (it can take an open file resource instead of a filename).

@WizKid

This comment has been minimized.

Show comment
Hide comment
@WizKid

WizKid Jan 20, 2014

Contributor

@simonwelsh Yea it is far from perfect but it usually give you a good start.

Contributor

WizKid commented Jan 20, 2014

@simonwelsh Yea it is far from perfect but it usually give you a good start.

@igorastds

This comment has been minimized.

Show comment
Hide comment
@igorastds

igorastds Feb 4, 2014

What's a reason for having Bountysource record for this one? Is it actual? (I can help with porting..)

What's a reason for having Bountysource record for this one? Is it actual? (I can help with porting..)

@scannell

This comment has been minimized.

Show comment
Hide comment
@scannell

scannell Feb 4, 2014

Contributor

@igorastds: There is a lot of otherwise relatively thankless work to do here that, once complete, will allow us to clean up the extensions story. It is to encourage community assistance with that.

Contributor

scannell commented Feb 4, 2014

@igorastds: There is a lot of otherwise relatively thankless work to do here that, once complete, will allow us to clean up the extensions story. It is to encourage community assistance with that.

@dewyatt

This comment has been minimized.

Show comment
Hide comment
@dewyatt

dewyatt Feb 13, 2014

These all-encompassing bug bounties are very off-putting.

dewyatt commented Feb 13, 2014

These all-encompassing bug bounties are very off-putting.

@scannell

This comment has been minimized.

Show comment
Hide comment
@scannell

scannell Feb 13, 2014

Contributor

@dewyatt, we've provided the feedback to Bountysource that it would be better to have an integrated way to do this without creating 70 additional open issues on the project. Unfortunately there's no support for that at the moment.

Contributor

scannell commented Feb 13, 2014

@dewyatt, we've provided the feedback to Bountysource that it would be better to have an integrated way to do this without creating 70 additional open issues on the project. Unfortunately there's no support for that at the moment.

@staabm

This comment has been minimized.

Show comment
Hide comment
@staabm

staabm Feb 26, 2014

Contributor

@simonwelsh do we need to add XSL 72ed649 to the todo-list?

Contributor

staabm commented Feb 26, 2014

@simonwelsh do we need to add XSL 72ed649 to the todo-list?

@simonwelsh

This comment has been minimized.

Show comment
Hide comment
@simonwelsh

simonwelsh Feb 26, 2014

Contributor

@staabm Already have. Has put the amount merged back below 15% :(

Contributor

simonwelsh commented Feb 26, 2014

@staabm Already have. Has put the amount merged back below 15% :(

sgolemon added a commit that referenced this issue Mar 27, 2014

Convert ext/mysql to HNI
Part of #1480

Also includes the commit in #2024

Closes facebook/hhvm#2027

#2027

Reviewed By: @ptarjan

Differential Revision: D1220239

Pulled By: @fredemmott
@staabm

This comment has been minimized.

Show comment
Hide comment
@staabm

staabm Apr 4, 2014

Contributor

@simonwelsh gd is missing in this list and was done by sarah in 5fc0301

Contributor

staabm commented Apr 4, 2014

@simonwelsh gd is missing in this list and was done by sarah in 5fc0301

@simonwelsh

This comment has been minimized.

Show comment
Hide comment
@simonwelsh

simonwelsh Apr 4, 2014

Contributor

@staabm This is a list of the IDL files. GD used image.idl.json.

Contributor

simonwelsh commented Apr 4, 2014

@staabm This is a list of the IDL files. GD used image.idl.json.

@simonwelsh

This comment has been minimized.

Show comment
Hide comment
@simonwelsh

simonwelsh Jul 18, 2014

Contributor

50% of the IDL files are now gone 🍰 😄

Contributor

simonwelsh commented Jul 18, 2014

50% of the IDL files are now gone 🍰 😄

facebook-github-bot added a commit that referenced this issue Jul 25, 2014

Convert ext/mysql to HNI
Summary: Part of #1480

Also includes the commit in #2024
Closes #2027

Reviewed By: ptarjan, joelm

Differential Revision: D1410521

Pulled By: @fredemmott

This was referenced Aug 16, 2014

hhvm-bot added a commit that referenced this issue Aug 17, 2014

Move debugger from IDL to HNI
Summary: Part of #1480.

This moves the functions from `debugger.idl.json` to HNI.
Closes #3491

Reviewed By: @ptarjan

Differential Revision: D1502583

Pulled By: svcscm

hhvm-bot added a commit that referenced this issue Aug 18, 2014

Move mailparse to HNI
Summary: This pull request moves the functions in mailparse.idl.json to HNI.

Also, `ezmlm_hash` now calculates the hash it should calculate.

Part of #1480
Closes #3505

Reviewed By: @ptarjan

Differential Revision: D1502587

This was referenced Aug 19, 2014

hhvm-bot added a commit that referenced this issue Aug 26, 2014

Rewrite misc.idl.json to HNI
Summary: This pull request moves the functions in misc.idl.json to HNI.

Part of #1480
Closes #3525

Reviewed By: @fredemmott

Differential Revision: D1506187

Pulled By: @sgolemon

hhvm-bot added a commit that referenced this issue Oct 30, 2014

Convert system/idl/function to HNI
Summary: Convert system/idl/function to HNI.
Also fixes compatibility with PHP, when func_get_arg() is called with non-integer parameter.

Part of #1480.
Closes #3942

Reviewed By: @​svcscm

Differential Revision: D1608060

Pulled By: @ptarjan
@Orvid

This comment has been minimized.

Show comment
Hide comment
@Orvid

Orvid Sep 25, 2015

Contributor

Just a poke to point out this hasn't been updated in almost a year. Also, I just noticed that this has a $1000 bounty on it on bountysource.

Contributor

Orvid commented Sep 25, 2015

Just a poke to point out this hasn't been updated in almost a year. Also, I just noticed that this has a $1000 bounty on it on bountysource.

@JoelMarcey

This comment has been minimized.

Show comment
Hide comment
@JoelMarcey

JoelMarcey Sep 25, 2015

Contributor

@sgolemon's got this! She has asio and simplexml in the pipeline; next up is collections.

Contributor

JoelMarcey commented Sep 25, 2015

@sgolemon's got this! She has asio and simplexml in the pipeline; next up is collections.

@simonwelsh

This comment has been minimized.

Show comment
Hide comment
@simonwelsh

simonwelsh Jan 29, 2016

Contributor

Only the remainder of collections left :D

Contributor

simonwelsh commented Jan 29, 2016

Only the remainder of collections left :D

@hhvm-bot hhvm-bot closed this in 1f9180e Apr 14, 2016

@simonwelsh

This comment has been minimized.

Show comment
Hide comment
@simonwelsh

simonwelsh Apr 14, 2016

Contributor

Woot woot :)

Contributor

simonwelsh commented Apr 14, 2016

Woot woot :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment