Perl CPAN Support, Fix All 5 Outstanding Issues, Second Try #1528

wbraswell · 2018-07-10T20:41:02Z

I tried this pull request once before, but forgot to clean up the code in the last commit: #1527

@jordansissel
Please merge this pull request, it solves all 5 outstanding Perl issues:

Issue: #1517
Fixed: wbraswell@a1afdb7

Issue: #1519
Fixed:
wbraswell@4756a45
wbraswell@373cb65

Issue: #1522
Fixed:
wbraswell@3171ee9
wbraswell@985c39b

Issue: #1523
Fixed: wbraswell@f4f4b56

Issue: #1526
Fixed:
wbraswell@c01f67a
wbraswell@1d37ec3

…xecmd(), Part 1

…xecmd(), Part 2

…ions

jordansissel · 2018-07-27T03:37:33Z

Testing manually bundle exec rspec:

Master: 263 examples, 9 failures
This PR: 263 examples, 45 failures

The following tests fail on this PR but do not fail on master.

+rspec ./spec/fpm/package/gem_spec.rb:26 # FPM::Package::Gem when :gem_version_bins? is true it should append the version to binaries
+rspec ./spec/fpm/package/gem_spec.rb:38 # FPM::Package::Gem when :gem_version_bins? is false it should not append the version to binaries
+rspec ./spec/fpm/package/gem_spec.rb:51 # FPM::Package::Gem when :gem_fix_name? is true and :gem_package_name_prefix is nil/default should prefix the package with 'gem-'
+rspec ./spec/fpm/package/gem_spec.rb:58 # FPM::Package::Gem when :gem_fix_name? is true and :gem_package_name_prefix is set should prefix the package name appropriately
+rspec ./spec/fpm/package/gem_spec.rb:72 # FPM::Package::Gem when :gem_fix_name? is false it should not prefix the name at all
+rspec ./spec/fpm/package/gem_spec.rb:83 # FPM::Package::Gem when :gem_shebang is nil/default should not change the shebang
+rspec ./spec/fpm/package/gem_spec.rb:96 # FPM::Package::Gem when :gem_shebang is set should change the shebang
+rspec ./spec/fpm/package/npm_spec.rb:17 # FPM::Package::NPM::default_prefix should provide a valid default_prefix
+rspec ./spec/fpm/package/pacman_spec.rb:100 # FPM::Package::Pacman#output Test that empty epoch is tested properly should have the correct iteration
+rspec ./spec/fpm/package/pacman_spec.rb:104 # FPM::Package::Pacman#output Test that empty epoch is tested properly should have the correct epoch
+rspec ./spec/fpm/package/pacman_spec.rb:186 # FPM::Package::Pacman#output normal tests script contents should be the same both with input as with original for after_install
+rspec ./spec/fpm/package/pacman_spec.rb:186 # FPM::Package::Pacman#output normal tests script contents should be the same both with input as with original for after_remove
+rspec ./spec/fpm/package/pacman_spec.rb:186 # FPM::Package::Pacman#output normal tests script contents should be the same both with input as with original for after_upgrade
+rspec ./spec/fpm/package/pacman_spec.rb:186 # FPM::Package::Pacman#output normal tests script contents should be the same both with input as with original for before_install
+rspec ./spec/fpm/package/pacman_spec.rb:186 # FPM::Package::Pacman#output normal tests script contents should be the same both with input as with original for before_remove
+rspec ./spec/fpm/package/pacman_spec.rb:186 # FPM::Package::Pacman#output normal tests script contents should be the same both with input as with original for before_upgrade
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /etc
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /etc/foo.conf
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /usr
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /usr/bin
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /usr/bin/foo
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /usr/lib
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /usr/lib/libfoo.so
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /usr/share
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /usr/share/doc
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /usr/share/doc/foo.txt
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /var
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /var/lib
+rspec ./spec/fpm/package/pacman_spec.rb:210 # FPM::Package::Pacman#output normal tests file permissions should preserve file permissions for /var/lib/foo
+rspec ./spec/fpm/package/pacman_spec.rb:218 # FPM::Package::Pacman#output normal tests package attributes should have the correct name
+rspec ./spec/fpm/package/pacman_spec.rb:222 # FPM::Package::Pacman#output normal tests package attributes should have the correct version
+rspec ./spec/fpm/package/pacman_spec.rb:226 # FPM::Package::Pacman#output normal tests package attributes should have the correct iteration
+rspec ./spec/fpm/package/pacman_spec.rb:230 # FPM::Package::Pacman#output normal tests package attributes should have the correct epoch
+rspec ./spec/fpm/package/pacman_spec.rb:234 # FPM::Package::Pacman#output normal tests package attributes should not have bogus dependencies, just correct dependencies
+rspec ./spec/fpm/package/pacman_spec.rb:92 # FPM::Package::Pacman#output Test that empty epoch is tested properly should have the correct name
+rspec ./spec/fpm/package/pacman_spec.rb:96 # FPM::Package::Pacman#output Test that empty epoch is tested properly should have the correct version

jordansissel · 2018-07-27T03:44:12Z

The pacman failures seem to be all identical:

  37) FPM::Package::Pacman#output normal tests package attributes should have the correct iteration
     Failure/Error: original.output(target)
     NoMethodError:
       undefined method `read' for nil:NilClass
     # ./lib/fpm/util.rb:267:in `block in safesystemout'
     # ./lib/fpm/util.rb:175:in `execmd'
     # ./lib/fpm/util.rb:266:in `safesystemout'
     # ./lib/fpm/package/pacman.rb:253:in `output'
     # ./spec/fpm/package/pacman_spec.rb:174:in `block (4 levels) in <top (required)>'

wbraswell · 2018-08-03T17:07:52Z

@jordansissel
Sorry I'm not a Ruby programmer, I'm not sure what these errors mean, can you please help me understand?

wbraswell · 2018-08-24T07:42:35Z

@liger1978
Can you please help me debug these failing pacman tests? I am not a Ruby programmer, but I think it might have something to do with Issue #1522 which we worked on together recently...

liger1978 · 2018-08-24T10:26:15Z

@wbraswell I can't really help. To be honest I'm not sure the work on #1522 is the right approach now. I think we need to go back to basics and workout why the gcc sub-process hangs during the build when triggered from the ruby code. I think this is going to need help from @jordansissel or someone else.

liger1978 · 2018-08-24T10:27:53Z

Running external programs seems to be a minefield in ruby and beyond my time/inclination/skills to fix this is a reliable non-hacky way.

wbraswell · 2018-08-24T18:07:24Z

@liger1978
Okay well I tried my best but was unable to figure out why GCC was hanging, this sounds like some kind of problem with Ruby internals which is probably beyond all of us to fix, which is why I proposed the somewhat hacky fix because at least it works... but these failing tests are the problem!

@jordansissel
Since this may be some kind of deep problem with Ruby itself, it may not be easy to fix the failing tests; however we do need some kind of solution and at this point my hack is the only proposal...
Since you are the author of FPM, I really think you can help us figure out what needs to happen if you just take a look at my code in relation to the failing tests.

Can you please give some guidance on how to proceed?

wbraswell · 2019-02-22T18:38:54Z

@jordansissel
Can you please spare a few minutes to review the code and give your input?

jordansissel · 2019-02-22T21:21:56Z

I will try to do soon. Other responsibilities have taken priority over this the past few months.

…

On Fri, Feb 22, 2019 at 10:38 AM William N. Braswell, Jr. < ***@***.***> wrote: @jordansissel <https://github.com/jordansissel> Can you please spare a few minutes to review the code and give your input? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1528 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIC6qt3-T16mlyardG8Zr4Fg75uJY-qks5vQDk_gaJpZM4VKFjX> .

wbraswell · 2019-02-22T21:26:37Z

@jordansissel
Thank you sir!

jordansissel · 2019-02-23T05:37:05Z

At first glance, this is a large PR which introduces many test failures and changes quite a number of things. I can spend some time trying to review, but with so much changing in a single PR with multiple objectives in this PR, it may be difficult for me to untangle all the changes.

jordansissel · 2019-02-23T05:41:09Z

For the Inline::CPP hang (which takes several minutes before hanging as it installs a dozen or so Perl modules), I can confirm that master hangs and the perl subprocess is blocked in a read:

% strace -p 729
strace: Process 729 attached
read(0,

wbraswell · 2019-02-23T05:43:16Z

@jordansissel
Yes please do look at the Ruby-specific issues with GCC hanging. I am not a Ruby programmer so I can't figure out that part at all.
If we don't get anywhere, then I can try to split apart the pull request into 5 separate pull requests, but that will take some time...

jordansissel · 2019-02-23T05:44:26Z

@wbraswell for tonight at least, I'm going to try and go through as many of the referenced issues as i can and figure out a solution (plus review the relevant changes in this PR, if need)

wbraswell · 2019-02-23T05:45:44Z

@jordansissel
Okay great thanks! :-)

jordansissel · 2019-02-23T05:49:35Z

@wbraswell do you know of any way to speed this up? Each fpm invocation is installing dependencies (Module::Build, etc) and it takes 2-3 minutes for each attempt before I can get to the freeze installing Inline::CPP. It's been about 10 years since I've used Perl, so I'm out of the loop.

jordansissel · 2019-02-23T05:54:45Z

@wbraswell figured out the performance as I remembered cpan runs the test suite by default. --no-cpan-test makes it fast.

I found that < /dev/null on fpm will make this work:

% time bin/fpm --debug -s cpan -t rpm --no-cpan-test Inline::CPP  < /dev/null
...
Created package {:path=>"perl-Inline-CPP-0.75-1.noarch.rpm", :file=>"clamp/command.rb", :line=>"63", :method=>"run"}
...
bin/fpm --debug -s cpan -t rpm --no-cpan-test Inline::CPP < /dev/null  5.16s user 1.30s system 50% cpu 12.672 total

If I have to guess right now, safesystem is not correctly closing (or redirecting /dev/null) for stdin. I'll keep posting notes here as I find new things.

jordansissel · 2019-02-23T06:09:32Z

This appears to fix Inline::CPP by closing stdin so that Inline::CPP's Makefile.PL will get nothing when reading (instead of blocking for reads):

diff --git a/lib/fpm/util.rb b/lib/fpm/util.rb
index 6ebfde2..117353c 100644
--- a/lib/fpm/util.rb
+++ b/lib/fpm/util.rb
@@ -153,9 +153,10 @@ module FPM::Util
     process.io.stdout = stdout_w
     process.io.stderr = stderr_w
 
-    if block_given? and opts[:stdin]
-      process.duplex = true
-    end
+    # Always use duplex mode because we will always
+    # want to either write to stdin (if block_given?)
+    # or we will want to close stdin to prevent subprocesses from blocking on it.
+    process.duplex = true
 
     process.start
 
@@ -170,10 +171,13 @@ module FPM::Util
 
       yield(*args3)
 
-      process.io.stdin.close        if opts[:stdin] and not process.io.stdin.closed?
+      process.io.stdin.close        if not process.io.stdin.closed?
       stdout_r.close                unless stdout_r.closed?
       stderr_r.close                unless stderr_r.closed?
     else
+      # If no block given (not interactive) we should close stdin.
+      process.io.stdin.close
+
       # Log both stdout and stderr as 'info' because nobody uses stderr for
       # actually reporting errors and as a result 'stderr' is a misnomer.
       logger.pipe(stdout_r => :info, stderr_r => :info)

% bin/fpm --debug -s cpan -t rpm --no-cpan-test Inline::CPP
,,,
Created package {:path=>"perl-Inline-CPP-0.75-1.noarch.rpm", :file=>"clamp/command.rb", :line=>"63", :method=>"run"}
...

jordansissel · 2019-02-23T06:11:10Z

The patch in #1528 (comment) appears to resolve #1519 and does not add any test failures (master currently has 13 failing tests on my workstation, and adding this patch keeps it at 13 failures aka no change)

jordansissel · 2019-02-23T06:16:34Z

For #1518, if I take just one line, I think it fixes it: https://github.com/jordansissel/fpm/pull/1518/files#diff-2533a90976db66b6aaa59e0763508b96R317

% git diff
diff --git a/lib/fpm/package/cpan.rb b/lib/fpm/package/cpan.rb
index cbb786f..64eec8d 100644
--- a/lib/fpm/package/cpan.rb
+++ b/lib/fpm/package/cpan.rb
@@ -304,7 +304,8 @@ class FPM::Package::CPAN < FPM::Package
     directory = build_path("module")
     ::Dir.mkdir(directory)
     args = [ "-C", directory, "-zxf", tarball,
-      "--strip-components", "1" ]
+     #      "--strip-components", "1" ]       # fails    on removing leading ./Foo/ in tarball paths
+            %q{--transform=s,[./]*[^/]*/,,} ]  # succeeds on removing leading ./Foo/ or /Foo/ or Foo/
     safesystem("tar", *args)
     return directory
   end

% bin/fpm --verbose --no-cpan-test -s cpan -t rpm Alien::astyle
Created package {:path=>"perl-Alien-astyle-0.010000-1.noarch.rpm"}

jordansissel · 2019-02-23T06:19:20Z

I took the above diff and committed it to a branch as @wbraswell 9d774843aebd565abea36357951ae31e10edb0bf

Two issues down. Three to go? ;)

jordansissel · 2019-02-23T06:34:46Z

@wbraswell The stdin one was kind of a fun bug! Thank you for working on all of this.

#1610 should fix many of these things. I excluded the SRPM because I'm unclear on the goal and also how not to confuse users, and I'd like us to discuss that separately from these CPAN bugs. Can you test #1610 and let me know how it goes?

wbraswell · 2019-03-10T06:23:36Z

@jordansissel
Okay great, thanks for helping solve that stdin bug! :-)
I will work on testing #1610 and report back here with the results.

wbraswell · 2022-10-13T21:14:35Z

In hindsight, it was foolish of me to try and fix more than 1 or 2 bugs per pull request.
I am closing this PR, which will be split into multiple PRs by @NicholasBHubbard starting with:
#1937

wbraswell added 8 commits July 4, 2018 00:56

Perl CPAN Support, Broken tar --strip-components Command, Fix

a1afdb7

Perl CPAN Support, Frozen cpanm Command, Fix Part 1

4756a45

Perl CPAN Support, Frozen cpanm Command, Fix Part 2

373cb65

Perl CPAN Support, Frozen cpanm Command, Fix Broken I/O Handling In e…

3171ee9

…xecmd(), Part 1

Perl CPAN Support, Missing 'author' Field, Fix

f4f4b56

Perl CPAN Support, Frozen cpanm Command, Fix Broken I/O Handling In e…

985c39b

…xecmd(), Part 2

RPM Support, Build Binary and/or Source Packages, Add -ba -bb -bs Opt…

c01f67a

…ions

RPM Support, Build Binary and/or Source Packages, Part 2

1d37ec3

wbraswell added 5 commits August 27, 2018 16:46

Perl CPAN Support, Correct Provides Data, Part 1

b901934

Perl CPAN Support, (Do Not) Rebuild Packages In Base Repo

0b03c7a

Perl CPAN Support, Correct Provides Data, Part 2

9621bf4

Perl CPAN Support, Strip Leading 'v' From Version Strings

78f9272

Perl CPAN Support, Work Around Broken Versions In RPM Base Repo

68bcc03

jordansissel mentioned this pull request Feb 23, 2019

Fix a few CPAN issues #1610

Closed

wbraswell closed this Oct 13, 2022

wbraswell mentioned this pull request Oct 13, 2022

Prevent crash if CPAN distribution has blank author field #1937

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Perl CPAN Support, Fix All 5 Outstanding Issues, Second Try #1528

Perl CPAN Support, Fix All 5 Outstanding Issues, Second Try #1528

wbraswell commented Jul 10, 2018

jordansissel commented Jul 27, 2018

jordansissel commented Jul 27, 2018

wbraswell commented Aug 3, 2018

wbraswell commented Aug 24, 2018

liger1978 commented Aug 24, 2018

liger1978 commented Aug 24, 2018

wbraswell commented Aug 24, 2018

wbraswell commented Feb 22, 2019

jordansissel commented Feb 22, 2019 via email

wbraswell commented Feb 22, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

wbraswell commented Feb 23, 2019

jordansissel commented Feb 23, 2019

wbraswell commented Feb 23, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

wbraswell commented Mar 10, 2019

wbraswell commented Oct 13, 2022

Perl CPAN Support, Fix All 5 Outstanding Issues, Second Try #1528

Perl CPAN Support, Fix All 5 Outstanding Issues, Second Try #1528

Conversation

wbraswell commented Jul 10, 2018

jordansissel commented Jul 27, 2018

jordansissel commented Jul 27, 2018

wbraswell commented Aug 3, 2018

wbraswell commented Aug 24, 2018

liger1978 commented Aug 24, 2018

liger1978 commented Aug 24, 2018

wbraswell commented Aug 24, 2018

wbraswell commented Feb 22, 2019

jordansissel commented Feb 22, 2019 via email

wbraswell commented Feb 22, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

wbraswell commented Feb 23, 2019

jordansissel commented Feb 23, 2019

wbraswell commented Feb 23, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

jordansissel commented Feb 23, 2019

wbraswell commented Mar 10, 2019

wbraswell commented Oct 13, 2022