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

Fix broken Dockerfile #300

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

Conversation

3nprob
Copy link

@3nprob 3nprob commented Aug 17, 2021

Dockerfile does not build anymore. Since it now copies a (missing from install dir) local copy of imapsync, switched to building from repo root so proper version can be copied. Also adds missing trailing \ that caused an issue.

@gilleslamiral A separate question I have after addressing this: The image now contains two independent versions of imapsync, one from the local repo and one downloaded release. Commonly I would expect a single Dockerfile to only contain the version built from local source.

I think this can be pretty confusing. Do you see a clear need for a single container to contain both versions? If not I'd propose (and happy to contribute if you agree) one of the following approaches:

  1. Build from source by default. Add build arg to override and download release instead. (This is what I suggest)
  2. Two separate Dockerfiles

This would allow easy building of both versions from the same repo.

* Add missing \
* Dockerfile now built from repo root
@florenthemmi
Copy link
Contributor

For my personal use, I am using a different Dockerfile (with latest Debian)
Maybe the current Dockerfile can be more simpler.

FROM debian:bullseye

RUN set -xe                   && \
	apt-get update            && \
	apt-get install -y           \
	libauthen-ntlm-perl          \
	libcgi-pm-perl               \
	libcrypt-openssl-rsa-perl    \
	libdata-uniqid-perl          \
	libencode-imaputf7-perl      \
	libfile-copy-recursive-perl  \
	libfile-tail-perl            \
	libio-socket-inet6-perl      \
	libio-socket-ssl-perl        \
	libio-tee-perl               \
	libhtml-parser-perl          \
	libjson-webtoken-perl        \
	libmail-imapclient-perl      \
	libparse-recdescent-perl     \
	libmodule-scandeps-perl      \
	libreadonly-perl             \
	libregexp-common-perl        \
	libsys-meminfo-perl          \
	libterm-readkey-perl         \
	libtest-mockobject-perl      \
	libtest-pod-perl             \
	libunicode-string-perl       \
	liburi-perl                  \
	libwww-perl                  \
	libtest-nowarnings-perl      \
	libtest-deep-perl            \
	libtest-warn-perl            \
	make                         \
	cpanminus                    \
	procps                       \
	wget

RUN wget -O /usr/bin/imapsync https://raw.githubusercontent.com/imapsync/imapsync/master/imapsync && \
	chmod +x /usr/bin/imapsync

USER nobody:nogroup

ENV HOME /var/tmp/
WORKDIR /var/tmp/

STOPSIGNAL SIGINT

ENTRYPOINT ["/usr/bin/imapsync"]

@gilleslamiral gilleslamiral self-assigned this Apr 3, 2022
@gilleslamiral
Copy link
Member

gilleslamiral commented Apr 3, 2022

Dockerfile does not build anymore. Since it now copies a (missing from install dir) local copy of imapsync,

It's fixed now with:

# I put a copy of the Dockerfile in the image itself
# It can help the maintenance, isn't it?
# Also put optionally my local and usually more recent imapsync on /, for testing purpose

COPY Dockerfile imapsyn[c] prerequisites_imapsyn[c] /

switched to building from repo root so proper version can be copied.

I disagree. The imapsync github releases are not tested in the docker context. They can work or they can fail.
I publish the imapsync Docker image after the tests pass.

@gilleslamiral A separate question I have after addressing this: The image now contains two independent versions of imapsync, one from the local repo and one downloaded release. Commonly I would expect a single Dockerfile to only contain the version built from local source.

I use the local imapsync for testing/fixing purposes. As long as the tests don't pass in a docker context I have to fix them. How? By modifying this local imapsync. Then I update this good imapsync upstream, which is not GitHub in my current workflow.

I think this can be pretty confusing. Do you see a clear need for a single container to contain both versions? If not I'd propose (and happy to contribute if you agree) one of the following approaches:

  1. Build from source by default. Add build arg to override and download release instead. (This is what I suggest)

The github source is not guaranteed to run under the docker context.

  1. Two separate Dockerfiles
    This would allow easy building of both versions from the same repo.

I have enough headaches with one Dockerfile to maintain.

Copy link
Member

@gilleslamiral gilleslamiral left a comment

Choose a reason for hiding this comment

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

I don't approve the merge because there is a newer Dockerfile and other reasons done in the conversation

@3nprob
Copy link
Author

3nprob commented Apr 4, 2022

Hi Gilles,

I disagree. The imapsync github releases are not tested in the docker context. They can work or they can fail. I publish the imapsync Docker image after the tests pass.

I don't understand this - Docker is a build and packaging tool. Just like a Makefile usually uses the local sources, so do Dockerfiles. You wouldn't expect a Makefile to pull in the sources from a remote endpoint and just copy the local one to the side for reference, right? Rather you'd expect the build to use the local source and the local source to be the version you want (usually by checking out a tagged release, not using master other than for dev and testing)

Typical workflow from a user would be: checkout a tagged release

Then I update this good imapsync upstream, which is not GitHub in my current workflow.

...Is the version at https://imapsync.lamiral.info/imapsync typically containing changes not present in the git repo source..?

So, if I may again, why not:

Build from local source by default. Add build arg to override and download release instead. (This is what I suggest)

It is up to the builder to make sure they're using the right source. Possibly by downloading it from your site

I have enough headaches with one Dockerfile to maintain.

One makes sense, I was (and still am btw) a bit confused why one docker image should contain two potentially different versions of the same application..

@gilleslamiral
Copy link
Member

I don't understand this - Docker is a build and packaging tool. Just like a Makefile usually uses the local sources, so do Dockerfiles.

Well, that's not my way for now.

You wouldn't expect a Makefile to pull in the sources from a remote endpoint and just copy the local one to the side for reference, right?

The local one is a temporary one, just for me. The Dockerfile is the same, it's for me first.

Rather you'd expect the build to use the local source and the local source to be the version you want (usually by checking out a tagged release, not using master other than for dev and testing)

I have only one branch.

Typical workflow from a user would be: checkout a tagged release

Is it? really? You know what a typical user workflow is? How do you know?
Or is it your typical workflow?

...Is the version at https://imapsync.lamiral.info/imapsync typically containing changes not present in the git repo source..?

Yes, that's imapsync latest published. Not Docker guaranteed either, except the one I use when I build the imapsync docker image.

So, if I may again, why not:

Build from local source by default. Add build arg to override and download release instead. (This is what I suggest)

It is up to the builder to make sure they're using the right source. Possibly by downloading it from your site

Or possibly using the old local one, or any one.

I have enough headaches with one Dockerfile to maintain.

One makes sense, I was (and still am btw) a bit confused why one docker image should contain two potentially different versions of the same application..

The imapsync docker image I provide usually contains the same releases, as it is the end of the docker building process I follow:
I iterate tests and bugfixes with the local then I publish it at https://imapsync.lamiral.info/imapsync, then I use the published to the build and publish the imapsync docker image on dockerhub.

@3nprob
Copy link
Author

3nprob commented Apr 4, 2022

Hey, you seem to be either defensive or upset - just leaving feedback here, assumed you're sharing the repo in public in order for it to be useful for others and appreciated some feedback to make it more-so 🤷

Is it? really? You know what a typical user workflow is? How do you know?

From experience building hundreds if not thousands of containerfiles from other projects and experience in the industry and open source community. Look around and you will probably find the same. The way it's done here is highly unusual and as you say, only really usable for you (made by you, for you, after all).

But that being said, that doesn't mean it's objectively "better" or "correct" or that you need to follow that - a lot of it is just cultural. It's your project and you do the call on it, after all.

I find imapsync a nice and useful tool, and would love for it to be more easy to use for others, without new people being confused and ending up either walking away from it or bothering you about the same thing.

@3nprob
Copy link
Author

3nprob commented Apr 4, 2022

Hope I'm not too grumpy back - again, want to reinforce I'm super grateful for your making and maintaining this neat software, whatever your workflow is. Keep it real.

(and in case it's of use:

docker run -it -v `pwd`:/path imapsync

to mount current working directory inside the containerfile - that way you get the local copy without having to rebuild even ;))

@gilleslamiral
Copy link
Member

From experience building hundreds if not thousands of container files from other projects and experience in the industry and open source community. Look around and you will probably find the same.

Ok, teacher!

The way it's done here is highly unusual and as you say, only really usable for you (made by you, for you, after all).

Well, you're unfair.
I updated the Dockerfile and the Dockerhub image to relate to the "two imapsync" problem:

https://hub.docker.com/r/gilleslamiral/imapsync

## Dockerfile for building a docker imapsync image

# $Id: Dockerfile,v 1.42 2022/04/04 19:16:50 gilles Exp gilles $
...
# This Dockerfile build an image with two imapsync
# 1) One comes from https://imapsync.lamiral.info/imapsync
#    and goes to /usr/bin/imapsync in the Docker image
#    It is used with the command:
#
#    docker run gilleslamiral/imapsync imapsync ...
# 
# 2) One comes from the local file ./imapsync
#    and goes to /imapsync in the Docker image
#    It is used with the command:
#
#    docker run gilleslamiral/imapsync /imapsync ...

@gilleslamiral
Copy link
Member

(and in case it's of use:

docker run -it -v `pwd`:/path imapsync

to mount current working directory inside the containerfile - that way you get the local copy without having to rebuild even ;))

$ docker run -it -v `pwd`:/path imapsync
Unable to find image 'imapsync:latest' locally
docker: Error response from daemon: pull access denied for imapsync, repository does not exist or may require 'docker login': denied: requested access to the resource is denied.
See 'docker run --help'.

$ docker run imapsync
Unable to find image 'imapsync:latest' locally
docker: Error response from daemon: pull access denied for imapsync, repository does not exist or may require 'docker login': denied: requested access to the resource is denied.
See 'docker run --help'.

I'm new to docker. I use it only to build the dockerhub imapsync image...

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.

None yet

3 participants