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

(FACT-1385) (FACT-1364) BigNum and FixedNum fix ups on Windows #1311

Conversation

HAIL9000
Copy link

This pull request addresses two Facter tickets. The first is FACT-1385 which
involved updating Facter so that our tests which relied on the rb_num2long
returning a long did not fail on Windows with Ruby 2.2.

The second is FACT-1364, which involved updating the printing methods
in Facter to recognize integers larger than 64-bits and display them
properly.

Both these changes rely on the new release of Leatherman.

Because the method `rb_num2long` was changed in Ruby 2.2. to return
a long, some unit tests were failing in Ruby 2.2. on windows because
they were expecting it to return a 64-bit type. Update facter to use
`num2size_t` and `rb_num2ll` methods instead in order to fix this
issue.

This fix also relies on an update to leatherman which adds the
num2size_t and rb_num2ll methods to the Ruby API.
Prior to this commit, when printing out fact values, facter only
supported numbers it could identify as a fixednum or a double. This
meant that anything that was a bignum would not be printed. Update
the write and to_json methods so that they support bignums.

This fix also relies on an update to leatherman which adds the
ability to recognize bignums to the ruby API.
@MikaelSmith
Copy link

We messed up tagging Leatherman 0.5.0, so you'll need to use 0.5.1.

@puppetcla
Copy link

CLA signed by all contributors.

Update .travis.yaml and appveyor.yml to use the newest version
of leatherman.
@HAIL9000 HAIL9000 force-pushed the issue/stable/FACT-1364_FACT-1385_integers_on_windows branch from 048dd55 to 7441c61 Compare April 19, 2016 03:14
@branan
Copy link

branan commented Apr 19, 2016

👍

@MikaelSmith
Copy link

I'm ok to merge this now so Branan can continue some other work using new Leatherman, and add tests in a follow-on PR.

@MikaelSmith MikaelSmith merged commit dd78260 into puppetlabs:stable Apr 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants