-
Notifications
You must be signed in to change notification settings - Fork 66
Conversation
source git: "https://github.com/chef/delivery-cli.git" | ||
|
||
dependency "openssl" | ||
dependency "rust" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does this end up in the build order? I'd like rust to be as early as possible so we cache it more often.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danielsdeleo I'm not sure that caching Rust matters all that much as we are just extracting pre-compiled binaries.
wouldn't it be better to have rust as part of our builders instead of in here? For example, we don't install gcc (except on windows where we want to ship it) as part of the omnibus phase |
@jdmundrawala That approach makes it very hard for project maintainers to rev the version of a build dependency like rust. It is also desirable to have ALL of a projects required dependencies (traditional or build) declared in a single location. If we introduce first class build deps into the Omnibus DSL we may declare things like GCC, @scotthain has toyed around with this very thing on our more esoteric Unix platforms. |
# limitations under the License. | ||
# | ||
|
||
name "rust" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That definition could arguably be made part of omnibus-software ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wk8 Definitely. I plan on moving it once we work all the kinks out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
One minor comment, otherwise LGTM |
In light of us not tagging releases of delivery-cli, 👍 |
👍 |
👍 LGTM! |
I'll merge this once master is green again and I can get a test build all the way through. |
b7da4f2
to
fc4c338
Compare
These should eventually be moved to omnibus-software.
fc4c338
to
c585baa
Compare
c585baa
to
326cbd1
Compare
326cbd1
to
637f0c1
Compare
This looks good from a CI perspective: http://manhattan.ci.chef.co/job/chefdk-build/339/ |
Ship the Delivery CLI in ChefDK
Revert "Merge pull request #394 from chef/schisamo/chefdk-delivery-cli"
After some discussions with the Delivery team this feels like the right place to package the
delivery
binary. From an end-user perspective it's delightful to not have to install yet another package to get the delivery-cli on your workstation. On the build node side most jobs that the delivery-cli executes rely on the other tools that ship in ChefDK.A couple of things to note:
Assuming everyone is happy with this approach I'll also update
chef verify
to be aware of thedelivery
binary./cc @chef/ociv @chef/delivery @chef/client-core @fnichol @tyler-ball @cwebberOps @adamhjk