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

Module list should check modules META.info is correct #255

Closed
ranguard opened this issue Sep 27, 2016 · 34 comments
Closed

Module list should check modules META.info is correct #255

ranguard opened this issue Sep 27, 2016 · 34 comments

Comments

@ranguard
Copy link

https://raw.githubusercontent.com/perl6/ecosystem/master/META.list

Includes:

https://raw.githubusercontent.com/MARTIMM/unicode-precis/master/META.info

Which currently looks like this

{
  "author": "Marcel Timmerman",
  "authors": [
    "Marcel Timmerman"
  ],
  "depends": [
  ],
  "description": "Implements PRECIS framework, rfc7564 and others. Obsoletes stringprep and saslprep",
  "license": "The Artistic License 2.0",
  "name": "Unicode::PRECIS",
  "perl": "6.c",
  "provides": {
    "Unicode::PRECIS": "lib/Unicode/PRECIS.pm6",

    "Unicode::PRECIS::FreeForm": "lib/Unicode/PRECIS/FreeForm.pm6",
    "Unicode::PRECIS::FreeForm::OpaqueString": "lib/Unicode/PRECIS/FreeForm/OpaqueString.pm6",

    "Unicode::PRECIS::Identifier": "lib/Unicode/PRECIS/Identifier.pm6",
    "Unicode::PRECIS::Identifier::UsernameCaseMapped": "lib/Unicode/PRECIS/Identifier/UsernameCaseMapped.pm6",
    "Unicode::PRECIS::Identifier::UsernameCasePreserved": "lib/Unicode/PRECIS/Identifier/UsernameCasePreserved.pm6",

    "Unicode::PRECIS::Tables": "lib/Unicode/PRECIS/Tables.pm6"
  },
  "source-url": "git://github.com/MARTIMM/unicode-stringprep.git",
  "version": "v0.3.0"
}

The _source-url_ is wrong and 404's - it would be nice for the system maintaining the master list to verify this sort of thing automatically.

@jonathanstowe
Copy link
Contributor

The META.list is basically maintained manually. So is only checked when the list is built into the aggregate of the META files by some cron job, however it doesn't currently check the source-url for validity.

I guess we could rewrite the url to http:// and do a head on it to check it's okay.

@zoffixznet
Copy link
Contributor

It already does. You can grep http://modules.perl6.org/update.log for [error]

I'm unsure what more can be done about it. GitHub network errors are frequent, so it nagging somewhere automatically whenever it sees an error is LTA.

@zoffixznet
Copy link
Contributor

zoffixznet commented Sep 27, 2016

I stand corrected. It figures out the URL from the META.list META6.json URL, not from source-url field, so it doesn't check it.

@MARTIMM
Copy link
Contributor

MARTIMM commented Sep 27, 2016

I am sorry for this bug, I'll repair it as soon as possible next morning.
Regards,
Marcel

On September 27, 2016 22:42:47 Leo Lapworth notifications@github.com wrote:

https://raw.githubusercontent.com/perl6/ecosystem/master/META.list

Includes:

https://raw.githubusercontent.com/MARTIMM/unicode-precis/master/META.info

Which currently looks like this

{
  "author": "Marcel Timmerman",
  "authors": [
    "Marcel Timmerman"
  ],
  "depends": [
  ],
  "description": "Implements PRECIS framework, rfc7564 and others. Obsoletes 
  stringprep and saslprep",
  "license": "The Artistic License 2.0",
  "name": "Unicode::PRECIS",
  "perl": "6.c",
  "provides": {
    "Unicode::PRECIS": "lib/Unicode/PRECIS.pm6",

    "Unicode::PRECIS::FreeForm": "lib/Unicode/PRECIS/FreeForm.pm6",
    "Unicode::PRECIS::FreeForm::OpaqueString": 
    "lib/Unicode/PRECIS/FreeForm/OpaqueString.pm6",

    "Unicode::PRECIS::Identifier": "lib/Unicode/PRECIS/Identifier.pm6",
    "Unicode::PRECIS::Identifier::UsernameCaseMapped": 
    "lib/Unicode/PRECIS/Identifier/UsernameCaseMapped.pm6",
    "Unicode::PRECIS::Identifier::UsernameCasePreserved": 
    "lib/Unicode/PRECIS/Identifier/UsernameCasePreserved.pm6",

    "Unicode::PRECIS::Tables": "lib/Unicode/PRECIS/Tables.pm6"
  },
  "source-url": "git://github.com/MARTIMM/unicode-stringprep.git",
  "version": "v0.3.0"
}

The _source-url_ is wrong and 404's - it would be nice for the system
maintaining the master list to verify this sort of thing automatically.

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#255

@MARTIMM
Copy link
Contributor

MARTIMM commented Sep 28, 2016

Hi,
The link is repaired.
Btw, the check might also be done in Test::META module.
Greetings,
Marcel

P.s. Sorry for the mess i've got you into...

@ranguard
Copy link
Author

ranguard commented Oct 2, 2016

@MARTIMM thanks for fixing - leaving ticket open as it wasn't specifically about your module's problem, so much as there should be a safe place to use as the master - otherwise everyone has to implement their own validation

@ranguard
Copy link
Author

@drforr FYI https://raw.githubusercontent.com/drforr/perl6-Perl6-Parser/master/META.info - boom, missing commas

{
    "name"         : "Perl6-Parser",
    "version"      : "*",
    "description"  : "Perl 6 parsing",
    "test-depends" : [
        "Test"
    ],
    "provides"     : {
        "Perl6::Parser"        : "lib/Perl6/Parser.pm6"
        "Perl6::Parser::Factory" : "lib/Perl6/Parser/Factory.pm6"
        "Perl6::Parser::Validator" : "lib/Perl6/Parser/Validator.pm6"
    },
    "source-url"   : "git://github.com/drforr/perl6-Perl6-Parser.git"
}

@jonathanstowe
Copy link
Contributor

This is why https://github.com/jonathanstowe/Test-META :)

@ranguard
Copy link
Author

@jonathanstowe can you (ecosystem team) make that a pre-req going forward for any module that is going to be added to https://raw.githubusercontent.com/perl6/ecosystem/master/META.list ?

ranguard added a commit to ranguard/perl6-Perl6-Parser that referenced this issue Oct 16, 2016
@zoffixznet
Copy link
Contributor

zoffixznet commented Oct 16, 2016

can you (ecosystem team) make that a pre-req going forward for any module that is going to be added to

👎 on such puritanical requirements. Even with it added, there's no guarantee of lack of errors. Authors have to run those tests too whenever making changes. All such a requirement would accomplish is create lots of noise, lots of confused people, and lots of dead PRs to the ecosystem.

@zoffixznet
Copy link
Contributor

And more to it.... I don't think this Issue even belongs in the Ecosystem domain. This is more to the tune of what Perl 5 Testers service does. Our version is http://smoke.perl6.org/report in whatever shape it currently exists.

@shadowcat-mst
Copy link

Ok, so, the thing here is: we're trying, with PSIXDISTS, to do trial uploads so e.g. zef can work against cpan dists for testing, and so the perl6 metacpan indexer can run against cpan, and various other infrastructure can be trialed.

Requiring valid metadata to introspect doesn't really seem "puritanical" to me, it just seems like the absolute bare mininum required to be able to start building out downstream things from the ecosystem.

Comparing it to cpantesters is a massive category error. cpantesters only tests dists that are at least vaguely valid, after all.

@zoffixznet
Copy link
Contributor

zoffixznet commented Oct 16, 2016

Requiring valid metadata to introspect

Valid metadata doesn't require a module built by a third party that brings in 2 deps with it. Moreover, the module doesn't magically make that metadata valid. It requires module authors to execute the tests that use the module.

In your context, it would be the indexer's job to ensure metadata's validity and refuse to index a distro if it is invalid.

Forcing the users to depend on Meta-Checker-De-Jour is silly and doesn't even solve the actual problem.

@shadowcat-mst
Copy link

@zoffixznet You appear to have accidentally heard an argument I'm not making.

All I'm saying is that it would be nice to have an index where the metadata can be assumed valid and the code can be assumed to exist. Many downstream users could then consume said index.

@jonathanstowe
Copy link
Contributor

I wouldn't be in favour of mandating the use of Test::META for all modules either, I would be in favour of encouraging a culture whereby people value using the greatest range of tests they have available which may include testing the META data, testing the documentation etc etc, but this is a discussion for elsewhere.

As it currently stands, the best place to ensure the validity of the META files is at the point they are aggregated into the projects.json from the META.list. This is less than perfect (because there isn't a mechanism to feed back a failure to the user directly,) at present, but we can work on that.

So no to mandating a particular set of tests, yes to better checking in the back end and possibly feeding failures back to the user earlier.

@shadowcat-mst
Copy link

That sounds great. @jonathanstowe key thing here is that it's basically blowing up the PSIXDISTS process, and while we could hack around it locally, it seems like it'd be a better idea to have a central "this should be valid" list instead.

@zoffixznet
Copy link
Contributor

.oO( A GitHub bot that automatically checks the PRs to ecosystem, and merges them if the META is good, or comments on them if there are errors... )

@zoffixznet
Copy link
Contributor

Oh, wait, that still has the issues of changes to modules...

.oO( hack modules.perl6.org dist builder to automatically submit Ecosystem issues and ping authors when issues detected ).

@zoffixznet
Copy link
Contributor

zoffixznet commented Oct 25, 2016

@ranguard there was a brief discussion on IRC...

What's considered a "distro" for PSIXDISTS? That is, where would be the best place to put an automated META check if it's not in PSIXDISTS thing itself. How does it determine when a new release of a module appeared, etc?

The modules.perl6.org dist builder has an easy possibility to insert such a check as it already looks for whether a repo has got new commits. I'm just curious in which format PSIXDISTS stuff could find that useful.

zoffixznet added a commit to Raku/modules.raku.org that referenced this issue Oct 25, 2016
Add extra checks to validate distributions in the Ecosystem.

Part of the PSIXDISTS indexing issue
Raku/ecosystem#255
@zoffixznet
Copy link
Contributor

I've added a METAChecker postprocessor to modules.perl6.org builder that checks source-url URL points to something that gives a 200 code. What else is needed and how to make this info more useful?

Currently, it's visible for anyone curious on http://modules.perl6.org/update.log, but there's no automated checks for anything beyond that. Moreover, failures happen frequently, so GitHub may not return a 200 each and every time, so there needs to be some statistical (?) means to ascertain whether a distro's META is OK.

@ranguard
Copy link
Author

@zoffixznet I currently store the sha of the master branch of each git repo, if that changes then I know it's a new version (as far as releasing to CPAN is concerned).

https://github.com/perl6modules/perl6-module-uploader publish_to_cpan.pl is the code and upload_tracker.json is the tracker file.

https://github.com/perl6modules/dists-uploaded-to-cpan is the actual releases as well.

@zoffixznet
Copy link
Contributor

zoffixznet commented Oct 25, 2016

Sounds like Test::META6 (Test::Perl6::META? Test::Perl6META?) Perl 5 module would be the perfect solution and it can be run by that script when preparing an upload, as well as the modules.perl6.org dist builder to indicate any errors in update.log.

@jonathanstowe
Copy link
Contributor

I can look at the making a Perl 5 version if no-one else picks it up

@ranguard
Copy link
Author

@jonathanstowe that would help me lots thanks, even if not the ideal long term solution...

It is tricky, I was going to propose 'valid dists' be maintained in a separate list and updated (via cron) in the same way I'm checking (based on sha) but then you need to update all links to include the sha... which ultimately is why I'm putting these on CPAN in the first place (so there are snapshot versions of the entire repo at a known point in time and that safeguards someone deleting their repo or doing other funky stuff with it).

Hopefully the p6 community can then work on the tools that encourage authors to upload their own releases to CPAN and I can turn off my script :)

@drforr
Copy link
Contributor

drforr commented Oct 26, 2016

Mea culpa as well; I'm in a habit of cp -r perl6-Random-Dist perl6-My-New-Dist ; rm -rf .git ; , submit, without really looking at the metadata closely. While having to use p5 to maintain a p6 repo is not ideal, maybe creating a set of small plugins for Dist::Zilla would be a good approach, as it's already got helpers for PAUSE upload?

@jonathanstowe
Copy link
Contributor

I've actually got a script somewhere that I use to generate the META files for my modules, I could tosh it up a bit and add it to the META6 distribution, I think all that it needs is to be able to store a little bit of config (for author and the like.) I'll put it on the stack of things I'm planning to do (having a full-time job again sucks ....)

@ranguard
Copy link
Author

ranguard commented Jan 8, 2017

Using https://raw.githubusercontent.com/perl6/ecosystem/master/META.list as my source of info...

  • The following have metadata which points to sources which are not valid GH repos
p6-fcgi
p6-fcgi-psgi
p6-unix-privileges
p6-jdf
szabgab/Perl6-Maven

I'm going to make my code just skip these issues quietly from now on as my code is doing the right thing as far as I am concerned, but doesn't have a decent enough source to work from to ensure that all ecosystem modules are uploaded to CPAN.

@jonathanstowe
Copy link
Contributor

The META.list is probably not the place to be getting the data from as this is totally raw and unvarnished, it gets processed into some other file which will already have checked that the META file is retrievable and somewhat sane, That file may or may not be http://ecosystem-api.p6c.org/projects.json but you would be better checking which file the modules site uses.

I would thoroughly recommend getting on #perl6 on IRC as it will enable a quicker exchange of information :)

@stmuk
Copy link
Contributor

stmuk commented Jan 8, 2017 via email

@jonathanstowe
Copy link
Contributor

It would be better to use whatever the modules.perl6.org uses I would say :)

@ranguard
Copy link
Author

ranguard commented Jan 8, 2017

19:16 < ranguard> RabidGravy: I understood modules.perl6.org to be built from
https://raw.githubusercontent.com/perl6/ecosystem/master/META.list
19:16 < ranguard> ^^ anyone know different?
19:16 < notviki> ranguard: That's accurate

@jonathanstowe
Copy link
Contributor

There is a travis ci job that is run on every commit to the modules list that checks the META of the modules that are added now.

@ranguard
Copy link
Author

ranguard commented Sep 14, 2017 via email

@JJ
Copy link
Contributor

JJ commented Apr 11, 2018

Should this be closed?

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

No branches or pull requests

8 participants