Bug #4980

Darwin doesn't show architecture fact

Added by Nathan Rich over 1 year ago. Updated 8 months ago.

Status:Closed Start date:10/11/2010
Priority:Low Due date:
Assignee:Michael Kincaid % Done:

0%
Category:library
Target version:1.6.2
Keywords: Affected Facter version:
Branch:https://github.com/mkincaid/facter/tree/ticket/next/4980
Votes: 1

Description

lib/facter/architecture.rb:

Facter.add(:architecture) do
  confine :kernel => :openbsd
  setcode do
    architecture = Facter.value(:hardwaremodel)
  end
end

change to

Facter.add(:architecture) do
  confine :kernel => [:openbsd, :darwin]
  setcode do
    architecture = Facter.value(:hardwaremodel)
  end
end

Related issues

duplicated by Facter - Bug #6470: Ubuntu server architecture detected as "x86_64" while it ... Duplicate 02/24/2011

History

#1
Updated by Nathan Rich over 1 year ago

should have mentioned the issue: darwin doesn’t have the architecture, instead having hardwaremodel. it should have architecture, which can come from hardwaremodel like it does with openbsd

#2
Updated by James Turnbull over 1 year ago

  • Category set to library
  • Status changed from Unreviewed to Needs Decision
  • Assignee set to Nigel Kersten

#3
Updated by Jesse Wolfe over 1 year ago

Would it be alright to default to Facter.value(:hardwaremodel) on all OSes that don’t have a special case? We could simplify the whole fact to:

Facter.add(:architecture) do
    setcode do
        model = Facter.value(:hardwaremodel)
        case model
        # most linuxen use "x86_64"
        when "x86_64"
            case Facter.value(:operatingsystem)
            when "Debian", "Gentoo", "GNU/kFreeBSD"
                "amd64"
            else
                model
            end
        when /(i[3456]86|pentium)/
            case Facter.value(:operatingsystem)
            when "Gentoo"
                "x86"
            else
                "i386"
            end
        else
            model
        end
    end
end

#4
Updated by Nigel Kersten over 1 year ago

  • Status changed from Needs Decision to Accepted
  • Assignee deleted (Nigel Kersten)

That sounds entirely reasonable.

#5
Updated by James Turnbull about 1 year ago

  • Assignee set to Michael Kincaid

#6
Updated by James Turnbull about 1 year ago

  • Target version set to 1.5.9
  • Branch set to https://github.com/jamtur01/facter/tree/tickets/master/4980

#7
Updated by James Turnbull about 1 year ago

  • Status changed from Accepted to In Topic Branch Pending Review

#8
Updated by Michael Kincaid about 1 year ago

Works as expected on 10.6.6.

System-Operators-MacBook-Pro-4:facter michael$ facter architecture => i386

#9
Updated by Michael Kincaid about 1 year ago

Code from comment #3 also works on 10.6.6 (previous comment was testing the change at the top of the ticket).

#10
Updated by Michael Kincaid about 1 year ago

  • File architecture_spec.rb added

Here is a set of tests that basically describes what it’s currently doing. Couldn’t find any documentation of what values this field should take on when, so don’t know that this is the most intelligent possible choice of cases :–)

(see commit below for better tests)

#11
Updated by Michael Kincaid about 1 year ago

Changes (inc. comments to spec this out) at https://github.com/mkincaid/facter/commit/a60abd6f8cb502506939af4d521f0448bb24fd37 Tests to follow shortly.

#12
Updated by Michael Kincaid about 1 year ago

Tests at https://github.com/mkincaid/facter/commit/67e249fbb17fd3933c98255db4d28c730b2f1dd2

#13
Updated by Michael Kincaid about 1 year ago

  • File deleted (architecture_spec.rb)

#14
Updated by Daniel Pittman about 1 year ago

  • Status changed from In Topic Branch Pending Review to Tests Insufficient

There are a few, fairly minor, issues with the tests as written; the biggest headache they have is that they might fail on Debian/Ubuntu because of the special-case result not being taken into account. The comments are inline to the commit using the github, and are not reproduced here.

I also note that these are a couple of commits directly on top of the facter ‘master’ branch, which isn’t in line with our dev process. They need to be on a separate branch off of next, and to merge in when they are done. The details are here: http://projects.puppetlabs.com/projects/puppet/wiki/Development_Development_Lifecycle

Thanks very much for your great contribution to Facter; it is good to see this sort of thing getting attention.

#15
Updated by James Turnbull about 1 year ago

  • Target version changed from 1.5.9 to 1.6.0

#16
Updated by Michael Kincaid about 1 year ago

  • Status changed from Tests Insufficient to In Topic Branch Pending Review
  • Branch changed from https://github.com/jamtur01/facter/tree/tickets/master/4980 to https://github.com/mkincaid/facter/tree/ticket/next/4980

Thanks for the fixes and comments. Revised accordingly.

#17
Updated by Michael Kincaid about 1 year ago

  • Assignee changed from Michael Kincaid to James Turnbull

#18
Updated by James Turnbull about 1 year ago

  • Status changed from In Topic Branch Pending Review to Code Insufficient

Failing test:

Failures:

1) Architecture fact should be amd64 if os is Ubuntu and hardwaremodel is x86_64

 Failure/Error: Facter.fact(:architecture).value.should == result
   expected: "amd64"
        got: "x86_64" (using ==)
 # ./spec/unit/architecture_spec.rb:39

Finished in 5.52 seconds 383 examples, 1 failure

#19
Updated by Michael Kincaid about 1 year ago

  • Status changed from Code Insufficient to In Topic Branch Pending Review

Should be fixed now (commit a6c20f7).

#20
Updated by Josh Cooper about 1 year ago

Hey James,

When testing Michael’s initial patch, we don’t get the failure you reported with the Ubuntu, x86_64 case. Do you remember what platform you were running on, etc to help diagnose?

#21
Updated by James Turnbull about 1 year ago

Ubuntu 10.10 from memory.

#22
Updated by Josh Cooper about 1 year ago

  • Status changed from In Topic Branch Pending Review to Tests Insufficient

Hi Michael,

I’m getting test failures when running the complete suite of rspec tests. It appears virtual_spec should be stubbing Facter.value(:hardwaremodel)

 $ rake rspec
 ...
 1) Virtual fact on Solaris should be vmware with VMWare vendor name from prtdiag
     Failure/Error: Facter.fact(:virtual).value.should == "vmware"
     Mocha::ExpectationError:
       unexpected invocation: Facter::Util::Resolution.exec('uname -m', '/bin/sh')
       satisfied expectations:
       - allowed any number of times, not yet invoked: Facter::Util::Virtual.zlinux?(any_parameters)
       - allowed any number of times, not yet invoked: Facter::Util::Virtual.hpvm?(any_parameters)
       - allowed any number of times, not yet invoked: Facter::Util::Virtual.kvm?(any_parameters)
       - allowed any number of times, not yet invoked: Facter::Util::Virtual.xen?(any_parameters)
       - allowed any number of times, not yet invoked: Facter::Util::Virtual.vserver?(any_parameters)
       - allowed any number of times, not yet invoked: Facter::Util::Virtual.openvz?(any_parameters)
       - allowed any number of times, invoked once: Facter::Util::Virtual.zone?(any_parameters)
       - allowed any number of times, invoked 4 times: #.value(any_parameters)
       - allowed any number of times, not yet invoked: Facter::Util::Resolution.exec('prtdiag', '/bin/sh')
       - allowed any number of times, not yet invoked: Facter::Util::Resolution.exec('dmidecode')
       - allowed any number of times, not yet invoked: Facter::Util::Resolution.exec('lspci')

#23
Updated by Michael Stahnke 11 months ago

  • Target version changed from 1.6.0 to 1.6.x

#24
Updated by Michael Kincaid 8 months ago

  • Assignee changed from James Turnbull to Michael Kincaid

#25
Updated by Michael Kincaid 8 months ago

  • Status changed from Tests Insufficient to In Topic Branch Pending Review

Failing tests fixed in #66c551a320.

#26
Updated by Adrien Thebo 8 months ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release

Merged in commit:dfda9be074ebd42ea14fec98cca00247a891b7c4

#27
Updated by Michael Stahnke 8 months ago

  • Status changed from Merged - Pending Release to Closed
  • Target version changed from 1.6.x to 1.6.2

Released as part of 1.6.2.

Also available in: Atom PDF