diff mbox series

base-files: make os-release symbolic link absolute

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

Commit Message

Florian Eckert Sept. 8, 2021, 12:50 p.m. UTC
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(-)

Comments

Paul Spooren Sept. 8, 2021, 8:39 p.m. UTC | #1
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
Hauke Mehrtens Sept. 8, 2021, 9:06 p.m. UTC | #2
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
>
Jo-Philipp Wich Sept. 8, 2021, 9:07 p.m. UTC | #3
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
Florian Eckert Sept. 10, 2021, 5:53 a.m. UTC | #4
>> * 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
Paul Spooren Sept. 10, 2021, 7:55 p.m. UTC | #5
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 mbox series

Patch

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