peachpie icon indicating copy to clipboard operation
peachpie copied to clipboard

com_dotnet extension: basic implementation (barely working)

Open Indigo744 opened this issue 5 years ago • 20 comments

Implements #834

I started to work on a basic implementation on extension com_dotnet

What works:

  • Creating COM object
    • codepage / typelib not supported
  • Calling methods with arguments which returns a simple value type (not COM objects)
  • Getting / Setting properties for value types (not COM objects)
  • com_create_guid()

What is missing:

Almost everything...! But the main work would revolve around implementing the Variant class so the COM class could inherit from it. Also this class would be used for handling complex COM object which is not working at all right now.

What should not be supported:

  • Creating dotnet object as .Net framework 4.0 and later are not supported.

Indigo744 avatar Sep 03 '20 15:09 Indigo744

We should find a way to run the test only on Windows with specific extension activated? Not sure how to do this...

Indigo744 avatar Sep 03 '20 16:09 Indigo744

We should find a way to run the test only on Windows with specific extension activated? Not sure how to do this...

for now, at the beginning you can do something like

if (PHP_OS != "WINNT") exit("***SKIP***");

The test will be marked as skipped

jakubmisek avatar Sep 04 '20 11:09 jakubmisek

On the PeachPie side, should I add a test in the COM constructor (and in the function) where I test that we are on Windows and throw an NotSupportedException if not?

Indigo744 avatar Sep 07 '20 07:09 Indigo744

@Indigo744 I wouldn't check for Windows platform, what if it gets supported by .NET in the future (through magic) .. I guess .NET throws something anyways now

jakubmisek avatar Sep 07 '20 09:09 jakubmisek

Noted @jakubmisek

However, I'm not sure I'll be able to push this implementation further in the short term.

Would it be acceptable to merge it "as is"? Some stuff are working, so it would be better than nothing for people needing this. How could we warn about what is missing?

Indigo744 avatar Sep 07 '20 09:09 Indigo744

I guess tests :) and also before the release, we are generating the compatibility matrix on https://docs.peachpie.io/compatibility-status/ which you can generate for yourself at https://github.com/peachpiecompiler/peachpie-docs/tree/master/tools (it creates list of functions/classes in all loaded extensions - for PHP and PeachPie - and compares them)

jakubmisek avatar Sep 07 '20 09:09 jakubmisek

I guess tests :) and also before the release, we are generating the compatibility matrix on https://docs.peachpie.io/compatibility-status/ which you can generate for yourself at https://github.com/peachpiecompiler/peachpie-docs/tree/master/tools (it creates list of functions/classes in all loaded extensions - for PHP and PeachPie - and compares them)

I was able to generate a new updated list, but the com_dotnet extension sits at 0 / 107... I am missing the relevant package on the PeachPie side, but I couldn't get it to work, even with the 1.0.0-dev version..

Indigo744 avatar Sep 07 '20 10:09 Indigo744

you have to add a package/project reference into generate.msbuildproj

jakubmisek avatar Sep 07 '20 11:09 jakubmisek

@jakubmisek I was missing the lib from the local Nuget packages! Updating the script fixes this issue.

However, now I have the following error:

Unhandled exception. System.NotImplementedException: System.Reflection.BindingFlags
   at Pchp.Core.Dynamic.ConvertExpression.BindToValue(Expression expr)
   at Pchp.Core.Reflection.PhpPropertyInfo.ClrFieldProperty.CompileAccess(AccessMask access)
   at Pchp.Core.Reflection.PhpPropertyInfo.ClrFieldProperty.<.ctor>b__9_0()
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at System.Lazy`1.get_Value()
   at Pchp.Core.Reflection.PhpPropertyInfo.ClrFieldProperty.GetValue(Context ctx, Object instance)
   at Pchp.Library.Reflection.ReflectionClass.getConstants(Context ctx)
   at CallSite.Target(Closure , CallSite , PhpValue , Context )
   at System.Dynamic.UpdateDelegates.UpdateAndExecute2[T0,T1,TRet](CallSite site, T0 arg0, T1 arg1)
   at <Root>.program_php.collect(Context <ctx>) in peachpie-docs\tools\generate-compatibility\program.php:line 45
   at <Root>.program_php.<Main>(Context <ctx>, PhpArray <locals>, Object this, RuntimeTypeHandle <self>) in peachpie-docs\tools\generate-compatibility\program.php:line 65
   at <Script>.Main(String[] args)

I guess it doesn't know how to handle the BindingFlags type related to the constant MemberAccessCom...

Indigo744 avatar Sep 07 '20 16:09 Indigo744

@Indigo744 this is because PHP reflection tries to deal with the following constant:

const BindingFlags MemberAccessCom

which should not be visible to PHP code at all. Annotate the constant with [PhpHidden] attribute so it won't get into PHP runtime at all (or mark it as ~~internal protected~~).

EDIT: it should be private protected - this denotes a field or a method that is not visible in PHP context

jakubmisek avatar Sep 08 '20 12:09 jakubmisek

internal protected was not enough, I still had the error. Adding [PhpHidden] did the trick however!

Indigo744 avatar Sep 08 '20 13:09 Indigo744

@Indigo744 sorry, private protected hides the field/const/method from PHP reflection

jakubmisek avatar Sep 09 '20 13:09 jakubmisek

Do you want me to modify it further, or is it good enough for now?

Indigo744 avatar Sep 09 '20 14:09 Indigo744

Do you want me to modify it further, or is it good enough for now?

the attribute is ok for now

jakubmisek avatar Sep 09 '20 14:09 jakubmisek

Hi @Indigo744 - can I help with this somehow? :)

jakubmisek avatar Oct 20 '20 12:10 jakubmisek

Hey @jakubmisek! Sorry for not getting back. I am swamped in work right now 😞 so as you could see I wasn't able to contribute as much as I could.

However note that this PR could be merged as is. Some stuff are working. The rest could be done later. As you wish 😸

Indigo744 avatar Oct 20 '20 13:10 Indigo744

@Indigo744 just checking :) would be great to have a list of feature that are not implemented so we would have an overview.

I'd be happy to implement whats missing eventually, and merge the PR

jakubmisek avatar Oct 20 '20 13:10 jakubmisek

I've outlined in the PR description what is defined/what is missing.

An exact list is available in the docs PR: https://github.com/peachpiecompiler/peachpie-docs/pull/5.

Indigo744 avatar Oct 20 '20 16:10 Indigo744

@jakubmisek Hello! Wow can't believe it's already 3 years later. Time flies!

Should we try merging it? I can clean up/fix conflicts.

Indigo744 avatar Oct 31 '23 08:10 Indigo744

@jakubmisek Hello! Wow can't believe it's already 3 years later. Time flies!

Should we try merging it? I can clean up/fix conflicts.

that would be awesome :)

jakubmisek avatar Dec 10 '23 14:12 jakubmisek