Re: [Buildroot] [PATCH 2/3] core/pkg-generic: fixup all PPD paths in a generic fashion

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Arnout, All,

On 2022-01-12 21:42 +0100, Arnout Vandecappelle spake thusly:
> On 08/01/2022 18:35, Yann E. MORIN wrote:
> >Some files contain hard-coded absolute paths that point to the host
> >and/or staging directories.
> >
> >With per-package directories (aka. PPD), these paths point to the PPD
> >of the package that created the files, when we want them to point to the
> >PPD of the package that uses them.
[--SNIP--]
> >+define PPD_FIXUP_PATHS
> >+	$(Q)grep -lrZ '$(PER_PACKAGE_DIR)/[^/]\+/' $(HOST_DIR) \
>  This will trawl to large binary files and potentially take a long time...

I too thought of that, and I even discussed it slightly with Peter K.
here about what would be most efficient: grep-then-file, or file-then-grep?

But if we add --binary-files=without-match to grep, then it will
bail-out early on binary files, which would be more efficient. And then
we still pass that through file to really get only text files.

But a text-file with utf8 is really a binary file, so would grep ignore
it?

> >+	|while read -d '' f; do \
> >+		file -b --mime-type "$${f}" | grep -q '^text/' || continue; \
> ... just to be ignored here.

Again, not sure, because text/plain but utf8-encoded?

>  More importantly though: if a file is a symlink, it's going to be hit
> twice.

No, I don't think so:

    $ man grep
    [..]
    -r, --recursive
        Read all files under each directory, recursively, following
        symbolic links only if they are on the command line. [...]

And that seems to be the case:

    $ echo Pouet >foo
    $ ln -s foo bar
    $ grep -r Pouet .
    foo:Pouet

So, seems OK to me?

> Worse, if it's a symlink to an absolute path (which most likely
> points *outside* of STAGING_DIR), we may end up sed'ing something on the
> host...
>  I notice now that the same (theoretical) issue exists in relocate-sdk.sh.
> Obviously that script doesn't get thoroughly tested so it may very well be
> the wrong thing to do...
> 
>  Do you remember perhaps why we don't simply do
> 
> find $(HOST_DIR) -type f -print0 \

Maybe we should. But grep -r was an elegant way to combine the recursive
feature of find, and limit to the matching files at the same time...

I can try to time things to see what's the fastest / less-slow... But
given that the symlinks are not an issue in practice, so we care?

> ? I was going to say that we can skip the grep, but then we're back at the
> large (text) file thing.
> 
> >+		printf '%s\0' "$${f}"; \
> 
>  Why not do the sed right here, like is done in relocate-sdk.sh? In fact,
> I'd keep the code as close as possible to relocate-sdk.sh to make later
> refactoring easier.

Using xarg allows to spawn only a few sed. printf is a shell built-in so
it's basically free.

[--SNIP--]
> >  define FIXUP_PYTHON_SYSCONFIGDATA
>  Maybe rename to REMOVE_PYTHON_SYSCONFIGATA_PYC

OK

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot