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

require package name permissions to use a dist name #80

Closed
wants to merge 27 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@rjbs
Collaborator

rjbs commented Mar 13, 2014

This implements the long-discussed rule:

  • To upload a distribution named "Xyz" you must have permission on package "Xyz."
  • …and it's okay if you only get that permission by the upload under consideration.

The grandfather list needs to be fleshed out. We can do that programatically.

@rjbs

This comment has been minimized.

Show comment
Hide comment
@rjbs

rjbs Mar 14, 2014

Collaborator
  • 806 dists can be "fixed" by having a row inserted to assign permissions, though there is no upload
  • 5 more are in a similar state, but do not have dist names that form valid package names; these can probably be inserted as permissions anyway; examples: Net--RabbitMQ, Text-Tabs+Wrap
  • 54 dists can't be obviously made good, and should be left alone for manual correction as needed
Collaborator

rjbs commented Mar 14, 2014

  • 806 dists can be "fixed" by having a row inserted to assign permissions, though there is no upload
  • 5 more are in a similar state, but do not have dist names that form valid package names; these can probably be inserted as permissions anyway; examples: Net--RabbitMQ, Text-Tabs+Wrap
  • 54 dists can't be obviously made good, and should be left alone for manual correction as needed
@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Mar 14, 2014

Contributor

Should we consider a more aggressive conversion from distname to distpkg? E.g.

s{\.pm$}{};
s{[+-]+}{::}g;

It might be nice to preserve the notion that the perms table is indexed on package names and not arbitrary strings.

Contributor

ghost commented Mar 14, 2014

Should we consider a more aggressive conversion from distname to distpkg? E.g.

s{\.pm$}{};
s{[+-]+}{::}g;

It might be nice to preserve the notion that the perms table is indexed on package names and not arbitrary strings.

@karenetheridge

This comment has been minimized.

Show comment
Hide comment
@karenetheridge

karenetheridge Mar 14, 2014

Contributor

On Fri, Mar 14, 2014 at 07:21:08AM -0700, David Golden wrote:

It might be nice to preserve the notion that the perms table is indexed on package names and not arbitrary strings.

+1000

Contributor

karenetheridge commented Mar 14, 2014

On Fri, Mar 14, 2014 at 07:21:08AM -0700, David Golden wrote:

It might be nice to preserve the notion that the perms table is indexed on package names and not arbitrary strings.

+1000

@andk

This comment has been minimized.

Show comment
Hide comment
@andk

andk Mar 15, 2014

Owner

d0476f4 needs to be reminded that we want to inform the user about what we did. Something in the spirit of 406a89d. All kinds of fails within pause processing need an error category and must find their way to the affected users.

Owner

andk commented Mar 15, 2014

d0476f4 needs to be reminded that we want to inform the user about what we did. Something in the spirit of 406a89d. All kinds of fails within pause processing need an error category and must find their way to the affected users.

@karenetheridge

This comment has been minimized.

Show comment
Hide comment
@karenetheridge

karenetheridge Mar 15, 2014

Contributor

On Fri, Mar 14, 2014 at 07:07:17AM -0700, Ricardo Signes wrote:

  • 5 more are in a similar state, but do not have dist names that form valid package names; these can probably be inserted as permissions anyway; examples: Net--RabbitMQ, Text-Tabs+Wrap

I don't like the idea of inserting entries in the index (can't remember the exact db table names) that don't conform to module syntax restrictions. It feels like this might cause confusion or issues later on.

Contributor

karenetheridge commented Mar 15, 2014

On Fri, Mar 14, 2014 at 07:07:17AM -0700, Ricardo Signes wrote:

  • 5 more are in a similar state, but do not have dist names that form valid package names; these can probably be inserted as permissions anyway; examples: Net--RabbitMQ, Text-Tabs+Wrap

I don't like the idea of inserting entries in the index (can't remember the exact db table names) that don't conform to module syntax restrictions. It feels like this might cause confusion or issues later on.

@rjbs

This comment has been minimized.

Show comment
Hide comment
@rjbs

rjbs Mar 15, 2014

Collaborator

new permissions to add at http://gist.github.com/9567098

Collaborator

rjbs commented Mar 15, 2014

new permissions to add at http://gist.github.com/9567098

rjbs added some commits Apr 14, 2013

correct some test output from index tests
This seems to have been left in a bad state from my last
hunk of work on it! :(
make all mldistwatch tests pass...
...even though I think some of them are wrong-headed atm.
be more aggressive in converting distname to distpkg
This fixes Net--RabbitMQ and Text-Tabs+Wrap
@rjbs

This comment has been minimized.

Show comment
Hide comment
@rjbs

rjbs Mar 16, 2014

Collaborator

This has been merged, after some problems related to DBH disconnection management. We did use @dagolden's suggestion on conversion, which got us just a few more dists not needing special casing.

Collaborator

rjbs commented Mar 16, 2014

This has been merged, after some problems related to DBH disconnection management. We did use @dagolden's suggestion on conversion, which got us just a few more dists not needing special casing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment