Add support for minitar 1.x #9449
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update puppet to support minitar 1.0 which contains breaking changes. Note this important warning from https://github.com/halostatue/minitar Minitar does not perform validation of path names provided to the convenience classes Minitar::Output and Minitar::Input, which use Kernel.open for their underlying implementations when not given an IO-like object. As a result we always pass a reader/writer to the unpack/pack methods respectively.
joshcooper
force-pushed
the
minitar_1_0
branch
from
7b75c09
to
2f9092a
Compare
bastelfreak
reviewed
Aug 15, 2024
| @@ -27,7 +27,7 @@ group(:features) do | |||
| gem 'hocon', '~> 1.0', require: false | |||
| # requires native libshadow headers/libs | |||
| #gem 'ruby-shadow', '~> 2.5', require: false, platforms: [:ruby] | |||
| gem 'minitar', '~> 0.9', require: false | |||
| gem 'minitar', '~> 1.0', require: false | |||
There was a problem hiding this comment.
Is it required to list it here if it's also in the gemspec? Is that something special because it's a feature?
There was a problem hiding this comment.
We only add minitar as a hard runtime dependency when building windows specific puppet gems, for example, compare the runtime dependencies in https://rubygems.org/gems/puppet/versions/8.8.1 and https://rubygems.org/gems/puppet/versions/8.8.1-x64-mingw32. So it's a hard dependency on windows but soft on other platforms. And soft dependencies are managed via puppet's feature system, e.g. Puppet.features.minitar?
bastelfreak
reviewed
Aug 15, 2024
| @@ -39,6 +39,6 @@ Gem::Specification.new do |spec| | |||
| if platform == 'x64-mingw32' || platform == 'x86-mingw32' | |||
| # ffi 1.16.0 - 1.16.2 are broken on Windows | |||
| spec.add_runtime_dependency('ffi', '>= 1.15.5', '< 1.17.0', '!= 1.16.0', '!= 1.16.1', '!= 1.16.2') | |||
| spec.add_runtime_dependency('minitar', '~> 0.9') | |||
| spec.add_runtime_dependency('minitar', '~> 1.0') | |||
There was a problem hiding this comment.
to ensure we don't pull in the broken 1.0.0. should we enforce 1.0.1 or newer?
Suggested change
| spec.add_runtime_dependency('minitar', '~> 1.0') | |
| spec.add_runtime_dependency('minitar', '~> 1.0', '>= 1.0.1') |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update puppet to support minitar 1.0 which contains breaking changes.
Note this important warning from https://github.com/halostatue/minitar
As a result we always pass a reader/writer to the unpack/pack methods respectively.
Fixes #9446
This is blocked on adding minitar 1.0.x to the runtime.