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

QR code library adds several new files to vendor dir that is breaking build #13794

Closed
diosmosis opened this issue Dec 3, 2018 · 8 comments · Fixed by #13889
Closed

QR code library adds several new files to vendor dir that is breaking build #13794

diosmosis opened this issue Dec 3, 2018 · 8 comments · Fixed by #13889
Assignees
Labels
c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Milestone

Comments

@diosmosis
Copy link
Member

See https://travis-ci.org/matomo-org/matomo/jobs/462646517

Many files like /vendor/endroid/qrcode/assets/data/qrv36_2.dat are added. Not sure if they are needed for qr code generation or not.

CC @mattab / @tsteur

@diosmosis diosmosis added c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Dec 3, 2018
@diosmosis diosmosis added this to the 3.8.0 milestone Dec 3, 2018
@tsteur
Copy link
Member

tsteur commented Dec 4, 2018

I've quickly checked in the code and it looks like this code is needed :( might be wrong though. only alternative would be to maybe use another lib. I've simply reused what @sgiehl used for Google Authenticator.

@Findus23
Copy link
Member

Findus23 commented Dec 4, 2018

@tsteur Is there any reason to use 1.9 instead of the latest 3.5.0? I can't find the assets/data files anymore in the latest version:
https://github.com/endroid/qr-code

Update:
Okay this is a reason:
https://github.com/endroid/qr-code/blob/0095706c3bf2389f15f6c097bab4a00a48fc5ff7/composer.json#L15
so -> #13611

@sgiehl
Copy link
Member

sgiehl commented Dec 4, 2018

Version 2.0 already didn't contain those files anymore. But that requires PHP 5.6. Might be worth to check if that version works with PHP 5.5 as well. Or maybe it's an option to mark the 2FA plugin as compatible with PHP 5.6 or 7 only. So users with older PHP versions can't use it

@tsteur
Copy link
Member

tsteur commented Dec 20, 2018

Marking it as PHP 5.6 plus could be an idea. Actually never tried if we can require PHP version for a core plugin.

@tsteur
Copy link
Member

tsteur commented Dec 20, 2018

Just added this plugin.json

{
  "require": {
    "php": ">=7.2.0"
  }
}

as a test and it worked. However, then this error notification constantly appears which would be a bit annoying:

The plugin TwoFactorAuth could not be loaded as it has missing dependencies: TwoFactorAuth requires Php >=7.2.0. Please contact your Matomo administrator.

Looked for couple other QR Code libraries but looks like they either support 5.6+ or have those dat files. Also looked for some JavaScript alternatives... maybe https://davidshimjs.github.io/qrcodejs/ (https://github.com/davidshimjs/qrcodejs) would be a good solution?

@tsteur tsteur self-assigned this Dec 20, 2018
@tsteur
Copy link
Member

tsteur commented Dec 20, 2018

tested https://github.com/davidshimjs/qrcodejs but seems to print canvas and image etc.... Might try another JS lib

@tsteur
Copy link
Member

tsteur commented Dec 20, 2018

also tried https://github.com/jeromeetienne/jquery-qrcode but doesn't work well either. Example in readme has typos, and the number of open issues and PRs is big. Also hasn't had a change in years. So not quite surprised it isn't working.

@tsteur
Copy link
Member

tsteur commented Dec 20, 2018

Got it to work with https://github.com/davidshimjs/qrcodejs after all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants