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

COOK-4534: Add option to update apt cache at compiletime #75

Closed
wants to merge 1 commit into from

Conversation

ryandub
Copy link
Contributor

@ryandub ryandub commented May 8, 2014

https://tickets.opscode.com/browse/COOK-4534

This pull-request adds a compile_time_update attribute to the Apt cookbook to force an apt-get update at compile time in order to ensure cookbooks that have an option to run at compile time can install packages. I used compile_time_update because compiletime is already being used for a different functionality. However, in the future, it may be useful to refactor these into attributes that are more obvious.

History

Changes in the build-essential cookbook (discussed here) have removed build-essential's behavior of running apt-get update before attempting to install packages when invoked at compile time. Per the linked discussion in the build-essential cookbook, @sethvargo recommended updating the Apt cookbook to provide an option to run at compile time instead of build-essential being responsible for it.

@@ -23,6 +23,7 @@
default['apt']['cacher_port'] = 3142
default['apt']['caching_server'] = false
default['apt']['compiletime'] = false
default['apt']['compile_time_update'] = false
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we need another attribute for this. I would just use the existing compiletime attribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My only reason for making a new attribute is that I am unsure if current users of the compiletime attribute want it to also update the cache. The functionality of the apt cookbook's compiletime attribute is pretty specific and not necessarily part of what people want/need for running updates at compile time. As I discussed in my PR, I think this probably needs to be addressed by a larger refactor. This implementation adds the desired functionality without breaking anything for current users.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see both sides of the argument. @btm @someara what do y'all think?

Copy link
Contributor

Choose a reason for hiding this comment

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

It does seem in retrospect that it was a mistake to not have compiletime be more specific, e.g. cacher_compile_time + update_compile_time.

There have been lots of times I've done testing in EC2 on older instances and have had failures on bootstrap because the apt cache was out of date and pointed to old versions of a package. I would have wanted a compile time update, but the cacher business would have been unnecessary (or potentially a problem?).

I'm not in love with more attributes, but I don't see another way to do this.

@hhoover
Copy link

hhoover commented May 9, 2014

Corresponding Chef ticket: https://tickets.opscode.com/browse/COOK-4534

@btm btm changed the title Add option to update apt cache at compiletime COOK-4534: Add option to update apt cache at compiletime May 9, 2014
@someara
Copy link

someara commented May 15, 2014

merged in fa34970

@someara someara closed this May 15, 2014
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

5 participants