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

CPAN support - Fix broken tar command #1940

Conversation

NicholasBHubbard
Copy link
Contributor

@NicholasBHubbard NicholasBHubbard commented Oct 17, 2022

This PR fixes issue #1517 using the fix provided by #1518.

For handling CPAN tarballs with leading "./Foo/" directory structures, we must employ a tar --transform regex instead of a tar --strip-components command.

This bug can be reproduced by running the following command on the main branch:

fpm --no-cpan-test --cpan-verbose --verbose --debug-workspace --workdir $HOME/tmp -t rpm -s cpan -v 0.010000 Alien::astyle

I also added a test to cpan_spec.rb that ensures Alien::astyle version 0.010000 can be built, but it should be noted that (on my machine) this tests causes the test suite to take about 4 minutes to run versus only about 30 seconds without this test.

@NicholasBHubbard NicholasBHubbard marked this pull request as ready for review October 22, 2022 01:10
@wbraswell
Copy link
Contributor

@jordansissel
As with the other Perl-related code, I have personally reviewed this pull request and I approve it. Please approve and merge, thanks!

@wbraswell
Copy link
Contributor

Ping @jordansissel

@jordansissel
Copy link
Owner

jordansissel commented Oct 25, 2022 via email

Copy link
Owner

@jordansissel jordansissel 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 working on this!

This new test takes a pretty long time on my workstation. I tried timing it, but my attempt errored after 4 minutes because my test container was missing g++ -- it surely would have taken more time if I had that dependency.

Are there any other ways we can try test this that might be faster? It also adds additional external dependencies (g++, etc) which adds additional maintenance burdens to the test suite.

One thought is that we could test the unpack method directly. This method has private access, but we can still invoke it in Ruby using instance_eval, like this:

directory = subject.instance_eval { unpack("my.tar.gz") }

# The 'directory' variable is a string with the path to where the module was unpacked.
# Now we can list things in that directory and verify the paths are as expected.

If we include a .tar.gz in this test as a fixture with a known directory structure, specifically the relative ./foo/bar style paths, it could be tested without reaching out to the internet, without adding a dependency, and much faster.

Here's an example I came up with to test --

# Create a tarball with a file './foo/bar'
$ mkdir -p /tmp/z/foo
$ touch $_/bar
$ tar -C /tmp/z -cvzf hello.tar.gz .
./
./foo/
./foo/bar

$ bundle exec irb
...
irb(main):003:0> directory = subject.instance_eval { unpack("hello.tar.gz") }
=> "/tmp/package-cpan-build-c133f629e51c353b598c8924fe88767b6407896f2fa875e41b3eb2a58a7c/module"

The above shows how we can invoke unpack directly, and we can use other tools like ruby's Find.find method or similar to make assertions about the structure and contents of the unpacked tarball.

What do you think?

@NicholasBHubbard
Copy link
Contributor Author

Thanks for the idea @jordansissel! I used the example code you gave me to create a test that shows that cpan.rb's unpack method can handle tarballs containing ./ leading paths. Let me know what you think.

@jordansissel
Copy link
Owner

Nice! Much faster.

% bundle exec rspec spec/fpm/package/cpan_spec.rb
....

Finished in 11.32 seconds (files took 0.20792 seconds to load)
4 examples, 0 failures

@jordansissel jordansissel merged commit 110419b into jordansissel:main Oct 28, 2022
@wbraswell
Copy link
Contributor

Thanks @jordansissel ! :-)

@NicholasBHubbard NicholasBHubbard deleted the cpan-support-fix-broken-tar-command branch October 31, 2022 22:21
@jordansissel
Copy link
Owner

fpm 1.15.0 is released and contains this improvement.

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.

None yet

3 participants