New --owner and --group for when they can't be read from the system #49
Conversation
With these new options, its now possible to use `metastore' when `xgetpwuid' and `xgetgrgid' don't work (for example on a server when the users aren't listed in `/etc/passwd' directly, but hidden somewhere on the network.
There was a problem hiding this comment.
Thank you, @mohammad-akhlaghi, for your first contribution to metastore.
In general it looks ok, but there are some issues I pointed out in code.
There is also one additional general issue, though. Your solution solves some use cases, for sure, but it may not be future-proof. Having something like --usermap and --groupmap (that are available in rsync) for unknown IDs would be much more robust.
I'm reluctant in merging not well thought out solutions from the get go, because they make it harder to introduce better solutions later. If we would have --usermap-for-unknown-uid and --groupmap-for-unknown-gid later, they would collide with --owner-for-unknown-uid and --group-for-unknown-gid. Users hate when features change and they have to relearn/rescript their usages, so we would not be able to easily remove these options later, etc.
I don't want to discourage you. Your work is appreciated, I'm just being honest that maintaining software (I know, I took back seat when it comes to metastore in recent years for various reason) is not always about easy short-term wins.
I may need to think more if having suboptimal solution won't be somewhat detrimental later. Maybe it's a risk we can take.
| " -O, --owner Owner to use when internal check fails.\n" | ||
| " -G, --group Group to use when internal check fails.\n" |
There was a problem hiding this comment.
I'm not sure I like this option names, they're too generic.
To avoid being misused, I would possibly not have short options for them, only long ones.
Descriptions are also too generic. Internal check is simply user/group ID existence in system's user/group database.
Suggestion:
--owner-for-unknown-uid- Owner to store when user ID does not exist in system database--group-for-unknown-gid- Group to store when group ID does
Why we need to be so specific? Here we're dealing with storing.
But there is another case, when dealing with applying, that we may want to handle in future.
Like --uid-for-unknown-owner and --gid-for-unknown-group (or maybe: --owner-for-unknown-owner and --group-for-unknown-group), and they are not necessarily meant to have same arguments.
Using same -O and -G options for both storing and applying would be error-prone.
| char *owner; /* user-provided owner. */ | ||
| char *group; /* user-provided group. */ |
There was a problem hiding this comment.
Wrong white space, please use tabs for indentation.
With these new options, its now possible to use metastore when
getpwuid' andgetgrgid' don't work (for example on a server when the user list isn't present in `/etc/passwd' directly, but hidden somewhere on the network. This problem actually occurred occured for me while trying to use metastore on a server and didn't allow me to use it. So it would probably be very useful for other users in such scenarios also.I just haven't added the man-page entry for these options because I don't have any experience in them. If you feel its good to merge with master, could you please add man page entries for it too?