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

Correct template path filter logic to only include regular files #524

Merged
merged 4 commits into from
Jul 13, 2018

Conversation

nabertrand
Copy link
Contributor

It looks like the original logic in PDK::Module::TemplateDir.files_in_template was created to filter paths that weren't regular files, but that logic was altered in commit 7facba3#diff-f1961fa3f901b19ee585c1962e910d5eL193 and the filter no longer works as exptected. If a directory other than "spec" appears in the moduleroot or moduleroot_init folders, pdk new module fails with the error

pdk (FATAL): Failed to render template '<directory>'
ArgumentError: '/tmp/pdk-templates20180614-14695-18x9jy1/moduleroot_init/<directory>' is not a readable file

This patch restores the original behavior.

@coveralls
Copy link

coveralls commented Jun 14, 2018

Coverage Status

Coverage decreased (-0.01%) to 92.785% when pulling 0238158 on nabertrand:template_dir_fix into c9fb0ff on puppetlabs:master.

File.file? does not return true for directories
@nabertrand
Copy link
Contributor Author

It looks like this is same issue as #445 but solves the problem slightly differently than #512. #512 will create a possibly unbalanced dirlocs list @ https://github.com/puppetlabs/pdk/pull/512/files#diff-f1961fa3f901b19ee585c1962e910d5eL216

Copy link
Contributor

@scotje scotje left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@scotje scotje merged commit 4a73b87 into puppetlabs:master Jul 13, 2018
@nabertrand nabertrand deleted the template_dir_fix branch July 13, 2018 18:26
@thomas-illiet
Copy link

Thanks !!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants