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

[Jiri Pirko <jiri@resnulli.us>]

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
>