| Message ID | 20240108182405.1135436-2-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | include: move include/qapi/qmp/ to include/qobject/ | expand |
On Mon, Jan 08, 2024 at 06:23:37PM +0000, Daniel P. Berrangé wrote: > The general expectation is that header files should follow the same > file/path naming scheme as the corresponding source file. There are > various historical exceptions to this practice in QEMU, with one of > the most notable being the include/qapi/qmp/ directory. Most of the > headers there correspond to source files in qobject/. > > This patch corrects that inconsistency by creating include/qobject/. > The only outlier is include/qapi/qmp/dispatch.h which gets renamed > to include/qapi/qmp-registry.h. > > To allow the code to continue to build, symlinks are temporarily > added in $QEMU/qapi/qmp/ to point to the new location. They will > be removed in a later commit. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > MAINTAINERS | 5 +---- > include/qapi/{qmp/dispatch.h => qmp-registry.h} | 0 > include/{qapi/qmp => qobject}/json-parser.h | 0 > include/{qapi/qmp => qobject}/json-writer.h | 0 > include/{qapi/qmp => qobject}/qbool.h | 0 > include/{qapi/qmp => qobject}/qdict.h | 0 > include/{qapi/qmp => qobject}/qerror.h | 0 Of course just after sending this I decided that moving qerror.h to qobject/ is probably not optimal. It only contains a set of (deprecated) error message strings. Perhaps it could just move from qapi/qmp/qerror.h to just qapi/qerror.h ? Other suggestions ? > include/{qapi/qmp => qobject}/qjson.h | 0 > include/{qapi/qmp => qobject}/qlist.h | 0 > include/{qapi/qmp => qobject}/qlit.h | 0 > include/{qapi/qmp => qobject}/qnull.h | 0 > include/{qapi/qmp => qobject}/qnum.h | 0 > include/{qapi/qmp => qobject}/qobject.h | 0 > include/{qapi/qmp => qobject}/qstring.h | 0 With regards, Daniel
On Mon, Jan 08, 2024 at 06:46:38PM +0000, Daniel P. Berrangé wrote: > Date: Mon, 8 Jan 2024 18:46:38 +0000 > From: "Daniel P. Berrangé" <berrange@redhat.com> > Subject: Re: [PATCH 01/29] include: move include/qapi/qmp/ to > include/qobject/ > > On Mon, Jan 08, 2024 at 06:23:37PM +0000, Daniel P. Berrangé wrote: > > The general expectation is that header files should follow the same > > file/path naming scheme as the corresponding source file. There are > > various historical exceptions to this practice in QEMU, with one of > > the most notable being the include/qapi/qmp/ directory. Most of the > > headers there correspond to source files in qobject/. > > > > This patch corrects that inconsistency by creating include/qobject/. > > The only outlier is include/qapi/qmp/dispatch.h which gets renamed > > to include/qapi/qmp-registry.h. > > > > To allow the code to continue to build, symlinks are temporarily > > added in $QEMU/qapi/qmp/ to point to the new location. They will > > be removed in a later commit. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > MAINTAINERS | 5 +---- > > include/qapi/{qmp/dispatch.h => qmp-registry.h} | 0 > > include/{qapi/qmp => qobject}/json-parser.h | 0 > > include/{qapi/qmp => qobject}/json-writer.h | 0 > > include/{qapi/qmp => qobject}/qbool.h | 0 > > include/{qapi/qmp => qobject}/qdict.h | 0 > > include/{qapi/qmp => qobject}/qerror.h | 0 > > Of course just after sending this I decided that moving qerror.h > to qobject/ is probably not optimal. It only contains a set of > (deprecated) error message strings. Perhaps it could just move > from qapi/qmp/qerror.h to just qapi/qerror.h ? Other suggestions ? From the naming style ("q" + module name) and the content comments (descripted as a module), qerror.h (as an error module starting with q) seems to be more neatly put together with other qmodules such as qbool.h, qdirct.h, qlist.h, etc. There is already an error.h under the include/qapi, which is supposed to be the developer's first choice, and it seems a bit confusing to have qerror.h in the same directory as error.h (even though it states that qerror.h will be deprecated)? Regards, Zhao > > > include/{qapi/qmp => qobject}/qjson.h | 0 > > include/{qapi/qmp => qobject}/qlist.h | 0 > > include/{qapi/qmp => qobject}/qlit.h | 0 > > include/{qapi/qmp => qobject}/qnull.h | 0 > > include/{qapi/qmp => qobject}/qnum.h | 0 > > include/{qapi/qmp => qobject}/qobject.h | 0 > > include/{qapi/qmp => qobject}/qstring.h | 0 > > With regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| > >
I just realized I dropped this on the floor. I apologize for the delay. Daniel P. Berrangé <berrange@redhat.com> writes: > On Mon, Jan 08, 2024 at 06:23:37PM +0000, Daniel P. Berrangé wrote: >> The general expectation is that header files should follow the same >> file/path naming scheme as the corresponding source file. There are >> various historical exceptions to this practice in QEMU, with one of >> the most notable being the include/qapi/qmp/ directory. Most of the >> headers there correspond to source files in qobject/. >> >> This patch corrects that inconsistency by creating include/qobject/. Yes, there's inconsistency, but is it worth cleaning up? Since you did the work already, and sunk cost doesn't count, ... >> The only outlier is include/qapi/qmp/dispatch.h which gets renamed >> to include/qapi/qmp-registry.h. Good, as "QMP registry" is a more accurate description than "QMP dispatch". >> To allow the code to continue to build, symlinks are temporarily >> added in $QEMU/qapi/qmp/ to point to the new location. They will >> be removed in a later commit. Only necessary to let you split the patch updating #include directives. The update is entirely mechanical, isn't it? I doubt splitting is worth the trouble then. >> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> >> --- >> MAINTAINERS | 5 +---- >> include/qapi/{qmp/dispatch.h => qmp-registry.h} | 0 >> include/{qapi/qmp => qobject}/json-parser.h | 0 >> include/{qapi/qmp => qobject}/json-writer.h | 0 >> include/{qapi/qmp => qobject}/qbool.h | 0 >> include/{qapi/qmp => qobject}/qdict.h | 0 >> include/{qapi/qmp => qobject}/qerror.h | 0 > > Of course just after sending this I decided that moving qerror.h > to qobject/ is probably not optimal. It only contains a set of > (deprecated) error message strings. Perhaps it could just move > from qapi/qmp/qerror.h to just qapi/qerror.h ? Other suggestions ? qapi/qerror.h works for me. Could use the opportunity to rename it to qapi/deprecated-qerror.h. >> include/{qapi/qmp => qobject}/qjson.h | 0 >> include/{qapi/qmp => qobject}/qlist.h | 0 >> include/{qapi/qmp => qobject}/qlit.h | 0 >> include/{qapi/qmp => qobject}/qnull.h | 0 >> include/{qapi/qmp => qobject}/qnum.h | 0 >> include/{qapi/qmp => qobject}/qobject.h | 0 >> include/{qapi/qmp => qobject}/qstring.h | 0 > > With regards, > Daniel
Markus Armbruster <armbru@redhat.com> writes: > I just realized I dropped this on the floor. I apologize for the delay. > > Daniel P. Berrangé <berrange@redhat.com> writes: > >> On Mon, Jan 08, 2024 at 06:23:37PM +0000, Daniel P. Berrangé wrote: >>> The general expectation is that header files should follow the same >>> file/path naming scheme as the corresponding source file. There are >>> various historical exceptions to this practice in QEMU, with one of >>> the most notable being the include/qapi/qmp/ directory. Most of the >>> headers there correspond to source files in qobject/. >>> >>> This patch corrects that inconsistency by creating include/qobject/. > > Yes, there's inconsistency, but is it worth cleaning up? Since you did > the work already, and sunk cost doesn't count, ... Funny: commit 7b1b5d191385ca52e96caae2a05c64f3a63855d9 Author: Paolo Bonzini <pbonzini@redhat.com> Date: Mon Dec 17 18:19:43 2012 +0100 qapi: move include files to include/qobject/ That's what you want! However, ... Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> [...] qapi/qmp-core.h => include/qapi/qmp/dispatch.h | 6 +++--- json-lexer.h => include/qapi/qmp/json-lexer.h | 4 ++-- json-parser.h => include/qapi/qmp/json-parser.h | 4 ++-- json-streamer.h => include/qapi/qmp/json-streamer.h | 4 ++-- qbool.h => include/qapi/qmp/qbool.h | 2 +- qdict.h => include/qapi/qmp/qdict.h | 4 ++-- qerror.h => include/qapi/qmp/qerror.h | 6 +++--- qfloat.h => include/qapi/qmp/qfloat.h | 2 +- qint.h => include/qapi/qmp/qint.h | 2 +- qjson.h => include/qapi/qmp/qjson.h | 4 ++-- qlist.h => include/qapi/qmp/qlist.h | 2 +- qobject.h => include/qapi/qmp/qobject.h | 0 qstring.h => include/qapi/qmp/qstring.h | 2 +- qemu-objects.h => include/qapi/qmp/types.h | 16 ++++++++-------- [...] ... it's not what the patch does %-} [...]
On Thu, Apr 25, 2024 at 11:34:46AM +0200, Markus Armbruster wrote: > I just realized I dropped this on the floor. I apologize for the delay. > > Daniel P. Berrangé <berrange@redhat.com> writes: > > > On Mon, Jan 08, 2024 at 06:23:37PM +0000, Daniel P. Berrangé wrote: > >> The general expectation is that header files should follow the same > >> file/path naming scheme as the corresponding source file. There are > >> various historical exceptions to this practice in QEMU, with one of > >> the most notable being the include/qapi/qmp/ directory. Most of the > >> headers there correspond to source files in qobject/. > >> > >> This patch corrects that inconsistency by creating include/qobject/. > > Yes, there's inconsistency, but is it worth cleaning up? Since you did > the work already, and sunk cost doesn't count, ... The motivation is my own inability to remaember that the qboject/*.c header files are in include/qapi/qmp/. I only need to find them every 6-12 months or so, and thus I've always forgotten their wierd location by that point ! > >> To allow the code to continue to build, symlinks are temporarily > >> added in $QEMU/qapi/qmp/ to point to the new location. They will > >> be removed in a later commit. > > Only necessary to let you split the patch updating #include directives. > The update is entirely mechanical, isn't it? I doubt splitting is worth > the trouble then. Yes, it was to allow succesfully building at each patch. Changes were basically a sed/perl command IIRC. With regards, Daniel
diff --git a/MAINTAINERS b/MAINTAINERS index 395f26ba86..d72e040a9a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3154,8 +3154,6 @@ S: Supported F: qapi/ X: qapi/*.json F: include/qapi/ -X: include/qapi/qmp/ -F: include/qapi/qmp/dispatch.h F: tests/qapi-schema/ F: tests/unit/test-*-visitor.c F: tests/unit/test-qapi-*.c @@ -3178,8 +3176,7 @@ QObject M: Markus Armbruster <armbru@redhat.com> S: Supported F: qobject/ -F: include/qapi/qmp/ -X: include/qapi/qmp/dispatch.h +F: include/qobject/ F: scripts/coccinelle/qobject.cocci F: tests/unit/check-qdict.c F: tests/unit/check-qjson.c diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp-registry.h similarity index 100% rename from include/qapi/qmp/dispatch.h rename to include/qapi/qmp-registry.h diff --git a/include/qapi/qmp/json-parser.h b/include/qobject/json-parser.h similarity index 100% rename from include/qapi/qmp/json-parser.h rename to include/qobject/json-parser.h diff --git a/include/qapi/qmp/json-writer.h b/include/qobject/json-writer.h similarity index 100% rename from include/qapi/qmp/json-writer.h rename to include/qobject/json-writer.h diff --git a/include/qapi/qmp/qbool.h b/include/qobject/qbool.h similarity index 100% rename from include/qapi/qmp/qbool.h rename to include/qobject/qbool.h diff --git a/include/qapi/qmp/qdict.h b/include/qobject/qdict.h similarity index 100% rename from include/qapi/qmp/qdict.h rename to include/qobject/qdict.h diff --git a/include/qapi/qmp/qerror.h b/include/qobject/qerror.h similarity index 100% rename from include/qapi/qmp/qerror.h rename to include/qobject/qerror.h diff --git a/include/qapi/qmp/qjson.h b/include/qobject/qjson.h similarity index 100% rename from include/qapi/qmp/qjson.h rename to include/qobject/qjson.h diff --git a/include/qapi/qmp/qlist.h b/include/qobject/qlist.h similarity index 100% rename from include/qapi/qmp/qlist.h rename to include/qobject/qlist.h diff --git a/include/qapi/qmp/qlit.h b/include/qobject/qlit.h similarity index 100% rename from include/qapi/qmp/qlit.h rename to include/qobject/qlit.h diff --git a/include/qapi/qmp/qnull.h b/include/qobject/qnull.h similarity index 100% rename from include/qapi/qmp/qnull.h rename to include/qobject/qnull.h diff --git a/include/qapi/qmp/qnum.h b/include/qobject/qnum.h similarity index 100% rename from include/qapi/qmp/qnum.h rename to include/qobject/qnum.h diff --git a/include/qapi/qmp/qobject.h b/include/qobject/qobject.h similarity index 100% rename from include/qapi/qmp/qobject.h rename to include/qobject/qobject.h diff --git a/include/qapi/qmp/qstring.h b/include/qobject/qstring.h similarity index 100% rename from include/qapi/qmp/qstring.h rename to include/qobject/qstring.h diff --git a/qapi/qmp/dispatch.h b/qapi/qmp/dispatch.h new file mode 120000 index 0000000000..ffedc3971d --- /dev/null +++ b/qapi/qmp/dispatch.h @@ -0,0 +1 @@ +../../include/qapi/qmp-registry.h \ No newline at end of file diff --git a/qapi/qmp/json-parser.h b/qapi/qmp/json-parser.h new file mode 120000 index 0000000000..059cb73fa8 --- /dev/null +++ b/qapi/qmp/json-parser.h @@ -0,0 +1 @@ +../../include/qobject/json-parser.h \ No newline at end of file diff --git a/qapi/qmp/json-writer.h b/qapi/qmp/json-writer.h new file mode 120000 index 0000000000..3e952f4c97 --- /dev/null +++ b/qapi/qmp/json-writer.h @@ -0,0 +1 @@ +../../include/qobject/json-writer.h \ No newline at end of file diff --git a/qapi/qmp/qbool.h b/qapi/qmp/qbool.h new file mode 120000 index 0000000000..443c881cf8 --- /dev/null +++ b/qapi/qmp/qbool.h @@ -0,0 +1 @@ +../../include/qobject/qbool.h \ No newline at end of file diff --git a/qapi/qmp/qdict.h b/qapi/qmp/qdict.h new file mode 120000 index 0000000000..8183614eae --- /dev/null +++ b/qapi/qmp/qdict.h @@ -0,0 +1 @@ +../../include/qobject/qdict.h \ No newline at end of file diff --git a/qapi/qmp/qerror.h b/qapi/qmp/qerror.h new file mode 120000 index 0000000000..cf210737a3 --- /dev/null +++ b/qapi/qmp/qerror.h @@ -0,0 +1 @@ +../../include/qobject/qerror.h \ No newline at end of file diff --git a/qapi/qmp/qjson.h b/qapi/qmp/qjson.h new file mode 120000 index 0000000000..85b48c5bfd --- /dev/null +++ b/qapi/qmp/qjson.h @@ -0,0 +1 @@ +../../include/qobject/qjson.h \ No newline at end of file diff --git a/qapi/qmp/qlist.h b/qapi/qmp/qlist.h new file mode 120000 index 0000000000..d40db0a12b --- /dev/null +++ b/qapi/qmp/qlist.h @@ -0,0 +1 @@ +../../include/qobject/qlist.h \ No newline at end of file diff --git a/qapi/qmp/qlit.h b/qapi/qmp/qlit.h new file mode 120000 index 0000000000..5dd5ac8ccb --- /dev/null +++ b/qapi/qmp/qlit.h @@ -0,0 +1 @@ +../../include/qobject/qlit.h \ No newline at end of file diff --git a/qapi/qmp/qnull.h b/qapi/qmp/qnull.h new file mode 120000 index 0000000000..944769d44b --- /dev/null +++ b/qapi/qmp/qnull.h @@ -0,0 +1 @@ +../../include/qobject/qnull.h \ No newline at end of file diff --git a/qapi/qmp/qnum.h b/qapi/qmp/qnum.h new file mode 120000 index 0000000000..8038e2f4d6 --- /dev/null +++ b/qapi/qmp/qnum.h @@ -0,0 +1 @@ +../../include/qobject/qnum.h \ No newline at end of file diff --git a/qapi/qmp/qobject.h b/qapi/qmp/qobject.h new file mode 120000 index 0000000000..89d9118cfd --- /dev/null +++ b/qapi/qmp/qobject.h @@ -0,0 +1 @@ +../../include/qobject/qobject.h \ No newline at end of file diff --git a/qapi/qmp/qstring.h b/qapi/qmp/qstring.h new file mode 120000 index 0000000000..24f48de18a --- /dev/null +++ b/qapi/qmp/qstring.h @@ -0,0 +1 @@ +../../include/qobject/qstring.h \ No newline at end of file
The general expectation is that header files should follow the same file/path naming scheme as the corresponding source file. There are various historical exceptions to this practice in QEMU, with one of the most notable being the include/qapi/qmp/ directory. Most of the headers there correspond to source files in qobject/. This patch corrects that inconsistency by creating include/qobject/. The only outlier is include/qapi/qmp/dispatch.h which gets renamed to include/qapi/qmp-registry.h. To allow the code to continue to build, symlinks are temporarily added in $QEMU/qapi/qmp/ to point to the new location. They will be removed in a later commit. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- MAINTAINERS | 5 +---- include/qapi/{qmp/dispatch.h => qmp-registry.h} | 0 include/{qapi/qmp => qobject}/json-parser.h | 0 include/{qapi/qmp => qobject}/json-writer.h | 0 include/{qapi/qmp => qobject}/qbool.h | 0 include/{qapi/qmp => qobject}/qdict.h | 0 include/{qapi/qmp => qobject}/qerror.h | 0 include/{qapi/qmp => qobject}/qjson.h | 0 include/{qapi/qmp => qobject}/qlist.h | 0 include/{qapi/qmp => qobject}/qlit.h | 0 include/{qapi/qmp => qobject}/qnull.h | 0 include/{qapi/qmp => qobject}/qnum.h | 0 include/{qapi/qmp => qobject}/qobject.h | 0 include/{qapi/qmp => qobject}/qstring.h | 0 qapi/qmp/dispatch.h | 1 + qapi/qmp/json-parser.h | 1 + qapi/qmp/json-writer.h | 1 + qapi/qmp/qbool.h | 1 + qapi/qmp/qdict.h | 1 + qapi/qmp/qerror.h | 1 + qapi/qmp/qjson.h | 1 + qapi/qmp/qlist.h | 1 + qapi/qmp/qlit.h | 1 + qapi/qmp/qnull.h | 1 + qapi/qmp/qnum.h | 1 + qapi/qmp/qobject.h | 1 + qapi/qmp/qstring.h | 1 + 27 files changed, 14 insertions(+), 4 deletions(-) rename include/qapi/{qmp/dispatch.h => qmp-registry.h} (100%) rename include/{qapi/qmp => qobject}/json-parser.h (100%) rename include/{qapi/qmp => qobject}/json-writer.h (100%) rename include/{qapi/qmp => qobject}/qbool.h (100%) rename include/{qapi/qmp => qobject}/qdict.h (100%) rename include/{qapi/qmp => qobject}/qerror.h (100%) rename include/{qapi/qmp => qobject}/qjson.h (100%) rename include/{qapi/qmp => qobject}/qlist.h (100%) rename include/{qapi/qmp => qobject}/qlit.h (100%) rename include/{qapi/qmp => qobject}/qnull.h (100%) rename include/{qapi/qmp => qobject}/qnum.h (100%) rename include/{qapi/qmp => qobject}/qobject.h (100%) rename include/{qapi/qmp => qobject}/qstring.h (100%) create mode 120000 qapi/qmp/dispatch.h create mode 120000 qapi/qmp/json-parser.h create mode 120000 qapi/qmp/json-writer.h create mode 120000 qapi/qmp/qbool.h create mode 120000 qapi/qmp/qdict.h create mode 120000 qapi/qmp/qerror.h create mode 120000 qapi/qmp/qjson.h create mode 120000 qapi/qmp/qlist.h create mode 120000 qapi/qmp/qlit.h create mode 120000 qapi/qmp/qnull.h create mode 120000 qapi/qmp/qnum.h create mode 120000 qapi/qmp/qobject.h create mode 120000 qapi/qmp/qstring.h