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-1294) Use relative path in facter.rb #1291

Merged
merged 1 commit into from
Mar 21, 2016

Conversation

MikaelSmith
Copy link

Change facter.rb to always use a relative path to find libfacter, based
on the relative path of the install location of facter.rb and
CMAKE_INSTALL_PREFIX. This fixes libfacter lookup when doing make install on a Windows machine, and removes hard-coding the MSI layout.

@MikaelSmith
Copy link
Author

This requires a change to puppet-agent builds on Windows to set the CMAKE_INSTALL_PREFIX appropriately.

Change `facter.rb` to always use a relative path to find libfacter, based
on the relative path of the install location of facter.rb and
CMAKE_INSTALL_PREFIX. This fixes libfacter lookup when doing `make
install` on a Windows machine, and removes hard-coding the MSI layout.
@@ -343,6 +338,12 @@ else()
endif()

if(RUBY_VENDORDIR)
file(RELATIVE_PATH LIBFACTER_INSTALL_RELATIVE ${RUBY_VENDORDIR} ${CMAKE_INSTALL_PREFIX})
Copy link

Choose a reason for hiding this comment

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

Do you need to quote RUBY_VENDORDIR and CMAKE_INSTALL_PREFIX here? I can never remember how exactly CMake behaves with variable expansion at function call sites

Copy link
Author

Choose a reason for hiding this comment

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

We didn't other places, so I was hoping not. I'll try a run installing to "C:/Program Files" just to be sure.

Copy link
Author

Choose a reason for hiding this comment

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

Testing "C:/Program Files" worked fine.

@MikaelSmith
Copy link
Author

Did Windows and el-7 builds, both look good. puppet facts and irb 'facter' work correctly. Well, irb 'facter' didn't work on Windows, but that's because it doesn't know where facter.rb is (it's not in Ruby's lib path).

@branan
Copy link

branan commented Mar 18, 2016

👍

@MikaelSmith
Copy link
Author

PA-252 was merged, this should be ready to go.

branan added a commit that referenced this pull request Mar 21, 2016
@branan branan merged commit 7082edb into puppetlabs:stable Mar 21, 2016
@MikaelSmith MikaelSmith deleted the FACT-1294 branch March 21, 2016 21:58
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.

None yet

2 participants