Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Are 4.2.3 Dockerfiles still autogenerated? #835

Closed
infotroph opened this issue Jul 22, 2024 · 7 comments · Fixed by #839
Closed

Are 4.2.3 Dockerfiles still autogenerated? #835

infotroph opened this issue Jul 22, 2024 · 7 comments · Fixed by #839
Labels
bug Something isn't working CI pre-built images Related to pre-built images

Comments

@infotroph
Copy link

Container image name

rocker/tidyverse:4.2.3

Container image digest

none

What operating system related to this question?

macOS

System information

  • Docker Desktop 4.32.0 (157355)
  • MacOS 14.5
  • Macbook Air M2

Question

The Dockerfiles for 4.2.3 did not get the fixes that were applied to other Dockerfiles after #782, so they're now missing the following lines that are present for other releases:

ENV LANG=en_US.UTF-8
COPY scripts/bin/ /rocker_scripts/bin/
[...]
COPY scripts/install_s6init.sh /rocker_scripts/install_s6init.sh
COPY scripts/default_user.sh /rocker_scripts/default_user.sh
COPY scripts/init_set_env.sh /rocker_scripts/init_set_env.sh
COPY scripts/init_userconf.sh /rocker_scripts/init_userconf.sh
COPY scripts/pam-helper.sh /rocker_scripts/pam-helper.sh

I noticed this because the missing COPY scripts/bin/ ... leads to a build failure in all the tidyverse-containing images, because install_tidyverse.sh needs the Rocker version of install2.r for its --skipmissing option.

Looks like the difference emerged because generate-dockerfiles.R only updates the latest 2 releases plus devel, so 4.2.3[*] is skipped.

Is the intended behavior here that all releases older than the last two will revert to being only manually edited? If yes, I can PR a hand-edit of the affected 4.2.3 dockerfiles and a patch to dockerfiles/README.md that explains only some files are autogenerated -- can you advise on the right phrasing? Would it also be good to annotate the relevant files as something like "no longer updated automatically"?

[*]Since the release of R 4.4.1, 4.3.3 is skipped as well, but none of the 4.3.3 dockerfiles (yet) change if I force regenerate them.

@eitsupi
Copy link
Member

eitsupi commented Aug 3, 2024

Sorry for the late reply. Thanks for pointing this out!

You are completely correct in pointing out that this is a bug in the script. This version of the Dockerfiles are removed from the repository as intended.
I was aware of this but did not have the bandwidth to fix it. I hope to fix it eventually, but I'm busy for a while.

@eitsupi eitsupi added bug Something isn't working CI pre-built images Related to pre-built images and removed question labels Aug 3, 2024
@infotroph
Copy link
Author

This version of the Dockerfiles are removed from the repository as intended.

Sorry, are you saying that all the 4.2.3 Dockerfiles should have been removed and the bug is that they weren't? Or that 4.2.3 is still supposed to be updated, the bug is that it is not, and the removal of the Dockerfiles older than 4.2.3 was intended? I see that https://github.com/rocker-org/rocker-versioned2/wiki still lists 4.2.3 as updated, but maybe that table is autogenerated too and affected by the same bug?

In either case, would it be helpful for me to re-add the missing lines to the 4.2.3 Dockerfiles, even if you'll be removing/overwriting them when you find time to fix the script? Happy to do that if I'm not causing trouble.

@eitsupi
Copy link
Member

eitsupi commented Aug 5, 2024

Sorry, are you saying that all the 4.2.3 Dockerfiles should have been removed and the bug is that they weren't?

Your understanding is correct. These Dockerfiles should be deleted but are not currently.
This is a bug in the script that deletes the Dockerfile.

In either case, would it be helpful for me to re-add the missing lines to the 4.2.3 Dockerfiles, even if you'll be removing/overwriting them when you find time to fix the script?

Why do you want this?

@infotroph
Copy link
Author

Thank you very much for your work to keep so many builds running for so long. I really appreciate it and realize that all versions eventually have to pass away (in this case into static artifact form). It's clear the recent repo reorganization was necessary and well-thought-through, even though (as you've probably gathered) I wish it had kept a longer lookback window for new builds.

Your understanding is correct

Just to be very sure: When you say "These Dockerfiles" should be deleted, do you mean all the 4.2.3 files, all 4.2.3 and 4.3.3 files, or something else?

In any case I still don't understand the reason they're slated for deletion. The documentation I can find, and the discussions in #614 and the issues/PRs that mention it, all seem to say your rule is to keep building the final patch release of each minor version >= R 4.0. The removal of 4.0 and 4.1.3 can be explained as a decision to stop updating images built on Ubuntu 20 (right?) But 4.2.3 and later are on Ubuntu 22 -- is there another piece of the update-vs-freeze decision tree that I don't know about?

Why do you want this?

  • To squeeze out another build that will patch 455 known-to-Docker vulnerabilities in an image I use daily.
  • To resolve an inconsistency between files that cost me a bunch of time while trying to test local builds of a fix for rocker/rstudio:devel is a quite old R-devel daily snapshot #822 -- I was trying to find a solution that worked for every Dockerfile with install_rstudio.sh in it, and spent a long time being puzzled by failures from 4.2.3.
  • I was hoping this approach would do the above without demanding too much of your time like the more involved fixes would. I do realize the longer this thread goes the less true that will be 😓

@eitsupi
Copy link
Member

eitsupi commented Aug 8, 2024

Just to be very sure: When you say "These Dockerfiles" should be deleted, do you mean all the 4.2.3 files, all 4.2.3 and 4.3.3 files, or something else?

Here I am referring to the R 4.2 files, since R 4.2 is already unsupported, so all Dockerfiles are deleted as intended.
See #614 (comment)

It is currently outside the scope of this project to build all of each minor version of R. It would be far more efficient to use rig to install any version of the R binary.
https://github.com/r-lib/rig

@infotroph
Copy link
Author

Thank you for the fix! I think I almost understand the system now. One last clarification: Does this mean that going forward, the only versions that get build updates (and therefore the only ones that get Dockerfiles) will be "all patches of the current minor version, the last-released patch of the previous minor version, and--for only some images--also R devel"?

I'm sorry to be obtuse here -- I can tell the discussion in #614 was clear to y'all, but not to me.

@eitsupi
Copy link
Member

eitsupi commented Aug 9, 2024

Does this mean that going forward, the only versions that get build updates (and therefore the only ones that get Dockerfiles) will be "all patches of the current minor version, the last-released patch of the previous minor version, and--for only some images--also R devel"?

The actual logic is here:

supported_versions <- fs::dir_ls(path = "build/args", regexp = r"((\d+\.){3}json)") |>
stringr::str_extract(r"((\d+\.){2}\d)") |>
R_system_version() |>
sort() |>
tibble::tibble(r_version = _) |>
dplyr::mutate(
major = r_version$major,
minor = r_version$minor,
patch = r_version$patch
) |>
dplyr::slice_tail(n = 2, by = c(major, minor)) |>
dplyr::filter(
minor == dplyr::last(minor) | dplyr::lead(minor) == dplyr::last(minor) & patch >= dplyr::lead(patch)
) |>
dplyr::pull(r_version) |>
as.character()

And, devel Dockerfiles are always present.

For the other versions, the last two patch versions and the last patch version of the second newest minor version should be the build target.

For example:

  • 4.X.0, 4.Y.Z
  • 4.X.1, 4.X.0, 4.Y.Z
  • 4.X.2, 4.X.1, 4.Y.Z

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CI pre-built images Related to pre-built images
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants