-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
Fix alpine tests #133
Fix alpine tests #133
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #133 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 32 32
Lines 1345 1345
Branches 229 229
=========================================
Hits 1345 1345 ☔ View full report in Codecov by Sentry. |
d86a333
to
b601242
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, Alpine doesn't have locales and the musl-locales package is very limited.
See https://grrr.tech/posts/2020/add-locales-to-alpine-linux-docker-image/
The best way forward might be to not rely on system locale but to depend on and use an external locale-data such as what Babel does.
I am confident all our use cases can switch to it but it must be checked obviously.
I'd thus advise we:
- open a ticket to change i18n system
- mention Alpine in that ticket
- update the readme near Alpine part mentioning that locale stuff are not OK and point to the ticket.
WDYT?
Agreed regarding the approach for finishing Alpine support: #134 |
I don't know why the CI is not trigered anymore ; may I rebase/force-push before you review the last commits? |
Yes ; might be a GH issue ; I've had a similar pb on a different repo today |
python-magic is supporting Alpine Linux since 0.4.24 (hence the minimum version) while file-magic is still not supporting it.
9e52eb4
to
6654bd5
Compare
Force push done, CI is green, please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe there's a typo in the range
6654bd5
to
c0df148
Compare
c0df148
to
68d3e30
Compare
🤦🏻 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤓
Fix #114 (remaining part is now tracked in #134)
Original comment
Partial fix of #114, the following test is still failing ; looks like on Alpine setlocale is never raising an error, see issue for details. I don't know if we accept to live with it or add a special detection for Alpine Linux which would skip the test.
Rationale
Some tests were failing on Alpine Linux +
file-magic
is not well maintained / not supporting Alpine LinuxChanges
file-magic
bypython-magic
library, since the last one has support for Alpine + is better maintained