miyagawa / Carmel

CPAN Artifact Repository Manager
Other
138 stars 17 forks source link

Allow `-TRIAL` in installed distname #107

Closed leedo closed 2 weeks ago

Grinnz commented 2 weeks ago

-TRIAL should not be present in any installed metadata, particularly not the distname or version. What would this be needed for?

miyagawa commented 2 weeks ago

$_[0] here is Carmel::Artifact, and its distname simply returns the path of the artifact. https://github.com/miyagawa/Carmel/blob/master/lib/Carmel/Artifact.pm#L91-L93

I assume if the tarball actually contains -TRIAL in its path i think that would do it? Regularly, with tools like Dist::Zilla, -TRIAL should only be added when uploading the final tarball, but that's not a requirement.

I suppose the right fix for find_match is to actually correctly parse the metadata for the distribution but i am away from the code right now.

Grinnz commented 2 weeks ago

I don't know enough about what exactly is being compared here, but it would help to know the failure case/motivation.

miyagawa commented 2 weeks ago

the dist in question is https://metacpan.org/release/ETHER/Net-IDN-Encode-2.501-TRIAL and I confirmed that the files in the tarball contains -TRIAL in its directory name and i suppose that's what is causing it. Still not sure what the right fix should be.

@Grinnz yep i have some context from lee from work. He should be able to provide exact failure case and probably a test case. I feel like this is a one-off thing specific to this distribution, but there's a big assumption that a distribution name and tarball directory name exactly match.

leedo commented 2 weeks ago

I believe I tracked this down to what seems like a combination of what Miyagawa described and a manually edited cpanfile.snapshot. Here's a sample cpanfile and broken cpanfile.snapshot that demonstrates the failure:

cpanfile:

requires 'Net::IDN::Encode', '2.501';

cpanfile.snapshot

# carton snapshot format: version 1.0
DISTRIBUTIONS
  Net-IDN-Encode-2.501
    pathname: E/ET/ETHER/Net-IDN-Encode-2.501-TRIAL.tar.gz
    provides:
      Net::IDN::Encode 2.501
      Net::IDN::Punycode 2.501
      Net::IDN::Punycode::PP 2.501
      Net::IDN::UTS46 2.501
      Net::IDN::UTS46::_Mapping 10
    requirements:
      ExtUtils::CBuilder 0
      Unicode::Normalize 0
      perl 5.008005

Running carmel results in:

$ carmel install
---> Installing new dependencies: Net::IDN::Encode
Successfully installed Module-Build-0.4234
Successfully installed Net-IDN-Encode-2.501
2 distributions installed
Can't find an artifact for Net::IDN::Encode => 2.501
You need to run `carmel install` first to get the modules installed and artifacts built.

I believe what happened was the snapshot file was manually edited. If I delete the snapshot file and regenerate it, I get a dist name with -TRIAL in it:

# carton snapshot format: version 1.0
DISTRIBUTIONS
  Net-IDN-Encode-2.501-TRIAL
    pathname: E/ET/ETHER/Net-IDN-Encode-2.501-TRIAL.tar.gz
    provides:
      Net::IDN::Encode 2.501
      Net::IDN::Punycode 2.501
      Net::IDN::Punycode::PP 2.501
      Net::IDN::UTS46 2.501
      Net::IDN::UTS46::_Mapping 10
    requirements:
      ExtUtils::CBuilder 0
      Unicode::Normalize 0
      perl 5.008005

And then carmel succeeds:

$ carmel install
Using Net::IDN::Encode (2.501)
---> Complete! 1 cpanfile dependencies. 1 modules installed.
miyagawa commented 2 weeks ago

the tarball shouldn't have -TRIAL in its directory name, but yes, it definitely looked like manually updated snapshot to me.