-
-
Notifications
You must be signed in to change notification settings - Fork 77
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
Adding Windows support #17
Conversation
@op-ct thanks a lot. |
@mcanevet I'd rather have FacterDB be capable of handling UTF-16LE json files on its own. It will allow folks to drop in |
@op-ct well the problem is that I can't even see the .facts content as github detects it as Binary file |
what's the status here? I'd love to have this in so we can revert our workarounds in the theforeman-puppet tests for Windows. GH at least is viewing the file correctly with the "View" button in the diff. |
Are these issues getting resolved any time soon? Would greatly appreciate this fix. |
On Windows hosts, `facter -j` writes `.facts` files using Little-endian UTF-16 Unicode, prefixed with a JSON-unfriendly BOM. This commit updates the `database` method to correctly interpret these files. Includes spec test.
This should be run after collecting Windows facts Requires `vagrant plugin install vagrant-host-shell`
Some things were out of order, but now they are fixed.
c83a26d
to
2baf27c
Compare
host.vm.provision "shell", path: "get_facts.sh" | ||
host.vm.provision "shell", inline: "/sbin/shutdown -h now" | ||
end | ||
config.vm.define "centos-7-x86_64" do |host| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you change alignment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's some left-over silliness from separating the CentOS 7 fixes from the Windows; will be fixed in a bit.
@op-ct I still think the facts should be stored in plain text (UTF-8 or something) |
@mcanevet They should be stored in plain text after this (sanitized by |
@op-ct facts/3.1/windows-2008 R2-x86_64.facts is still in binary format |
@@ -2,11 +2,30 @@ | |||
require 'jgrep' | |||
|
|||
module FacterDB | |||
# Convenience method to handle odd files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it still needed now that the facts are sanitized?
bc2a4af
to
294c490
Compare
|
||
def self.database | ||
@database ||= "[#{Dir.glob("#{File.expand_path(File.join(File.dirname(__FILE__), '../facts'))}/*/*.facts").map { |f| File.read(f) }.join(',')}]\n" | ||
fs = File.expand_path('../facts/*/*.facts',File.dirname(__FILE__)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get this change in the library...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does a few things, but the intent was to be more semantic and less error-prone when a file contains bad data (or a different encoding).
A convenient side effect is that self.buld_database()
makes a nice place to insert code for inspecting incoming data. I've already done this to identify broken data files during development. In the future, we might consider using it to identify facter data that doesn't conform to FacterDB conventions (e.g., an fqdn
of foo.example.com
)
The Windows `fqdn` and `domain` facts now default to the facterdb convention of `foo.example.com`.
Before this patch, it wasn't clear which versions of each OS had data for any given version of facter. This patch addresses that problem by adding a **Facter version and Operating System coverage** table to the `README.md`. The markdown for the table can be maintained using the new rake task, `rake table`.
294c490
to
bda0a45
Compare
Closing this out as there hasn't been any response and I believe that all the windows related improvements have been added in other PRs since this was opened. |
This patch adds:
Vagrantfile
facts/get_facts.ps1
to gather Windows factsfacts/clean_facts.rb
, that cleans upfacts/
files (e.g., YAML-only output from early Windows Facter versions) before they are entered into the database.rake table
) to generate a markdown matrix of OS versions x Facter versions currently supported byfacterdb
(to help maintain theREADME.md
).README.md
Note:
facts/clean_facts.rb
requires the Vagrant plugin vagrant-host-shell