(PDK-446) Fix pathing on Debian/Ubuntu #69
Conversation
configs/components/shellpath.rb
Outdated
| pkg.install_file('./50-pdk', '/etc/paths.d/50-pdk') | ||
| elsif platform.is_deb? | ||
| pkg.link File.join(settings[:prefix], 'bin', 'pdk'), File.join(settings[:main_bin], 'pdk') |
There was a problem hiding this comment.
We could just do this on all posix platforms and not bother with the /etc/profile.d/pdk.sh file to change PATH?
There was a problem hiding this comment.
That would make the code easier to follow. We can still go back on this if it turns out to have some unforeseen consequences.
There was a problem hiding this comment.
Did we decide this was better than just including the script in both profile.d and Xsession.d?
There was a problem hiding this comment.
See comments on puppetlabs/pdk#272 - the symlink should work on all platforms and shells
There was a problem hiding this comment.
Yeah, seems like /usr/local/bin is a reasonable approach.
There was a problem hiding this comment.
I also think we could just do this on all linux platforms at least.
pdk/bin should only contain pdk itself and not dependencies like curl.
Move pdk from /opt/puppetlabs/bin to /opt/puppetlabs/pdk/bin Fix adding pdk to PATH on debian|ubuntu
| ["#{platform[:make]} #{install_prefix} install", | ||
| "rm -f #{settings[:prefix]}/bin/openssl", | ||
| "rm -f #{settings[:prefix]}/bin/c_rehash", | ||
| ] |
There was a problem hiding this comment.
This needs careful testing because I seem to remember things breaking when I tried to get rid of these before. But I could be wrong. :)
There was a problem hiding this comment.
On EL7, Ubuntu 16.04 and Windows 2012 R2 it passes our beaker tests. I'm getting an error trying to build OSX 12.10
Is there anything outside our existing beaker tests (install the package, create a new module, and validate it) that might not work? If so, such cases are probably high priority to add to the beaker tests
There was a problem hiding this comment.
My main concern would be places where Ruby needs to go out and download something new (particularly over HTTPS) and I think with the level of vendoring we do now, the default use case doesn't go out to the network anymore. Easy fix would be to add a test which adds a new gem to a generated module Gemfile and then tries to run pdk validate or whatever.
There was a problem hiding this comment.
Built packages for latest macOS and Ubuntu and everything seems to work fine even when you add new dependencies to the Gemfile.
There was a problem hiding this comment.
Thanks @scotje ! The remaining part of the ticket is to update the beaker code; so I can add that new test at the same time. I think it would be enough of an ongoing concern to warrant adding a beaker test for.
configs/components/shellpath.rb
Outdated
| pkg.install_file('./50-pdk', '/etc/paths.d/50-pdk') | ||
| elsif platform.is_deb? | ||
| pkg.link File.join(settings[:prefix], 'bin', 'pdk'), File.join(settings[:main_bin], 'pdk') |
There was a problem hiding this comment.
Did we decide this was better than just including the script in both profile.d and Xsession.d?
|
@scotje @DavidS - PR updated to use the /usr/local/bin/pdk symlink on all Linux variants. |
|
LGTM, are the test changes a pre-req for this to get merged, or is this good to go? |
|
@DavidS It's good to go - the beaker change will be to stop beaker using a full path to pdk and instead rely on it being on the path. |
Move the PDK exe from /opt/puppetlabs/bin to /opt/puppetlabs/pdk/bin
On Debian/Ubuntu add a symlink from /opt/puppetlabs/pdk/bin/pdk to /usr/local/bin/pdk
Removes binaries left in /opt/puppetlabs/pdk/bin by the curl and openssl components