Skip to content

Fix resolution of the virtual fact on FreeBSD #16

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

Merged
merged 1 commit into from
Jun 26, 2025

Conversation

smortex
Copy link
Member

@smortex smortex commented Jun 16, 2025

We should not call Linux-specific code from a non-Linux system. Before #15, the Linux-specific code was unexpectedly loaded, and the Linux resolver was non-functional on FreeBSD, causing a warning to be output. With #15, the Linux specific code is not loaded anymore, breaking further the fact resolution.

In order to fix this, the virtual_detector code needs to either be split into platform-specific pieces, or take care to only call platform-specific code on these platforms. Given this is a utility class, I chose the second route, and since the Linux-specific code is now only loaded on Linux, rely on the namespace existing or not to determine if we should run that code.

Originally submitted as:
puppetlabs/facter#2744

bastelfreak
bastelfreak previously approved these changes Jun 16, 2025
@smortex
Copy link
Member Author

smortex commented Jun 17, 2025

I fixed the mistakes that caused the jRuby failures. Interesting it did not failed with other implementations of Ruby 🤷

@smortex smortex added the bug Something isn't working label Jun 26, 2025
jessereynolds
jessereynolds previously approved these changes Jun 26, 2025
Copy link
Contributor

@jessereynolds jessereynolds left a comment

Choose a reason for hiding this comment

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

I like this mechanism to detect if an implementation for the given os type exists.

@smortex smortex dismissed jessereynolds’s stale review June 26, 2025 11:42

The merge-base changed after approval.

jessereynolds
jessereynolds previously approved these changes Jun 26, 2025
@smortex smortex dismissed jessereynolds’s stale review June 26, 2025 11:48

The merge-base changed after approval.

bastelfreak
bastelfreak previously approved these changes Jun 26, 2025
@smortex smortex dismissed bastelfreak’s stale review June 26, 2025 12:29

The merge-base changed after approval.

bastelfreak
bastelfreak previously approved these changes Jun 26, 2025
@smortex smortex dismissed bastelfreak’s stale review June 26, 2025 12:37

The merge-base changed after approval.

bastelfreak
bastelfreak previously approved these changes Jun 26, 2025
@smortex smortex dismissed bastelfreak’s stale review June 26, 2025 16:35

The merge-base changed after approval.

bastelfreak
bastelfreak previously approved these changes Jun 26, 2025
bastelfreak
bastelfreak previously approved these changes Jun 26, 2025
@bastelfreak bastelfreak self-requested a review June 26, 2025 16:46
We should not call Linux-specific code from a non-Linux system.  This
cross-platform utility class needs to skip Linux code when running on a
non-linux platform.
@smortex smortex merged commit 3fd1732 into main Jun 26, 2025
21 checks passed
@smortex smortex deleted the fix-freebsd-virtual-fact branch June 26, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants