[PR PATCH] base-files: remove non-portable behaviour

[PR PATCH] base-files: remove non-portable behaviour

[paper42]

[-- Attachment #1: Type: text/plain, Size: 526 bytes --]

There is a new pull request by paper42 against master on the void-packages repository

https://github.com/paper42/void-packages base-files-posix-sh
https://github.com/void-linux/void-packages/pull/33088

base-files: remove non-portable behaviour
On some minimal systems like OpenWrt, the install command doesn't exist
which results in xbps not creating the required directories when creating
a void chroot.

This PR needs testing.

A patch file from https://github.com/void-linux/void-packages/pull/33088.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-base-files-posix-sh-33088.patch --]
[-- Type: text/x-diff, Size: 3927 bytes --]

From afea35b6452d7798df25f2d90b819f8e4ce71181 Mon Sep 17 00:00:00 2001
From: Michal Vasilek <michal@vasilek.cz>
Date: Sat, 18 Sep 2021 21:27:09 +0200
Subject: [PATCH] base-files: remove non-portable behaviour

On some minimal systems like OpenWrt, the install command doesn't exist
which results in xbps not creating the required directories when creating
a void chroot.
---
 srcpkgs/base-files/INSTALL  | 49 ++++++++++++++++++-------------------
 srcpkgs/base-files/template |  2 +-
 2 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/srcpkgs/base-files/INSTALL b/srcpkgs/base-files/INSTALL
index 85a7f92cd4ee..e3b01ab9bb36 100644
--- a/srcpkgs/base-files/INSTALL
+++ b/srcpkgs/base-files/INSTALL
@@ -4,58 +4,57 @@ make_system_dirs() {
 	#
 	for d in boot etc etc/modprobe.d etc/modules-load.d \
 		etc/skel home dev proc usr mnt opt sys media var run/lock; do
-		[ ! -d ${d} ] && install -d ${d}
+		mkdir -p ${d}
 	done
 
-	[ ! -d root ] && install -dm750 root
+	mkdir -p root && chmod 750 root
 
 	# Don't try to create var/mail in the correct place if the user
 	# is updating from an old system that has var/mail as a symlink
-	[ ! -L var/mail ] && [ ! -d var/mail ] && install -dm1777 var/mail
+	[ ! -L var/mail ] && mkdir -p var/mail && chmod 1777 var/mail
 
-	[ ! -d var/spool ] && install -d var/spool
+	mkdir -p var/spool
 
 	for d in local local/bin local/sbin local/include local/lib \
 		bin include lib src; do
-		[ ! -d usr/${d} ] && install -d usr/${d}
+		mkdir -p usr/${d}
 	done
 
 	for d in locale misc terminfo zoneinfo doc info; do
-		[ ! -d usr/share/${d} ] && install -d usr/share/${d}
-		[ ! -d usr/local/share/${d} ] && install -d usr/local/share/${d}
+		mkdir -p usr/share/${d}
+		mkdir -p usr/local/share/${d}
 	done
 
 	for d in 1 2 3 4 5 6 7 8; do
-		[ ! -d usr/share/man/man${d} ] && \
-			install -d usr/share/man/man${d}
-		[ ! -d usr/local/share/man/man${d} ] && \
-			install -d usr/local/share/man/man${d}
+		mkdir -p usr/share/man/man${d}
+		mkdir -p usr/local/share/man/man${d}
 	done
 
 	for d in empty log opt cache lib; do
-		[ ! -d var/${d} ] && install -d var/${d}
+		mkdir -p var/${d}
 	done
 
 	# Create /var/run and /var/lock symlinks.
 	for d in run lock; do
-		if [ ! -h "var/$d" -a -d var/${d} ]; then
+		if [ ! -h "var/$d" ] && [ -d var/${d} ]; then
 			echo "/${d} must not be a directory, exiting!"
 			exit 1
 		fi
 	done
 
-	cd var
-	ln -sf ../run .
-	ln -sf ../run/lock .
-	[ ! -d spool/mail ] && ln -sfn ../mail spool/mail
-	if [ -L spool/mail/mail -a "$(readlink spool/mail/mail)" = spool/mail ]; then
-		# Get rid of broken symlink created by older versions of base-files.
-		rm spool/mail/mail
-	fi
-	cd ..
+	(
+		cd var || return
+		ln -sf ../run .
+		ln -sf ../run/lock .
+		[ ! -d spool/mail ] && ln -sfn ../mail spool/mail
+		if [ -L spool/mail/mail ] && [ "$(readlink spool/mail/mail)" = spool/mail ]; then
+			# Get rid of broken symlink created by older versions of base-files.
+			rm spool/mail/mail
+		fi
+	)
 
-	install -dm1777 tmp
-	install -dm1777 var/tmp
+	mkdir -p tmp && chmod 1777 tmp
+	mkdir -p var/tmp && chmod 1777 var/tmp
 
 	# remove leftover polkit rules from live systems
 	[ -e etc/polkit-1/rules.d/void-live.rules ] && rm etc/polkit-1/rules.d/void-live.rules
@@ -73,7 +72,7 @@ post)
 	echo "Creating system directories/symlinks..."
 	make_system_dirs
 	# Enable shadow passwd/groups.
-	if [ -x bin/pwconv -a -x bin/grpconv -a "$(id -u)" -eq 0 ]; then
+	if [ -x bin/pwconv ] && [ -x bin/grpconv ] && [ "$(id -u)" -eq 0 ]; then
 		pwconv && grpconv
 	fi
 	;;
diff --git a/srcpkgs/base-files/template b/srcpkgs/base-files/template
index 758cd54ad147..6dddef3ef43f 100644
--- a/srcpkgs/base-files/template
+++ b/srcpkgs/base-files/template
@@ -1,7 +1,7 @@
 # Template file for 'base-files'
 pkgname=base-files
 version=0.142
-revision=11
+revision=12
 bootstrap=yes
 depends="xbps-triggers"
 short_desc="Void Linux base system files"

Re: [PR REVIEW] base-files: remove non-portable behaviour

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 194 bytes --]

New review comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#discussion_r715657880

Comment:
Nit: I think this change merits a version bump.

Re: base-files: remove non-portable behaviour

[q66]

[-- Attachment #1: Type: text/plain, Size: 197 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926667706

Comment:
sounds like a problem with openwrt rather than with our file

Re: base-files: remove non-portable behaviour

[q66]

[-- Attachment #1: Type: text/plain, Size: 230 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926668372

Comment:
also why do you mess with the conditionals? calling `test` once is better than multiple times

Re: base-files: remove non-portable behaviour

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 411 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926669597

Comment:
But yeah, partly agree with q66 here. It should be possible to obtain `install(1)` from somewhere, busybox has one, at least. If we use weird options that can be fixed, though.

And the requirement is currently documented in the README:

> install(1) - GNU coreutils

Re: base-files: remove non-portable behaviour

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 534 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926669597

Comment:
But yeah, partly agree with q66 here. It should be possible to obtain `install(1)` from somewhere, busybox has one, at least. If we use weird options that can be fixed, though.

And the requirement is currently documented in the README:

> install(1) - GNU coreutils

Hmm, that requirement is documented here, not for system installation. That could be documented in void-docs too, then.

Re: base-files: remove non-portable behaviour

[q66]

[-- Attachment #1: Type: text/plain, Size: 185 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926671761

Comment:
it's also compatible with BSD-style `install(1)`

Re: base-files: remove non-portable behaviour

[q66]

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926671761

Comment:
it's also compatible with BSD-style `install(1)` (the main difference between these is the behavior of `-D`, but that's not used here)

Re: base-files: remove non-portable behaviour

[paper42]

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

New comment by paper42 on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926673388

Comment:
> sounds like a problem with openwrt rather than with our file

The install command is common, but not always available everywhere (it's not required by POSIX), so I think we should avoid it in base-files. On routers which often have a very limited space, a command that doesn't make sense to be used on small routers like install shouldn't be installed. Bigger, more powerful routers also run the same version of OpenWrt maybe with some extra packages, but they still use the same busybox.

> also why do you mess with the conditionals? calling test once is better than multiple times

The last change? That's a mistake that I shouldn't have pushed.

Re: base-files: remove non-portable behaviour

[q66]

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926674576

Comment:
there's multiple conditionals that you changed from `[ x -a y ]` to `[ x ] && [ y ]` which is a worse way to write it in general

Re: [PR PATCH] [Updated] base-files: remove non-portable behaviour

[paper42]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

There is an updated pull request by paper42 against master on the void-packages repository

https://github.com/paper42/void-packages base-files-posix-sh
https://github.com/void-linux/void-packages/pull/33088

base-files: remove non-portable behaviour
On some minimal systems like OpenWrt, the install command doesn't exist
which results in xbps not creating the required directories when creating
a void chroot.

This PR needs testing.

A patch file from https://github.com/void-linux/void-packages/pull/33088.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-base-files-posix-sh-33088.patch --]
[-- Type: text/x-diff, Size: 3625 bytes --]

From 5ccf29dda562c835409fff6e365396b381fda0ed Mon Sep 17 00:00:00 2001
From: Michal Vasilek <michal@vasilek.cz>
Date: Sat, 18 Sep 2021 21:27:09 +0200
Subject: [PATCH] base-files: remove non-portable behaviour

On some minimal systems like OpenWrt, the install command doesn't exist
which results in xbps not creating the required directories when creating
a void chroot.
---
 srcpkgs/base-files/INSTALL  | 47 ++++++++++++++++++-------------------
 srcpkgs/base-files/template |  2 +-
 2 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/srcpkgs/base-files/INSTALL b/srcpkgs/base-files/INSTALL
index 85a7f92cd4ee..fe0abe08d401 100644
--- a/srcpkgs/base-files/INSTALL
+++ b/srcpkgs/base-files/INSTALL
@@ -4,58 +4,57 @@ make_system_dirs() {
 	#
 	for d in boot etc etc/modprobe.d etc/modules-load.d \
 		etc/skel home dev proc usr mnt opt sys media var run/lock; do
-		[ ! -d ${d} ] && install -d ${d}
+		mkdir -p ${d}
 	done
 
-	[ ! -d root ] && install -dm750 root
+	mkdir -p root && chmod 750 root
 
 	# Don't try to create var/mail in the correct place if the user
 	# is updating from an old system that has var/mail as a symlink
-	[ ! -L var/mail ] && [ ! -d var/mail ] && install -dm1777 var/mail
+	[ ! -L var/mail ] && mkdir -p var/mail && chmod 1777 var/mail
 
-	[ ! -d var/spool ] && install -d var/spool
+	mkdir -p var/spool
 
 	for d in local local/bin local/sbin local/include local/lib \
 		bin include lib src; do
-		[ ! -d usr/${d} ] && install -d usr/${d}
+		mkdir -p usr/${d}
 	done
 
 	for d in locale misc terminfo zoneinfo doc info; do
-		[ ! -d usr/share/${d} ] && install -d usr/share/${d}
-		[ ! -d usr/local/share/${d} ] && install -d usr/local/share/${d}
+		mkdir -p usr/share/${d}
+		mkdir -p usr/local/share/${d}
 	done
 
 	for d in 1 2 3 4 5 6 7 8; do
-		[ ! -d usr/share/man/man${d} ] && \
-			install -d usr/share/man/man${d}
-		[ ! -d usr/local/share/man/man${d} ] && \
-			install -d usr/local/share/man/man${d}
+		mkdir -p usr/share/man/man${d}
+		mkdir -p usr/local/share/man/man${d}
 	done
 
 	for d in empty log opt cache lib; do
-		[ ! -d var/${d} ] && install -d var/${d}
+		mkdir -p var/${d}
 	done
 
 	# Create /var/run and /var/lock symlinks.
 	for d in run lock; do
-		if [ ! -h "var/$d" -a -d var/${d} ]; then
+		if [ ! -h "var/$d" ] && [ -d var/${d} ]; then
 			echo "/${d} must not be a directory, exiting!"
 			exit 1
 		fi
 	done
 
-	cd var
-	ln -sf ../run .
-	ln -sf ../run/lock .
-	[ ! -d spool/mail ] && ln -sfn ../mail spool/mail
-	if [ -L spool/mail/mail -a "$(readlink spool/mail/mail)" = spool/mail ]; then
-		# Get rid of broken symlink created by older versions of base-files.
-		rm spool/mail/mail
-	fi
-	cd ..
+	(
+		cd var || return
+		ln -sf ../run .
+		ln -sf ../run/lock .
+		[ ! -d spool/mail ] && ln -sfn ../mail spool/mail
+		if [ -L spool/mail/mail -a "$(readlink spool/mail/mail)" = spool/mail ]; then
+			# Get rid of broken symlink created by older versions of base-files.
+			rm spool/mail/mail
+		fi
+	)
 
-	install -dm1777 tmp
-	install -dm1777 var/tmp
+	mkdir -p tmp && chmod 1777 tmp
+	mkdir -p var/tmp && chmod 1777 var/tmp
 
 	# remove leftover polkit rules from live systems
 	[ -e etc/polkit-1/rules.d/void-live.rules ] && rm etc/polkit-1/rules.d/void-live.rules
diff --git a/srcpkgs/base-files/template b/srcpkgs/base-files/template
index 758cd54ad147..6dddef3ef43f 100644
--- a/srcpkgs/base-files/template
+++ b/srcpkgs/base-files/template
@@ -1,7 +1,7 @@
 # Template file for 'base-files'
 pkgname=base-files
 version=0.142
-revision=11
+revision=12
 bootstrap=yes
 depends="xbps-triggers"
 short_desc="Void Linux base system files"

Re: base-files: remove non-portable behaviour

[paper42]

[-- Attachment #1: Type: text/plain, Size: 280 bytes --]

New comment by paper42 on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926677623

Comment:
> there's multiple conditionals that you changed from `[ x -a y ]` to `[ x ] && [ y ]` which is a worse way to write it in general

fixed

Re: base-files: remove non-portable behaviour

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926679487

Comment:
Is this the only thing standing in the way of bootstrapping a void install on a vanilla OpenWRT system?

Re: [PR REVIEW] base-files: remove non-portable behaviour

[q66]

[-- Attachment #1: Type: text/plain, Size: 189 bytes --]

New review comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#discussion_r715670555

Comment:
what's the point of wrapping this whole block?

Re: [PR REVIEW] base-files: remove non-portable behaviour

[q66]

[-- Attachment #1: Type: text/plain, Size: 166 bytes --]

New review comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#discussion_r715670821

Comment:
you still left one here

Re: base-files: remove non-portable behaviour

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

New comment by ahesford on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926685501

Comment:
Regardless of the availability of `install`, I generally prefer things like `mkdir` especially when the permissions are don't-care conditions. One can argue that `install` is preferable in the cases where you want specific permissions, but there are only two here and you've already shown that dropping `install` worksa round at least one issue.

You may want to set a umask in this function, since `install` guarantees a default mode of `755` but the `mkdir` might inherit a umask from the environment.

Re: base-files: remove non-portable behaviour

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

New comment by ahesford on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926685501

Comment:
Regardless of the availability of `install`, I generally prefer things like `mkdir` especially when the permissions are don't-care conditions. One can argue that `install` is preferable in the cases where you want specific permissions, but there are only two here and you've already shown that dropping `install` works around at least one issue.

You may want to set a umask in this function, since `install` guarantees a default mode of `755` but the `mkdir` might inherit a umask from the environment.

Re: base-files: remove non-portable behaviour

[paper42]

[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]

New comment by paper42 on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-926695350

Comment:
> Is this the only thing standing in the way of bootstrapping a void install on a vanilla OpenWRT system?

It can be already bootstrapped without this change, but the installation will be slightly broken. I had to reconfigure base-files from inside the chroot and then everything works fine.

I am also getting a warning that's not fixed by this PR:
```
base-files-0.142_11: configuring ...
./usr/libexec/xbps-triggers/system-accounts: 20: cannot create /dev/null: Directory nonexistent
./usr/libexec/xbps-triggers/system-accounts: 67: cannot create /dev/null: Directory nonexistent
./usr/libexec/xbps-triggers/system-accounts: 71: cannot create /dev/null: Directory nonexistent
./usr/libexec/xbps-triggers/system-accounts: 38: cannot create /dev/null: Directory nonexistent
WARNING: cannot create kvm:24 system group (missing groupadd)
The following group must be created manually: kvm:24
```
I am not sure what's happening here, because running the commands manually works fine, `groupadd` exists and `command -v groupadd` works fine. I will look into it further.

Re: [PR REVIEW] base-files: remove non-portable behaviour

[paper42]

[-- Attachment #1: Type: text/plain, Size: 290 bytes --]

New review comment by paper42 on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#discussion_r715691117

Comment:
When `cd var` fails, we run `cd ..` which could go out of the chroot. I didn't observe this on a real system, but it sounds like a possibility.

Re: [PR REVIEW] base-files: remove non-portable behaviour

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 206 bytes --]

New review comment by ahesford on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#discussion_r715692590

Comment:
Limiting a PWD change to a subshell is always a good idea.

Re: [PR PATCH] [Updated] base-files: remove non-portable behaviour

[paper42]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

There is an updated pull request by paper42 against master on the void-packages repository

https://github.com/paper42/void-packages base-files-posix-sh
https://github.com/void-linux/void-packages/pull/33088

base-files: remove non-portable behaviour
On some minimal systems like OpenWrt, the install command doesn't exist
which results in xbps not creating the required directories when creating
a void chroot.

This PR needs testing.

A patch file from https://github.com/void-linux/void-packages/pull/33088.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-base-files-posix-sh-33088.patch --]
[-- Type: text/x-diff, Size: 3554 bytes --]

From 1d3edc9017a69c26bf1457f04b51c8e17b564694 Mon Sep 17 00:00:00 2001
From: Michal Vasilek <michal@vasilek.cz>
Date: Sat, 18 Sep 2021 21:27:09 +0200
Subject: [PATCH] base-files: remove non-portable behaviour

On some minimal systems like OpenWrt, the install command doesn't exist
which results in xbps not creating the required directories when creating
a void chroot.
---
 srcpkgs/base-files/INSTALL  | 46 ++++++++++++++++++-------------------
 srcpkgs/base-files/template |  4 ++--
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/srcpkgs/base-files/INSTALL b/srcpkgs/base-files/INSTALL
index 85a7f92cd4ee..29e8c7412684 100644
--- a/srcpkgs/base-files/INSTALL
+++ b/srcpkgs/base-files/INSTALL
@@ -2,38 +2,37 @@ make_system_dirs() {
 	#
 	# Install FHS system directories.
 	#
+	umask 022
 	for d in boot etc etc/modprobe.d etc/modules-load.d \
 		etc/skel home dev proc usr mnt opt sys media var run/lock; do
-		[ ! -d ${d} ] && install -d ${d}
+		mkdir -p ${d}
 	done
 
-	[ ! -d root ] && install -dm750 root
+	mkdir -p root && chmod 750 root
 
 	# Don't try to create var/mail in the correct place if the user
 	# is updating from an old system that has var/mail as a symlink
-	[ ! -L var/mail ] && [ ! -d var/mail ] && install -dm1777 var/mail
+	[ ! -L var/mail ] && mkdir -p var/mail && chmod 1777 var/mail
 
-	[ ! -d var/spool ] && install -d var/spool
+	mkdir -p var/spool
 
 	for d in local local/bin local/sbin local/include local/lib \
 		bin include lib src; do
-		[ ! -d usr/${d} ] && install -d usr/${d}
+		mkdir -p usr/${d}
 	done
 
 	for d in locale misc terminfo zoneinfo doc info; do
-		[ ! -d usr/share/${d} ] && install -d usr/share/${d}
-		[ ! -d usr/local/share/${d} ] && install -d usr/local/share/${d}
+		mkdir -p usr/share/${d}
+		mkdir -p usr/local/share/${d}
 	done
 
 	for d in 1 2 3 4 5 6 7 8; do
-		[ ! -d usr/share/man/man${d} ] && \
-			install -d usr/share/man/man${d}
-		[ ! -d usr/local/share/man/man${d} ] && \
-			install -d usr/local/share/man/man${d}
+		mkdir -p usr/share/man/man${d}
+		mkdir -p usr/local/share/man/man${d}
 	done
 
 	for d in empty log opt cache lib; do
-		[ ! -d var/${d} ] && install -d var/${d}
+		mkdir -p var/${d}
 	done
 
 	# Create /var/run and /var/lock symlinks.
@@ -44,18 +43,19 @@ make_system_dirs() {
 		fi
 	done
 
-	cd var
-	ln -sf ../run .
-	ln -sf ../run/lock .
-	[ ! -d spool/mail ] && ln -sfn ../mail spool/mail
-	if [ -L spool/mail/mail -a "$(readlink spool/mail/mail)" = spool/mail ]; then
-		# Get rid of broken symlink created by older versions of base-files.
-		rm spool/mail/mail
-	fi
-	cd ..
+	(
+		cd var || return
+		ln -sf ../run .
+		ln -sf ../run/lock .
+		[ ! -d spool/mail ] && ln -sfn ../mail spool/mail
+		if [ -L spool/mail/mail -a "$(readlink spool/mail/mail)" = spool/mail ]; then
+			# Get rid of broken symlink created by older versions of base-files.
+			rm spool/mail/mail
+		fi
+	)
 
-	install -dm1777 tmp
-	install -dm1777 var/tmp
+	mkdir -p tmp && chmod 1777 tmp
+	mkdir -p var/tmp && chmod 1777 var/tmp
 
 	# remove leftover polkit rules from live systems
 	[ -e etc/polkit-1/rules.d/void-live.rules ] && rm etc/polkit-1/rules.d/void-live.rules
diff --git a/srcpkgs/base-files/template b/srcpkgs/base-files/template
index 758cd54ad147..06022fadb48d 100644
--- a/srcpkgs/base-files/template
+++ b/srcpkgs/base-files/template
@@ -1,7 +1,7 @@
 # Template file for 'base-files'
 pkgname=base-files
-version=0.142
-revision=11
+version=0.143
+revision=1
 bootstrap=yes
 depends="xbps-triggers"
 short_desc="Void Linux base system files"

Re: [PR REVIEW] base-files: remove non-portable behaviour

[CameronNemo]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

New review comment by CameronNemo on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#discussion_r720928121

Comment:
But this does not do the same thing. Previously if the root already existed with permissions of (for example) 0700, it would be left alone. Now you are forcibly changing the mode regardless of whether it already exists or not.

Also FWIW busybox and toybox mkdir support the `-m` flag. I would rather just use that as it does exactly what we want here: leaves the mode alone if the directory already exists.

My comment here applies for all of the mkdir && chmod lines you added, except the last two. I guess for the tmp dirs it is desired to always set the mode to 1777. So you can leave those two lines how you have them.

Re: [PR REVIEW] base-files: remove non-portable behaviour

[CameronNemo]

[-- Attachment #1: Type: text/plain, Size: 784 bytes --]

New review comment by CameronNemo on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#discussion_r720928121

Comment:
But this does not do the same thing. Previously if the "root" dir already existed with permissions of (for example) 0700, it would be left alone. Now you are forcibly changing the mode regardless of whether it already exists or not.

Also FWIW busybox and toybox mkdir support the `-m` flag. I would rather just use that as it does exactly what we want here: leaves the mode alone if the directory already exists.

My comment here applies for all of the mkdir && chmod lines you added, except the last two. I guess for the tmp dirs it is desired to always set the mode to 1777. So you can leave those two lines how you have them.

Re: base-files: remove non-portable behaviour

[CameronNemo]

[-- Attachment #1: Type: text/plain, Size: 661 bytes --]

New comment by CameronNemo on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-933039843

Comment:
>I had to reconfigure base-files from inside the chroot and then everything works fine.

FWIW I would recommend *always* re-configuring base-files from within the chroot.

>I am not sure what's happening here, because running the commands manually works fine

Uh did you mount /dev into the chroot? You need to have /dev and /proc in the chroot.

When I was not mounting /proc into my chroot, I saw breakage with the GNU install command: https://lists.gnu.org/archive/html/bug-coreutils/2020-09/msg00009.html

Re: base-files: remove non-portable behaviour

[CameronNemo]

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

New comment by CameronNemo on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-933039843

Comment:
>I had to reconfigure base-files from inside the chroot and then everything works fine.

FWIW I would recommend *always* re-configuring base-files from within the chroot.

>I am not sure what's happening here, because running the commands manually works fine

Uh did you mount /dev into the chroot? You need to have /dev and /proc in the chroot.

When I was not mounting /proc into my chroot, I saw breakage with the GNU install command: 

>mounting /proc may be your best bet. I vaguely recall there are other places in glibc that assume /proc

-- Paul Eggert, 2020

https://lists.gnu.org/archive/html/bug-coreutils/2020-09/msg00009.html

Re: base-files: remove non-portable behaviour

[github-actions]

[-- Attachment #1: Type: text/plain, Size: 305 bytes --]

New comment by github-actions[bot] on void-packages repository

https://github.com/void-linux/void-packages/pull/33088#issuecomment-1146725056

Comment:
Pull Requests become stale 90 days after last activity and are closed 14 days after that.  If this pull request is still relevant bump it or assign it.