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

ostree: move admindir to /etc/alternatives.admindir #135

Merged
merged 2 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ install:
[ -d $(DESTDIR)/$(SBINDIR) ] || mkdir -p $(DESTDIR)/$(SBINDIR)
[ -d $(DESTDIR)/$(MANDIR) ] || mkdir -p $(DESTDIR)/$(MANDIR)
[ -d $(DESTDIR)/$(MANDIR)/man8 ] || mkdir -p $(DESTDIR)/$(MANDIR)/man8
[ -d $(DESTDIR)/$(ALTDIR) ] || mkdir -p -m 755 $(DESTDIR)/$(ALTDIR)
[ -d $(DESTDIR)/$(ALTDATADIR) ] || mkdir -p -m 755 $(DESTDIR)/$(ALTDATADIR)
[ -d $(DESTDIR)/$(SYSTEMDUTILDIR) ] || mkdir -p -m 755 $(DESTDIR)/$(SYSTEMDUTILDIR)

Expand Down
2 changes: 2 additions & 0 deletions alternatives.8
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ A directory, by default
containing
.BR alternatives '
state information.
/etc/alternatives.admindir on OSTree-based systems.
.TP
link group
A set of related symlinks, intended to be updated as a group.
Expand Down Expand Up @@ -416,6 +417,7 @@ option.
.TP
.I /var/lib/alternatives/
The default administration directory.
/etc/alternatives.admindir on OSTree-based systems.
Can be overridden by the
.B --admindir
option.
Expand Down
34 changes: 30 additions & 4 deletions alternatives.c
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,15 @@ static int fileExists(char *path) {
return !stat(path, &sbuf);
}

static int dirExists(char *path) {
struct stat sbuf;

if (stat(path, &sbuf))
return 0;

return !!S_ISDIR(sbuf.st_mode);
}

static int facilityBelongsToUs(char *facility, const char *altDir) {
char buf[PATH_MAX];
if (readlink(facility, buf, sizeof(buf)) <= 0)
Expand Down Expand Up @@ -1321,6 +1330,10 @@ static int listServices(const char *altDir, const char *stateDir, int flags) {
return 0;
}

static int isOSTree() {
return fileExists("/run/ostree-booted") || isLink("/ostree");
}

int main(int argc, const char **argv) {
const char **nextArg;
char *end;
Expand All @@ -1331,8 +1344,7 @@ int main(int argc, const char **argv) {
struct alternative newAlt = {-1, {NULL, NULL, NULL}, NULL, NULL, 0, NULL};
int flags = 0;
char *altDir = "/etc/alternatives";
char *stateDir = "/var/lib/alternatives";
struct stat sb;
char *stateDir= NULL;
struct linkSet newSet = {NULL, NULL, NULL};

setlocale(LC_ALL, "");
Expand Down Expand Up @@ -1454,12 +1466,26 @@ int main(int argc, const char **argv) {
}
}

if (stat(altDir, &sb) || !S_ISDIR(sb.st_mode) || access(altDir, F_OK)) {
if (!dirExists(altDir)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The access() check should remain, I think.

Copy link
Member

Choose a reason for hiding this comment

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

We discussed this with @msekletar, and I don't think it is needed. With F_OK, it will just check if it exists, but that is already done through stat. If there had been W_OK, that would have been a different story.

Copy link
Contributor

Choose a reason for hiding this comment

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

True... But then it'd be better to remove it in a separate commit.

fprintf(stderr, _("altdir %s invalid\n"), altDir);
exit(2);
}

if (stat(stateDir, &sb) || !S_ISDIR(sb.st_mode) || access(stateDir, F_OK)) {
// if the stateDir is not explicitly set, we will use /var/lib/alternatives on normal systems
// and /etc/alternatives-admindir on OSTree systems, if the dir does not exist, we will create it
// if the stateDir is explicitly set, we will *not* try to create the dir and fail immediately if it does not exist
if (!stateDir) {
stateDir = "/var/lib/alternatives";
if (!dirExists(stateDir)) {
if (isOSTree())
stateDir = "/etc/alternatives-admindir";

if (mkdir(stateDir, 0755) < 0 && errno != EEXIST) {
fprintf(stderr, _("failed to create admindir: %s\n"), strerror(errno));
exit(2);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

access() check missing here too, in both branches.

Best to do that just once after the whole if block, I think (that is, beyond line 1491). Or even change the else if at 1488 back to if. It could mean a duplicate existence check, but I don't think this code is performance-critical... 😉

Copy link
Member

Choose a reason for hiding this comment

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

I already talked about the acccess and I like performant code :-)

} else if (!dirExists(stateDir)) {
fprintf(stderr, _("admindir %s invalid\n"), stateDir);
exit(2);
}
Expand Down
5 changes: 3 additions & 2 deletions chkconfig.spec
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Provides: /sbin/chkconfig
%description
Chkconfig is a basic system utility. It updates and queries runlevel
information for system services. Chkconfig manipulates the numerous
symbolic links in /etc/rc.d, to relieve system administrators of some
symbolic links in /etc/rc.d, to relieve system administrators of some
of the drudgery of manually editing the symbolic links.

%package -n ntsysv
Expand Down Expand Up @@ -95,11 +95,12 @@ mkdir -p $RPM_BUILD_ROOT/etc/chkconfig.d
%files -n alternatives
%license COPYING
%dir /etc/alternatives
%ghost %dir %attr(755, root, root) /etc/alternatives.admindir
%ghost %dir %attr(755, root, root) /var/lib/alternatives
%{_sbindir}/update-alternatives
%{_sbindir}/alternatives
%{_mandir}/*/update-alternatives*
%{_mandir}/*/alternatives*
%dir /var/lib/alternatives

%changelog
* Tue Jul 30 2024 Jan Macku <jamacku@redhat.com> - 1.29-1
Expand Down
Loading