Message ID | 20240129180629.1631473-1-alex@linutronix.de |
---|---|
State | Accepted, archived |
Commit | 6d9fe2623c37e405a80acf71633f7291ecdde533 |
Headers | show |
Series | classes/package_rpm: correctly escape percent characters | expand |
This looks better to me. Hopefully two % is all we need now. Thanks! --Mark On 1/29/24 12:06 PM, Alexander Kanavin wrote: > This many characters doesn't work with rpm 4.19 packaging > (as shown by nodejs recipes), and per documentation a single escape > is enough: > > https://github.com/rpm-software-management/rpm/blob/rpm-4.19.x/docs/manual/spec.md#shell-globbing > > It also should be done in a function, and just before writing out the > corrected filename to .spec, not earlier where the path may still > be needed for file operations (such as gettings file attributes). > > Signed-off-by: Alexander Kanavin <alex@linutronix.de> > --- > meta/classes-global/package_rpm.bbclass | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/meta/classes-global/package_rpm.bbclass b/meta/classes-global/package_rpm.bbclass > index e0f4de42a15..819ee502783 100644 > --- a/meta/classes-global/package_rpm.bbclass > +++ b/meta/classes-global/package_rpm.bbclass > @@ -216,10 +216,12 @@ python write_specfile () { > raise e > return "%attr({:o},{},{}) ".format(mode, owner, group) > > + def escape_chars(p): > + return p.replace("%", "%%") > + > 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,29 +240,27 @@ 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 + '"') > + target.append(get_attr(dir) + '%dir "' + escape_chars(p) + '"') > else: > # packages own only empty directories or explict directory. > # This will prevent the overlapping of security permission. > attr = get_attr(path) > if path and not files and not dirs: > - target.append(attr + '%dir "' + path + '"') > + target.append(attr + '%dir "' + escape_chars(path) + '"') > elif path and path in dirfiles: > - target.append(attr + '%dir "' + path + '"') > + target.append(attr + '%dir "' + escape_chars(path) + '"') > > 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): > - target.append(attr + '%config "' + p + '"') > + target.append(attr + '%config "' + escape_chars(p) + '"') > else: > - target.append(attr + '"' + p + '"') > + target.append(attr + '"' + escape_chars(p) + '"') > > # Prevent the prerm/postrm scripts from being run during an upgrade > def wrap_uninstall(scriptvar): > > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#194473): https://lists.openembedded.org/g/openembedded-core/message/194473 > Mute This Topic: https://lists.openembedded.org/mt/104036885/3616948 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [mark.hatle@kernel.crashing.org] > -=-=-=-=-=-=-=-=-=-=-=- >
> -----Original Message----- > From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Alexander Kanavin > Sent: den 29 januari 2024 19:06 > To: openembedded-core@lists.openembedded.org > Cc: Alexander Kanavin <alex@linutronix.de> > Subject: [OE-core] [PATCH] classes/package_rpm: correctly escape percent characters > > This many characters doesn't work with rpm 4.19 packaging > (as shown by nodejs recipes), and per documentation a single escape > is enough: > > https://github.com/rpm-software-management/rpm/blob/rpm-4.19.x/docs/manual/spec.md#shell-globbing > > It also should be done in a function, and just before writing out the > corrected filename to .spec, not earlier where the path may still > be needed for file operations (such as gettings file attributes). > > Signed-off-by: Alexander Kanavin <alex@linutronix.de> > --- > meta/classes-global/package_rpm.bbclass | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/meta/classes-global/package_rpm.bbclass b/meta/classes-global/package_rpm.bbclass > index e0f4de42a15..819ee502783 100644 > --- a/meta/classes-global/package_rpm.bbclass > +++ b/meta/classes-global/package_rpm.bbclass > @@ -216,10 +216,12 @@ python write_specfile () { > raise e > return "%attr({:o},{},{}) ".format(mode, owner, group) > > + def escape_chars(p): > + return p.replace("%", "%%") Based on the documentation linked to above, wouldn't it be a good idea to handle \ and " as well? E.g.: return p.replace("%", "%%").replace("\\", "\\\\").replace('"', '\\"')) > + > 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,29 +240,27 @@ 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 + '"') > + target.append(get_attr(dir) + '%dir "' + escape_chars(p) + '"') > else: > # packages own only empty directories or explict directory. > # This will prevent the overlapping of security permission. > attr = get_attr(path) > if path and not files and not dirs: > - target.append(attr + '%dir "' + path + '"') > + target.append(attr + '%dir "' + escape_chars(path) + '"') > elif path and path in dirfiles: > - target.append(attr + '%dir "' + path + '"') > + target.append(attr + '%dir "' + escape_chars(path) + '"') The above can be simplified to: elif path: # packages own only empty directories or explict directory. # This will prevent the overlapping of security permission. attr = get_attr(path) if (not files and not dirs) or path in dirfiles: target.append(attr + '%dir "' + escape_chars(path) + '"') > > 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): > - target.append(attr + '%config "' + p + '"') > + target.append(attr + '%config "' + escape_chars(p) + '"') > else: > - target.append(attr + '"' + p + '"') > + target.append(attr + '"' + escape_chars(p) + '"') > > # Prevent the prerm/postrm scripts from being run during an upgrade > def wrap_uninstall(scriptvar): > -- > 2.39.2 //Peter
Thanks, I sent patches for both. Alex On Wed, 31 Jan 2024 at 02:07, Peter Kjellerstedt <peter.kjellerstedt@axis.com> wrote: > > > -----Original Message----- > > From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Alexander Kanavin > > Sent: den 29 januari 2024 19:06 > > To: openembedded-core@lists.openembedded.org > > Cc: Alexander Kanavin <alex@linutronix.de> > > Subject: [OE-core] [PATCH] classes/package_rpm: correctly escape percent characters > > > > This many characters doesn't work with rpm 4.19 packaging > > (as shown by nodejs recipes), and per documentation a single escape > > is enough: > > > > https://github.com/rpm-software-management/rpm/blob/rpm-4.19.x/docs/manual/spec.md#shell-globbing > > > > It also should be done in a function, and just before writing out the > > corrected filename to .spec, not earlier where the path may still > > be needed for file operations (such as gettings file attributes). > > > > Signed-off-by: Alexander Kanavin <alex@linutronix.de> > > --- > > meta/classes-global/package_rpm.bbclass | 16 ++++++++-------- > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/meta/classes-global/package_rpm.bbclass b/meta/classes-global/package_rpm.bbclass > > index e0f4de42a15..819ee502783 100644 > > --- a/meta/classes-global/package_rpm.bbclass > > +++ b/meta/classes-global/package_rpm.bbclass > > @@ -216,10 +216,12 @@ python write_specfile () { > > raise e > > return "%attr({:o},{},{}) ".format(mode, owner, group) > > > > + def escape_chars(p): > > + return p.replace("%", "%%") > > > Based on the documentation linked to above, wouldn't it be a good idea to > handle \ and " as well? E.g.: > > return p.replace("%", "%%").replace("\\", "\\\\").replace('"', '\\"')) > > > + > > 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,29 +240,27 @@ 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 + '"') > > + target.append(get_attr(dir) + '%dir "' + escape_chars(p) + '"') > > else: > > # packages own only empty directories or explict directory. > > # This will prevent the overlapping of security permission. > > attr = get_attr(path) > > if path and not files and not dirs: > > - target.append(attr + '%dir "' + path + '"') > > + target.append(attr + '%dir "' + escape_chars(path) + '"') > > elif path and path in dirfiles: > > - target.append(attr + '%dir "' + path + '"') > > + target.append(attr + '%dir "' + escape_chars(path) + '"') > > The above can be simplified to: > > elif path: > # packages own only empty directories or explict directory. > # This will prevent the overlapping of security permission. > attr = get_attr(path) > if (not files and not dirs) or path in dirfiles: > target.append(attr + '%dir "' + escape_chars(path) + '"') > > > > > 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): > > - target.append(attr + '%config "' + p + '"') > > + target.append(attr + '%config "' + escape_chars(p) + '"') > > else: > > - target.append(attr + '"' + p + '"') > > + target.append(attr + '"' + escape_chars(p) + '"') > > > > # Prevent the prerm/postrm scripts from being run during an upgrade > > def wrap_uninstall(scriptvar): > > -- > > 2.39.2 > > //Peter >
diff --git a/meta/classes-global/package_rpm.bbclass b/meta/classes-global/package_rpm.bbclass index e0f4de42a15..819ee502783 100644 --- a/meta/classes-global/package_rpm.bbclass +++ b/meta/classes-global/package_rpm.bbclass @@ -216,10 +216,12 @@ python write_specfile () { raise e return "%attr({:o},{},{}) ".format(mode, owner, group) + def escape_chars(p): + return p.replace("%", "%%") + 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,29 +240,27 @@ 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 + '"') + target.append(get_attr(dir) + '%dir "' + escape_chars(p) + '"') else: # packages own only empty directories or explict directory. # This will prevent the overlapping of security permission. attr = get_attr(path) if path and not files and not dirs: - target.append(attr + '%dir "' + path + '"') + target.append(attr + '%dir "' + escape_chars(path) + '"') elif path and path in dirfiles: - target.append(attr + '%dir "' + path + '"') + target.append(attr + '%dir "' + escape_chars(path) + '"') 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): - target.append(attr + '%config "' + p + '"') + target.append(attr + '%config "' + escape_chars(p) + '"') else: - target.append(attr + '"' + p + '"') + target.append(attr + '"' + escape_chars(p) + '"') # Prevent the prerm/postrm scripts from being run during an upgrade def wrap_uninstall(scriptvar):
This many characters doesn't work with rpm 4.19 packaging (as shown by nodejs recipes), and per documentation a single escape is enough: https://github.com/rpm-software-management/rpm/blob/rpm-4.19.x/docs/manual/spec.md#shell-globbing It also should be done in a function, and just before writing out the corrected filename to .spec, not earlier where the path may still be needed for file operations (such as gettings file attributes). Signed-off-by: Alexander Kanavin <alex@linutronix.de> --- meta/classes-global/package_rpm.bbclass | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)