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

Non Drupal 8 projects checking fails if PHP doesn't have iconv enabled #33

Open
hkirsman opened this issue Oct 10, 2019 · 3 comments
Open

Comments

@hkirsman
Copy link
Collaborator

hkirsman commented Oct 10, 2019

I added code-quality to my Drupal 7 project but it fails when commiting:

git commit                                                         
GrumPHP detected a pre-commit command.
GrumPHP is sniffing your code!
Running task 1/8: EcsTask... ✘
Aborted ...
PHP Warning:  Use of undefined constant ICONV_IMPL - assumed 'ICONV_IMPL' (this will throw an Error in a future version of PHP) in /home/hannes/Sites/tekla/campus/vendor/nette/utils/src/Utils/Strings.php on line 154
PHP Fatal error:  Uncaught Error: Call to undefined function Nette\Utils\iconv() in /home/hannes/Sites/tekla/campus/vendor/nette/utils/src/Utils/Strings.php:168
Stack trace:
#0 /home/hannes/Sites/tekla/campus/vendor/nette/utils/src/Utils/Strings.php(180): Nette\Utils\Strings::toAscii('/home/hannes/Si...')
#1 /home/hannes/Sites/tekla/campus/vendor/symplify/easy-coding-standard/packages/ChangedFilesDetector/src/Cache/Simple/FilesystemCacheFactory.php(22): Nette\Utils\Strings::webalize('/home/hannes/Si...')
#2 /tmp/easy_coding_standard/ContainerZdGlkCq/getFilesystemCacheService.php(9): Symplify\EasyCodingStandard\ChangedFilesDetector\Cache\Simple\FilesystemCacheFactory->create()
#3 /tmp/easy_coding_standard/ContainerZdGlkCq/HttpKernelSymplify_EasyCodingStandard_HttpKernel_EasyCodingStandardKernelProdd90fcfcbe4265705e72551ee048dbb9063446Container.php(152): require('/tmp/easy_codin...')
#4 /tmp/easy_coding_standard/ContainerZdGlkCq/getSimpleCacheAdapterService.php(9): ContainerZdGlkCq\HttpKernelSymplify_EasyCodingStandard_HttpKerne in /home/hannes/Sites/tekla/campus/vendor/nette/utils/src/Utils/Strings.php on line 168
To skip commit checks, add -n or --no-verify flag to commit command

My system doesn't seem to have php module iconv enabled and configured. But then again it works fine for Drupal 8. When checking deeper, then I found that Drupal 8 has symfony/polyfill-iconv package installed. This solved my issue:

 composer require symfony/polyfill-iconv:^1 

It's missing --dev intentionally because Drupal 8 web/core/composer.json file has the dependency also in require section as:

"symfony/polyfill-iconv": "^1.0",
hkirsman added a commit that referenced this issue Nov 11, 2019
Code Quality tool needs PHP iconv module enabled but
it's not always available by default. That's why
Drupal 8 has added polyfill package for it and it
makes sense to add it to Code Quality too. We want
this tool to be installed as easily as possible.
@hkirsman
Copy link
Collaborator Author

1 more comment on the issue that it in case you have for example PHP 7.3 installed but not the iconv module enabled, then it will fail silently as it will install 1.0.3 version of Code Quality tool. You'll then don't have the phpstan package installed and the line from documentation to copy it's config will fail:

cp vendor/wunderio/code-quality/config/phpstan.neon ./phpstan.neon

hkirsman added a commit that referenced this issue Nov 11, 2019
Code Quality tool needs PHP iconv module enabled but
it's not always available by default. That's why
Drupal 8 has added polyfill package for it and it
makes sense to add it to Code Quality too. We want
this tool to be installed as easily as possible.
hkirsman added a commit that referenced this issue Nov 11, 2019
Code Quality tool needs PHP iconv module enabled but
it's not always available by default. That's why
Drupal 8 has added polyfill package for it and it
makes sense to add it to Code Quality too. We want
this tool to be installed as easily as possible.
hkirsman added a commit that referenced this issue Nov 11, 2019
Code Quality tool needs PHP iconv module enabled but
it's not always available by default. That's why
Drupal 8 has added polyfill package for it and it
makes sense to add it to Code Quality too. We want
this tool to be installed as easily as possible.
hkirsman added a commit that referenced this issue Nov 11, 2019
Code Quality tool needs PHP iconv module enabled but
it's not always available by default. That's why
Drupal 8 has added polyfill package for it and it
makes sense to add it to Code Quality too. We want
this tool to be installed as easily as possible.
hkirsman added a commit that referenced this issue Nov 11, 2019
Code Quality tool needs PHP iconv module enabled but
it's not always available by default. That's why
Drupal 8 has added polyfill package for it and it
makes sense to add it to Code Quality too. We want
this tool to be installed as easily as possible.
hkirsman added a commit that referenced this issue Nov 11, 2019
Code Quality tool needs PHP iconv module enabled but
it's not always available by default. That's why
Drupal 8 has added polyfill package for it and it
makes sense to add it to Code Quality too. We want
this tool to be installed as easily as possible.
hkirsman added a commit that referenced this issue Nov 11, 2019
Code Quality tool needs PHP iconv module enabled but
it's not always available by default. That's why
Drupal 8 has added polyfill package for it and it
makes sense to add it to Code Quality too. We want
this tool to be installed as easily as possible.
hkirsman added a commit that referenced this issue Nov 11, 2019
Code Quality tool needs PHP iconv module enabled but
it's not always available by default. That's why
Drupal 8 has added polyfill package for it and it
makes sense to add it to Code Quality too. We want
this tool to be installed as easily as possible.
@hkirsman
Copy link
Collaborator Author

hkirsman commented Nov 11, 2019

I was not able to solve it from composer.json.

Tried:

  1. Install the polyfill but for Composer this is not the same as ext-iconv
"require": {
    "symfony/polyfill-iconv": "^1",
  1. Created fake iconv that has provide entry in it, still Composer did not like it.
    "require": {
        "symfony/polyfill-iconv": "^1",
        "wunderio/fake-iconv": "^1",
  1. Tried provide in the package itself. Nothing.
    "provide": {
        "ext-iconv": "*"
    },
  1. Faked the platform but I guess it works from the composer.json thay user has in his system.
    "config": {
        "platform": {
            "ext-iconv": "0"
        }
    },

All in all, it's advised to use the PHP iconv library as it's faster. Some more options to propose here
5. Still add the polyfill and update readme install section to ignore the requirements with --ignore-platform-reqs
6. Add note to readme about the issue and say why it is the way it is.

Something else?

@guncha25 @mgalang

hkirsman added a commit that referenced this issue Nov 11, 2019
PHP iconv extension is needed on the system for this plugin to work.
We'll add polyfill-iconv here in case you are working with Lando
and the plugin is availabble in Docker container but
git commit fails as the extension is not available in the
host system.
hkirsman added a commit that referenced this issue Nov 11, 2019
PHP iconv extension is needed on the system for this plugin to work.
We'll add polyfill-iconv here in case you are working with Lando
and the plugin is availabble in Docker container but
git commit fails as the extension is not available in the
host system.
hkirsman added a commit that referenced this issue Nov 11, 2019
PHP iconv extension is needed on the system for this plugin to work.
We'll add polyfill-iconv here in case you are working with Lando
and the plugin is availabble in Docker container but
git commit fails as the extension is not available in the
host system.
@hkirsman
Copy link
Collaborator Author

hkirsman commented Nov 11, 2019

Ok, here's my last thought on this. Here's the situation that I would like to avoid. I've installed the latest code-quality in lando using:

lando composer require wunderio/code-quality --dev

Everything goes fine but now when trying to commit it yells about the iconv library:

alt text

Wouldn't hurt to add this safe fallback for projects like Drupal 7 (it's in core drupal 8 composer.json)?

guncha25 added a commit that referenced this issue Nov 16, 2019
mitrpaka pushed a commit to mitrpaka/code-quality that referenced this issue Nov 28, 2020
PHP iconv extension is needed on the system for this plugin to work.
We'll add polyfill-iconv here in case you are working with Lando
and the plugin is availabble in Docker container but
git commit fails as the extension is not available in the
host system.
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

No branches or pull requests

1 participant