ldmx-sw icon indicating copy to clipboard operation
ldmx-sw copied to clipboard

just adopt a task runner

Open tomeichlersmith opened this issue 1 year ago • 7 comments

After I started looking into adopting denv within ldmx-sw I realized that I was just writing custom shell scripts to run various common commands that ldmx-sw developers run. In the software biz, this is called "task running" and there are a plethora of solutions already available. After some browsing, I can across the task runner just which I find to be beneficial for a few reasons:

  • Very simple syntax associating a task name and the command(s) that the task runs
  • All tasks located in a single file which could be reference if the command itself is not available
  • Natural and easy self-documentation of tasks via comments in the file defining them
  • (meme) Written in :fire: blazing fast :rocket: Rust (but in all seriousness, this makes it incredibly cross platform and easy to install)
  • I like the name and the mechanic of just <task> e.g. just build

On the branch associated with this issue, I've included a justfile that I haven't thoroughly tested but shows what I'm imagining could be used. The terminal output of running just without any arguments (and the default task being just --list) gives the help message.

tom@appa:~/code/ldmx/ldmx-sw$ just
just --list
Available recipes:
    build             # compile and install ldmx-sw
    check             # check that the necessary programs for running ldmx-sw are present
    configure *CONF   # configure how ldmx-sw will be built
    fire config *ARGS # run ldmx-sw with the input configuration script
    init              # initialize a containerized development environment
    mount *DIR        # mount a directory into the denv
    pull *IMAGE       # make sure the image is pulled down
    setenv *ENVVAR    # pass an environment variable into the denv
    use *IMAGE        # change which image is used for the denv

I also want to include an image of this because it is :sparkles: pretty :sparkles:

image

In the SW Dev Meeting where I first brought up just, the request was made to include the possibility of still have ldmx be the root command. I also like this and so the solution I'm imagining is to simply have ldmx be an alias for just.

tom@appa:~/code/ldmx/ldmx-sw$ alias ldmx=just
tom@appa:~/code/ldmx/ldmx-sw$ ldmx
just --list
Available recipes:
    build             # compile and install ldmx-sw
    check             # check that the necessary programs for running ldmx-sw are present
    configure *CONF   # configure how ldmx-sw will be built
    fire config *ARGS # run ldmx-sw with the input configuration script
    init              # initialize a containerized development environment
    mount *DIR        # mount a directory into the denv
    pull *IMAGE       # make sure the image is pulled down
    setenv *ENVVAR    # pass an environment variable into the denv
    use *IMAGE        # change which image is used for the denv

Related Discussions for Background

  • https://github.com/LDMX-Software/ldmx-sw/issues/1232
  • https://github.com/LDMX-Software/ldmx-sw/pull/1269

tomeichlersmith avatar Jul 18 '24 17:07 tomeichlersmith

ldmx be an alias for just.

and that's something we can do when sourcing the environment, right? I think this will make it almost transparent to most users.

Also can you add the compile and the recompFire commands to your justfile, please?

tvami avatar Jul 18 '24 17:07 tvami

can you add the compile and the recompFire commands to your justfile, please?

Just did :+1: https://github.com/LDMX-Software/ldmx-sw/commit/d084d1e2a43c354e64608a074da998142dc18687

we can do when sourcing the environment, right?

yes, I think the ldmx-env.sh would evolve into only alias ldmx=just after some time allowing for folks to transition. I could add a check for denv and just to the header of ldmx-env.sh and then if they both pass, we would alias ldmx=just and if they don't there would be a warning.

tomeichlersmith avatar Jul 18 '24 18:07 tomeichlersmith

Just did

thanks, but recompFire only does fire like this now, no?

I could add a check for

yeah I think that sounds good!

tvami avatar Jul 18 '24 18:07 tvami

It calls the fire task which calls the build task as a dependency so both build and fire will be run.

I may want to rewrite this if I find while testing that recompiling before firing every time is too onerous.

tomeichlersmith avatar Jul 18 '24 18:07 tomeichlersmith

Testing Notes

  • [x] just lists the possible recipes and a help message
    • Can we deduce argv[0] within a justfile so I can have the help message conform to the user's choice of just or ldmx?

check

  • [x] just check fails if denv is not found
  • [x] just check passes if denv is found and it finds a container runner

building

  • [x] just configure passes argments to cmake and calls it appropriately
  • [x] just build defaults to building with all cores, can set number of cores with integer e.g. just build 2

init

  • [x] just init successfully creates a denv on computer that passes just check
  • [ ] just init fails helpfully if specific parts are not available

fire

  • [x] just fire runs a config from the ldmx-sw directory
  • [x] run from somewhere else (i.e. testing [no-cd] attribute), need to use just -f path/to/ldmx-sw/justfile fire config.py so its probably easier to use denv for this part but c'est la vie

format

  • [x] formats all c++ files with clang-format within the denv
  • [x] formats justfile using its own canonical formatting

miscellanea

  • Need to fully-qualify, add an alias to /etc/containers/registries.conf.d/shortnames.conf, or add docker.io to unqualified-search-registries in /etc/containers/registries.conf for podman to find DockerHub images. Not a big issue since most folks won't use podman but would be nice to point out. (I chose the last option so the justfile can remain unchanged)
  • The minimum just version is 1.23.0 since I use the confirm(PROMPT) attribute for clean.
    • https://just.systems/man/en/chapter_34.html
    • I'm thinking about removing the confirm(PROMPT) attribute since then we'd drop to 1.9 which is available on many package managers; however, the pre-built binaries are already very cross platform and would have the user installing 1.32 or newer.

tomeichlersmith avatar Jul 24 '24 19:07 tomeichlersmith

image

New screenshot with updates. Want to look into deducing argv[0] so the help message can have ldmx or just depending on user's choice. Not sure if that is possible though.

tomeichlersmith avatar Jul 24 '24 19:07 tomeichlersmith

More thoughts while on the train, mostly in regards to environment variables we can define in the justfile to apply to all the recipes it runs.

  • [x] should we set LDMX_BASE to something helpful?
  • [x] should we set the APPTAINER_CACHEDIR environment variable to something helpful?
    • would need to be modify-able by user since there are still locations where ldmx-sw and apptainer caches differ

:stop_sign: Will Not :stop_sign:

  • implement a from-ldmx-env recipe. Thought about it, but denv pretty much requires the ldmx/dev image to be one of the newest available and other updates to ldmx-sw deprecate non-v4 images, so encouraging updating is a good way to go.
  • put in python formatting right now. This is a bigger discussion that we can add later.
  • fully test all command orderings. Further bugs can be patched as we encounter them, going to focus on making sure new developers can understand how to get started.

tomeichlersmith avatar Jul 25 '24 22:07 tomeichlersmith