diff mbox series

classes/package_rpm: do not mangle filenames with % in them

Message ID 20240129115709.3517246-1-alex@linutronix.de
State New
Headers show
Series classes/package_rpm: do not mangle filenames with % in them | expand

Commit Message

Alexander Kanavin Jan. 29, 2024, 11:57 a.m. UTC
This causes problems in rpm 4.19 packaging of recipes such as nodejs,
and isn't actually necessary anymore (was added many years ago
against rpm 5.x fork).

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 meta/classes-global/package_rpm.bbclass | 3 ---
 1 file changed, 3 deletions(-)

Comments

Mark Hatle Jan. 29, 2024, 4:42 p.m. UTC | #1
Due to the RPM macro language, you can end up with something like this:

%define foo bar

(files section):

/path/dir/%foo

Then when the %files is parsed you end up with:

/path/dir/bar

The processing of the files says "well that doesn't exist, so I won't package it."


For RPM 5.... it was determined the only save way to prevent the expansion was 
that horrible set of components.  (And I mean HORRIBLE!)


So before this is removed, we really need someone to test the RPM 4.19 behavior 
and verify that it does NOT expand the '%" in ways that we don't want.

(the above was all from memory.  So there may have been other nuances involved.

If I remember correctly the preferred format for regular replacement macros 
should be "%{macro}".  But there are/were internals in place like sections 
heads, functional macros (like %configure) and similar that were used.


I wish I could remember the exact place this failed in the past, but that 
appears to be lost in time.  For all I know it was "/path/dir/foo %bar".  (Yes 
I've seen some horrible filenames in my time.

--Mark

On 1/29/24 5:57 AM, Alexander Kanavin wrote:
> This causes problems in rpm 4.19 packaging of recipes such as nodejs,
> and isn't actually necessary anymore (was added many years ago
> against rpm 5.x fork).
> 
> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> ---
>   meta/classes-global/package_rpm.bbclass | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/meta/classes-global/package_rpm.bbclass b/meta/classes-global/package_rpm.bbclass
> index e0f4de42a15..767f1e21b6e 100644
> --- a/meta/classes-global/package_rpm.bbclass
> +++ b/meta/classes-global/package_rpm.bbclass
> @@ -219,7 +219,6 @@ python write_specfile () {
>               path = rootpath.replace(walkpath, "")
>               if path.endswith("DEBIAN") or path.endswith("CONTROL"):
>                   continue
> -            path = path.replace("%", "%%%%%%%%")
>   
>               # Treat all symlinks to directories as normal files.
>               # os.walk() lists them as directories.
> @@ -238,7 +237,6 @@ python write_specfile () {
>                   for dir in dirs:
>                       if dir == "CONTROL" or dir == "DEBIAN":
>                           continue
> -                    dir = dir.replace("%", "%%%%%%%%")
>                       p = path + '/' + dir
>                       # All packages own the directories their files are in...
>                       target.append(get_attr(dir) + '%dir "' + p + '"')
> @@ -254,7 +252,6 @@ python write_specfile () {
>               for file in files:
>                   if file == "CONTROL" or file == "DEBIAN":
>                       continue
> -                file = file.replace("%", "%%%%%%%%")
>                   attr = get_attr(file)
>                   p = path + '/' + file
>                   if conffiles.count(p):
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#194456): https://lists.openembedded.org/g/openembedded-core/message/194456
> Mute This Topic: https://lists.openembedded.org/mt/104029566/3616948
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [mark.hatle@kernel.crashing.org]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Alexander Kanavin Jan. 29, 2024, 5:27 p.m. UTC | #2
The documentation says:

The usual rules for shell globbing apply (see `glob(7)`), including brace
expansion.  Metacharacters can be escaped by prefixing them with a backslash
(`\`).  Spaces are used to separate file names and must also be escaped.
Enclosing a file name in double quotes (`"`) preserves the literal value of all
characters within the quotes, with the exception of `\` and the percent sign
(`%`).  A `\` or `%` can be escaped with an extra `\` or `%`, respectively.  A
double quote can be escaped with a `\`.

https://github.com/rpm-software-management/rpm/blob/rpm-4.19.x/docs/manual/spec.md#shell-globbing

So I think we simply need to replace % with %%, and only just before
it's written into the spec file - current code does it far too early,
rendering the paths unusable in regular file operations.


Alex



On Mon, 29 Jan 2024 at 17:43, Mark Hatle <mark.hatle@kernel.crashing.org> wrote:
>
> Due to the RPM macro language, you can end up with something like this:
>
> %define foo bar
>
> (files section):
>
> /path/dir/%foo
>
> Then when the %files is parsed you end up with:
>
> /path/dir/bar
>
> The processing of the files says "well that doesn't exist, so I won't package it."
>
>
> For RPM 5.... it was determined the only save way to prevent the expansion was
> that horrible set of components.  (And I mean HORRIBLE!)
>
>
> So before this is removed, we really need someone to test the RPM 4.19 behavior
> and verify that it does NOT expand the '%" in ways that we don't want.
>
> (the above was all from memory.  So there may have been other nuances involved.
>
> If I remember correctly the preferred format for regular replacement macros
> should be "%{macro}".  But there are/were internals in place like sections
> heads, functional macros (like %configure) and similar that were used.
>
>
> I wish I could remember the exact place this failed in the past, but that
> appears to be lost in time.  For all I know it was "/path/dir/foo %bar".  (Yes
> I've seen some horrible filenames in my time.
>
> --Mark
>
> On 1/29/24 5:57 AM, Alexander Kanavin wrote:
> > This causes problems in rpm 4.19 packaging of recipes such as nodejs,
> > and isn't actually necessary anymore (was added many years ago
> > against rpm 5.x fork).
> >
> > Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> > ---
> >   meta/classes-global/package_rpm.bbclass | 3 ---
> >   1 file changed, 3 deletions(-)
> >
> > diff --git a/meta/classes-global/package_rpm.bbclass b/meta/classes-global/package_rpm.bbclass
> > index e0f4de42a15..767f1e21b6e 100644
> > --- a/meta/classes-global/package_rpm.bbclass
> > +++ b/meta/classes-global/package_rpm.bbclass
> > @@ -219,7 +219,6 @@ python write_specfile () {
> >               path = rootpath.replace(walkpath, "")
> >               if path.endswith("DEBIAN") or path.endswith("CONTROL"):
> >                   continue
> > -            path = path.replace("%", "%%%%%%%%")
> >
> >               # Treat all symlinks to directories as normal files.
> >               # os.walk() lists them as directories.
> > @@ -238,7 +237,6 @@ python write_specfile () {
> >                   for dir in dirs:
> >                       if dir == "CONTROL" or dir == "DEBIAN":
> >                           continue
> > -                    dir = dir.replace("%", "%%%%%%%%")
> >                       p = path + '/' + dir
> >                       # All packages own the directories their files are in...
> >                       target.append(get_attr(dir) + '%dir "' + p + '"')
> > @@ -254,7 +252,6 @@ python write_specfile () {
> >               for file in files:
> >                   if file == "CONTROL" or file == "DEBIAN":
> >                       continue
> > -                file = file.replace("%", "%%%%%%%%")
> >                   attr = get_attr(file)
> >                   p = path + '/' + file
> >                   if conffiles.count(p):
> >
> >
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#194456): https://lists.openembedded.org/g/openembedded-core/message/194456
> > Mute This Topic: https://lists.openembedded.org/mt/104029566/3616948
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [mark.hatle@kernel.crashing.org]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
diff mbox series

Patch

diff --git a/meta/classes-global/package_rpm.bbclass b/meta/classes-global/package_rpm.bbclass
index e0f4de42a15..767f1e21b6e 100644
--- a/meta/classes-global/package_rpm.bbclass
+++ b/meta/classes-global/package_rpm.bbclass
@@ -219,7 +219,6 @@  python write_specfile () {
             path = rootpath.replace(walkpath, "")
             if path.endswith("DEBIAN") or path.endswith("CONTROL"):
                 continue
-            path = path.replace("%", "%%%%%%%%")
 
             # Treat all symlinks to directories as normal files.
             # os.walk() lists them as directories.
@@ -238,7 +237,6 @@  python write_specfile () {
                 for dir in dirs:
                     if dir == "CONTROL" or dir == "DEBIAN":
                         continue
-                    dir = dir.replace("%", "%%%%%%%%")
                     p = path + '/' + dir
                     # All packages own the directories their files are in...
                     target.append(get_attr(dir) + '%dir "' + p + '"')
@@ -254,7 +252,6 @@  python write_specfile () {
             for file in files:
                 if file == "CONTROL" or file == "DEBIAN":
                     continue
-                file = file.replace("%", "%%%%%%%%")
                 attr = get_attr(file)
                 p = path + '/' + file
                 if conffiles.count(p):