[Buildroot] [PATCH 1/1] package/dhcp: fix dhcp symlink in target/var/lib/dhcp

[Yann E. MORIN]

Thomas, Chrles, Al,

On 2023-07-10 20:47 +0200, Thomas Petazzoni spake thusly:
> +Yann in Cc. I will ask a few questions below, but honestly I'm not
> super familiar with this part of the code base, so I might be asking
> some stupid questions.

There are no stupid questions, only stupid answers (which is customary
for me to provide).

> On Tue, 20 Jun 2023 10:07:26 -0700
> Charles Hardin <ckhardin at gmail.com> wrote:
> > the var directories to the usr/share/factory and the run a command
> > like this:
> > 
> >    mv $(TARGET_DIR)/var $(TARGET_DIR)/usr/share/factory/var
> >    mkdir -p $(TARGET_DIR)/var
> >    for i in $(TARGET_DIR)/usr/share/factory/var/* \
> >             $(TARGET_DIR)/usr/share/factory/var/lib/* \
> >             $(TARGET_DIR)/usr/share/factory/var/lib/systemd/*; do
> > 
> > If the symlink is to the /tmp directory the glob compare on the
> > dhcp link will evaluate and result in an incorrect entry being
> > created in the resulting tmpfs mount on var.
> I'm not able to parse your explanation here. Which glob compare? Which
> evaluation?
> > 
> >     L+! /var/lib/dhcp - - - - ../usr/share/factory//var/lib/dhcp
> Not obvious to me what is invalid in this entry, but admittedly I have
> not done a test build/test run to investigate more.

This line is indeed incorrect: it means to create /var/lib/dhcp as a
symlink to ../usr/share/factory//var/lib/dhcp, i.e. to
/var/lib/../usr/share/factory//var/lib/dhcp, which in turn is made
cannonical as /var/usr/share/factory//var/lib/dhcp, which indeed does
not exist.

> Perhaps you can give a little bit more details to make the issue
> obvious to the reviewer, and therefore help your patch to be merged?

I think a better solution is to actually fix the factory:

   41 »   for i in $(TARGET_DIR)/usr/share/factory/var/* \
   42 »   »    $(TARGET_DIR)/usr/share/factory/var/lib/* \
   43 »   »    $(TARGET_DIR)/usr/share/factory/var/lib/systemd/*; do \
   44 »   »   [ -e "$${i}" ] || continue; \
   45 »   »   j="$${i#$(TARGET_DIR)/usr/share/factory}"; \
   46 »   »   if [ -L "$${i}" ]; then \
   47 »   »   »   printf "L+! %s - - - - %s\n" \
   48 »   »   »   »   "$${j}" "../usr/share/factory/$${j}" \

The ../ here should be replaced with as-many ../ as the fiel is deep
relative to /usr/share/factory/var/

i.e. probably something (untested) like:

       for i in $(TARGET_DIR)/usr/share/factory/var/* \
            $(TARGET_DIR)/usr/share/factory/var/lib/* \
            $(TARGET_DIR)/usr/share/factory/var/lib/systemd/*; do \
           [ -e "$${i}" ] || continue; \
           j="$${i#$(TARGET_DIR)/usr/share/factory}"; \
           if [ -L "$${i}" ]; then \
               d="$$(sed -r -e 's,([^./]+/),../,g' <<<$${j%/*}/)" \
               printf "L+! %s - - - - %s\n" \
                   "$${j}" "${d}/usr/share/factory/$${j}" \

This way, that should fix all absolute synlinks that are in an arbitrary
depth in the factory.

Regards,
Yann E. MORIN.

> Thanks!
> 
> Thomas
> -- 
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'