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

Bump toolchain/luet to 0.32.0 #1268

Merged
merged 6 commits into from
Apr 29, 2022
Merged

Conversation

cOS-cibot
Copy link
Contributor

Signed-off-by: cOS-cibot [bot] cOScibot@gmail.com

Answer: Because Oct 31 = Dec 25

Signed-off-by: cOS-cibot [bot] <cOScibot@gmail.com>
Signed-off-by: cOS-cibot [bot] <cOScibot@gmail.com>
Signed-off-by: cOS-cibot [bot] <cOScibot@gmail.com>
Signed-off-by: cOS-cibot [bot] <cOScibot@gmail.com>
@mudler
Copy link
Contributor

mudler commented Apr 28, 2022

@davidcassany seems now during luet bumps it tries to upgrade from the local repo but fails as we have an hard dep on the toolchain versions in the repo:

Old version             | New version           | License
toolchain/luet-0.31.2-1 | toolchain/luet-0.32.0 |        

 INFO   Copying file luet-toolchain-0.32.0.package.tar.zst from /home/runner/work/cOS-toolkit/cOS-toolkit/build
 INFO   SUCCESS 📦  Package  toolchain/luet-0.32.0 downloaded
 INFO   Checking for file conflicts..
 INFO   Done checking for file conflicts..
 INFO   📦  Package  toolchain/luet-0.32.0 installed
  ERROR    Error: Failed solving solution for package: could not satisfy the constraints: 
          luet-makeiso-toolchain-0.4.0-2 and 
          luet-toolchain-0.32.0 and 
          !(luet-makeiso-toolchain-0.4.0-2) or luet-toolchain-0.31.2-1 and 
          !(luet-toolchain-0.32.0) or !(luet-toolchain-0.32.0) or !(luet-toolchain-0.31.2-1)
panic: fatal error

On one side it is very handy that it will build everything with the new version, so maybe we can be less restrict on the version being pinpointed here?

@davidcassany
Copy link
Contributor

@mudler I am not sure I fully get it, but to me it feels there might be something wrong in our build process, if we build a new luet all packages requiring a new luet should be rebuilt too. Looks like we have little issues in toolchain package definition since those dependencies while they seam to be fine they are fake. luet-makeiso is on build.yaml does not force the build against the required luet version and luet-makeiso does not have a real runtime dependency to luet. Having a closer look to these dependencies I have the feeling they are no accurate, even wrong. It did not pop up and it is hard to turn it into an issue because the current setup forces everything to be more or less aligned so they can't deviate from each other, but the requirement is no accurate.

I'd consider it a small bug in our package definitions.

As an example lets consider the luet-makeiso case:

  • toolchain runtime requires:
    • luet >= whatever defined in go.mod for luet-makeiso
    • luet-makeiso
  • luet-makeiso build requires:
    • luet == whatever defined in go.mod (I don't know how to handle this without hacking, any idea?)

The runtime toolchain constraint is based under the assumption that we keep luet backward compatible. If we assume that luet is not backward compatible on major release we could write it as:

  • toolchain runtime requires:
    • luet < next major release

@mudler
Copy link
Contributor

mudler commented Apr 29, 2022

yep, the problem at hand is that https://github.com/rancher-sandbox/cOS-toolkit/blob/master/packages/toolchain/collection.yaml#L34 uses an anchor to luet, so it hardly binds luet-makeiso <-> luet version, similarly for the other toolchain pieces, I think we had an issue around it because it bubbled up also in another occasion: #944

Indeed the dependency versions can be optimized, at the end luet-makeiso does not have any strict requirements in runtime as there are no fancy features relied upon. We can safely assume that there is backward compatibility in a <1.0.0 range.

For the build time, I wouldn't pin any version as the go.mod will pin by itself internally, so for us it's just required to be specific on the luet-makeiso version used

@mudler
Copy link
Contributor

mudler commented Apr 29, 2022

what is true is also that we don't have to pull anymore luet-makeiso during make deps as well :)

@davidcassany
Copy link
Contributor

Indeed the dependency versions can be optimized, at the end I luet-makeiso does not have any strict requirements in runtime as there are no fancy features relied upon. We can safely assume that there is backward compatibility in a <1.0.0 range.

Yes, just keeping this single constraint to limit the backward compatibility to some reasonable range sounds like a great option and forget about buildtime dependencies of go modules.

mudler added 2 commits April 29, 2022 10:54
And make it bound to luet <1.0.0

Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>
Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>
@mudler mudler linked an issue Apr 29, 2022 that may be closed by this pull request
@mudler
Copy link
Contributor

mudler commented Apr 29, 2022

@davidcassany I've pushed two commits here on this branch:

  1. removes luet-makeiso from Makefiles
  2. puts a pin to luet "<1.0.0" on luet-makeiso

If looks good, I'd go with auto-merge afterwards

Copy link
Contributor

@davidcassany davidcassany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good here

@mudler mudler enabled auto-merge (squash) April 29, 2022 09:00
@mudler
Copy link
Contributor

mudler commented Apr 29, 2022

btw this version brings in subpackages(https://luet.io/docs/concepts/packages/specfile/#subpackages) that was a very much wanted feature since a while 🙂 , we can now refactor few packages out like cloud-config, grub, etc.

@mudler mudler merged commit 69fc833 into rancher:master Apr 29, 2022
@mudler mudler deleted the bump_luet_toolchain branch April 29, 2022 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Required version to luet is pinned
3 participants