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

(PDK-1264) Display a nicer error when tarring long paths #663

Merged
merged 1 commit into from
May 21, 2019

Conversation

rodjek
Copy link
Contributor

@rodjek rodjek commented May 10, 2019

Ideally we'd upgrade minitar to 0.8 which supports long paths, but the rest of the Puppet ecosystem is using minitar 0.6.1, which would result in building packages that r10k or PMT couldn't install. Also, upgrading minitar to 0.8 causes dependency hell with all the gems that have a ~> 0.6.1 minitar dependency.

Instead, we now display an error explaining the problem and asking the user to rename the file or exclude the file from the build before trying again.

@coveralls
Copy link

coveralls commented May 10, 2019

Coverage Status

Coverage increased (+0.03%) to 93.377% when pulling 939b58f on rodjek:pdk-1264 into 8635c97 on puppetlabs:master.

@rodjek rodjek force-pushed the pdk-1264 branch 2 times, most recently from 42e9bbb to 5a9367d Compare May 13, 2019 05:19
@rodjek rodjek changed the title (PDK-1264) Support long paths when building modules (PDK-1264) Display a nicer error when tarring long paths May 13, 2019
lib/pdk/module/build.rb Outdated Show resolved Hide resolved
rescue ArgumentError => e
raise PDK::CLI::ExitWithError, _(
'%{message} Please rename the file or exclude it from the package ' \
'by adding it to the .pdkignore file in your module.',
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the subtle distinction between the two failure cases is probably not worth surfacing to the user in this message and we can just say that the path is too long here. We can log the distinction at DEBUG perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking that, but say there is a file in the root of the module called this_is_a_file_name_that_goes_over_100_characters_oh_no.md, it would fail but would clearly be < 256 bytes which would make a generic error confusing.

@scotje
Copy link
Contributor

scotje commented May 21, 2019

Forced pushed an amended commit with the comment fixed up. Will merge after CI passes.

@scotje scotje merged commit ea1e415 into puppetlabs:master May 21, 2019
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.

4 participants