Added JPEGPhoto and createhomedir #14
Conversation
| @@ -36,16 +35,20 @@ def main(): | |||
| required_user_options = optparse.OptionGroup( | |||
| parser, 'Required User Options') | |||
| required_user_options.add_option( | |||
| '--name', '-n', help='User shortname. REQUIRED.') | |||
| '--name', '-n', | |||
| help='User shortname. REQUIRED.') | |||
| required_user_options.add_option( | |||
There was a problem hiding this comment.
You've made a bunch of changes unrelated to your core changes. This makes reviewing your changes harder.
There was a problem hiding this comment.
sorry about that, thought it would be easier to read
createuserpkg
Outdated
| @@ -119,7 +135,10 @@ def main(): | |||
| user_data['shell'] = options.shell | |||
| if options.hidden: | |||
| user_data['IsHidden'] = u'YES' | |||
|
|
|||
| if options.picture: | |||
| from locallibs import imageutil | |||
There was a problem hiding this comment.
imports really belong at the top of a Python file unless you have a really good reason to do them conditionally.
There was a problem hiding this comment.
I thought i only import it if there is a image as argument, but i will put it on top
locallibs/imageutil.py
Outdated
| image_path, | ||
| temp_image) | ||
| try: | ||
| subprocess.call(cmd, shell=True) |
There was a problem hiding this comment.
Please don't use shell=True with subprocess. Properly format your command arguments into a list and use the command list with subprocess.call.
locallibs/userpkg.py
Outdated
| @@ -88,6 +89,9 @@ | |||
| /usr/bin/killall DirectoryService 2>/dev/null || /usr/bin/killall opendirectoryd 2>/dev/null | |||
| """ | |||
|
|
|||
| PI_CREATEHOMEDIR = """ | |||
| /usr/sbin/createhomedir -c | |||
| """ | |||
There was a problem hiding this comment.
Using createhomedir will only work when a package is installed to the current boot volume. All the other options for pycreateuserpkg work no matter if the package target is the current boot volume or an alternate volume (as when installing into an AutoDMG image, or via a Imagr or DeployStudio or Bootstrappr workflow). I'm hesitant to merge a feature that works only sometimes.
There was a problem hiding this comment.
In the latest 10.13.4 beta 4 there is also a new behavior to createmobileaccount that prompts in the cli run for an admin account to set the SecureToken. There is a -D option to suppress it. There should be some logic to detect the OS and act accordingly.
-Eric
There was a problem hiding this comment.
createmobileaccount and createhomedir are different tools; I don't know if there are similar new issues in 10.13.4 that would affect createhomedir.
There was a problem hiding this comment.
I will put in into the if block, under the killall statements
- Imports of modules on top - Changed to subprocess call - Put the createhomedir into the if-block
| @@ -88,6 +88,9 @@ | |||
| /usr/bin/killall DirectoryService 2>/dev/null || /usr/bin/killall opendirectoryd 2>/dev/null | |||
| """ | |||
|
|
|||
| PI_CREATEHOMEDIR = """ | |||
| /usr/sbin/createhomedir -c | |||
| """ | |||
There was a problem hiding this comment.
I don't think you've addressed my issue with createhomedir not working for anything other than installs to /.
I think the postinstall script bit needs to test $3 and do the right thing, and I think the overall createuserpkg tool needs to warn people in the help output that this option works only in some scenarios; etc.
There was a problem hiding this comment.
I will add the createhomedir command to the pi_live_actions, so it gets written into the if [ "$3" == "/" ] block. Additionally i will add a notice to the help text that says that it only work an a boot volume.
Would that be ok?
Thanks again
Gerd
|
Have you had time to look at my latest pull request Thanks in advance |
|
Not yet, no. |
|
Sorry, coming back to this now after a billion-year break; did you want to still work on merging this? |
--createhome creates the user's home directory and is largely based on the work and discussion in gregneagle#14 Changes for this option are to the OptionParser and creation of pkg_data in createuserpkg, make_config_file in userpkg.py and the postinstall script The postinsstall script now checks for this option and if found runs: /usr/sbin/createhomedir -c -u $USERNAME --key will add the user's public key to ~/.ssh/authorized_keys. Specifying --key implies --createhome. Changes for this option are in the same locations as --createhome The postinstall script will add the key to the user's authorized_keys file, creating it (and ~/.ssh) if it doesn't exist. It will also set permissions on the files. These commands are run as the user in order to expand ~ and pick up the user's GID when the files are created. This seems to require the use of eval which means you need to trust the content of the --key variable. If there is a safer way to do this that would be a good change.
My first try to build a "real" python program, so please be patient :)
I've added the support for creating JPEGPhoto, also added support to create the home directory
Thanks you!
best regards
Gerd