Closed nya3jp closed 4 years ago
Good idea.
One suggestion: At build time you use two paths off of the root (/build and /app). I did the same in my C++ Dockerfile (/source and /build) but now I am regretting it. It makes it inconvenient to test the build script without docker. Instead we could use two subdirectories, e.g. /app/build and /app/run. If the build script does things like "cp some_file ../run" it will work outside of docker as well. Also, files that only need to exist in the run directory, like run.sh or a pre-built binary, can be placed there in the repository and not need to be copied by the build script at all.
For example:
FROM icfpcontest2020/${platform}-build AS build # This line varies by platforms
WORKDIR /app
COPY . .
RUN mkdir -p /app/run
WORKDIR /app/build
RUN ./build.sh # This script should build the solution and install it to /app/run
FROM icfpcontest2020/${platform}-run # This line varies by platforms
WORKDIR /app/run
COPY --from=build /app/run .
ENTRYPOINT ["./run.sh"]
And with a single image:
FROM icfpcontest2020/${platform} # This is the only line that varies by platforms
WORKDIR /app
COPY . .
RUN mkdir -p /app/run
WORKDIR /app/build
RUN ./build.sh # This script should build the solution
WORKDIR /app/run
ENTRYPOINT ["./run.sh"]
Brilliant idea! Can I ask any one of you to rewrite a single platform of your choice to @jeremysawicki's second option (single image)? We'll verify that everything works fine: that it's comfortable for you as the developers and doesn't break our submission system. Then I'll try to align all the other platforms.
So is there no longer a concern about keeping run images small, as you previously expressed (https://github.com/icfpcontest2020/dockerfiles/issues/8#issuecomment-651982458)? Or you've decided to sacrifice size for simplicity?
Sure, I can work on updating C++.
Simplicity is a strong argument, yes. I think we can handle a limited amount of large base images in our submission system.
I really like @jeremysawicki's proposal. I agree that it's great to do everything under the checkout directory to simplify local verification process. And given that @beevee is fine with the second approach to reuse build images as run images, resulting Dockerfile
would be a lot simpler.
I have very minor revision proposal: what do you think about placing build.sh
at the top-level directory? This is because build.sh
is an important entry point and always provided statically in the source repository (participants can't generate it dynamically). We can keep run.sh
in a subdirectory because it might be generated by build.sh
; separating source files and generated files is generally a good practice.
So Dockerfile
would be as simple as:
FROM icfpcontest2020/${platform} # This is the only line that varies by platforms
WORKDIR /app
COPY . .
RUN ./build.sh
WORKDIR /app/run
ENTRYPOINT ["./run.sh"]
And finally, it's great if @jeremysawicki could work on applying this idea to the cpp
platform. Currently it is the only platform having separated build/run images, so it would be the most interesting one to try converting to the new approach.
On second thought, I feel like moving run.sh
to the top-level directory as well. It's also an important entry point so having it in the top-level directory would make it more prominent. In the case that a participant wants to dynamically generate a run script, they can just exec a generated script from run.sh
.
In the end, Dockerfile
is much simpler:
FROM icfpcontest2020/${platform} # This is the only line that varies by platforms
WORKDIR /app
COPY . .
RUN ./build.sh
ENTRYPOINT ["./run.sh"]
Yes, I was about to agree with you about build.sh
and to suggest the same for run.sh
. The main reason for a run
directory was to identify which files should be copied to the run image, which is not needed if we use a single image. In the event that some platform needs separate build and run images, we can just copy everything over. So I think your Dockerfile
is good.
Here is an example of how build.sh
and run.sh
might look:
#!/bin/sh
cd app
make
#!/bin/sh
cd app
exec ./app "$@"
If someone wants to put output files in a separate directory, they can. If someone wants to put their source code at the top level and avoid the need to cd app
, they can. We don't need to mandate anything other than the entry points.
We do have to choose how to structure the starter kits, though. Should we put the code at the top level or in a subdirectory? If a subdirectory, should it be called app
or src
or something else? I guess the current starter kits use app
so it is simplest to stick with that.
I strongly prefer leaving this application in a subdirectory (/app). A team can store lots of auxiliary stuff in this repo, and I’d like to keep it separate from the application.
I’m also not sure about make. Less people are familiar with make than with simple bash scripts. Make can complicate things for beginners.
Regarding the app
subdirectory, I'm not quite sure what you are proposing:
app
subdirectory in the starter kits, but teams can do whatever they want?app
subdirectory by only copying app
, build.sh
, and run.sh
into the container?build.sh
and run.sh
in the app
subdirectory, and only copy app
to the container?It is starting to sound like a good idea to isolate everything in the app
subdirectory. It is a one line change to the Dockerfile
:
FROM icfpcontest2020/${platform} # This is the only line that varies by platforms
WORKDIR /app
COPY app .
RUN ./build.sh
ENTRYPOINT ["./run.sh"]
Also it makes cd app
unnecessary in build.sh
and run.sh
.
Regarding make
, that was just an example of something teams might do. The starter kits can do the simplest thing possible, like calling g++
directly in starterkit-cpp
.
Regarding the app subdirectory, I'm not quite sure what you are proposing
I imagine that team's repository may contain multiple applications. One of them being the submission, other some helpful auxilliary apps for development. These applications may have some common libraries. Resulting directory structure can look like this:
/lib ← required to build the submission, but also required to build helper apps
/app ← submission
/helper_app_1 ← not required for submission to build or run
/helper_app_2
...
That's why I think we should copy everything and let teams do whatever they want. But keep /app
in starterkits as a default place to put the submission application to make them consistent and predictable.
But copying everything will probably make submissions quite large, since teams can and will store various massive binary files in their repos... Now I'm confused.
OK, let's go with a single image and copy everything.
I made some changes to dockerfiles and starterkit-cpp
app
-> solution
- it was strange to have /app/app
directory, now we have /solution/app
build
directory and added this directory to .gitignore
Please @nya3jp and @jeremysawicki review this changes, I'm not a cpp developer at all )
@jeremysawicki: Thanks for updating the cpp platform!
@spaceorc: Renaming the root directory to /solution
sounds fine to me. Typically build.sh
and run.sh
operate on relative paths, so the root directory path doesn't matter anyway. I'm not sure about the build
directory, I believe @jeremysawicki knows best.
I will make PRs for some platforms I'm familiar with.
@spaceorc: I don't have a strong opinion about either of these changes.
* renamed root directory from `app` -> `solution` - it was strange to have `/app/app` directory, now we have `/solution/app`
Yes, I thought /app/app was bit strange as well, so this is good.
* moved build output to `build` directory and added this directory to `.gitignore`
Personally I would not have made this change as it adds a small incremental complexity, but I don't think it matters much either way. I think most teams will use their own build system that they are comfortable with.
Done all the remaining platforms.
As initially proposed in #11, some platforms now support customizing the build/run process with participant-supplied shell scripts (e.g. #21). But other platforms don't.
To avoid confusing participants, I propose to align the build/run protocol among all platforms. In other words, all
Dockerfile
in this repository should be identical except for the base image reference (e.g.icfpcontest2020/python
).I imagine that such
Dockerfile
would be like this:Also, if we're fine with reusing build-time images as run-time images,
Dockerfile
can be simplied to something like the following. Actually this looks better to me, unless it has big implication to the evaluation infrastructure, because the difference of build-time dependencies and run-time dependencies is often tricky and hard to debug.