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

(maint) Pass the appropriate Puppetfile path to module loader #1203

Merged

Conversation

mwaggett
Copy link
Contributor

see individual commit messages for details

@mwaggett mwaggett requested a review from a team August 12, 2021 16:51
@mwaggett mwaggett force-pushed the maint/main/fix-puppetfile-path branch from 9729808 to 8b9830c Compare August 13, 2021 18:33
This commit moves the Puppetfile logging to the `R10K::ModuleLoader::Puppetfile`
since that is what does the actual loading.
@mwaggett mwaggett force-pushed the maint/main/fix-puppetfile-path branch from 8b9830c to 7880c0c Compare August 13, 2021 18:35
@mwaggett
Copy link
Contributor Author

This should be good to go now.

@mwaggett mwaggett changed the title (maint) Readability + debugging improvements (maint) Pass the appropriate Puppetfile path to module loader Aug 13, 2021
@mwaggett
Copy link
Contributor Author

This passed for me locally.

@mwaggett
Copy link
Contributor Author

well.. some iteration of it lol 👀

@mwaggett mwaggett force-pushed the maint/main/fix-puppetfile-path branch from 1eef305 to 8d37831 Compare August 13, 2021 20:32
@mwaggett
Copy link
Contributor Author

okay that should be better 😅

Copy link
Collaborator

@Magisus Magisus left a comment

Choose a reason for hiding this comment

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

I think this looks alright, but Justin has more context.

@mwaggett mwaggett force-pushed the maint/main/fix-puppetfile-path branch from 44cd72b to b79efdf Compare August 13, 2021 21:13
@justinstoller
Copy link
Member

Do you want to add a unit test for the puppetfile spec that passing { puppetfile_path: "foo/Puppetfile" } etc is honored?

@mwaggett mwaggett force-pushed the maint/main/fix-puppetfile-path branch from b79efdf to 6de38b1 Compare August 13, 2021 22:11
@mwaggett
Copy link
Contributor Author

@justinstoller tests added 😊

@mwaggett mwaggett force-pushed the maint/main/fix-puppetfile-path branch from 6de38b1 to ac97fc5 Compare August 13, 2021 23:28
Previously, we always passed the `puppetfile_name` from the `R10K::Puppetfile`
class to `R10K::ModuleLoader::Puppetfile`. That was an attempt to fix a bug
where relative Puppetfile paths were not getting resolved correctly by the
module loader (puppetlabs@9082511).
That, however, unearthed another bug: when instantiating a new instance of
`R10K::Puppetfile` with the `puppetfile_path` option, that path would get
completely ignored in favor of the default `puppetfile_name` value. This broke
usage of the `r10k puppetfile purge` command.

This commit ensures that we pass the appropriate Puppetfile path to the module
loader - we pass the `puppetfile_path` if it exists, otherwise we pass the
`puppetfile_name`. In order to maintain the fix for the original bug around
relative Puppetfile paths, we no longer attempt to resolve the `puppetfile_path`
in the `R10K::Puppetfile` class, since `R10K::ModuleLoader::Puppetfile` already
handles that resolution.
Previously, we were using the remote version of the Puppetfile to test the
`r10k puppetfile purge` command. This commit updates the test to use the
deployed version of the Puppetfile, which is what a user would actually use.
@mwaggett mwaggett force-pushed the maint/main/fix-puppetfile-path branch from ac97fc5 to 7cd14c0 Compare August 13, 2021 23:40
@justinstoller justinstoller merged commit e9bdb69 into puppetlabs:main Aug 13, 2021
@mwaggett mwaggett deleted the maint/main/fix-puppetfile-path branch August 27, 2021 18:03
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