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

Ownership of files #3

Closed
gaborcsardi opened this issue Feb 25, 2014 · 10 comments · Fixed by #6
Closed

Ownership of files #3

gaborcsardi opened this issue Feb 25, 2014 · 10 comments · Fixed by #6

Comments

@gaborcsardi
Copy link

bdist_mpkg sets the group of the files in the mpkg, but it does not set the owner. So the files installed by mpkg might have a random owner.

~$ ls -l /Library/Python/2.7/site-packages/igraph/
total 4112
-rw-rw-r--   1 _coremediaiod  admin   175198 Feb 25 10:38 __init__.py
-rw-rw-r--   1 _coremediaiod  admin   165526 Feb 25 10:41 __init__.pyc
-rw-rw-r--   1 _coremediaiod  admin    59926 Feb 25 10:41 __init__.pyo
-rwxrwxr-x   1 _coremediaiod  admin  1076740 Feb 25 10:41 _igraph.so
drwxrwxr-x   8 gaborcsardi    admin      272 Feb 25 10:48 app
...

Also, I am not sure if setting the group to admin is the best practice, because other python packages have group wheel:

~$ ls -l /Library/Python/2.7/site-packages/
total 904
...
drwxr-xr-x   4 root           wheel             136 Nov 11 19:54 Pygments-1.6-py2.7.egg
drwxr-xr-x   4 root           wheel             136 Nov 11 18:15 epydoc-3.0.1-py2.7.egg
drwxr-xr-x   5 root           wheel             170 Nov 12 17:07 html2text-3.200.3-py2.7.egg
drwxrwxr-x  44 gaborcsardi    admin            1496 Feb 25 10:48 igraph
drwxr-xr-x   5 root           wheel             170 Feb  7 13:58 ipython-1.1.0-py2.7.egg
...
@matthew-brett
Copy link
Contributor

Sounds reasonable; you are suggesting set owner to root, and group to wheel?

@gaborcsardi
Copy link
Author

Yes, that's what I am suggesting. You can do this after creating the mpkg, with reown_mpkg, but still, bist_mpkg could do this as well. Btw. reown_mpkg could have root:wheel as default I guess, but that is a minor issue.

@matthew-brett
Copy link
Contributor

Hum. By default it seems that an 'Admin' user is a member of the 'admin' group, but not the 'wheel' group. This means that this will fail, by default, without sudo

chgrp wheel myfile

This will also fail:

chown root:wheel myfile

So - if I attempt these it means the bdist_mpkg command has to be run as sudo. That seems to strict to me. Other options:

  • leave as is
  • change default for reown_mpkg to root, wheel, requiring sudo by default for reown_mpkg
  • chown root:wheel if bdist_mpkg is run with sudo, warn otherwise

@gaborcsardi
Copy link
Author

Hmmm, it is already not optimal that the building user has to have admin rights, and it is even worse if one has to do sudo to build. But it seems like there is no other way to get proper user:group settings. I really don't know what is the good solution here.

@matthew-brett
Copy link
Contributor

@gaborcsardi
Copy link
Author

root:admin is fine with me as well. I just saw that the other Python packages were installed as root:wheel, that's why I thought that that's the standard.

Btw. you can try running 'Verify permissions' or 'Repair permissions' (or whatever it is called) on the Volume you install the package to, and then it will tell you if root:admin is inappropriate. Probably it is fine.

@gaborcsardi
Copy link
Author

Although you link points to a retired, outdated document.....

@matthew-brett
Copy link
Contributor

I think the best thing is to leave the current behavior as is, which is just to change the installable files to admin group if the building user has permission.

We can add a warning to run sudo reown_mpkg dist/yourpackage-version.mpkg to fix the permissions on the package.

The preserves backwards compatibility, doesn't require sudo for building packages, and at least warns the user about the permission problems.

Do you agree?

@gaborcsardi
Copy link
Author

Well, I am not happy, but what can we do? So yes, I agree.

It would be really nice to be able to build an mpkg on my build server, by a non-admin user, but looks like it is impossible right now. I am looking into building a dmg, that contains the mpkg, and modifying the user/group/permission in the dmg should be definitely possible, because it is just a file. But this is another issue.....

@gaborcsardi
Copy link
Author

Thanks for taking the time to work this out, btw.

matthew-brett added a commit to matthew-brett/bdist_mpkg that referenced this issue Mar 6, 2014
Suggest running `reown_mpkg` on the built archive. Closes MacPythongh-3 - thanks
to @gaborcsardi for pointing out the problem.
matthew-brett added a commit that referenced this issue Mar 10, 2014
RF: warn about permissions of archive

Suggest running reown_mpkg on the built archive. Closes gh-3 - thanks to
@gaborcsardi for pointing out the problem.
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

Successfully merging a pull request may close this issue.

2 participants