Improve .build caching for faster builds. Part 2.

[Sencudra]

In #63, the first step was made toward speeding up the build process. In this PR, I’m trying to get closer to the limit. TLDR; this is achived by merging exercise source code into warmed package to limit changes in build graph.

Final results

This are the results of running bin/benchmark-in-docker.sh for old version, version from #63 and current PR:

The measurement results fluctuate a lot, within about half a second, so your results could differ.

What is changed:

1. Renamed WarmUp package to TestEnvironment to hide WarmUp name feedback user feedback (see compile-error test exected results). Think this could be misleading for students.
2. Exercise source code is merged into TestEnvironment, meaning minor changes in build graph.
3. compile-error test is adjusted for TestEnvironment package.
4. Few new imports are added in source files of using-library to create more work for swift-frontend.

Why not using swift package dump-package to get info about package?

It's more bulletproof, but also slower. Running dump-package eats up more than half a second

What if exercise contains more targets than just main and test?

For this situation the whole Source and Test folders are copied. Then, the main target folders are renamed to TestEnvironment to be consistent with Package.swift. The names and paths of other possible targets are not modified.

P.S. Please, consider merging with squashing. Some of last PRs were merged with full commit history which is frustrating for main branch.

[github-actions]

Hello 👋 Thanks for your PR.

This repo does not currently have dedicated maintainers. Our guardians team will attempt to review and merge your PR, but it will likely take longer for your PR to be reviewed.

If you enjoy contributing to Exercism and have a track-record of doing so successfully, you might like to become an Exercism maintainer for this track.

Please feel free to ask any questions, or chat to us about anything to do with this PR or the reviewing process on the Exercism forum.

(cc @exercism/guardians)

[github-actions]

Hello. Thanks for opening a PR on Exercism 🙂

We ask that all changes to Exercism are discussed on our Community Forum before being opened on GitHub. To enforce this, we automatically close all PRs that are submitted. That doesn't mean your PR is rejected but that we want the initial discussion about it to happen on our forum where a wide range of key contributors across the Exercism ecosystem can weigh in.

You can use this link to copy this into a new topic on the forum. If we decide the PR is appropriate, we'll reopen it and continue with it, so please don't delete your local branch.

If you're interested in learning more about this auto-responder, please read this blog post.

---

Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it.

[Sencudra]

Also added set -eu cause the last PR run was green, but the result contains error with jq.

[IsaacG]

Thank you for your patience with me

[Sencudra]

For whoever merges this, please don’t forget to squash the commits.

[IsaacG]

+cc @exercism/swift

[meatball133]

Any particular reason why this is moved outside of the build stage?

[Sencudra]

Yes, this is the main idea of this PR. Instead of building and copying some minor parts of the artifacts, it builds so-called "TestEnvionment" package inside the final image, leaving all artifacts there, so it’s basically a cold build. Then, when the exercise is run, there is only a minor diff to apply and build time is much faster.

[meatball133]

I mean my general sense on the pr is that the core changes (the new test environment logic) have I nothing to comment against. But as I have said before I generally don't believe editing any of the bash files (outside run.sh and build-test-runner.sh). Some tracks have chosen to implement their own logic around that and as a non-track maintainer for those track it is quite often annoying to get them working, while those which follow more or less the structure of https://github.com/[exercism/generic-test-runner](https://github.com/exercism/generic-test-runner), just works. And I don't really see the changes in those files as necessary even if they might look nicer.

As for macOS support, I generally believe that the testrunner is only suppose to be a tooling for the servers and is not meant for an end user (again some test runner on exercism is very annoying to get started if you don't know how it is setup, and they are often not that well documented). Thereby I don't really see the point of it and I also believe a problem which can occur is that a track such a Swift gets very macOS focused, which is in itself might not be a bad thing, but how things was before so was Windows and Linux treated a bit like 2nd class citizens so it is a balancing act. My point is mostly that if we decide to make the test runner macOS compatible, why not Windows compatible? I also like to mention that the ci is supposed to run in docker so it shouldn't matter what the underlying os is really (as long as it has the same compile target, e.g x86). But from a developer perspective might it be worth exploring if the project can be run through Apple's new container tool for macOS.

[Sencudra]

@meatball133

I mean my general sense on the pr is that the core changes (the new test environment logic) have I nothing to comment against. But as I have said before I generally don't believe editing any of the bash files (outside run.sh and build-test-runner.sh). Some tracks have chosen to implement their own logic around that and as a non-track maintainer for those track it is quite often annoying to get them working, while those which follow more or less the structure of [https://github.com/exercism/generic-test-runner](https://github.com/%5Bexercism/generic-test-runner%5D(https://github.com/exercism/generic-test-runner)), just works. And I don't really see the changes in those files as necessary even if they might look nicer.

I understand your logic and reasoning, but I strongly disagree with the approach. I think the chosen strategy is not the right one. Let me explain my view and suggest some changes:

1. From what I see, the template is just a starting point, a quick way to build a new test runner. But I don’t see anywhere that test runners must always follow the same structure or stay in sync with the template. And even if that was the idea, it should have been part of the design from the start — for example, by putting all test runners on one shared codebase. Trying to control this only through a disclaimer in code/docs or PR reviews doesn’t work well, because it requires constant attention and increases the cognitive load on the maintainer. On the other hand, an approach of common structure would also limit developers in what they can do with the test runner, and I don’t think it’s worth it. I suggest solving the maintenance issues in another way.

2. How can we make things easier? First, by making the runner simpler. For example, we could remove scripts that run outside of Docker. In practice, they don’t give useful results and are mostly pointless. Second, we could add more PR checks to make sure all scripts work in a selected set of environments, so no one has to fix them manually and after they appear in main branch. And finally, we could add more documentation — though to be honest, I’m not a fan of too much documentation, since it also has to be kept up to date.

3. Why are the current changes not just “for a nicer look”? The template has real problems with how tests are run. When tests run from a mounted folder, it’s fine. But the current PR requires copying tests into Docker and running them there. inside. When they are copied inside Docker (which is the main part of the suggested optimization), problems appear with test purity — the Docker image has to be reset to its initial state after each test. That’s why these changes are not just cosmetic; they are needed for the runner to work efficiently.

4. About the cleanup: most of the bash script changes in this PR came from IsaacG’s review, and I really appreciate that. His current PR to the generic-test-runner also shows that the existing scripts are far from ideal.

As for macOS support, I generally believe that the testrunner is only suppose to be a tooling for the servers and is not meant for an end user (again some test runner on exercism is very annoying to get started if you don't know how it is setup, and they are often not that well documented). Thereby I don't really see the point of it and I also believe a problem which can occur is that a track such a Swift gets very macOS focused, which is in itself might not be a bad thing, but how things was before so was Windows and Linux treated a bit like 2nd class citizens so it is a balancing act. My point is mostly that if we decide to make the test runner macOS compatible, why not Windows compatible? I also like to mention that the ci is supposed to run in docker so it shouldn't matter what the underlying os is really (as long as it has the same compile target, e.g x86). But from a developer perspective might it be worth exploring if the project can be run through Apple's new container tool for macOS.

The end user for the test runner is, of course, not an Exercism user, but the maintainer and a potential contributor. I believe that the script wrapper around Docker should work on Linux, macOS, or Windows, so the entry barrier for contributors is low. As I've mentioned before it's possible to add verification checks for this. We could also add some information about environment setup — I’m happy to do that.

At the moment, macOS is somewhat treated as a second-class citizen: Windows has WSL, Linux works fine, but on macOS it doesn’t work due to some minor quirks with sed, find, etc. Swift is still primarily an Apple-specific language, and most people who want to contribute to the code will be using macOS. Forcing them to install a WSL-like workaround seems weird, especially since the differences are minimal compared to Windows, where the native tools are way much different.

The current tests run in a Linux environment in Docker, and I don’t think anything needs to be changed. This is possibly a more lightweight approach than using a macOS host. If the goal were to save resources, running tests on a Linux host seems to be much more efficient.

Ovarall, I’m open to discussion and ready to do everything properly.

[meatball133]

For the first section: I relize that I am leaving maintainer roll of this track soon (and you will likely take over). So me puting in roadblock in how you want to manage things are a bit futail. I just want you to have this in mind to not make the files too cluterd for future contributors and maintainers.

For the 2nd section: I don't agree with the fact that macOS users are treated as 2nd class citizens. I know other maintainers on Exercism which doesnt have a special setup for macOS for their testrunner and they can test through docker desktop. Even if I use a mix of operating systems on a day to day basis, when I work with these test runners I do 90% of the time through github codespaces. Since most of the time there is no setup needed and "it just works". So I don't really agree with that reasoning. And even if I will accept these changes, I am not really thinking adding macOS ci has any real need.

Also for the changes to the dockerfile. Having it like this is fine (since they likely doesn't change the size that much), but just also want to point out that the larger the docker image is, the more it costs for Exercism (and the Swift test runner is among the larger).

[Sencudra]

how you want to manage things are a bit futail.

No, I'm not that kind of guy who wants to change things to make them more comfortable for myself :) I'm always open to discussion. So yeah, I'll keep your advice in mind.

but just also want to point out that the larger the docker image is

This is probably one of things I want to look next. Should I make a proposal on forum for that before even starting to research? Probably it's better to update the exercises and the the test runner to 6.2 before researching how to reduce size of docker image.

[meatball133]

I will say it is likely pretty tricky to make the image smaller. You are free to try though, and you don't have to write on the forum before reaserching. But if you want to have a "docker base", e.g: https://github.com/exercism/nim-docker-base. You will likely have to make a post requesting a new repo