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

Configurable tools #140

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Configurable tools #140

wants to merge 10 commits into from

Conversation

damphyr
Copy link

@damphyr damphyr commented Oct 20, 2016

Pulls out the tool configuration in a YAML file under config.

Also makes all constant values (CACHE_DIR, ZIP_EXE etc.) overridable through environment variables (which you can use on the rake command line)

Updates all software to the latest versions.

@damphyr
Copy link
Author

damphyr commented Oct 20, 2016

Not ready yet, PR opened to be able to clarify a few things

@damphyr
Copy link
Author

damphyr commented Oct 20, 2016

Q1: What are you doing with that 'includes' parameter?

Vassilis Rizopoulos added 2 commits October 20, 2016 13:38
… YAML configuration. All chef, vagrant and atom extras are conditional on the presence of the tool in the configuration
@damphyr
Copy link
Author

damphyr commented Oct 20, 2016

That Appveyor build is going to block forever because it's asking for the git name and email

@damphyr
Copy link
Author

damphyr commented Oct 20, 2016

Torben I need your input here, there's a few things that do not work due to environment scripts:

  • Asking for the name and email for git in set-env blocks all builds.
  • Atom plugin installation fails.

On the other hand I've restructured most of the Rakefile so that we can use it as a basis for all devpacks and handle exceptions (like the double unpack for Chef).

Basically I broke the kitchen and took home the oven to bake my own cookies.

Copy link
Owner

@tknerr tknerr left a comment

Choose a reason for hiding this comment

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

LGTM, but no idea why the appveyor build is failing yet. Suspect this is a side effect of the tool updates. Will have a look....

Thanks for the improvements!

def target_dir
ddir=ENV["TARGET_DIR"]
ddir||=File.join(base_dir,"target")
File.expand_path(ddir)
Copy link
Owner

Choose a reason for hiding this comment

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

how about File.expand_path(ENV["TARGET_DIR"] || File.join(base_dir, "target") instead?

for my own taste a bit more readable as a one-liner, but not being picky here

#chefdk install package contains a zip file
extra_pkg="#{target_dir}/chefdk/opscode/chefdk.zip"
tgt_dir="#{target_dir}/chefdk/opscode/"
unpack(extra_pkg, tgt_dir, includes = [])
Copy link
Owner

Choose a reason for hiding this comment

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

need the extra_pkg and tgt_dir vars here? I'd rather inline them here...

end

desc 'downloads required resources and builds the devpack binary'
task :build => :clean do
tools_config=YAML.load(File.read("#{base_dir}/config/tools.yaml"))
Copy link
Owner

Choose a reason for hiding this comment

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

yes, I like that! :-)

move_chefdk
fix_chefdk
download_tools(tools_config)
if tools_config.keys.include?("chefdk")
Copy link
Owner

Choose a reason for hiding this comment

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

wonder if we could make this a bit more generic and pluggable. not necessarily for bills-kitchen here, but if we want to use that as a minimal template for future devpacks

Copy link
Owner

Choose a reason for hiding this comment

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

All the ifs in the :build task I mean...

File.write(file, File.read(file).gsub('@"C:\opscode\chefdk\embedded\bin\ruby.exe" "%~dpn0" %*', '@"%~dp0\..\..\..\..\..\bin\ruby.exe" "%~dpn0" %*'))
end
end

def install_knife_plugins
def install_knife_plugins tool_config
Copy link
Owner

Choose a reason for hiding this comment

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

nice one

fail "knife plugin installation failed" unless system(command)
end
end

def install_vagrant_plugins
def install_vagrant_plugins tool_config
Copy link
Owner

Choose a reason for hiding this comment

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

nice one

fail "vagrant plugin installation failed" unless system(command)
end
end

def install_atom_plugins
def install_atom_plugins tool_config
Copy link
Owner

Choose a reason for hiding this comment

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

nice one, too.

both the build_dir and cache_dir were nested under target_dir originally. restoring that for now...
(earlier this was cdk, hope it won't hit the max path length issue with chef_dk...)
@tknerr
Copy link
Owner

tknerr commented Oct 25, 2016

@damphyr while I believe we could get this to work / pass the appveyor build again, I'm not sure what to do with it.

Instead of doing something half-assed, I'd rather focus on making https://github.com/Zuehlke/z-devpack reusable / modular enough (see Zuehlke/z-devpack#1) so it can be the new basis for bills-kitchen.

What do you think?

@damphyr
Copy link
Author

damphyr commented Oct 26, 2016

What I think is : https://github.com/Zuehlke/z-devpack :)

@damphyr
Copy link
Author

damphyr commented May 14, 2019

Do we scrap this or should I bring it up to date?

@tknerr
Copy link
Owner

tknerr commented May 14, 2019

I should probably add some large banner stating that this is unmaintained (ever since I switched to macOS about 1.5 years ago...)

I'm currently maintaining https://github.com/tknerr/linus-kitchen/ instead, which is a Linux Developer VM variant with the same intent / goal as Bill's Kitchen.

So yes, I'm in favor of ditching this, unless someone wants to continue to maintain it

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.

2 participants