Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve unzip error on applying new patches when composer cache is cleared #107

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

adamj88
Copy link
Contributor

@adamj88 adamj88 commented Apr 4, 2023

Fixes #103

@adamj88
Copy link
Contributor Author

adamj88 commented Apr 4, 2023

@zamoroka would you be able to merge this PR, please?

It addresses a bug in the patch process detailed in #103 which is fairly common.

@ElSamhaa
Copy link

What is the status of this fix? I'm facing the bug for some time now.

@hostep
Copy link
Contributor

hostep commented Sep 1, 2023

Hi @zamoroka, have you guys already had some time to look into this?

We keep running into #103 from time to time. Re-running composer install a second time (or sometimes 3 or 4 times more) always fixes it, but it's annoying that this keeps happening every now and again.

I haven't verified if these changes fixes the problem, but I assume Adam tested this.

@zamoroka-vaimo zamoroka-vaimo merged commit 6d8e2de into vaimo:master Nov 22, 2023
@zamoroka-vaimo
Copy link
Contributor

Hi all, PR released in 5.1.2
sorry for the delay :)

@fredden
Copy link

fredden commented Dec 5, 2023

This change seems related to #111, and #112 looks like it will revert at least part of this change. There doesn't seem to be any in-depth explanation for why this change is the correct thing to fix #103. Can @adamj88 provide some insight into the reasoning for this change, please?

@adamj88
Copy link
Contributor Author

adamj88 commented Dec 5, 2023

@fredden from #103 the explanation was that the uninstall method inside the promise was at fault:

If we replace it with the code

 $installationManager
            ->uninstall($repository, $uninstallOperation);

        return $installer
                ->download($package)
            ->then(function () use ($installationManager, $installOperation, $repository) {
                $installationManager->install($repository, $installOperation);
            });

the patches apply properly even when composer cache is cleared.
It looks like the promise is not working for the uninstall function, but I need more input here.

@hostep
Copy link
Contributor

hostep commented Dec 18, 2023

We also keep running into #111 after upgrading to 5.1.2, so it sounds like this fix causes more issues than it fixes, we have to downgrade to 5.1.1 to even get the module to work at all in certain cases unfortunately :(

@zamoroka-vaimo: would be appreciated if you find some more time to look into this a bit and if #112 would fix things or not...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unzip error on applying new patches when composer cache is cleared.
5 participants