[PATCH 0/2] tools: ynl: two patches to ease building with rpmbuild

[PATCH 0/2] tools: ynl: two patches to ease building with rpmbuild

[Jan Stancek]

I'm looking to build and package ynl for Fedora and Centos Stream users.
Default rpmbuild has couple hardening options enabled by default [1][2],
which currently prevent ynl from building.

This series contains 2 small patches to address it.

[1] https://fedoraproject.org/wiki/Changes/Harden_All_Packages
[2] https://fedoraproject.org/wiki/Changes/PythonSafePath

Jan Stancek (2):
  tools: ynl: add script dir to sys.path
  tools: ynl: extend CFLAGS to keep options from environment

 tools/net/ynl/cli.py             | 3 +++
 tools/net/ynl/ethtool.py         | 2 ++
 tools/net/ynl/generated/Makefile | 2 +-
 tools/net/ynl/lib/Makefile       | 2 +-
 tools/net/ynl/samples/Makefile   | 2 +-
 tools/net/ynl/ynl-gen-c.py       | 3 +++
 6 files changed, 11 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH 1/2] tools: ynl: add script dir to sys.path

[Jan Stancek]

Python options like PYTHONSAFEPATH or -P [1] do not add script
directory to PYTHONPATH. ynl depends on this path to build and run.

[1] This option is default for Fedora rpmbuild since introduction of
    https://fedoraproject.org/wiki/Changes/PythonSafePath

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 tools/net/ynl/cli.py       | 3 +++
 tools/net/ynl/ethtool.py   | 2 ++
 tools/net/ynl/ynl-gen-c.py | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/tools/net/ynl/cli.py b/tools/net/ynl/cli.py
index b8481f401376..873463dbdcc0 100755
--- a/tools/net/ynl/cli.py
+++ b/tools/net/ynl/cli.py
@@ -3,9 +3,12 @@
 
 import argparse
 import json
+import pathlib
 import pprint
+import sys
 import time
 
+sys.path.append(pathlib.Path(__file__).resolve().parent.as_posix())
 from lib import YnlFamily, Netlink, NlError
 
 
diff --git a/tools/net/ynl/ethtool.py b/tools/net/ynl/ethtool.py
index 63c471f075ab..ebb0a11f67bf 100755
--- a/tools/net/ynl/ethtool.py
+++ b/tools/net/ynl/ethtool.py
@@ -3,11 +3,13 @@
 
 import argparse
 import json
+import pathlib
 import pprint
 import sys
 import re
 import os
 
+sys.path.append(pathlib.Path(__file__).resolve().parent.as_posix())
 from lib import YnlFamily
 
 def args_to_req(ynl, op_name, args, req):
diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 717530bc9c52..a86e88019e22 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -4,12 +4,15 @@
 import argparse
 import collections
 import filecmp
+import pathlib
 import os
 import re
 import shutil
+import sys
 import tempfile
 import yaml
 
+sys.path.append(pathlib.Path(__file__).resolve().parent.as_posix())
 from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, SpecEnumEntry
 
 
-- 
2.43.0

[PATCH 2/2] tools: ynl: extend CFLAGS to keep options from environment

[Jan Stancek]

Package build environments like Fedora rpmbuild introduced hardening
options (e.g. -pie -Wl,-z,now) by passing a -spec option to CFLAGS
and LDFLAGS.

ynl Makefiles currently override CFLAGS but not LDFLAGS, which leads
to a mismatch and build failure:
        CC sample devlink
  /usr/bin/ld: devlink.o: relocation R_X86_64_32 against symbol `ynl_devlink_family' can not be used when making a PIE object; recompile with -fPIE
  /usr/bin/ld: failed to set dynamic section sizes: bad value
  collect2: error: ld returned 1 exit status

Extend CFLAGS to support hardening options set by build environment.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 tools/net/ynl/generated/Makefile | 2 +-
 tools/net/ynl/lib/Makefile       | 2 +-
 tools/net/ynl/samples/Makefile   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/net/ynl/generated/Makefile b/tools/net/ynl/generated/Makefile
index 713f5fb9cc2d..7db5240de58a 100644
--- a/tools/net/ynl/generated/Makefile
+++ b/tools/net/ynl/generated/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 CC=gcc
-CFLAGS=-std=gnu11 -O2 -W -Wall -Wextra -Wno-unused-parameter -Wshadow \
+CFLAGS += -std=gnu11 -O2 -W -Wall -Wextra -Wno-unused-parameter -Wshadow \
 	-I../lib/ -idirafter $(UAPI_PATH)
 ifeq ("$(DEBUG)","1")
   CFLAGS += -g -fsanitize=address -fsanitize=leak -static-libasan
diff --git a/tools/net/ynl/lib/Makefile b/tools/net/ynl/lib/Makefile
index 2887cc5de530..94c49cca3dca 100644
--- a/tools/net/ynl/lib/Makefile
+++ b/tools/net/ynl/lib/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 CC=gcc
-CFLAGS=-std=gnu11 -O2 -W -Wall -Wextra -Wno-unused-parameter -Wshadow
+CFLAGS += -std=gnu11 -O2 -W -Wall -Wextra -Wno-unused-parameter -Wshadow
 ifeq ("$(DEBUG)","1")
   CFLAGS += -g -fsanitize=address -fsanitize=leak -static-libasan
 endif
diff --git a/tools/net/ynl/samples/Makefile b/tools/net/ynl/samples/Makefile
index e194a7565861..c9494a564da4 100644
--- a/tools/net/ynl/samples/Makefile
+++ b/tools/net/ynl/samples/Makefile
@@ -3,7 +3,7 @@
 include ../Makefile.deps
 
 CC=gcc
-CFLAGS=-std=gnu11 -O2 -W -Wall -Wextra -Wno-unused-parameter -Wshadow \
+CFLAGS += -std=gnu11 -O2 -W -Wall -Wextra -Wno-unused-parameter -Wshadow \
 	-I../lib/ -I../generated/ -idirafter $(UAPI_PATH)
 ifeq ("$(DEBUG)","1")
   CFLAGS += -g -fsanitize=address -fsanitize=leak -static-libasan
-- 
2.43.0

Re: [PATCH 0/2] tools: ynl: two patches to ease building with rpmbuild

[Jakub Kicinski]

On Mon, 11 Nov 2024 14:04:43 +0100 Jan Stancek wrote:
> I'm looking to build and package ynl for Fedora and Centos Stream users.

Great to hear!

> Default rpmbuild has couple hardening options enabled by default [1][2],
> which currently prevent ynl from building.

Could you rebase on:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/
and resend? I see some fuzz:

Applying: tools: ynl: add script dir to sys.path
Using index info to reconstruct a base tree...
M	tools/net/ynl/cli.py
M	tools/net/ynl/ynl-gen-c.py
Falling back to patching base and 3-way merge...
Auto-merging tools/net/ynl/ynl-gen-c.py
Auto-merging tools/net/ynl/cli.py
Applying: tools: ynl: extend CFLAGS to keep options from environment


With that fixed feel free to add to the patches:

Acked-by: Jakub Kicinski <kuba@kernel.org>


One thing I keep thinking about, maybe you already read this, is to
add  some sort of spec search path and install the specs under /usr.
So the user can simply say --family X on the CLI without specifying 
the fs full path to the YAML file. Would you be willing to send a patch
for this?

Re: [PATCH 0/2] tools: ynl: two patches to ease building with rpmbuild

[Jan Stancek]

On Tue, Nov 12, 2024 at 12:52 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 11 Nov 2024 14:04:43 +0100 Jan Stancek wrote:
> > I'm looking to build and package ynl for Fedora and Centos Stream users.
>
> Great to hear!
>
> > Default rpmbuild has couple hardening options enabled by default [1][2],
> > which currently prevent ynl from building.
>
> Could you rebase on:
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/
> and resend? I see some fuzz:
>
> Applying: tools: ynl: add script dir to sys.path
> Using index info to reconstruct a base tree...
> M       tools/net/ynl/cli.py
> M       tools/net/ynl/ynl-gen-c.py
> Falling back to patching base and 3-way merge...
> Auto-merging tools/net/ynl/ynl-gen-c.py
> Auto-merging tools/net/ynl/cli.py
> Applying: tools: ynl: extend CFLAGS to keep options from environment
>
>
> With that fixed feel free to add to the patches:
>
> Acked-by: Jakub Kicinski <kuba@kernel.org>

Will do.

>
>
> One thing I keep thinking about, maybe you already read this, is to
> add  some sort of spec search path and install the specs under /usr.
> So the user can simply say --family X on the CLI without specifying
> the fs full path to the YAML file. Would you be willing to send a patch
> for this?

I can look at adding--family option (atm. for running ynl in-tree).

One thing I wasn't sure about (due to lacking install target) was whether
you intend to run ynl always from linux tree.

If you're open to adding 'install' target, I think that should be something
to look at as well. It would make packaging less fragile, as I'm currently
handling all that on spec side.

Thanks,
Jan

Re: [PATCH 0/2] tools: ynl: two patches to ease building with rpmbuild

[Donald Hunter]

Jan Stancek <jstancek@redhat.com> writes:

> On Tue, Nov 12, 2024 at 12:52 AM Jakub Kicinski <kuba@kernel.org> wrote:
>>
>> One thing I keep thinking about, maybe you already read this, is to
>> add  some sort of spec search path and install the specs under /usr.
>> So the user can simply say --family X on the CLI without specifying
>> the fs full path to the YAML file. Would you be willing to send a patch
>> for this?
>
> I can look at adding--family option (atm. for running ynl in-tree).
>
> One thing I wasn't sure about (due to lacking install target) was whether
> you intend to run ynl always from linux tree.
>
> If you're open to adding 'install' target, I think that should be something
> to look at as well. It would make packaging less fragile, as I'm currently
> handling all that on spec side.

Hi Jan,

I am happy to work with you on adding an install target, plus some other
UX improvements like --family.

Thanks,
Donald.

Re: [PATCH 0/2] tools: ynl: two patches to ease building with rpmbuild

[Jakub Kicinski]

On Tue, 12 Nov 2024 09:16:07 +0100 Jan Stancek wrote:
> One thing I wasn't sure about (due to lacking install target) was whether
> you intend to run ynl always from linux tree.
> 
> If you're open to adding 'install' target, I think that should be something
> to look at as well. It would make packaging less fragile, as I'm currently
> handling all that on spec side.

Yes, definitely open to adding an install target.

Assume that whatever is missing or done strangely from packaging
perspective is due to incompetence rather than intentional design :)