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.

Sign up for GitHub

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

New --owner and --group for when they can't be read from the system #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

New --owner and --group for when they can't be read from the system #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions src/metaentry.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ mentries_print(const struct metahash *mhash)

/* Creates a metaentry for the file/dir/etc at path */
struct metaentry *
mentry_create(const char *path)
mentry_create(const char *path, msettings *st)
{
#if !defined(NO_XATTR) || !(NO_XATTR+0)
ssize_t lsize, vsize;
Expand All @@ -214,24 +214,26 @@ mentry_create(const char *path)
}

pbuf = xgetpwuid(sbuf.st_uid);
if (!pbuf) {
msg(MSG_ERROR, "getpwuid failed for %s: uid %i not found\n",
if (!pbuf && !st->owner) {
msg(MSG_ERROR, "getpwuid failed for %s: uid %i not found, "
"you can use `--owner' (`-O') to manually specify it.\n",
path, (int)sbuf.st_uid);
return NULL;
}

gbuf = xgetgrgid(sbuf.st_gid);
if (!gbuf) {
msg(MSG_ERROR, "getgrgid failed for %s: gid %i not found\n",
if (!gbuf && !st->group) {
msg(MSG_ERROR, "getgrgid failed for %s: gid %i not found, "
"you can use `--group' (`-G') to manually specify it.\n",
path, (int)sbuf.st_gid);
return NULL;
}

mentry = mentry_alloc();
mentry->path = xstrdup(path);
mentry->pathlen = strlen(mentry->path);
mentry->owner = xstrdup(pbuf->pw_name);
mentry->group = xstrdup(gbuf->gr_name);
mentry->owner = pbuf ? xstrdup(pbuf->pw_name) : st->owner;
mentry->group = gbuf ? xstrdup(gbuf->gr_name) : st->group;
mentry->mode = sbuf.st_mode & 0177777;
mentry->mtime = sbuf.st_mtim.tv_sec;
mentry->mtimensec = sbuf.st_mtim.tv_nsec;
Expand Down Expand Up @@ -359,7 +361,7 @@ mentries_recurse(const char *path, struct metahash *mhash, msettings *st)
return;
}

mentry = mentry_create(path);
mentry = mentry_create(path, st);
if (!mentry)
return;

Expand Down
2 changes: 1 addition & 1 deletion src/metaentry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ struct metahash {
};

/* Create a metaentry for the file/dir/etc at path */
struct metaentry *mentry_create(const char *path);
struct metaentry *mentry_create(const char *path, msettings *st);

/* Recurses opath and adds metadata entries to the metaentry list */
void mentries_recurse_path(const char *opath, struct metahash **mhash,
Expand Down
17 changes: 13 additions & 4 deletions src/metastore.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
/* metastore settings */
static struct metasettings settings = {
.metafile = METAFILE,
.owner = NULL,
.group = NULL,
.do_mtime = false,
.do_emptydirs = false,
.do_removeemptydirs = false,
Expand Down Expand Up @@ -285,7 +287,7 @@ compare_fix(struct metaentry *real, struct metaentry *stored, int cmp)
* recreating them.
*/
static void
fixup_emptydirs(void)
fixup_emptydirs(msettings *st)
{
struct metaentry *entry;
struct metaentry *cur;
Expand Down Expand Up @@ -363,7 +365,7 @@ fixup_emptydirs(void)
}
msg(MSG_QUIET, "ok\n");

new = mentry_create(cur->path);
new = mentry_create(cur->path, st);
if (!new) {
msg(MSG_QUIET, "Failed to get metadata for %s\n", cur->path);
continue;
Expand Down Expand Up @@ -462,6 +464,8 @@ usage(const char *arg0, const char *message)
" -E, --remove-empty-dirs Remove extra empty directories\n"
" -g, --git Do not omit .git directories\n"
" -f, --file=FILE Set metadata file (" METAFILE " by default)\n"
" -O, --owner Owner to use when internal check fails.\n"
" -G, --group Group to use when internal check fails.\n"
Comment on lines +467 to +468
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

);

exit(message ? EXIT_FAILURE : EXIT_SUCCESS);
Expand All @@ -482,6 +486,8 @@ static struct option long_options[] = {
{ "remove-empty-dirs", no_argument, NULL, 'E' },
{ "git", no_argument, NULL, 'g' },
{ "file", required_argument, NULL, 'f' },
{ "owner", required_argument, NULL, 'O' },
{ "group", required_argument, NULL, 'G' },
{ NULL, 0, NULL, 0 }
};

Expand All @@ -498,7 +504,7 @@ main(int argc, char **argv)
i = 0;
while (1) {
int option_index = 0;
c = getopt_long(argc, argv, "csadVhvqmeEgf:",
c = getopt_long(argc, argv, "csadVhvqmeEgf:O:G:",
long_options, &option_index);
if (c == -1)
break;
Expand All @@ -517,11 +523,14 @@ main(int argc, char **argv)
break;
case 'g': /* git */ settings.do_git = true; break;
case 'f': /* file */ settings.metafile = optarg; break;
case 'O': /* owner */ settings.owner = optarg; break;
case 'G': /* group */ settings.group = optarg; break;
default:
usage(argv[0], "unknown option");
}
}


/* Make sure only one action is specified */
if (i != 1)
usage(argv[0], "incorrect option(s)");
Expand Down Expand Up @@ -573,7 +582,7 @@ main(int argc, char **argv)
case ACTION_APPLY:
mentries_compare(real, stored, compare_fix, &settings);
if (settings.do_emptydirs)
fixup_emptydirs();
fixup_emptydirs(&settings);
if (settings.do_removeemptydirs)
fixup_newemptydirs();
break;
Expand Down
2 changes: 2 additions & 0 deletions src/settings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
/* Data structure to hold metastore settings */
struct metasettings {
char *metafile; /* path to the file containing the metadata */
char *owner; /* user-provided owner. */
char *group; /* user-provided group. */
Comment on lines +26 to +27
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong white space, please use tabs for indentation.

bool do_mtime; /* should mtimes be corrected? */
bool do_emptydirs; /* should empty dirs be recreated? */
bool do_removeemptydirs; /* should new empty dirs be removed? */
Expand Down