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

Fixing puppet apt module requirement to < 6.0.0 #714

Merged
merged 2 commits into from
Aug 18, 2018
Merged

Fixing puppet apt module requirement to < 6.0.0 #714

merged 2 commits into from
Aug 18, 2018

Conversation

meltingrobot
Copy link
Contributor

@meltingrobot meltingrobot commented Aug 17, 2018

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

@@ -10,7 +10,7 @@
install_module_dependencies

# Install aditional modules for soft deps
install_module_from_forge('puppetlabs-apt', '>= 4.1.0 < 5.0.0') if fact('os.family') == 'Debian'
install_module_from_forge('puppetlabs-apt', '>= 4.1.0 < 5.1.0') if fact('os.family') == 'Debian'
Copy link
Member

Choose a reason for hiding this comment

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

can you please update this to < 6.0.0? We always try to support the whole major release range.

@meltingrobot meltingrobot changed the title Fixing puppet apt module requirement to < 5.1.0 Fixing puppet apt module requirement to < 6.0.0 Aug 17, 2018
@meltingrobot
Copy link
Contributor Author

Okay, I think I fixed the commit to be 6.0.0 now.

@wyardley
Copy link
Contributor

Puppet librarian shouldn't be looking at the spec_helper for acceptance tests, should it? I don't see an objection to merging this, but I don't see how it can help either. @bastelfreak?

I could see it installing deps from metadata but from spec_helper_acceptance?

@wyardley
Copy link
Contributor

@meltingrobot can you squash that commit, if you know how?

@meltingrobot
Copy link
Contributor Author

@wyardley I'm pretty sure it is the reason our Jenkins build is failing. When I did a grep through all the module files, that was the only place that the puppet-apt module was mentioned. Below is what I am seeing:

[Librarian] Conflict between puppetlabs-apt (< 5.0.0, >= 2.0.0) https://forgeapi.puppetlabs.com and puppetlabs-apt/5.0.1 https://forgeapi.puppetlabs.com

Not sure how to squash a commit, will look for directions to see if I can do that.

@wyardley
Copy link
Contributor

I believe that file should be only looking at metadata.json and the like, but it will pull in sub dependencies and their dependencies. So it’s possible you have a module that depends on a module that depends on apt (btw, apt is supposed to be a soft dep, and shouldn’t be listed in metadata).

@meltingrobot
Copy link
Contributor Author

Okay, I'm not sure how to squash a commit. I've read some guides, but I'm not sure what I'm doing on that.

@wyardley
Copy link
Contributor

@meltingrobot maybe better not to do it on your master branch. Normally, in a feature branch, you could do something like git rebase -i HEAD~2, change pick to f (for fixup), then git push --force-with-lease. I'll just squash-merge this one, so don't worry about it.

Feel free to check out #voxpupuli on IRC / Slack if you have other questions.

I will go ahead and merge this, but I have a feeling a nested dependency might be your actual issue.

@wyardley wyardley merged commit c19355a into voxpupuli:master Aug 18, 2018
@meltingrobot
Copy link
Contributor Author

@wyardley You were completely correct. It did not fix the issue. I discovered the real culprit after more research into the problem. It is actually the elastic-elasticsearch module for the 5.4.3 version we have that has a hard dependency of puppetlabs-apt (< 5.0.0, >= 2.0.0) I will look at updating that module to a 6.X version that drops that dependency. Thank you for your help.

@wyardley
Copy link
Contributor

wyardley commented Aug 18, 2018

@meltingrobot Awesome, glad we figured that all out.

cegeka-jenkins pushed a commit to cegeka/puppet-rabbitmq that referenced this pull request Mar 26, 2021
* Fixing puppet apt module requirement to < 6.0.0
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.

3 participants