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

Edit Dockerfile from debian to buster #51

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SofiyanIfren
Copy link

Made two moidifications to make it working, as discussed in the following issue : #49

  1. From Debian Jessie to Buster, because the binary-amd64 was not available inside the Jessie image, here it is for Buster : /debian-security/dists/buster/updates/main/binary-amd64
  2. Removed the '\r' character which was causing this bug : /usr/bin/env: 'perl\r': No such file or directory (the '\r' was not removed from the perl files)

The image is now working fine al hamdouliLlah

Copy link
Contributor

@ahmedre ahmedre left a comment

Choose a reason for hiding this comment

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

jazakumAllah khairan! 2 minor comments

@@ -1,4 +1,4 @@
FROM debian:jessie
FROM debian:buster
Copy link
Contributor

Choose a reason for hiding this comment

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

how about debian:buster-slim instead?

Copy link
Contributor

@ahmedre ahmedre Apr 9, 2024

Choose a reason for hiding this comment

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

or bookworm-slim even?

@@ -35,6 +35,8 @@ RUN cd /app && \
make && \
make install

RUN find /app -type f -name '*.pl' -exec sed -i 's/\r$//' {} \;
Copy link
Contributor

Choose a reason for hiding this comment

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

can we change the script/generate.pl directly just remove the \r instead of running this every time?

Copy link
Contributor

Choose a reason for hiding this comment

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

actually not seeing a \r here - i'll look into this some more.

@ahmedre
Copy link
Contributor

ahmedre commented Apr 9, 2024

jazakumAllah khairan for the PR - I merged #50, which had some of these changes along with bumping mysql. it's working for me now on macOS. where did you find this \r fix is needed since things are ok for me without it?

@SofiyanIfren
Copy link
Author

Assalamou aleikoum sorry for the response time, I didn't receive the notifications...

  • I d'idn't try slim images for buster, I can do it if you want
  • For the \r char, it may be because I'm a windows user, it is common to have similar issues of misunderstood characters between the apple and the window ; I don't think modifying the file will be appropriate, as if I do it, it will be done from a different OS

May Allah agree our deeds

@ahmedre
Copy link
Contributor

ahmedre commented May 8, 2024

wa3laikum alsalam,
no problem - i think the main branch should be working now without this patch, please give it a shot in sha' Allah.
jazakumAllah khairan

@SofiyanIfren
Copy link
Author

SofiyanIfren commented May 8, 2024

Just tried, the build is OK but I have a new error on the main branch, maybe the doc. is not updated :

λ docker-compose run gen /app/script/generate.pl --width 1300 --output ./output/ --pages 50
[+] Running 1/0

  • Container qurancom-images-mysql-1 Created 0.0s [+] Running 1/1
  • Container qurancom-images-mysql-1 Started 1.0s DBI connect('database=nextgen;host=mysql;port=3306','nextgen',...) failed: Can't connect to MySQL server on 'mysql' (115) at /app/script/../lib/Quran/DB.pm line 20.
    Died at /app/script/../lib/Quran/DB.pm line 20.

Shall I open a new issue ?

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.

2 participants