-
-
Notifications
You must be signed in to change notification settings - Fork 352
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
chore: ensure in CI that there is no dependency to snapshots #5395
Conversation
Changing the spoon-pom version is done by the release workflow now. If we do not bump that to a new snapshot version, we are unable to change anything in there during development, as spoon-core will use the old, released version. What is the reason for this change? Is it necessary, that spoon snapshot versions (i.e. currently being worked on) never refer to spoon in a snapshot state? I would think that only the final release version must point to fixed, stable version numbers. |
The primary reason is that this is the cause for which many jobs our integration CI (checking that client code still works on SNAPSHOT) are broken: https://ci.inria.fr/sos/. The underlying root cause is that To fix the integration CI, I see two options:
I prefer this one because 1) it minimizes coupling, 2) this check is very meaningful and useful in general, beyond this current breakage. |
The problem with your solution is that changes in spoon-pom (e.g. to update dependencies) will be built in the release commit for the first time. As an example, consider this sequence of events:
To ensure changes in spoon-pom are directly applied, we would need to leave the version of the spoon-pom in this repository the same after a release. This is not a good idea though, IMHO, because then we have multiple spoon-poms with the same version tag but different contents. Btw, https://ci.inria.fr/sos/ seems to only show builds if you are logged in — and I don't believe I have an account :) |
This would work, the CI would build what's written in the code for a given commit. And then Renovate would update spoon-parent one's newly released.
Yes, we would exclude this workflow on purpose. We would change spoon-pom, release it and then update in another commit. On this topic, I've created long time overdue #5397 |
any idea why and how github-actions[bot] reacted with thumbs up emoji? |
I am not sure I follow. How would the changes in spoon-pom be picked up by spoon-core, if spoon-core refers to an old version of the parent? I think spoon-core would either use the previous versions spoon-pom (if we only update the parent after the release) or might fail in the release commit for the first time. I am not really keen on the version of the parent and core going out of sync in a way that delays dependency errors until a release happens. Maybe I am just missing something?
Qodana found no issues (and used the github actions token to create the reaction) |
It would not, and that's exactly the point. If we want to change the parent, the workflow is:
Fully reproducible dependency. The parent always point to an addressable, immutable, reproducible dependency (think NIX :). On a related note, #5397 is the best option for making everything explicit with the same properties. |
I see, I didn't understand that you actually wanted that behaviour.
It doesn't help much currently as our SNAPSHOT versions are not bumped for every commit, so you have no idea what "spoon-core-SNAPSHOT" refers to without a git hash. And if you have the git hash, you also have the used spoon-pom version and the reactor build should resolve the parent properly, I think. For spoon-core releases we already point to fixed spoon-pom versions in the parent.
I agree. I think that's the easiest option… This PR necessitates changes in our release workflows. Currently they bump the spoon-pom parent version and release spoon-pom in the same CI afaik. What you propose would require one release workflow for spoon-pom and one for spoon-core. We can make those changes after (maybe) merging the Nix PR, we just likely do not want to make an automatic release after this PR is merged. |
fix #5340