Skip to content

Fix duplicate package detection bug on import. #3

Closed
wants to merge 1 commit into from

2 participants

@throughnothing

This fixes an issue where in certain circumstances an import
will fail with "Invalid version format (non-numeric data)"
because $done{$name} contains a hash instead of a version
object. Makes sure $done{$name} is set to a version object.

@throughnothing throughnothing Fix duplicate package detection bug on import.
This fixes an issue where in certain circumstances an import
will fail with "Invalid version format (non-numeric data)"
because $done{$name} contains a hash instead of a version
object.  Makes sure $done{$name} is set to a version object.
95c9e9c
@thaljef
Owner
thaljef commented on 95c9e9c Mar 3, 2012

Thanks! Can you tell me which distribution you were importing that caused the error?

It is an internal non-public distribution. It has a lot of dependencies, and I couldn't track down the specific origin dependency that was causing it. It was failing ultimately on common::sense. I would like to be able to add a test for this, but I'm not 100% sure as to the exact scenario that reproduces it as I'm brand new to the code. Thoughts?

@thaljef
Owner
thaljef commented Mar 4, 2012

If you give me the list of dependencies, then I can probably track it down.

@throughnothing

Here are the deps from Makefile.PL:

Business::CreditCard
Business::OnlinePayment
DBIx::Class::Core
DBIx::Class::Schema
Dancer
Dancer::Plugin
Dancer::Plugin::DBIC
Dancer::Plugin::Stomp
Data::Dumper
Data::GUID
DateTime
DateTime::Duration
DateTime::Format::ISO8601
DateTime::Format::Pg
Digest::MD5
Digest::SHA
Digest::SHA2
Exporter
File::Temp
FindBin
HTML::Entities
HTML::Strip
Imager
Imager::QRCode
JSON
JSON::Schema
LWP
LWP::Protocol::https
LWP::UserAgent
Math::Round
Modern::Perl
Moose
Net::Stomp
Switch
Template
Text::Trim
Text::Unidecode
Try::Tiny
URI::Escape
Web::Scraper
WebService::Rackspace::CloudFiles
YAML
base
strict
utf8
warnings

@thaljef
Owner
thaljef commented Mar 4, 2012

Thanks! I'll look into this in the next couple days. Meanwhile, can you shoot me an email and describe how you are using (or plan to use) Pinto? I'm still trying to figure out how best to incorporate Pinto into the development cycle. So I'm really curious to hear what people are doing in the wild.

thaljef@cpan.org

@throughnothing

Have you had a chance to look into/try and reproduce this?

@thaljef
Owner
thaljef commented Mar 15, 2012

This particular bug has magically disappeared and I'm not sure why. I did make some modifications to the import code, but I didn't expect them to impact this bug. So I clearly don't fully understand my own code :(

But as of Pinto-0.033, I can import all the packages you listed above without any errors. This was the only warning:

Unable to extract prerequisites from /Users/jeff/Development/Pinto/Pinto-Core/TEST/authors/id/K/KU/KURIHARA/Imager-QRCode-0.033.tar.gz: Unable to extract prerequisites from /Users/jeff/Development/Pinto/Pinto-Core/TEST/authors/id/K/KU/KURIHARA/Imager-QRCode-0.033.tar.gz: Failed to configure /var/folders/jS/jS5l-KjoEFy6KPsU0X1bP++++TI/-Tmp-/RFGXY9sWBh/Imager-QRCode-0.033 at /Users/jeff/.perlbrew/libs/perl-5.14.2@dev/lib/perl5/Pinto/PackageExtractor.pm line 78

That probably has more to do with the state of the Imager-QRCode distribution.

The import code is somewhat broken in other ways though. When faced with a list of packages to import (such as yours), it may re-examine an archive's dependencies over and over. This doesn't change the outcome -- it just makes the process slower. Correcting this will require some interface changes. I'll have to think it over.

Thanks for reporting this!

@thaljef thaljef closed this Mar 15, 2012
@throughnothing

Thanks for looking into this. If you just look at the logic of the code, it certainly seems like it must still be an issue, just no longer produced by these set of dependencies for some reason. I'll try and figure out if I can come up with a test case to fix it, but it does look like everything is working for my use case for now, so thanks for that :)

@thaljef
Owner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.