Skip to content

Edit Dockerfile from debian to buster#51

Open
SofiyanIfren wants to merge 1 commit into
quran:masterfrom
SofiyanIfren:master
Open

Edit Dockerfile from debian to buster#51
SofiyanIfren wants to merge 1 commit into
quran:masterfrom
SofiyanIfren:master

Conversation

@SofiyanIfren

Copy link
Copy Markdown

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

@ahmedre ahmedre left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread Dockerfile
@@ -1,4 +1,4 @@
FROM debian:jessie
FROM debian:buster

Copy link
Copy Markdown
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?

@ahmedre ahmedre Apr 9, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

or bookworm-slim even?

Comment thread Dockerfile
make && \
make install

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

Copy link
Copy Markdown
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
Copy Markdown
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

ahmedre commented Apr 9, 2024

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

ahmedre commented May 8, 2024

Copy link
Copy Markdown
Contributor

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

SofiyanIfren commented May 8, 2024

Copy link
Copy Markdown
Author

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