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

A Docker Container, tool for prezto development #1753

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

hlecuanda
Copy link

Proposed Changes

  • Provides a tool for rapid testing and development without disrupting your own einvironment
  • Most CI tools are based don docker containers so this could help get on the CI and QA bandwagon
  • I'm not exactly sure where to place this, and i'm sure is a lot of room for improvement. the Makefile and Dockerfile are heavily commented, plus i added a container-README.md that walks through the features already on the container if you wish to try it out

here's a quick screencast too https://asciinema.org/a/277054

@belak
Copy link
Collaborator

belak commented Nov 7, 2019

Sorry, I've been super busy - I really like the idea behind this, especially making some issues easier to reproduce. Thanks for taking the time to do this!

@hlecuanda
Copy link
Author

My pleasure! and I'm sorry too, year end is coming up, and it's hectic around here this time of the year.
I'm sure there is a lot of room for improvement for this container, and i'd be happy to work on any feedback to make it more useful for the prezto project.

Do you think this is the right place for this? or is this better suited for a separate branch, or an external contrubution? maybe a plugin? please let me know what ypu think

@hlecuanda hlecuanda changed the title Container devel A Docker Container, tool for prezto development Nov 12, 2019
Copy link
Contributor

@casaper casaper left a comment

Choose a reason for hiding this comment

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

Just two minor things I noticed in the docker file, in case that helps

@@ -0,0 +1,24 @@
FROM alpine as prezto-devel
LABEL maintainer="[email protected]"
RUN apk --no-cache update && apk upgrade && \
Copy link
Contributor

@casaper casaper Feb 11, 2020

Choose a reason for hiding this comment

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

Instead of --no-cache the common recommendation is this at the end

I've just noticed that --no-cache you renders the clean-up part I wanted to mention obsolete.

And Why do you do apk upgrade ? Since you imply alpine:latest in FROM.
Because If you didn't do that, it would probably shrink the image size significantly, without you having any flaws I can see.

WORKDIR /home/prezto
USER prezto
RUN cd /home/prezto && mkdir src
COPY . src
Copy link
Contributor

Choose a reason for hiding this comment

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

If you used ADD instead of COPY it would speed up things, because then the files would not really be copied into the container.
Or do you need it like that so that your local sources won't be changed with the later make runs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please tell me how ADD will speed things up? According to docker's best practices individual COPY statements are preferred.

Copy link
Contributor

@casaper casaper Feb 13, 2020

Choose a reason for hiding this comment

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

ADD adds the directory as a docker volume.
COPY really creates copies of the files into the docker image.

At least that was how I understood it so far...

Copy link
Contributor

@wadkar wadkar Feb 13, 2020

Choose a reason for hiding this comment

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

I don't think it works like that. Neither statements add a directory as a volume. Docker volumes are different directives manipulated by VOLUME statement in Dockerfile or -v flag at runtime.

Both ADD and COPY statements copy the source from the build context (first argument) to the destination in docker image (second argument).

Specifically, as the best practices link explains, the ADD statement has additional magic to it - it will automagically uncompress a compressed file, or if the first argument is a URL it will download it.

COPY . src is good first pass for a Dockerfile. Though ideally, one would prefer to have separate statements for each directory, so that a change in one file in one directory will not invalidate the entire cache. But then we run into the problem of how we will order individual COPY statements. So, I am okay with COPY . src for now.

Edit: Just saw your below comments. At first glance, I like the volumes approach with the caveat that modification from within docker container will modify the host file. I'll check out this PR over the weekend and give it a shot.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well however, it doesn't really matter to me actually if ADD or COPY is used in the end here.

My main point is not running stuff in the image building process, but rather run it on the finished image...
Its just quicker, and sllicker. If you prefer to build the image newly each time. you run something, or change code and then test it, and fail and change it again... up to you. 😉

Copy link
Contributor

@casaper casaper left a comment

Choose a reason for hiding this comment

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

I would just suggest to not run the most things I've realized so far from within the Dockerfile. For reasons stated within the Dockerfile comment I just made.

I don't want to block you or anything. I just think it'll be smoother to do it with a container approach, and not with an image approach.

COPY . src
RUN cp src/Makefile .
RUN make .clone
RUN make src .homercs
Copy link
Contributor

@casaper casaper Feb 13, 2020

Choose a reason for hiding this comment

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

What I don't understand is why you run make inside the docker file.

Without fully understanding your makefile and your shell script, I just try to tell you how I would approach that what I assume you want to achieve. If that assumptions are wrong or if I'm anyhow wrong, no big deal. I only try to help.

My Dockerfile would be like this:

FROM alpine

RUN apk --no-cache add \
  util-linux \
  pciutils \
  usbutils \
  coreutils \
  binutils \
  findutils \
  grep \
  man \
  man-pages \
  mdocml-apropos \
  less \
  less-doc \
  make \
  grep \
  zsh \
  zsh-vcs \
  zsh-zftp \
  zsh-calendar \
  zsh-doc \
  git \
  vim \
  git-zsh-completion \
  tmux \
  tmux-zsh-completion \
  tree \
  docker-zsh-completion \
  && addgroup prezto \
  && adduser -D prezto -G prezto -S /bin/zsh

WORKDIR /home/prezto
USER prezto

CMD ["/bin/zsh"]

And then I would add a docker-compose.yml file and run the stuff through that:

version: "3.7"

services:
  prezto:
    build: .
    volumes:
      - ./:/home/prezto/.zprezto
      - ./my-test-stuff:/home/prezto/test
      - ./runcoms/zshenv:/home/prezto/.zshenv
      - ./runcoms/zprofile:/home/prezto/.zprofile
      - ./runcoms/zshrc:/home/prezto/.zshrc
      - ./runcoms/zpreztorc:/home/prezto/.zpreztorc
      - ./runcoms/zlogin:/home/prezto/.zlogin
      - ./runcoms/zlogout:/home/prezto/.zlogout

Now you can set that up with:

docker-compose build  # builds your docker file
docker-compose run --rm prezto /bin/zsh
Couldn't read file /home/prezto/.zprezto/modules/prompt/functions/prompt_agnoster_setup containing theme agnoster.
Couldn't read file /home/prezto/.zprezto/modules/prompt/functions/prompt_powerlevel10k_setup containing theme powerlevel10k.
Couldn't read file /home/prezto/.zprezto/modules/prompt/functions/prompt_powerlevel9k_setup containing theme powerlevel9k.
Couldn't read file /home/prezto/.zprezto/modules/prompt/functions/prompt_powerline_setup containing theme powerline.
Couldn't read file /home/prezto/.zprezto/modules/prompt/functions/prompt_pure_setup containing theme pure.
prompt_sorin_setup:7: async: function definition file not found
prompt_sorin_async_tasks:4: command not found: async_start_worker
prompt_sorin_async_tasks:5: command not found: async_register_callback
prompt_sorin_async_tasks:10: command not found: async_flush_jobs
prompt_sorin_async_tasks:13: command not found: async_job
~ ❯❯❯

I did exactly this here, and it is partial C&P from my shell. So I'm sure that it works.

This has several benefits:

  1. your image will not have to check out the prezto repository, because that you allready have in your dev dir
  2. /home/prezto has all the dotfiles like .zshenv allready in there, directly from your dev machines runcoms
  3. You can use the same image, it's smaller, and you only need to create new containers

You can have some test scripts inside the prezto project like this:

# remember that i mounted `./my-test-stuff:/home/prezto/test` in the docker-compose.yml
docker-compose run --rm prezto /home/prezto/test/some-script.sh
I'm echoed by  some-script.sh doing the stuff you want to do...

This way you can then run technically anything in that container and get that container flushed right after your done.
And you're working with your current WIP directories code.

I just recon it is not really the way to run your tests within the Dockerfile's RUN statements.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeap, this makes a lot more sense than building prezto inside the image.

@wadkar
Copy link
Contributor

wadkar commented Feb 13, 2020

Looks good after reading through the diff @hlecuanda , let me follow the README and make the container over the weekend. Does that sound good to you?

Copy link
Contributor

@casaper casaper left a comment

Choose a reason for hiding this comment

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

I didn't want to block you or anything. Just help you.

But if you guys prefer to build the image newly for any change... nock your cpu's out.. 😉

@wadkar
Copy link
Contributor

wadkar commented Feb 17, 2020

So I played with thee Dockerfile over the weekend and it was fun. I think it'll help us develop/test prezto without messing with our day-to-day config changes.

I am more inclined towards following @casaper 's suggestion to mount the locally cloned prezto repository instead of using Makefiles to install it. I was on a limited bandwidth over the weekend, and my developer experience was severely hampered every time I needed to build the image with some modifications to the Dockerfile of my own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants