who -b etc in facter's uptime (#13535)

[Alex Harvey]

Hi all,

I've spent the day reading through the Wiki on writing tests (http://projects.puppetlabs.com/projects/puppet/wiki/Development_Writing_Tests) and doing the RSpec tutorial that was linked, and otherwise examining rspec tests in ./spec/unit/util/uptime_spec.rb in facter.  I am working on a fix and have recently updated the following bug:  http://projects.puppetlabs.com/issues/13535. 

I have established that the existing RSpec test relating to who -b is flawed, and I've come to the conclusion that the who -b calls are probably unused, no matter what OS is used.  Of course, I can't get access to absolutely every OS, but I've certainly ruled out a pretty large set.  Nonetheless because I am highly risk averse I would like to discuss before proposing to completely remove the calls to who -b. :)

My conclusion is that the method uptime_who_dash_b could only work in theory on one OS, namely Linux, and that in the case of Linux, it will never be called, because Linux is catered for by the method uptime_proc_uptime.  Furthermore, I have established that the RSpec test of the who -b functionality is flawed, based on the contents of the fixture file who_b_boottime, and works only because only the case of testing one hour into the future was considered.  The test would have failed if any time between Jan 1 and Aug 1 had been considered, based on the contents

The fixture file appears to contain output from probably OSX or OpenBSD.

$ cat ./spec/fixtures/unit/util/uptime/who_b_boottime
reboot   ~        Aug  1 14:13

In the case of OSX, and other BSD derivatives, the code will use the call to sysctl -n kern.boottime and never reach the problematic who -b call.  That said, I don't have access to OpenBSD to check but comments in the RSpec tests tell me that OpenBSD goes to sysctl -n kern.boottime.

In the case of Linux, this who -b code does work, but the code is redundant, because Linux uses the uptime_proc_uptime method instead.

In the case of all other OSes I know of, the who -b code will fail, because Linux seems to be the only OS that provides the year in who -b output.

Linux

$ who -b
         system boot  2012-08-27 14:41

OSX/NetBSD/PCBSD

$ who -b
reboot   ~        Jul 28 16:34

AIX

$ who -b
   .        system boot Jun 27 12:05

Solaris

$ who -b
   .       system boot  May 31 17:42

HP-UX

$ who -b
   .       system boot  Aug 13 08:05

$ who -b
   .        system boot Aug  7 13:40

Now, next problem is that in the case of Solaris prior to Solaris 8, uptime_kstat also won't work because kstat wasn't introduced until Solaris 8.  Nonetheless, the fix for AIX proposed by Malcolm Howe will work on Solaris 10 as well.  Thus, it seems simpler and easier if we let Solaris use the same solution as AIX & HP-UX. 

So I am proposing the following changes as a fix to the bug I am working on -

- remove the who -b code altogether, including the associated RSpec test.
- add a case statement to make it explicit as to which OS uses which method
- add a method uptime_uptime per Malcolm Howe's suggestion that will work for all commercial Unix.
- remove the uptime_kstat altogether, including the associated RSpect tests as it will no longer be needed.
- restructure the RSpec tests to follow the new code structure*

* I am aware of TDD, although it seems the existing tests really follow the implementation?

i.e.

Thus, instead of

  def self.get_uptime_seconds_unix
    uptime_proc_uptime or uptime_sysctl or uptime_kstat or uptime_who_dash_b
  end

I would replace this with

  def self.get_uptime_seconds_unix
    case Facter.value(:kernel)
    when "Linux"
      uptime_proc_uptime
    when "FreeBSD", "OpenBSD", "NetBSD", "GNU/kFreeBSD", "DragonFly", "Darwin"
      uptime_sysctl
    when "SunOS", "AIX", "HP-UX"
      uptime_uptime
    else
      nil
    end
  end

I have access to all of these platforms to test on except for OpenBSD and DragonFly.  '

I'd like some feedback to make sure I'm not being too aggressive in my plan for changes here.

Of course, another solution that involves fewer changes is just do exactly what Malcolm Howe suggested.

Thanks in advance,
Alex Harvey

[Andy Parker]

On Mon, Sep 17, 2012 at 7:39 PM, Alex Harvey <alexh...@gmail.com> wrote:
> Hi all,
>
> I've spent the day reading through the Wiki on writing tests
> (http://projects.puppetlabs.com/projects/puppet/wiki/Development_Writing_Tests)
> and doing the RSpec tutorial that was linked, and otherwise examining rspec
> tests in ./spec/unit/util/uptime_spec.rb in facter. I am working on a fix
> and have recently updated the following bug:
> http://projects.puppetlabs.com/issues/13535.
>
> I have established that the existing RSpec test relating to who -b is
> flawed, and I've come to the conclusion that the who -b calls are probably
> unused, no matter what OS is used. Of course, I can't get access to
> absolutely every OS, but I've certainly ruled out a pretty large set.
> Nonetheless because I am highly risk averse I would like to discuss before
> proposing to completely remove the calls to who -b. :)
>

The OpenBSD man page doesn't mention it
(http://www.openbsd.org/cgi-bin/man.cgi?query=who&apropos=0&sektion=0&manpath=OpenBSD+Current&arch=i386&format=html).

> My conclusion is that the method uptime_who_dash_b could only work in theory
> on one OS, namely Linux, and that in the case of Linux, it will never be
> called, because Linux is catered for by the method uptime_proc_uptime.
> Furthermore, I have established that the RSpec test of the who -b
> functionality is flawed, based on the contents of the fixture file
> who_b_boottime, and works only because only the case of testing one hour
> into the future was considered. The test would have failed if any time
> between Jan 1 and Aug 1 had been considered, based on the contents
>
> The fixture file appears to contain output from probably OSX or OpenBSD.
>

Probably OSX since that is what most of the Puppet Labs employees use.

What is the advantage of this over just checking what commands are
available? My fear with something like this is that as the list of
kernels changes this will get out of sync. It also seems to be that
the kernel isn't really the determining facter about how to get
uptime, rather the available command is.

> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Developers" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/puppet-dev/-/Zj0P4fHpEUMJ.
> To post to this group, send email to puppe...@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-dev+...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-dev?hl=en.

[Alex Harvey]

On Wednesday, September 19, 2012 5:06:56 AM UTC+10, Andy Parker wrote:

Probably OSX since that is what most of the Puppet Labs employees use.

Sounds good.  I'm forced to use Windows XP and I don't have local administrator access. :-)
 

On Mon, Sep 17, 2012 at 7:39 PM, Alex Harvey wrote:

> So I am proposing the following changes as a fix to the bug I am working on

> - remove the who -b code altogether, including the associated RSpec test.
> - add a case statement to make it explicit as to which OS uses which method

What is the advantage of this over just checking what commands are
available? My fear with something like this is that as the list of
kernels changes this will get out of sync. It also seems to be that
the kernel isn't really the determining facter about how to get
uptime, rather the available command is.

 

Yes, I take your point.  I guess there are advantages and disadvantages in both approaches.  Checking only for the availability of a command can lead to similar problems when the same commands get implemented differently on different OSes.  Obviously, who -b is one example of this.  Or consider the Solaris 'kstat' command.  At the moment, it only exists in Solaris, as far as I can tell.  But Oracle is porting a whole lot of Solaris features into Oracle Linux.  And the open source community often ports features from OpenSolaris into various Linux distributions.  If hypothetically kstat gets ported, it probably won't behave in quite the same way as it does in Solaris. 

All said though I am happy to omit the case statement and follow the structure of the existing code.  I spent yesterday afternoon fine tuning Malcolm Howe's suggestion so as to cope with the differences in output of uptime on Solaris, AIX & HP-UX.  It means I'll need to test about 64 or more different cases of 'uptime' output ('up 1 day', 'up 1 day, 1 min', 'up 1 day, 1 hr', 'up 1 day, 1:01', 'up 1 day(s)', lots of fun.  I think it will actually work on Linux & BSD as well but perhaps it's best not to change the /proc/uptime & sysctl methods).

Anyhow how do you feel about the rest of my plan?  Would the following be okay -

Instead of

def self.get_uptime_seconds_unix
     uptime_proc_uptime or uptime_sysctl or uptime_kstat or uptime_who_dash_b
end

we could have

def self.get_uptime_seconds_unix
     uptime_proc_uptime or uptime_sysctl or uptime_uptime
end

TIA,
Alex

[Alex Harvey]

I have made the following changes and tested to confirm it fixes the bug and tested on AIX 5.3 & 6.1, HP-UX 11.23, and Solaris 2.6 & 7.  I have also created a fairly exhaustive set of RSpec tests I think. 

I am still wondering if people will object to the fact I've removed the kstat method for Solaris - I think the uptime method is better and generally more reliable (I've seen kstat broken before a number of times in Solaris).  That said, it's easy to leave the kstat in if people prefer.

Changes to RSpec tests / new tests -

# diff -u ./spec/unit/util/uptime_spec.rb.orig ./spec/unit/util/uptime_spec.rb
--- ./spec/unit/util/uptime_spec.rb.orig        Wed Sep 19 22:26:16 2012
+++ ./spec/unit/util/uptime_spec.rb     Thu Sep 20 17:18:31 2012
@@ -41,36 +41,50 @@

       describe "nor is 'sysctl kern.boottime'" do
         before :each do
-          Facter::Util::Uptime.stubs(:uptime_sysctl_cmd).returns("cat \"#{@nonexistent_file}\"")
+          Facter::Util::Uptime.stubs(:uptime_proc_uptime).returns(false)
+          Facter::Util::Uptime.stubs(:uptime_sysctl).returns(false)
+          Facter.fact(:kernel).stubs(:value).returns('SunOS')
         end

-        it "should use 'kstat -p unix:::boot_time'" do
-          kstat_output_file = my_fixture('kstat_boot_time') # unix:0:system_misc:boot_time    1236919980
-          Facter::Util::Uptime.stubs(:uptime_kstat_cmd).returns("cat \"#{kstat_output_file}\"")
-          Time.stubs(:now).returns Time.at(1236923580) #one hour later
-          Facter::Util::Uptime.get_uptime_seconds_unix.should == 60 * 60
-        end
+        describe "should use '/usr/bin/uptime'" do
+          # Note about uptime variations.
+          # Solaris (5.6, 5.7, 5.8, 5.9, 5.10 & 5.11) and HP-UX (11.00, 11.11, 11.23, 11.31) have time right justified at
+          #   at 8 characters, and two spaces before 'up'.
+          # Solaris differs from all other Unices (and Linux) in that the plural/singular case of minutes/hours/days are
+          #   written min(s)/hr(s)/day(s) instead of min/mins/hr/hrs etc., e.g. 1 min(s), 2 min(s) as opposed to
+          #   1 min, 2 mins, etc.
+          # AIX (4.3.3, 5.2, 5.3, 6.1) differs from other SysV Unices in that times are padded with a leading 0 in the
+          #   hour column where necessary, and have AM/PM in uppercase, and there are three spaces before 'up'.
+          # Tru64 (4.0, 5.1) differs from other SysV Unices in that times are in 24 hour format, and there are no
+          #   leading spaces.
+          # Linux (RHEL 5) differs from the Unices in that seconds are given in the time, time is right justified at
+          #   9 characters, one space before up.
+          test_cases = [
+            ['  4:42pm  up 1 min(s),  0 users,  load average: 0.95, 0.25, 0.09',                                         1*60],
+            [' 10:14pm  up 3 hr(s),  0 users,  load average: 0.00, 0.00, 0.00',                               3*60*60        ],
+            ['  9:01pm  up  1:47,  0 users,  load average: 0.00, 0.00, 0.00',                                 1*60*60 + 47*60],
+            ['  1:56pm  up 25 day(s),  2 users,  load average: 0.59, 0.56, 0.50',              25*24*60*60                   ],
+            ['  2:23pm  up 25 day(s), 27 min(s),  2 users,  load average: 0.49, 0.45, 0.46',   25*24*60*60            + 27*60],
+            ['  1:07pm  up 174 day(s), 16 hr(s),  0 users,  load average: 0.05, 0.04, 0.03',  174*24*60*60 + 16*60*60        ],
+            ['  8:59pm  up 94 day(s),  3:17,  46 users,  load average: 0.66, 0.67, 0.70',      94*24*60*60 +  3*60*60 + 17*60],
+            ['  02:42PM   up 1 day, 39 mins,  0 users,  load average: 1.49, 1.74, 1.80',        1*24*60*60            + 39*60],
+            ['  02:34PM   up 621 days, 18 hrs,  0 users,  load average: 2.67, 2.52, 2.56',    621*24*60*60 + 18*60*60        ],
+            ['  02:42PM   up 41 days,   2:38,  0 users,  load average: 0.38, 0.70, 0.55',      41*24*60*60 +  2*60*60 + 38*60],
+            ['  1:29pm  up 485 days,  0 users,  load average: 0.00, 0.01, 0.01',              485*24*60*60                   ],
+            ['  3:30am  up 108 days, 1 hr,  31 users,  load average: 0.39, 0.40, 0.41',       108*24*60*60 +  1*60*60        ],
+            ['13:16  up 58 mins,  2 users,  load average: 0.00, 0.02, 0.05',                                            58*60],
+            ['13:18  up 1 hr,  1 user,  load average: 0.58, 0.23, 0.14',                                      1*60*60        ],
+            ['13:19  up  1:01,  1 user,  load average: 0.10, 0.26, 0.21',                                     1*60*60 +  1*60],
+            ['15:56  up 152 days, 17 hrs,  0 users,  load average: 0.01, 0.06, 0.07',         152*24*60*60 + 17*60*60        ],
+            [' 13:36:05 up 118 days,  1:15,  1 user,  load average: 0.00, 0.00, 0.00',        118*24*60*60 +  1*60*60 + 15*60]
+          ]

-        describe "nor is 'kstat -p unix:::boot_time'" do
-          before :each do
-            Facter::Util::Uptime.stubs(:uptime_kstat_cmd).returns("cat \"#{@nonexistent_file}\"")
-          end
-
-          it "should use 'who -b'" do
-            who_b_output_file = my_fixture('who_b_boottime') # Aug 1 14:13
-            Facter::Util::Uptime.stubs(:uptime_who_cmd).returns("cat \"#{who_b_output_file}\"")
-            Time.stubs(:now).returns Time.parse("Aug 01 15:13") # one hour later
-            Facter::Util::Uptime.get_uptime_seconds_unix.should == 60 * 60
-          end
-
-          describe "nor is 'who -b'" do
-            before :each do
-              Facter::Util::Uptime.stubs(:uptime_who_cmd).returns("cat \"#{@nonexistent_file}\"")
+          test_cases.each do |uptime_output, expected|
+            it "should return #{expected} for #{uptime_output}" do
+              Facter::Util::Resolution.stubs(:exec).with('/usr/bin/uptime 2>/dev/null').returns(uptime_output)
+              Facter.fact(:uptime_seconds).value.should == expected
             end
-
-            it "should return nil" do
-              Facter::Util::Uptime.get_uptime_seconds_unix.should == nil
-            end
           end
         end
       end

# diff -u ./lib/facter/util/uptime.rb.orig ./lib/facter/util/uptime.rb
--- ./lib/facter/util/uptime.rb.orig    Wed Sep 19 22:26:11 2012
+++ ./lib/facter/util/uptime.rb Thu Sep 20 18:28:01 2012
@@ -4,7 +4,7 @@
 #
 module Facter::Util::Uptime
   def self.get_uptime_seconds_unix
-    uptime_proc_uptime or uptime_sysctl or uptime_kstat or uptime_who_dash_b
+    uptime_proc_uptime or uptime_sysctl or uptime_uptime
   end

   def self.get_uptime_seconds_win
@@ -31,18 +31,33 @@
     end
   end

-  def self.uptime_kstat
-    if output = Facter::Util::Resolution.exec("#{uptime_kstat_cmd} 2>/dev/null")
-      compute_uptime(Time.at(output.chomp.split(/\s/).last.to_i))
+  def self.uptime_uptime
+    if output = Facter::Util::Resolution.exec("#{uptime_uptime_cmd} 2>/dev/null")
+      up=0
+      if output =~ /(\d+) day(?:s|\(s\))?,\s+(\d+):(\d+)/
+        # Regexp handles Solaris, AIX, HP-UX, and Tru64.
+        # 'day(?:s|\(s\))?' says maybe 'day', 'days',
+        #   or 'day(s)', and don't set $2.
+        up=86400*$1.to_i + 3600*$2.to_i + 60*$3.to_i
+      elsif output =~ /(\d+) day(?:s|\(s\))?,\s+(\d+) hr(?:s|\(s\))?,/
+        up=86400*$1.to_i + 3600*$2.to_i
+      elsif output =~ /(\d+) day(?:s|\(s\))?,\s+(\d+) min(?:s|\(s\))?,/
+        up=86400*$1.to_i + 60*$2.to_i
+      elsif output =~ /(\d+) day(?:s|\(s\))?,/
+        up=86400*$1.to_i
+      elsif output =~ /up\s+(\d+):(\d+),/
+        # must anchor to 'up' to avoid matching time of day
+        # at beginning of line.
+        up=3600*$1.to_i + 60*$2.to_i
+      elsif output =~ /(\d+) hr(?:s|\(s\))?,/
+        up=3600*$1.to_i
+      elsif output =~ /(\d+) min(?:s|\(s\))?,/
+        up=60*$1.to_i
+      end
+      up
     end
   end

-  def self.uptime_who_dash_b
-    if output = Facter::Util::Resolution.exec("#{uptime_who_cmd} 2>/dev/null")
-      compute_uptime(Time.parse(output))
-    end
-  end
-
   def self.compute_uptime(time)
     (Time.now - time).to_i
   end
@@ -55,11 +70,7 @@
     'sysctl -n kern.boottime'
   end

-  def self.uptime_kstat_cmd
-    'kstat -p unix:::boot_time'
+  def self.uptime_uptime_cmd
+    "/usr/bin/uptime"
   end
-
-  def self.uptime_who_cmd
-    'who -b'
-  end
 end

If there are no objections I guess I should submit the change.

[Andy Parker]

Go ahead and submit this as a pull request against facter, we'll
continue the code review using that. Be sure to pay attention to the
CONTRIBUTING.md for how to submit the pull request and write the
commit messages.

Thanks for all of the work you've put into this! We can't support all
of these platforms without people like you taking the time to submit
patches against your platforms.

> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Developers" group.
> To view this discussion on the web visit

> https://groups.google.com/d/msg/puppet-dev/-/Y8OHNXCw6HoJ.

[Alex Harvey]

On Friday, September 21, 2012 2:08:57 AM UTC+10, Andy Parker wrote:

Go ahead and submit this as a pull request against facter, we'll
continue the code review using that. Be sure to pay attention to the
CONTRIBUTING.md for how to submit the pull request and write the
commit messages.

Thanks for all of the work you've put into this! We can't support all
of these platforms without people like you taking the time to submit
patches against your platforms.

No problem, it is an exciting project from my perspective. 

As far as submitting the patch I have got myself stuck in a corner using GitHub.  I did read CONTRIBUTING.md but apparently too quickly because I missed the bit about the need to create a topic branch when I submitted my patch for #14674.  So I would really like to just blow everything away and clone a new copy of facter, and create my topic branch properly this time.  Trouble is, I am waiting on the code review for #14674 and GitHub doesn't easily let you create multiple forked copies of the repo. 

So, I followed instructions by Adrian Short on how to create multiple forks of a repository.  But after following these instructions I seem to only have the master branch in my second repo!  I wonder can I simply delete my forked repository https://github.com/alexharv074/facter?  I don't really understand git or GitHub that well so I'm not sure if the "pull request" resulted in a copy of my changes being made somewhere.

If not, what other options do I have here?  Sorry, I should have read the instructions more carefully the first time.

Here is full documentation of what I did while following Adrian Short's page.  Maybe I'm just a few steps away from being able to submit my patch to someone who understands git. :)

Created a second empty repo called 'facter2.git' using the "New Repository" button at GitHub.

Cloned the puppet labs repository:

-bash-3.2$ git clone https://github.com/puppetlabs/facter.git facter.secondfork
Cloning into facter.secondfork...
remote: Counting objects: 9691, done.
remote: Compressing objects: 100% (3797/3797), done.
remote: Total 9691 (delta 5960), reused 9265 (delta 5622)
Receiving objects: 100% (9691/9691), 1.99 MiB | 172 KiB/s, done.
Resolving deltas: 100% (5960/5960), done.

-bash-3.2$ cd facter.secondfork/

-bash-3.2$ git remote -v
origin  https://github.com/puppetlabs/facter.git (fetch)
origin  https://github.com/puppetlabs/facter.git (push)

-bash-3.2$ git remote rename origin upstream

-bash-3.2$ git remote add origin https://github.com/alexharv074/facter2.git

-bash-3.2$ git remote -v
origin  https://github.com/alexharv074/facter2.git (fetch)
origin  https://github.com/alexharv074/facter2.git (push)
upstream        https://github.com/puppetlabs/facter.git (fetch)
upstream        https://github.com/puppetlabs/facter.git (push)

-bash-3.2$ git remote set-url origin https://alexh...@github.com/alexharv074/facter2.git

-bash-3.2$ git remote -v
origin  https://alexh...@github.com/alexharv074/facter2.git (fetch)
origin  https://alexh...@github.com/alexharv074/facter2.git (push)
upstream        https://github.com/puppetlabs/facter.git (fetch)
upstream        https://github.com/puppetlabs/facter.git (push)

-bash-3.2$ git push -u origin master
Password:
Counting objects: 8662, done.
Delta compression using up to 32 threads.
Compressing objects: 100% (3262/3262), done.
Writing objects: 100% (8662/8662), 1.54 MiB | 337 KiB/s, done.
Total 8662 (delta 5161), reused 8647 (delta 5151)
To https://alexh...@github.com/alexharv074/facter2.git
 * [new branch]      master -> master
Branch master set up to track remote branch master from origin.

[Andy Parker]

On Fri, Sep 21, 2012 at 12:59 AM, Alex Harvey <alexh...@gmail.com> wrote:
> As far as submitting the patch I have got myself stuck in a corner using
> GitHub. I did read CONTRIBUTING.md but apparently too quickly because I
> missed the bit about the need to create a topic branch when I submitted my
> patch for #14674. So I would really like to just blow everything away and
> clone a new copy of facter, and create my topic branch properly this time.
> Trouble is, I am waiting on the code review for #14674 and GitHub doesn't
> easily let you create multiple forked copies of the repo.
>

Being on the branch is nice, but not critical. It really matters if
you are going to be continue to work on facter, though, otherwise your
new commits start to show up in the pull request that you submitted.

There should be no need to creating a new fork to fix this problem.
From your original fork you should be able to do (assuming you have
upstream that points to the puppetlabs facter repo)

git fetch upstream
git checkout -b
ticket/1.6.x/ensure-processor-calls-only-done-on-correct-platform
upstream/1.6.x
git cherry-pick 1.6.x^^^...1.6.x
git checkout 1.6.x
git reset --hard upstream/1.6.x

That should pull all of your commits onto a new branch. Be certain
that it has worked before you do that last command because that will
throw away the commits on the 1.6.x branch (they are actually still
around, but hard to get at).

I also noticed that you reference ticket #14764
(https://projects.puppetlabs.com/issues/14764) in your commits. Are
you sure that is the right bug?

> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Developers" group.
> To view this discussion on the web visit

> https://groups.google.com/d/msg/puppet-dev/-/dkGHVck9Sa8J.

[Alex Harvey]

Argh, I did all this with some minor variations and thought I'd got my repo into a sane state.  Then I've played around and I'm afraid I think it's still completely wrong.

-bash-3.2$ git branch -av
  1.6.x                                                     47fd300 Merge pull request #324 from haus/maint/1.6.x/fixup_install_rb
* 13535_facter_uptime_can_be_wrong_on_AIX                   871ecb5 [ahead 3, behind 21] refactor, change case of ProcessorCount for sunos Facter.add block for consistency.
  14674_ensure_processor_call_only_done_on_correct_platform 871ecb5 refactor, change case of ProcessorCount for sunos Facter.add block for consistency.
  master                                                    e865cbe Merge remote-tracking branch 'origin/2.x'
  remotes/origin/1.6.x                                      871ecb5 refactor, change case of ProcessorCount for sunos Facter.add block for consistency.
  remotes/origin/1.6rc                                      5a3e537 Updating FACTERVERSION and CHANGELOG for 1.6.12
  remotes/origin/2.x                                        a47c37b Merge pull request #307 from Evan-Pierce/ticket/2.x/15585_ext-facts-parse-incorrectly-equal-sign
  remotes/origin/HEAD                                       -> origin/master
  remotes/origin/gh-pages                                   693fad0 Automatic update of YARD documentation by Jenkins
  remotes/origin/integration/fact_definition                e594f27 (#14623) Add warning for facts without definitions
  remotes/origin/master                                     e865cbe Merge remote-tracking branch 'origin/2.x'
  remotes/upstream/1.6.x                                    47fd300 Merge pull request #324 from haus/maint/1.6.x/fixup_install_rb
  remotes/upstream/1.6rc                                    5a3e537 Updating FACTERVERSION and CHANGELOG for 1.6.12
  remotes/upstream/2.x                                      a47c37b Merge pull request #307 from Evan-Pierce/ticket/2.x/15585_ext-facts-parse-incorrectly-equal-sign
  remotes/upstream/gh-pages                                 693fad0 Automatic update of YARD documentation by Jenkins
  remotes/upstream/integration/fact_definition              e594f27 (#14623) Add warning for facts without definitions
  remotes/upstream/master                                   e865cbe Merge remote-tracking branch 'origin/2.x'

I guess I need to learn git...
 

I also noticed that you reference ticket #14764
(https://projects.puppetlabs.com/issues/14764) in your commits. Are
you sure that is the right bug?

Okay, I've made a typo there - it should be #14674, not #14764...

Is blowing everything away and starting again, on both bugs, an option at this point? :)

[Alex Harvey]

On Saturday, September 22, 2012 3:42:23 PM UTC+10, Alex Harvey wrote:

Is blowing everything away and starting again, on both bugs, an option at this point? :)

It's okay, disregard this.  I must have got smarter over the weekend because when I looked at it this morning git started to make sense. :)  I think I've fixed my repo and will submit the patch shortly.