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

Composer: let mcrypt_compat "provide" mcrypt req #2

Closed
wants to merge 1 commit into from

Conversation

terrafrost
Copy link
Member

No description provided.

@Alanaktion
Copy link

This really adds a lot of value to this package. Without it, it's possible to work around an ext-mcrypt requirement using --ignore-platform-reqs, but that can have other side effects that wouldn't be an issue if this were merged.

@terrafrost
Copy link
Member Author

The problem I have with merging this is that it breaks the unit tests. In particular, I get the following error:

[InvalidArgumentException]                     
  Package is not installed: ext-mcrypt-7.0.26.0

Any ideas as to how I can fix this?

@ravage84 ravage84 mentioned this pull request Feb 9, 2018
@ravage84
Copy link

ravage84 commented Feb 9, 2018

@terrafrost, I couldn't reproduce your issue. Though I've used PHP 7.1 on Windows for a quick setup. How did you get that error?

I also think many could benefit from your compat library, as I mentioned in #13.

@terrafrost
Copy link
Member Author

@ravage84 - it's reproducible on Travis CI. Even your PR (#13) failed..

@ravage84
Copy link

ravage84 commented Feb 9, 2018

@terrafrost excuse my ignorance 🙇

It seems it's a Composer bug. I did some debugging and confirmed a behavior that has already been mentioned by someone else:

composer/composer#7086

I guess Composer should ignore php extensions when looking which package (library) makes which package obsolete.

@Alanaktion
Copy link

There is another option that works reasonably well if you just need to install other Composer libraries that depend on ext-mcrypt. Requiring this library, then adding a config section to your composer.json can work to tell Composer to assume the extension is actually installed.

{
    "require": {
        "phpseclib/mcrypt_compat": "^1.0",
        // ...
    },
    "config": {
        "platform": {
            "ext-mcrypt": "1.0.0"
        }
    }
}

@@ -32,5 +32,8 @@
},
"autoload": {
"files": ["lib/mcrypt.php"]
},
"provide": {
"ext-mcrypt": "*"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the value should be version, so use 0 or minimum (uppper?) php api you support

the native extension matches PHP_VERSION:

$ php -r '$r = new ReflectionExtension("mcrypt"); var_dump($r->getVersion());'
Command line code:1:
string(6) "7.1.25"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seemed to do the trick! I went ahead and made it provide 5.6.40 (the last 5.6 version) since mcrypt was removed in 7.0.0. I've gone ahead and made a new release as well.

Thanks!

@glensc
Copy link

glensc commented Feb 26, 2019

for travis. add this fragment:

$ composer config platform.ext-mcrypt '0'

then downstream projects do not need to hack with their composer.json, as that solution is not flexible as composer.json/composer.lock are committed to repo usually, and i have no choice to use real extension, this compat lib or have a failure in dependency resolution.

@glensc
Copy link

glensc commented Feb 26, 2019

here's project that uses ext provide successfully:

@terrafrost terrafrost closed this May 26, 2019
@terrafrost
Copy link
Member Author

4c97e9b does this.

Thanks!

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.

None yet

4 participants