Squeak: Always update images for builds #567
Conversation
Older Squeak images too can (and sometimes will) receive updates. Resolves hpi-swa#560.
Performance reportComparing the build times from the self-tests before/after this patch:
Overall, I don't really see a significant increase in build times. Note that some build times even were reduced, and that the trunk build times (which may serve as a rough reference point) increased by up to ~40%. Of course, the build times vary greatly on GitHub Actions Cloud. Given that the added update step actually adds value (= removes bugs, see hpi-swa/Squot#349 for a motivating example), my proposal would be to accept this possible increase in build time. Looking forward to your opinion @fniephaus! /cc @j4yk |
|
I get what problem this is trying to solve but I'd say it's an edge case. More importantly, it's changing semantics and adds another source of failures (if your project doesn't need access to source.squeak.org, it's not needed for non-trunk builds). Having said that, what do you think about adding an options for this? Something like |
|
Hmm, from my naive perspective of a CI user running a build on smalltalkCI for Squeak x.y should always have the same effect as downloading the latest image for Squeak x.y from squeak.org ... shouldn't it? So I'm not sure whether this is really only an edge case. But I also see your point and an |
|
I doubt that naive CI users care about random updates for old Squeak releases. Also, we don't even update the website on backport releases. If only we could base such a decision on some user feedback... maybe @ekrebs5, @marceltaeumel, or @j4yk have an opinion on this? |
|
@marceltaeumel's position was to make updates for release images opt-in, also considering that every update comes with an (even though low) risk of breaking things (e.g., a broken update stream or any conflicting change with smalltalkCI). Yes, most CI users will probably not even expect any updates for a release image. On the other hand, if I download a Squeak 5.3 release image today and set it up (by following the recommendations of the welcome wizard) and then later find out that the CI behaves differently for the same version of Squeak, I will be somewhat confused. In the end, smalltalkCI is only maintaining a cache of files.squeak.org that should be invisible to users - or should it? (I'm actually surprised that https://squeak.org/downloads/ is not updated for backports. @marceltaeumel Is this just a trade-off for the maintainability of the website or an explicit statement of "what is Squeak 5.3"?) So in the end, while understanding the arguments for opt-in, my personal opinion would still be to enforce image updates (or at least make them opt-out). If @j4yk or @ekrebs5 don't have to add anything to this, it's probably on you @fniephaus to make a final decision. :-) |
|
I'm still not convinced it's a good idea to update release images by default. You're most likely an expert user, core dev, or even the author of the backported fix that you want in the release image. I don't mind an update option that is opt-in and we can bump the base images on request. One good thing about bumping manually is that we can always easily roll back. |
|
Can we just make this an option, which users can enable if needed? |
|
Sounds fair. Is this urgent for anyone? If not, I will do this soon (tm). If urgent, I don't bother if you or anyone else wants to proceed on this PR instead of me. |
|
Nope, it's not urgent. |
|
@fniephaus I'm wondering what would be the best way to configure this behavior:
While a command line option would be the simplest to implement, I'm afraid that it unnecessarily increases the usage complexity for CI users who previously only had to maintain a config file and an image version. For instance, Looking forward to your opinion! :-) |
Updating needs to happen within Smalltalk anyway, so there's no need to write any Bash code to make this work. SmalltalkCISpec {
// #loading, #testing, ...
#options : {
#forceImageUpdate : true
}
}What do you think? |
My first take would have been to add a check here: Line 381 in 854de5a Which would actually make a bash argument simpler to implement ... on the other hand, a ston option might be more convenient ... I'm not sure ... Maybe a command line argument could actually be more convenient because users can set it conditionally, which is not possible with ease for ston files? In that case we would still have to decide |
You could also use different .ston config files and use them conditionally. Besides, I'm not sure users would ever want to test both with and without, so not sure this is something we need to optimize for. |
To clarify that: A common use case that I would have in mind for the requested functionality would be a CI setup like that:
Yes, I could always install the latest updates, but this would slow down the other builds in this example. I would consider something like |
We could also support both ways, but the more we spread this, the more users will ask for support for other dialects, which you probably don't plan to implement?
What you could also do today is use a simple |
Fair ... But if we just support and document
Or a Thinking aloud ... (too much text)At the moment, the "image updating" logic resists in https://github.com/hpi-swa/smalltalkCI/blob/6d6c79b14f0fc577f7bdcf27fee9b39509cf532a/squeak/prepare.st which would make a bash-side implementation simpler and help us avoid duplication. I just realize that the bash script already does some kind of ston parsing:Lines 261 to 263 in 854de5a This is most likely all but not idiomatic? Just imagine someone had a I think it is crucial that image updating should happen before anything else because class definitions, preambles, etc. might depend on new behavior. Thus, if we wanted to handle a Lines 381 to 388 in 854de5a Since loading is optional, this would require an extra invocation of the VM ... thus implementing updates inside
tl;dr: After a first look at the code, my preference would be |
That's another solution for the problem, but I'm not sure we want to encode an option (run image update) with a label (latest) in the image selection. That opens up a new dimension for configuration and we need to communicate this somehow to users. What if we add support for fully-qualified Squeak versions, something like |
Yes, I see that. Also, this could be a compatibility issue, I don't know how many places check that smalltalk_version else - just as one example, SqueakByExample's CI does.
Wow, this reminds me a lot of #506 and #471. :-) Some notes:
In summary, personally, I do not have any use case for full-qualified versions. Given your fair criticism against |
Again, this requires Bash code for a feature that can purely be written and maintained in Smalltalk, simply because you want to conveniently create build matrices with GitHub Actions. Also, update mechanisms depends on the selected image, so I don't see how it will work with an user-provided image. I don't want to needlessly block you on this, but why can't it be an option to auto-update in the config? Can't you just turn on auto-updates for all your images, even if only one really depends on backports? Updating other images shouldn't take long and ensures that other backports don't break your app. I just find the mix of trunk, updated, and stock images a bit questionable. |
Simply because we never worked out a good way to automate the hosting part: creating and updating GitHub releases is a bit tricky, and so is finding the latest 5.3 image. Right now, it's just very simple and hard coded. Suggestions and ideas are welcome. |
|
Maybe we could even find an (existing?) or smart way to allow for dynamic option values, as in instead of accepting true or false, the option could also accept an expression that is evaluated and the result is used for the option. That way, you could write an inline check to control auto updates in your ston config. |
I am hesitating to reimplement a feature in Smalltalk that already exists in bash (as described in the details section of #567 (comment)). However, if you deem it more important not to increase the bash logic further, yes, we can also do it on the image side.
Fair point, agreed. :-) Maybe "dynamic configurations" should become an own issue - similar to a |
You are referring to the prepare.st, which really only is for trunk builds. Yes, we could share some logic, but I rather see that prepare.st in the smalltalkCI Smalltalk code base at some point. 😉 Should be fairly simple as "trunk" would imply "auto-updates on".
SGTM, feel free to create another feature request in case you think this could be useful. |
Creating and updating GitHub releases: There seem to be plenty GitHub actions for this. I have used softprops/action-gh-release successfully in a couple of projects to create releases, I hope that updating them later should not be more complicated. Finding the latest image: This is what I did in create-image: |
|
Alright, will try to implement a |
Thing is this does not work for GitHub releases. You can use the GitHub API, but this would add another request, and another dependency (GitHub API). And the GitHub API can be rate-limited. |
To which step are you referring? Inside smalltalk CI's release job (cron):
On the CI user's side, nothing changes, smalltalkCI still refers to a hard-coded release asset URL. Alternatively, we could create a new GitHub release every time and delete the previous one. Might be more elegant but indeed increase complexity on the user side for finding the latest release. As further alternatives, we could store the artifacts on files.squeak.org or in a separate repo/branch (deleting the ancestry and force-pushing with each commit to avoid large storage consumption) and download it from there. So, with the first option (edit the latest existing release), the complexity (runtime/dependencies) on the CI user side does not increase. Same if we use a separate repo/branch. For files.squeak.org, we would indeed introduce a new dependency, yes. On the other hand, complexity would indeed be increased in our new release job (because it previously did not exist). I have been using the GitHub API for automating releases through a GitHub action for almost two years with many commits per day and never hit any specific outages or rate limits. For this use case, we might run the script once a day/once a week (depends on how we weigh our costs against the costs of CI users for manual updates), so I would not be afraid of rate limits. Also, if anything in the automated release breaks, we would still remain at the status quo, right? |
I don't think this is a good idea: what if there's an issue and you need a particular image, but now it's overwritten? I believe we'd need to publish binaries and include the update level. This means that you need some sort of lookup. Right now, this is part of smalltalkCI's bash scripts. Maybe we could do this somehow with a moving tag that identifies the latest and gives us a stable URL, but I haven't tried this yet. I don't think this is impossible to solve. A proper solution just needs exploration and experimentation for which I don't have the time at the moment.
Right. |
Older Squeak images too can (and sometimes will) receive updates. Resolves #560.