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

Manually install iconv as the one provided in Alpine does not work #166

Closed
wants to merge 3 commits into from

Conversation

tfmm
Copy link

@tfmm tfmm commented Nov 5, 2020

linuxserver.io


  • I have read the contributing guideline and understand that I have made the correct modifications

Description:

This manually installs the PHP iconv extension, as the one provided in Alpine repositories does not work correctly.

Benefits of this PR and context:

As Mentioned in #165 iconv on alpine is not working properly, causing an issue for some extensions and NC functionality.

Closes #165

How Has This Been Tested?

Built all 3 images without issue.
Ran the standard image without issue.
Reviewed NC functionality across multiple portions of the application, no issues found.

Source / References:

Ref: docker-library/php#240 (comment)
Ref: #165
Ref: nextcloud/mail#3999
Ref: nextcloud/mail#3908

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thanks for opening this pull request! Be sure to follow the pull request template!

@aptalca
Copy link
Member

aptalca commented Nov 5, 2020

I really think it would be much simpler to PR a fix to the alpine repo package rather than compiling here and setting php versions manually

@tfmm
Copy link
Author

tfmm commented Nov 5, 2020

While I do not disagree, from multiple references around the internet, the Alpine maintainers do not appear to be willing to fix the issue on their end.

@aptalca
Copy link
Member

aptalca commented Nov 5, 2020

We've actually fixed quite a few upstream bugs there via PRs. Their maintainers may not be super proactive but they are very much open to community PRs.

With that said, I've been on mobile and have not looked into what the upstream issue is.

@tfmm
Copy link
Author

tfmm commented Nov 5, 2020

Still testing, but I may have found a cleaner way to handle this, will update the PR or close it and open a new one if the testing works out.

@tfmm
Copy link
Author

tfmm commented Nov 5, 2020

Updated the PR as there is a work-around that only involves installing gnu-libiconv from community repos and not having to manually compile.

@tobbenb
Copy link
Member

tobbenb commented Nov 5, 2020

Updated the PR as there is a work-around that only involves installing gnu-libiconv from community repos and not having to manually compile.

There was a user on the unraid forum I asked to install the gnu-libiconv package, but it didn't fix the issue. php7-iconv is installed when we build the container if I'm not mistaken.

@tfmm
Copy link
Author

tfmm commented Nov 6, 2020

I found the same thing when I installed gnu-libiconv after the container was built. However, I found that if I installed it during build, and then installed php7-iconv afterwards, and also from the community repo vs the main repo, it worked. I also set the LD_PRELOAD env variable to load the extension.

@DataHearth
Copy link

Hello there, is this PR will be merged soon ? I'm really looking forward to get rid of my custom image on my unRAID server.
That could be a cool a update for unRAID users. That would avoid doing a custom image based on this one with the fix 👌.

@Manu3l0us
Copy link

Any chance to get this merged?

@aptalca
Copy link
Member

aptalca commented Dec 7, 2020

This is too hacky for my taste, as the main and edge repos provide different versions of php. This really should be fixed upstream as I suggested before. Did you submit a PR there or open an issue?

@ageisen2000
Copy link

@aptalca docker-library/php#240 (comment)

is this the correct spot to look for? if so, this maintainer doesn't seem to want to change the functionality?

@aptalca
Copy link
Member

aptalca commented Dec 25, 2020

Correct spot is the alpine gitlab repo: https://gitlab.alpinelinux.org/alpine/aports

@itsthejb
Copy link

YMMV, whilst I'm a huge fan of Linux server images, decided to move over to the official Debian image for this one. Seems nextcloud is getting a bit huge and feature rich these days, and the alpine base might not be enough going forward (just my 10c)

@Manu3l0us
Copy link

@itsthejb
I'm considering this step too. Still a bit worried about the migration though, is there a guide or do you have written down the process by any chance?

@aptalca
Copy link
Member

aptalca commented Dec 25, 2020

A lot of folks complaining in this thread about alpine, yet not a single person even attempted to submit a patch upstream. It shouldn't be that hard as the edge version works, but not the 3.12 version. Looking at the build differences would likely easily reveal the necessary fix.

@ageisen2000
Copy link

Now that I know where to look I will try to see if I can look at submitting a pr, but with the holidays and my very tiny amount of knowledge around alpine it'll be difficult.

I agree that an upstream fix seems a lot better. Thanks for pointing out where the repo was for me!

@Intenos
Copy link

Intenos commented Jan 2, 2021

Has this fix already been requested from the Alpine developers? I´m also interested in having it working.

@aptalca
Copy link
Member

aptalca commented Jan 2, 2021

Well, if no one does anything, the version on edge will end up on alpine 3.13 any day now

@Intenos
Copy link

Intenos commented Jan 2, 2021

I have not added a request on Github yet. What would I need to exactly ask for? Is it sufficient to add an issue named "Please add/fix iconv extension" and link also to this issue here?

@Poltergeisen
Copy link

@aptalca when you say any day now do you think within the next week or two? That might be faster than I can get to the PR anyways

@aptalca
Copy link
Member

aptalca commented Jan 2, 2021

@github-actions
Copy link

github-actions bot commented Feb 9, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@homerr homerr closed this Aug 7, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nextcloud Mail broken, messages not loading (Nextcloud 20.0.1/Mail 1.5.1)