com-lihaoyi / mill

Your shiny new Java/Scala build tool!
https://mill-build.com/
MIT License
2k stars 308 forks source link

For Discussion: Devcontainer #3108

Open Quafadas opened 3 months ago

Quafadas commented 3 months ago

This adds a dev container file which stands up a one click cleanroom development environment in GitHub code spaces.

Stuff that seems to work:

To my surprise, testing seems flaky in this environment ...

X mill.api.PathRefTests.json.ref 20ms 
  utest.AssertionError: assert(json == expected)
  json: String = "ref:v0:89fa1bc2:/tmp/13626813947610103712/foo.txt"
  expected: String = "ref:v0:4c7ef487:/tmp/13626813947610103712/foo.txt"

I think this is one of the quite fundamental tests - I'm surprised it fails here, but not in CI! I think this PR can have value none-the-less, although I'm not sure if I would be able to get mill to the stage where it passes all tests here without (some quite significant) help.

Many of the test suites (run inside the container) do pass, however, so it's at least close-to-right

Quafadas commented 2 months ago

Hmmmm.... that is... a great point. I'll do some research in the coming days.

Quafadas commented 2 months ago

@lefou I evolved this is a little after some extra research and taking a little step back to think "what I would want" too.

In respect of your comment about CI - I added an additional workflow https://github.com/Quafadas/mill/blob/codespace/.github/workflows/container.yml

Which rebuilds the container on push to main. It checks only that mill __.compile is successful in the container and that that command exits with a 0.

i.e. assumes that a merge to main compiles. This is quite a low baseline, but I think is about right for what we want, as all the "real" testing is done elsewhere... we only need a basic check for the container itself.

According to the docs, that workflow should also cache that container, so that a cold start doesn't need to build the environment from scratch. https://github.com/devcontainers/ci/blob/main/docs/github-action.md

In the dev container itself, I also added coursier and millw... https://github.com/Quafadas/mill/blob/codespace/.devcontainer/features/mill/install.sh

So that one can type in cs, and mill to the command line and they work straight up from a cold start. Then, installed metals dependancies via coursier, so that metals has it's dependancies when the container is instantiated.

Finally, when the container is created, this runs. onCreateCommand": "mill -j 0 __.prepareOffline && mill -j 0 __.compile && mill mill.bsp.BSP/install && mill --import ivy:com.lihaoyi::mill-contrib-bloop: mill.contrib.bloop.Bloop/install

I hope that again, this reduces "cold start" time for a contributor by getting mills dependancies. It puts the first compile in place and gets the BSP support ready. I'm not sure if it's easily possible to do more than this - I hope it helps as those steps were quite time consuming first time out.

Currently, CI in this repo is red (I think one of the test suites are flaky and times out)?

On my fork, the CI is also red :-(, I believe because the job doesn't have permission to add this container to mills artefact repository. https://github.com/Quafadas/mill/actions/runs/9157845979/job/25175008941

Which ... seems reasonable. I don't really know how to make this green though without merging to main, as I don't think my fork should have those permissions? I'd welcome advice here...

In that job, the container building does appears successful, and that configuration appears to have worked successfully in one of my many toy projects...

https://github.com/Quafadas/live-server-scala-cli-js/blob/main/.github/workflows/container.yml https://github.com/Quafadas/live-server-scala-cli-js/actions/runs/9155999012

One final consideration: as the container is cached, I fear it may contribute to the resource usage of the com.lihaoyi ecosystem. I would not wish to inadvertently create a problem here.

I have written a bible. I am sorry... if there are parts of this you don't like, I'm happy to adjust further.