Skip to content

Conversation

victormlg
Copy link
Contributor

No description provided.

Copy link

Marking this PR as stale due to inactivity; it will be closed in 7 days.

@github-actions github-actions bot added the stale Pull requests with no recent activity label May 26, 2025
Copy link
Contributor

@larsewi larsewi 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 🚀 Would be nice if you could add an acceptance test. You can use os_version_major.cf as inspiration.

@github-actions github-actions bot removed the stale Pull requests with no recent activity label May 27, 2025
@victormlg victormlg force-pushed the os_version_minor branch 2 times, most recently from b01c5ad to 77e6a40 Compare June 10, 2025 14:40
@victormlg victormlg requested a review from larsewi June 10, 2025 15:06
@larsewi larsewi requested a review from jakub-nt June 11, 2025 07:16
Copy link
Contributor

@larsewi larsewi left a comment

Choose a reason for hiding this comment

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

I think you will have to derive os_version_minor from somewhere else on Windows. We don't want it to be "Unknown" on supported platforms.

@victormlg
Copy link
Contributor Author

@cf-bottom jenkins, please

@cf-bottom
Copy link

@larsewi
Copy link
Contributor

larsewi commented Jun 24, 2025

Build for Windows (no tests)
Build Status
Packages here

@olehermanse olehermanse requested a review from larsewi June 24, 2025 13:50
@victormlg
Copy link
Contributor Author

I tested sys.os_version_minor on a win-2019-dev-v8 ami and got the following:

image

It looks like it is working. It's difficult to make sense of a "os version minor" on this type of machine. Here is the version of the OS that I got by running winver

image

@victormlg victormlg requested a review from olehermanse June 24, 2025 14:29
@olehermanse
Copy link
Member

That's okay, I guess. From your screenshot I would kind of expect the minor version to be 1809.

Copy link
Contributor

@larsewi larsewi left a comment

Choose a reason for hiding this comment

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

Great work @victormlg 🚀 Only some smaller comments. Not sure what to do about Windows. Maybe $(sys.os_version_minor) -> Unknown is okey?

@olehermanse olehermanse requested a review from larsewi June 26, 2025 17:01
Copy link
Contributor

@larsewi larsewi 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 🚀 The fixes to the C code should be amended to the first commit though

@larsewi
Copy link
Contributor

larsewi commented Jun 27, 2025

@cf-bottom Jenkins with exotics please :)

@cf-bottom
Copy link

Copy link
Member

@olehermanse olehermanse left a comment

Choose a reason for hiding this comment

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

@victormlg victormlg force-pushed the os_version_minor branch 5 times, most recently from 8ae3de9 to 2b94fb1 Compare August 6, 2025 09:01
@larsewi
Copy link
Contributor

larsewi commented Aug 6, 2025

AIX only Build Status

@larsewi
Copy link
Contributor

larsewi commented Aug 7, 2025

^ AIX failure is unrelated

@larsewi
Copy link
Contributor

larsewi commented Aug 7, 2025

Build Status
This build includes fix from cfengine/buildscripts#1834

Copy link
Contributor

@larsewi larsewi left a comment

Choose a reason for hiding this comment

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

Great work @victormlg 🚀 Added some smaller comments that you'll need to address.

@victormlg victormlg force-pushed the os_version_minor branch 2 times, most recently from 976558c to 4610916 Compare August 13, 2025 13:17
@larsewi
Copy link
Contributor

larsewi commented Aug 13, 2025

AIX only
Build Status

@larsewi
Copy link
Contributor

larsewi commented Aug 14, 2025

@cf-bottom Jenkins with exotics please :)

@cf-bottom
Copy link

@larsewi
Copy link
Contributor

larsewi commented Aug 19, 2025

@cf-bottom Jenkins with exotics please :)

@cf-bottom
Copy link

Copy link
Contributor

@larsewi larsewi left a comment

Choose a reason for hiding this comment

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

Failed on Solaris 11

R: Defined classes: Afternoon
R: Defined classes: Min32
R: Defined classes: GMT_Tuesday
R: Defined classes: feature_tls_1_0
R: Defined classes: zone_s11_build
R: Defined classes: ipv4_192_168
R: Defined classes: loghost
R: Defined classes: feature_copyfrom_restrict_keys
R: Defined classes: Lcycle_0
R: Defined classes: GMT_Yr2025
R: Defined classes: feature_def
R: Defined classes: sunos_sun4v_5_11_11_1
R: Defined classes: cfengine_3_27
R: Defined classes: ipv4_192_168_253
R: Defined classes: feature_yaml
R: Defined classes: GMT_Day19
R: Defined classes: s11_build
R: Defined classes: 192_168_253_163
R: Defined classes: error_mode
R: Defined classes: warning_mode
R: Defined classes: GMT_Lcycle_0
R: Defined classes: sunos_5_11
R: Defined classes: Day19
R: Defined classes: Tuesday
R: Defined classes: ipv4_127_0
R: Defined classes: Hr13_Q3
R: Defined classes: sunos_sun4v
R: Defined classes: net_iface_lo0_1
R: Defined classes: cfengine_3
R: Defined classes: 4_cpus
R: Defined classes: GMT_Morning
R: Defined classes: feature_def_json_preparse
R: Defined classes: feature_host_specific_data_load
R: Defined classes: feature_host
R: Defined classes: GMT_August
R: Defined classes: GMT_Min30_35
R: Defined classes: DEBUG
R: Defined classes: GMT_Hr11_Q3
R: Defined classes: feature_host_specific_data
R: Defined classes: feature
R: Defined classes: cfengine
R: Defined classes: 64_bit
R: Defined classes: notice_mode
R: Defined classes: feature_curl
R: Defined classes: feature_tls_1_3
R: Defined classes: feature_tls_1
R: Defined classes: agent
R: Defined classes: cfengine_3_27_0a_aa1825e77
R: Defined classes: ipv4_192_168_253_163
R: Defined classes: 127_0_0_1
R: Defined classes: any
R: Defined classes: feature_copyfrom
R: Defined classes: Hr13
R: Defined classes: feature_copyfrom_restrict
R: Defined classes: net_iface_net0_1
R: Defined classes: sparc
R: Defined classes: ipv4_127_0_0_1
R: Defined classes: cfengine_3_27_0a
R: Defined classes: sun4v
R: Defined classes: ipv4_192
R: Defined classes: feature_xml
R: Defined classes: solaris
R: Defined classes: inform_mode
R: Defined classes: August
R: Defined classes: Min30_35
R: Defined classes: ipv4_127_0_0
R: Defined classes: feature_def_json
R: Defined classes: feature_tls_1_1
R: Defined classes: compiled_on_solaris2_11
R: Defined classes: feature_host_specific
R: Defined classes: GMT_Hr11
R: Defined classes: sunos_sun4v_5_11
R: Defined classes: GMT_Q3
R: Defined classes: feature_pam
R: Defined classes: mac_00_14_4f_f9_9a_c8
R: Defined classes: feature_tls
R: Defined classes: AUTO
R: Defined classes: community_edition
R: Defined classes: ipv4_127
R: Defined classes: Yr2025
R: Defined classes: Q3
R: Defined classes: feature_tls_1_2
R: Defined classes: GMT_Min32
R: /export/home/jenkins/workspace/testing-pr/label/PACKAGES_sparc64_solaris_11/cfengine-3.27.0a.aa1825e77/tests/acceptance/./01_vars/01_basic/os_version_minor.cf Found: Unknown
R: /export/home/jenkins/workspace/testing-pr/label/PACKAGES_sparc64_solaris_11/cfengine-3.27.0a.aa1825e77/tests/acceptance/./01_vars/01_basic/os_version_minor.cf FAIL
R: Version number extracted from class: $(test.os_class)
R: /export/home/jenkins/workspace/testing-pr/label/PACKAGES_sparc64_solaris_11/cfengine-3.27.0a.aa1825e77/tests/acceptance/./01_vars/01_basic/os_version_minor.cf Expected: $(test.expected)

@victormlg
Copy link
Contributor Author

victormlg commented Aug 29, 2025

Merged here: #5861

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants