Add Dockerfile to enable running end-to-end tests with docker

[AgustinRamiroDiaz]

Context

Fixes #1225

Reviewer tips

The commit history is a bit dirty (if we go forward we should squash it), so look at the final picture

• Added Dockerfile and .dockerignore
• Documented a bit in the README.md

I also encourage you to run the tests in your local machine to see how it works. Documentation on how to do this in the README.md

Notes

• This is my first time contributing here, I'm open to receive all your feedback
• The tests are failing at the moment in the tip of master. If someone is able to run vagrant, please notify me if this is also the case so I know if it's an issue with the Dockerfile configuration

[mentalisttraceur]

Nice! Thanks for doing this!

I hope it gets merged (once everything is verified working as-it's-supposed-to) because I also couldn't get the in-Vagrant tests running, and it might be nicer to have both as options (or whichever single option is most widely available if Exa doesn't want to maintain both).

Disclaimer: I'm just passing through, I'm not one of the Exa maintainers. So don't take my words as Official, I'm just commenting on a couple things to help out.

In the big picture, the only concern I have is that if the VM tests currently exercise listing things belonging to different users/groups, we should probably make sure the Docker container is able to do the same.

A few detailed/small/nitpicky comments inline. Overall great contribution though, in my opinion!

[polarathene]

I'm a bit busy this week, but I could contribute a review with feedback for changes if you ping me next week if you'd like.

---

Your .dockerignore is fairly extensive, if you don't need as much in the image, you can ignore everything and then add some exceptions of folders / files to include in the docker context. See this example for reference.

I help maintain a Docker focused project so Dockerfile is something I know quite well. Assuming anyone that is building the image is ok with doing so with a fairly recent version of Docker (v23+, and depending on features versions released in 2022), there is a few features you could take advantage of.

• HereDoc for many batching commands into a single layer without the prior drawback of chaining && \ at the end of each line. Example.
• It's ok to extract some scripts out to their own shell script file if appropriate, sometimes it's just noise to the Dockerfile and doesn't belong there, better linting too. Example.
• You may want to use --link with your COPY instructions, it can provide an optimization that minimizes invalidating layers. Example. Might be good for the series of --from stage copies you have at the end.
• You create a few shell files (t, rexa, etc) to run different commands, but could use a single shell script with a switch to support that. Example (not the best).
• I think all the rust:1.71.1 could reference a single ARG with default value instead.
• apt-get could use a cache mount if the image was built often enough and you wanted to minimize this step. From the looks of it the cache mount may not be too useful here, but it is for anything with longer build times that can leverage cache from prior builds. This is different from docker build leveraging cached layers and is mostly beneficial for parts of a build that are not needed in the image like package cache, build cache etc.

[AgustinRamiroDiaz]

@polarathene thanks a lot for your feedback! I'll be addressing it in my next commits

chaining && \

Where do you think we could use this feature? We don't have the command chaining, we only have some multi-line commands only for readability (install a lot of apt packages)

It's ok to extract some scripts out to their own shell script file if appropriate, sometimes it's just noise to the Dockerfile and doesn't belong there, better linting too. Example.

Yeah I agree, but for this PR I don't want to be too disruptive, I think this is great as a post iterative improvement

You create a few shell files (t, rexa, etc) to run different commands, but could use a single shell script with a switch to support that. Example (not the best).

Sorry, I didn't understand the example. If you mean to modify the current scripts, I do not want to modify the internal behavior because I cannot guarantee that they'll behave correctly in Vagrant (couldn't make it work)

[polarathene]

Really shouldn't be necessary to rely on sudo if the intent is to replace Vagrantfile with Dockerfile.

[AgustinRamiroDiaz]

I agree, I just don't want to touch all the internal files because a) I'm not too familiar with them and b) I'm not sure it'll behave the same as I couldn't run the Vagrant tests. I think we should go iteratively on this, by first making it compatible and afterwards refactoring the sudo stuff

[polarathene]

kconv should be located down here with the rest? I assume the concern was due to the package install portion for kconv and moving the layer(s) down to the end here being problematic when an earlier instruction invalidates the Dockerfile cache?

[polarathene]

You could actually shift these up to after kconv, shouldn't be an issue given what the rest of the stage is doing?

[AgustinRamiroDiaz]

kconv should be located down here with the rest?

Actually it's needed for RUN cargo kcov --print-install-kcov-sh | sh here

You could actually shift these up to after kconv, shouldn't be an issue given what the rest of the stage is doing?

That's true, do you think it's useful? I don't understand the benefit

[polarathene]

I don't understand the benefit

The layers that are more frequently to change should come later. The bulk of this stage is something that could run in a separate script via RUN, but for now you've stated you want to keep it in the Dockerfile.

You could move these COPY lines up before kconv for better co-location, but right now no major concern either way.

[polarathene]

Where do you think we could use this feature? We don't have the command chaining, we only have some multi-line commands only for readability (install a lot of apt packages)

I see now that you've mostly taken the scripts from the Vagrantfile as-is. I provided some suggestions of where you could use it.

If both files are to be kept, it probably should be it's own script file that is shared between the two. Personally I'd be in favor of removing the Vagrantfile and simplify the `Dockerfile.

Yeah I agree, but for this PR I don't want to be too disruptive, I think this is great as a post iterative improvement
I do not want to modify the internal behavior because I cannot guarantee that they'll behave correctly in Vagrant (couldn't make it work)

I haven't looked too much into how those were being used, but since this is just taking lines from the Vagrantfile I can understand why you want to take that approach.

[polarathene]

I'm not sure if you can use mknod with the image build to get the /dev special files created:

exa/devtools/dev-create-test-filesystem.sh

Lines 126 to 128 in f039202

	sudo mknod "$TEST_ROOT/specials/block-device" b 3 60
	sudo mknod "$TEST_ROOT/specials/char-device" c 14 40
	sudo mknod "$TEST_ROOT/specials/named-pipe" p

[AgustinRamiroDiaz]

Sorry, I don't understand. Do you mean that mknod won't work in docker?

[polarathene]

Yeah, I don't think it works, but I could be mistaken, I haven't checked 🤷‍♂️

[polarathene]

Ignore this

This also has some odd locale logic at the end. I'm wondering if that's necessary?

In docker-mailserver image we have a similar locale section:

RUN <<EOF
  rm -rf /usr/share/locale/*
  rm -rf /usr/share/man/*
  rm -rf /usr/share/doc/*
  update-locale
EOF

For exa, I tracked it down to a specific test for different locales, so ignore this feedback 😅

115315a

[AgustinRamiroDiaz]

@polarathene @mentalisttraceur thanks a lot for your time and feedback. I think I've addressed it all, please ping me if not

[polarathene]

Suggested change

	```fish
	```sh

[polarathene]

Thanks for addressing the feedback :)

I'm quite busy so might not have much more feedback for now, especially since further work is being deferred to a future PR.

I haven't tried building and running it yet, but I'm curious if it appears to be working with tests passing? Especially those related to the usage of mknod (might work if that's run at runtime if build time isn't persisting /dev).

[AgustinRamiroDiaz]

I haven't tried building and running it yet, but I'm curious if it appears to be working with tests passing?

@polarathene actually that's my main concern, tests do NOT pass at the moment of writing this, it'd be great if someone with Vagrant could check them to see if it's a problem from the Dockerfile or from the tests. The output is huge so I didn't want to paste it here, but can do it if you need it

[polarathene]

@AgustinRamiroDiaz I don't get a good impression that it'll be easy to resolve that. It may be better to drop pursuing this further.

Are you aware of eza? It's a recent fork of exa where development is moving at a much faster pace and addressing many issues and PRs that have been open / stale for exa.

They've been discussing the test suite as well with its issues, using Docker / Nix and migrating the test suite to something else that isn't as problematic for them.

[ariasuni]

Closing this since exa is unmaintained (see #1243), you should take a look at the active fork eza (an alternative strategy has been used for tests).