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

Update ensure_packages->stdlib::ensure_packages; require stdlib 9 #249

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

bastelfreak
Copy link
Member

The ensure_packages() function is deprecated with stdlib 9. It triggers a catalog compilation warning on Puppet 7 and a catgalog compilation error on Pupppet 8. This PR switches to the successor, stdlib::ensure_packages(). This function got introduced in stdlib 9.

The `ensure_packages()` function is deprecated with stdlib 9. It
triggers a catalog compilation warning on Puppet 7 and a catgalog
compilation error on Pupppet 8. This PR switches to the successor,
`stdlib::ensure_packages()`. This function got introduced in stdlib 9.
@evgeni
Copy link
Member

evgeni commented Mar 26, 2024

This should then update the minimum version of stdlib to 9?
But also, I thought the non namespaced versions are supposed to still work?!

@bastelfreak bastelfreak mentioned this pull request Mar 26, 2024
@bastelfreak
Copy link
Member Author

bastelfreak commented Mar 26, 2024

They do work, but you will see this on Puppet 7:

2024-03-26T13:00:16.305+01:00 WARN  [qtp1279219621-121] [puppetserver] Puppet This function is deprecated, please use stdlib::ensure_packages instead. at ["/etc/puppetlabs/code/environments/production/modules/dns/manifests/install.pp", 5]:["/etc/puppetlabs/code/environments/production/modules/dns/manifests/init.pp", 185]

And on Puppet 8 it's an error (and if you switch strict mode to on on puppet 7, it will fail there as well). I suggest to merge #250 for a new minor release, then merge #249 and do a major release.

@evgeni
Copy link
Member

evgeni commented Mar 26, 2024

Why does CI then pass on Puppet 8 without that change?

@bastelfreak
Copy link
Member Author

bastelfreak commented Mar 26, 2024

Apparently this is a whole clusterfuck that broke so much stuff that there's now a patch within the deprecation method:
https://github.com/puppetlabs/puppetlabs-stdlib/blob/main/lib/puppet/functions/deprecation.rb#L27-L35

And the old ensure_packages() hacks around the fail() call by passing false as last argument to deprecation():

https://github.com/puppetlabs/puppetlabs-stdlib/blob/main/lib/puppet/functions/ensure_packages.rb#L10

This got implemented in stdlib 9.something. But 9.0.0 + Puppet 8 will actually fail.

edit:

Got implemented as workaround in Stdlib 9.2.0: puppetlabs/puppetlabs-stdlib@f7dd14a

@ekohl
Copy link
Member

ekohl commented Mar 26, 2024

If only there had been a release of Puppet 8 where the methods were deprecated and replacements available. That would have made the whole migration so much less painful.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

In other places we've treated this as a major version bump, given the pain of stdlib 9. At this point I'm not too worried about it anymore given our other modules have made the jump too.

@bastelfreak
Copy link
Member Author

bastelfreak commented Mar 26, 2024

If only the modules would be more flexible and would have done a new stdlib 8 release with the new namespaced functions after everybody noticed that the 9.0 release was shit. That's what annoys me even more. This issue slipped through and we all noticed it only after the release. But refusing the backport the changes is just a shitmove.

Here is a picture of a dog to keep us happy :
PXL_20240217_114423269

@evgeni
Copy link
Member

evgeni commented Mar 26, 2024

So we just forbid 9.x < 9.2.0 and are done?

@bastelfreak
Copy link
Member Author

bastelfreak commented Mar 26, 2024

So we just forbid 9.x < 9.2.0 and are done?

We had that idea but if I remember it correctly metadata.json doesn't support that properly. Also the whole hack in stdlib is a workaround and ideally we move away from the old functions.

Edit: The issue isn't that the old functions are deprecated, the issue is that the successors got introduced in the same major release as the old ones were deprecated. They should have been introduced earlier.

bastelfreak added a commit to bastelfreak/puppet-puppetserver_foreman that referenced this pull request Apr 2, 2024
@ekohl ekohl merged commit 2bde91c into theforeman:master Jul 15, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants