[PATCH net-next] tools: ynl-gen: fix uAPI generation after tempfile changes

[PATCH net-next] tools: ynl-gen: fix uAPI generation after tempfile changes

[Jakub Kicinski]

We use a tempfile for code generation, to avoid wiping the target
file out if the code generator crashes. File contents are copied
from tempfile to actual destination at the end of main().

uAPI generation is relatively simple so when generating the uAPI
header we return from main() early, and never reach the "copy code
over" stage. Since commit under Fixes uAPI headers are not updated
by ynl-gen.

Move the copy/commit of the code into CodeWriter, to make it
easier to call at any point in time. Hook it into the destructor
to make sure we don't miss calling it.

Fixes: f65f305ae008 ("tools: ynl-gen: use temporary file for rendering")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: jiri@resnulli.us
---
 tools/net/ynl/ynl-gen-c.py | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 9209bdcca9c6..897af958cee8 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -1045,14 +1045,30 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
 
 
 class CodeWriter:
-    def __init__(self, nlib, out_file):
+    def __init__(self, nlib, out_file=None):
         self.nlib = nlib
 
         self._nl = False
         self._block_end = False
         self._silent_block = False
         self._ind = 0
-        self._out = out_file
+        if out_file is None:
+            self._out = os.sys.stdout
+        else:
+            self._out = tempfile.TemporaryFile('w+')
+            self._out_file = out_file
+
+    def __del__(self):
+        self.close_out_file()
+
+    def close_out_file(self):
+        if self._out == os.sys.stdout:
+            return
+        with open(self._out_file, 'w+') as out_file:
+            self._out.seek(0)
+            shutil.copyfileobj(self._out, out_file)
+            self._out.close()
+        self._out = os.sys.stdout
 
     @classmethod
     def _is_cond(cls, line):
@@ -2313,11 +2329,9 @@ _C_KW = {
     parser.add_argument('--source', dest='header', action='store_false')
     parser.add_argument('--user-header', nargs='+', default=[])
     parser.add_argument('--exclude-op', action='append', default=[])
-    parser.add_argument('-o', dest='out_file', type=str)
+    parser.add_argument('-o', dest='out_file', type=str, default=None)
     args = parser.parse_args()
 
-    tmp_file = tempfile.TemporaryFile('w+') if args.out_file else os.sys.stdout
-
     if args.header is None:
         parser.error("--header or --source is required")
 
@@ -2341,7 +2355,7 @@ _C_KW = {
         print(f'Message enum-model {parsed.msg_id_model} not supported for {args.mode} generation')
         os.sys.exit(1)
 
-    cw = CodeWriter(BaseNlLib(), tmp_file)
+    cw = CodeWriter(BaseNlLib(), args.out_file)
 
     _, spec_kernel = find_kernel_root(args.spec)
     if args.mode == 'uapi' or args.header:
@@ -2590,10 +2604,6 @@ _C_KW = {
     if args.header:
         cw.p(f'#endif /* {hdr_prot} */')
 
-    if args.out_file:
-        out_file = open(args.out_file, 'w+')
-        tmp_file.seek(0)
-        shutil.copyfileobj(tmp_file, out_file)
 
 if __name__ == "__main__":
     main()
-- 
2.41.0

Re: [PATCH net-next] tools: ynl-gen: fix uAPI generation after tempfile changes

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 24 Aug 2023 14:24:31 -0700 you wrote:
> We use a tempfile for code generation, to avoid wiping the target
> file out if the code generator crashes. File contents are copied
> from tempfile to actual destination at the end of main().
> 
> uAPI generation is relatively simple so when generating the uAPI
> header we return from main() early, and never reach the "copy code
> over" stage. Since commit under Fixes uAPI headers are not updated
> by ynl-gen.
> 
> [...]

Here is the summary with links:
  - [net-next] tools: ynl-gen: fix uAPI generation after tempfile changes
    https://git.kernel.org/netdev/net-next/c/a02430c06f56

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next] tools: ynl-gen: fix uAPI generation after tempfile changes

[Jiri Pirko]

Thu, Aug 24, 2023 at 11:24:31PM CEST, kuba@kernel.org wrote:
>We use a tempfile for code generation, to avoid wiping the target
>file out if the code generator crashes. File contents are copied
>from tempfile to actual destination at the end of main().
>
>uAPI generation is relatively simple so when generating the uAPI
>header we return from main() early, and never reach the "copy code
>over" stage. Since commit under Fixes uAPI headers are not updated
>by ynl-gen.

Ah, I missed that part, sorry. My python is not so fluent :)

>
>Move the copy/commit of the code into CodeWriter, to make it
>easier to call at any point in time. Hook it into the destructor
>to make sure we don't miss calling it.
>
>Fixes: f65f305ae008 ("tools: ynl-gen: use temporary file for rendering")
>Signed-off-by: Jakub Kicinski <kuba@kernel.org>
>---
>CC: jiri@resnulli.us
>---
> tools/net/ynl/ynl-gen-c.py | 30 ++++++++++++++++++++----------
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
>diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
>index 9209bdcca9c6..897af958cee8 100755
>--- a/tools/net/ynl/ynl-gen-c.py
>+++ b/tools/net/ynl/ynl-gen-c.py
>@@ -1045,14 +1045,30 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
> 
> 
> class CodeWriter:
>-    def __init__(self, nlib, out_file):
>+    def __init__(self, nlib, out_file=None):
>         self.nlib = nlib
> 
>         self._nl = False
>         self._block_end = False
>         self._silent_block = False
>         self._ind = 0
>-        self._out = out_file
>+        if out_file is None:
>+            self._out = os.sys.stdout
>+        else:
>+            self._out = tempfile.TemporaryFile('w+')
>+            self._out_file = out_file
>+
>+    def __del__(self):
>+        self.close_out_file()
>+
>+    def close_out_file(self):
>+        if self._out == os.sys.stdout:
>+            return
>+        with open(self._out_file, 'w+') as out_file:
>+            self._out.seek(0)
>+            shutil.copyfileobj(self._out, out_file)
>+            self._out.close()
>+        self._out = os.sys.stdout
> 
>     @classmethod
>     def _is_cond(cls, line):
>@@ -2313,11 +2329,9 @@ _C_KW = {
>     parser.add_argument('--source', dest='header', action='store_false')
>     parser.add_argument('--user-header', nargs='+', default=[])
>     parser.add_argument('--exclude-op', action='append', default=[])
>-    parser.add_argument('-o', dest='out_file', type=str)
>+    parser.add_argument('-o', dest='out_file', type=str, default=None)
>     args = parser.parse_args()
> 
>-    tmp_file = tempfile.TemporaryFile('w+') if args.out_file else os.sys.stdout
>-
>     if args.header is None:
>         parser.error("--header or --source is required")
> 
>@@ -2341,7 +2355,7 @@ _C_KW = {
>         print(f'Message enum-model {parsed.msg_id_model} not supported for {args.mode} generation')
>         os.sys.exit(1)
> 
>-    cw = CodeWriter(BaseNlLib(), tmp_file)
>+    cw = CodeWriter(BaseNlLib(), args.out_file)
> 
>     _, spec_kernel = find_kernel_root(args.spec)
>     if args.mode == 'uapi' or args.header:
>@@ -2590,10 +2604,6 @@ _C_KW = {
>     if args.header:
>         cw.p(f'#endif /* {hdr_prot} */')
> 
>-    if args.out_file:
>-        out_file = open(args.out_file, 'w+')
>-        tmp_file.seek(0)
>-        shutil.copyfileobj(tmp_file, out_file)
> 
> if __name__ == "__main__":
>     main()
>-- 
>2.41.0
>