Jira (PUP-11663) max/min core functions are incorrect for Semver types

[Sean Millichamp (Jira)]

Sean Millichamp created an issue

Puppet / PUP-11663 max/min core functions are incorrect for Semver types Issue Type: Improvement Affects Versions: PUP 7.20.0 Assignee: Henrik Lindberg Components: Functions Created: 2022/10/25 11:39 AM Priority: Normal Reporter: Sean Millichamp The Puppet max/min functions do not handle Semver types correctly. The "Any" data type function signature matches, they are converted to strings, compare as strings, and are often incorrect. Based on the spec tests for max/min it looks to be explicitly called out behavior, which I assume it means that it can't change until a new major Puppet version: https://github.com/puppetlabs/puppet/blob/main/spec/unit/functions/max_spec.rb#L77 It would be great if min/max would support all the applicable data types in their native/correct fashion. At least Semver, Timestamp, and Timespan should all be added and the deprecated Any behavior removed. Add Comment

This message was sent by Atlassian Jira (v8.20.11#820011-sha1:0629dd8)

[Morgan Rhodes (Jira)]

Morgan Rhodes assigned an issue to Unassigned

Puppet / PUP-11663 max/min core functions are incorrect for Semver types

Change By: Morgan Rhodes Assignee: Henrik Lindberg

Add Comment

[Morgan Rhodes (Jira)]

Morgan Rhodes updated an issue

Puppet / PUP-11663 max/min core functions are incorrect for Semver types

Change By: Morgan Rhodes Team: Phoenix

Add Comment

[Morgan Rhodes (Jira)]

Morgan Rhodes updated an issue

Puppet / PUP-11663 max/min core functions are incorrect for Semver types

Change By: Morgan Rhodes Epic Link: PA-4664

Add Comment

[Ben Ford (Jira)]

Ben Ford commented on PUP-11663

Re: max/min core functions are incorrect for Semver types Sean Millichamp this doesn't fix the issue, but I've got a simple module that makes versioncmp() easier to use. A PR to add Semver support would be rad. https://github.com/binford2k/binford2k-version

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-11663

max/min core functions are incorrect for Semver types

Change By: Josh Cooper Sprint: Phoenix 2023-03-29

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-11663 max/min core functions are incorrect for Semver types Change By: Josh Cooper

The Puppet max/min functions do not handle Semver types correctly. The "Any" data type function signature matches, they are converted to strings, compare as strings, and are often incorrect. Based on the spec tests for max/min it looks to be explicitly called out behavior, which I assume it means that it can't change until a new major Puppet version: https://github.com/puppetlabs/puppet/blob/main/spec/unit/functions/max_spec.rb#L77 It would be great if min/max would support all the applicable data types in their native/correct fashion. At least Semver, Timestamp, and Timespan should all be added and the deprecated Any behavior removed.

Current behavior: {noformat} $ bundle exec puppet apply -e 'notice(max(Semver.new("2.0.0"), Semver.new("10.0.0")))' Warning: The max() function's auto conversion of Any to String is deprecated - change to convert input before calling, or use lambda    (file & line not available) Notice: Scope(Class[main]): 2.0.0 {noformat} Desired behavior: {noformat} $ bundle exec puppet apply -e 'notice(max(Semver.new("2.0.0"), Semver.new("10.0.0")))' Notice: Scope(Class[main]): 10.0.0 {noformat} Similarly for the {{min}} function and when comparing {{Timestamp}} and {{Timespan}}

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-11663 max/min core functions are incorrect for Semver types

Change By: Josh Cooper Story Points: 3

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-11663 max/min core functions are incorrect for Semver types Change By: Josh Cooper

The Puppet max/min functions do not handle Semver types correctly. The "Any" data type function signature matches, they are converted to strings, compare as strings, and are often incorrect. Based on the spec tests for max/min it looks to be explicitly called out behavior, which I assume it means that it can't change until a new major Puppet version: https://github.com/puppetlabs/puppet/blob/main/spec/unit/functions/max_spec.rb#L77 It would be great if min/max would support all the applicable data types in their native/correct fashion. At least Semver, Timestamp, and Timespan should all be added and the deprecated Any behavior removed. Current behavior: {noformat} $ bundle exec puppet apply -e 'notice(max(Semver.new("2.0.0"), Semver.new("10.0.0")))' Warning: The max() function's auto conversion of Any to String is deprecated - change to convert input before calling, or use lambda    (file & line not available) Notice: Scope(Class[main]): 2.0.0 {noformat} Desired behavior: {noformat} $ bundle exec puppet apply -e 'notice(max(Semver.new("2.0.0"), Semver.new("10.0.0")))' Notice: Scope(Class[main]): 10.0.0 {noformat}

Similarly for the {{min}} function and when comparing {{Timestamp}} and {{Timespan}} . Note {{Semver}}, {{Timestamp}} and {{Timespan}} are already comparable so the following returns 1 because the {noformat} $ bundle exec puppet apply -e "notice(compare(Semver.new('10.0.0'), SemVer('2.0.0')))"

Notice: Scope(Class[main]): 1

{noformat}

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-11663 max/min core functions are incorrect for Semver types Change By: Josh Cooper

The Puppet max/min functions do not handle Semver types correctly. The "Any" data type function signature matches, they are converted to strings, compare as strings, and are often incorrect. Based on the spec tests for max/min it looks to be explicitly called out behavior, which I assume it means that it can't change until a new major Puppet version: https://github.com/puppetlabs/puppet/blob/main/spec/unit/functions/max_spec.rb#L77 It would be great if min/max would support all the applicable data types in their native/correct fashion. At least Semver, Timestamp, and Timespan should all be added and the deprecated Any behavior removed. Current behavior: {noformat} $ bundle exec puppet apply -e 'notice(max(Semver.new("2.0.0"), Semver.new("10.0.0")))' Warning: The max() function's auto conversion of Any to String is deprecated - change to convert input before calling, or use lambda    (file & line not available) Notice: Scope(Class[main]): 2.0.0 {noformat} Desired behavior: {noformat} $ bundle exec puppet apply -e 'notice(max(Semver.new("2.0.0"), Semver.new("10.0.0")))' Notice: Scope(Class[main]): 10.0.0 {noformat} Similarly for the {{min}} function and when comparing {{Timestamp}} and {{Timespan}}.

Note {{Semver}}, {{Timestamp}} and {{Timespan}} are can already comparable so be compared using the {{compare}} function, for example, the following " returns - 1 because , 0 or 1 if first value is smaller, equal or larger than the second value." So this change will mean {{min}} and {{max}} behave consistently with {{compare}}:

{noformat} $ bundle exec puppet apply -e "notice(compare(Semver.new('10.0.0'), SemVer('2.0.0')))" Notice: Scope(Class[main]): 1 {noformat}

Add Comment

[Michael Hashizume (Jira)]

Michael Hashizume assigned an issue to Michael Hashizume

Puppet / PUP-11663 max/min core functions are incorrect for Semver types

Change By: Michael Hashizume Assignee: Michael Hashizume

Add Comment

[Michael Hashizume (Jira)]

Michael Hashizume updated an issue

Puppet / PUP-11663 max/min core functions are incorrect for Semver types

Change By: Michael Hashizume Release Notes: Enhancement Release Notes Summary: The max and min functions now recognize SemVer, Timespan, and Timestamp types properly.

Add Comment

[Aria Li (Jira)]

Aria Li updated an issue

Puppet / PUP-11663 max/min core functions are incorrect for Semver types

Change By: Aria Li Fix Version/s: PUP 8.0.0

Add Comment