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.

Sign up for GitHub

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

Jump to bottom

Add dockerization option? #36

Open
MatanKribus opened this issue Mar 1, 2022 · 7 comments
Open

Add dockerization option? #36

MatanKribus opened this issue Mar 1, 2022 · 7 comments

Comments

@MatanKribus
Copy link

I think a simple dockerfile will suffice :)
will help with stability across diff environments.

I messed around and made a new repo which is basically a clone of this with a dockerfile. all you need to do is build :P

I'll close it(And the issue) if you think it's inappropriate obviously, just say so :)

https://github.com/MatanKribus/dockerized-mandown
@Titor8115
Copy link
Owner

@MatanKribus Thanks for the docker support. Im just not too familiar with docker. Is it possible to maybe add docker as an build option? Cause I was hoping to make something that can be used across most of the OS (even Windows eventually). If so Id be really happy to merge your's into the project.

@MatanKribus
Copy link
Author

@Titor8115 I should have been more clear in what I meant by "all you need to do is build"

docker is a virtualization tool, it build's images, to run on containers.
you can think of containers as self contained environments. (not a virtual machine)

what docker can do is create a image specific to your application/service/project , the way you do that is with a Dockerfile, there you specify the steps a system takes to be able to do what you want it to.

as you can see, it's a seperate process then building the project :)
what I was thinking is maybe making a script that will let people choose either a virtualized option, or an "on host" install option.

@Titor8115
Copy link
Owner

Sorry for the late reply. Docker definitely sounds interesting and I think it should be an option for the project. But let me test this out once I get back before merge it.

Ill be back to my workstation probably tomorrow or the day after.

@Titor8115
Copy link
Owner

Hi I just finished testing the Dockerfile from your repo, for some reason the program doesn't seem to run past the beginning where ncurses starts doing it's thing.

sudo docker run 4d92912d458f README.md
<html><head><title>(7)</title></head><body></body></html>

This is as far as the program runs. Is it something else I should setup before or perhaps docker run is not the best option here?

@MatanKribus
Copy link
Author

MatanKribus commented Mar 13, 2022 •
edited

Hi, I'm glad to see you decided to add docker, a few things to remember regarding docker:

  1. each "run" command will start a container, which is a sep env (complete with it's own file system and all!)
  2. to use files from the current host, we want to either copy them, or "share" them in a volume(think of volumes as mounted storage for you container, from the host.)
  3. we avoid using the ID of the container(as you did), mostly for readability. and sanity

try
docker run -v $PWD:/app -w /app -it mandown:1.0.1 README.md
from the directory where README.md exist. notice the $PWD, the path should be absolute.
what it does:
-v is for volume, we mount the working directory from our host, on to the /app directory on the container.
-w sets the working directory on the container, to /app. think of it as "cd /app".
notice that the pathing will begin from the root of the container. but we don't care that much because it's a container, and it's disposable.

edit:
the -it is important too, it tells docker to run the command in interactive mode. hence giving us the option to play around in the container before quitting the root process, at which point the container closes itself because the root process have died.
edit2:
notice the name of my container, "mandown:1.0.1" it's actually the name I gave it while building the image.
docker build -t mandown:1.0.1 .
the -t option gives the ability to name and tag the image, so name:tag

@Titor8115
Copy link
Owner

OK, It worked. One last issue, the docker doesn't seem to use the locale, setlocale(LC_ALL, ""); I set for the system (meaning some character aren't rendered correctly). Perhaps It's just someone docker user can setup themselves?

The rest worked fine. You could send a Pull Request for merging whenever you want. While you are at it, I apologize that I need your help with the docker section for the README. Would you mind to add to README.md, in that Pull Request, anything you felt needed for docker?

Thanks a lot for everything!

@MatanKribus
Copy link
Author

OK, It worked. One last issue, the docker doesn't seem to use the locale, setlocale(LC_ALL, ""); I set for the system (meaning some character aren't rendered correctly). Perhaps It's just someone docker user can setup themselves?

The rest worked fine. You could send a Pull Request for merging whenever you want. While you are at it, I apologize that I need your help with the docker section for the README. Would you mind to add to README.md, in that Pull Request, anything you felt needed for docker?

Thanks a lot for everything!

Sure, I would like to solve the issue with the setlocate, could you show me what should be displayed when it works correctly so I could debug it on my machine? I think maybe playing around with the base image might solve the issue, but I can't be sure of that until I test it.

99.99% sure it's not something the end user should config themselfs.

I will edit the README.md on my clone after I finished debugging the problem with setlocal can I also ask what machine are you using to develop/test/run the program? I assume a distro of linux, which one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Titor8115 @MatanKribus