phar-io / composer-distributor

Distribute PHAR files via Composer
Other
12 stars 2 forks source link

Discussion: Testing with PharIO\GnuPG #26

Open sebastianfeldmann opened 1 year ago

sebastianfeldmann commented 1 year ago

I'm a bit stuck. I'm trying to fix the current tests but I keep running into problems.

PHPUnit\Framework\MockObject\IncompatibleReturnValueException: Method import may not return value of type array, its declared return type is "void"

Was playing around with it a bit but always running into expected GnuPg got PharIO\GnuPg issues when testing without the extension.

I was thinking about hiding the two GnuPG implementations behind a GnuPGProxy and only type hint and Mock the proxy. So I don't have to deal with the type jougling.

But I'm not sure if if makes sense to implement it in this repo.

Maybe implementing it in phar-io/gnupg makes more sense. Then even phive could implement against the proxy api und would not care about the \GnuPg or PharIO\GnuPg magic. It would always be GnuPgProxy (name still up for discussion).

Or am I just too blind to figure this little type mess out another way.

@theseer @heiglandreas what do you think.

theseer commented 1 year ago

We really just need to rework (to not say rewrite ;-) the gnupg library. I have no idea why I/we considered a good idea to implement it the way we did...

heiglandreas commented 1 year ago

IMO the GnuPH lib should donthe heavy lifting of figuring out whether the lib is available or not. So in essence that already should be the proxy.

How that is then handled internally is not an issue of this package 🙈

heiglandreas commented 1 year ago

We are already using a factory to fetch the GnuPG class.

I'd propose to use a GnuPG interface and the factory then returns one of three implementations: one based on the GnuPG extension, one based on the gpg exectutable and one fallback that will never validate (or it will but output an appropriate message?)

sebastianfeldmann commented 1 year ago

The question would be if we hand out the implementations to users of the lib, or if we hide that stuff in the background.

I have to admit I'm still far from being an expert with all the gpg stuff. Maybe we define multiple interfaces depending on the use case.

PharIO\GnuPG\Verifyer
PharIO\GnuPG\KeyImporter
...

I think I'm in favor of hiding the "complexity" for lib users and internally take care of everything that is needed to make the key handling and verification work. So internally do exactly what you described only difference not handing over different implementations to the user instead handling them internally.

I would love to work on this (need to finally become at least decent with this gpg stuff). Happy to video call in the upcoming days to plan the course of action.

theseer commented 1 year ago

I very much like the idea of growing phar-io/gnupg to become a more complete solution.

heiglandreas commented 1 year ago

I wouldn't leave anything to the user!

We are currently using phario/gpg. And I'd only refactor that lib to handle the stuff differently and especially to handle cases where neither the gpg extension nor the gpg binary are present

theseer commented 1 year ago

Do you plan to plan to have a full userland implementation? :open_mouth:

heiglandreas commented 1 year ago

NO!

More like not verifying at all but telling the user that they are on a system without either and that that is insecure and they are on their own from here!

theseer commented 1 year ago

Okay, given the meeting in Munich didn't work, let's try to schedule a video call :-)

I'll send a link later today to find time slots.