Message ID | 20210908125002.9433-1-fe@dev.tdt.de |
---|---|
State | Rejected |
Delegated to: | Paul Spooren |
Headers | show |
Series | base-files: make os-release symbolic link absolute | expand |
On 9/8/21 02:50, Florian Eckert wrote: > The file `/etc/os-release` is currently a relative link to > `../usr/lib/os-release`. > > The follwing links on my `/etc` are also absolute: > * localtime -> /tmp/localtime > * resolv.conf -> /tmp/resolv.conf > * /tmp/TZmtab -> /proc/mounts > > For consistency, this should also apply to `/usr/lib/os-release`. > > Signed-off-by: Florian Eckert <fe@dev.tdt.de> > --- Did you check that none of the build scripts try to access this file? In case they do, they'd fail since an absolute path wouldn't exists on the building host system. > package/base-files/files/etc/os-release | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/package/base-files/files/etc/os-release b/package/base-files/files/etc/os-release > index c4c75b419c..d98fc7a7ac 120000 > --- a/package/base-files/files/etc/os-release > +++ b/package/base-files/files/etc/os-release > @@ -1 +1 @@ > -../usr/lib/os-release > \ No newline at end of file > +/usr/lib/os-release > \ No newline at end of file
On 9/8/21 2:50 PM, Florian Eckert wrote: > The file `/etc/os-release` is currently a relative link to > `../usr/lib/os-release`. > > The follwing links on my `/etc` are also absolute: > * localtime -> /tmp/localtime > * resolv.conf -> /tmp/resolv.conf > * /tmp/TZmtab -> /proc/mounts > > For consistency, this should also apply to `/usr/lib/os-release`. Is there any other reason than consistency to change this? I prefer relative links because they also work when the file system is accessed by the build system like Paul said. Hauke > > Signed-off-by: Florian Eckert <fe@dev.tdt.de> > --- > package/base-files/files/etc/os-release | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/package/base-files/files/etc/os-release b/package/base-files/files/etc/os-release > index c4c75b419c..d98fc7a7ac 120000 > --- a/package/base-files/files/etc/os-release > +++ b/package/base-files/files/etc/os-release > @@ -1 +1 @@ > -../usr/lib/os-release > \ No newline at end of file > +/usr/lib/os-release > \ No newline at end of file >
Hi, > Did you check that none of the build scripts try to access this file? In > case they do, they'd fail since an absolute path wouldn't exists on the > building host system. apart from that it might break 3rd party workflows for no obvious reason. The existing absolute symlinks mentioned all point to ephemeral resources while os-release points to an actual fixed file. In doubt, I'd personally favor keeping things as-is as this change is not actually fixing or improving things, just breaking potential use-cases (e.g. `cat /some/mounted_rootfs/etc/os-release`) ~ Jo
>> * resolv.conf -> /tmp/resolv.conf >> * /tmp/TZmtab -> /proc/mounts >> >> For consistency, this should also apply to `/usr/lib/os-release`. > > Is there any other reason than consistency to change this? Yes, I have to elaborate on that. I have a small script that boots the system into a called it "sandbox" mode. If I do not save the sandbox config changes the system boots with old configuration. This means that the changes to the configurations under /etc/ are not permanently saved, because I put a tmpfs over the /etc/. For this to work, however, I have to copy the data somewhere else beforehand and then mount it. mkdir -p /tmp/permetc mount --bind /etc /tmp/permetc mount -t tmpfs -o size="3M" none /etc cp -r /tmp/permetc/* /etc Now I can configure the system as I need it. If I make a mistake, I can reboot the system and the old configuration will be reloaded. So if I want to check what has changed (sandbox config vs. bootup config). I display a diff with the command diff -Naur /etc/permetc/ /etc The problem now is that the file "../usr/lib/os-release" is always shown as not existing, because it is a relative path and the file under /tmp/permetc/../usr/lib/os-release/ does not exist. > I prefer relative links because they also work when the file system is > accessed by the build system like Paul said. But if that is the case, could we not write the data into the target file under /usr/lib/os-release and not into the linked file? This would make such simple changes possible. The normal case should be that the folder structure should look like on the target system just as we need it. By the way, this is the only link that is relative (at least on my system) all others are absolute. However thanks for the feedback and your time -- Florian
On 9/9/21 19:53, Florian Eckert wrote: >>> * resolv.conf -> /tmp/resolv.conf >>> * /tmp/TZmtab -> /proc/mounts >>> >>> For consistency, this should also apply to `/usr/lib/os-release`. >> >> Is there any other reason than consistency to change this? > > Yes, I have to elaborate on that. > I have a small script that boots the system into a called it "sandbox" > mode. > If I do not save the sandbox config changes the system boots with old > configuration. > This means that the changes to the configurations under /etc/ are not > permanently saved, because I put a tmpfs over the /etc/. > > For this to work, however, I have to copy the data somewhere else > beforehand and then mount it. > > mkdir -p /tmp/permetc > mount --bind /etc /tmp/permetc > mount -t tmpfs -o size="3M" none /etc > cp -r /tmp/permetc/* /etc If you already run such script, can't you hard link os-release anyway? > > Now I can configure the system as I need it. If I make a mistake, I > can reboot the system and the old configuration will be reloaded. > > So if I want to check what has changed (sandbox config vs. bootup > config). > I display a diff with the command > > diff -Naur /etc/permetc/ /etc > > The problem now is that the file "../usr/lib/os-release" is always > shown as not existing, because it is a relative path and the file > under /tmp/permetc/../usr/lib/os-release/ does not exist. > >> I prefer relative links because they also work when the file system is >> accessed by the build system like Paul said. > > But if that is the case, could we not write the data into the target > file under /usr/lib/os-release and not into the linked file? > This would make such simple changes possible. > The normal case should be that the folder structure should look like > on the target system just as we need it. > > By the way, this is the only link that is relative (at least on my > system) all others are absolute. > Looking at this I'm wondering if there is a good reason to have the file twice? From my understanding there should be only a single place to have this file?
diff --git a/package/base-files/files/etc/os-release b/package/base-files/files/etc/os-release index c4c75b419c..d98fc7a7ac 120000 --- a/package/base-files/files/etc/os-release +++ b/package/base-files/files/etc/os-release @@ -1 +1 @@ -../usr/lib/os-release \ No newline at end of file +/usr/lib/os-release \ No newline at end of file
The file `/etc/os-release` is currently a relative link to `../usr/lib/os-release`. The follwing links on my `/etc` are also absolute: * localtime -> /tmp/localtime * resolv.conf -> /tmp/resolv.conf * /tmp/TZmtab -> /proc/mounts For consistency, this should also apply to `/usr/lib/os-release`. Signed-off-by: Florian Eckert <fe@dev.tdt.de> --- package/base-files/files/etc/os-release | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)