Introspectable interface file not generated in some cases. #17

Closed
opened 5 years ago by MicheleC · 13 comments
Owner

Using attached file test1.xml and running

dbusxml2qt3 test1.xml

prints out

[2019/05/04 22:05:10.933] ClassGenerator: processing interface 'org.freedesktop.DBus.Introspectable'
[2019/05/04 22:05:10.933] ClassGenerator: processing interface 'org.freedesktop.DBus.ObjectManager'

but the file "introspectableInterface.h" is not created, although it is included into "objectmanagerNode.cpp".

Using attached file tde.xml, the "introspectableInterface.h" file is correctly created.

Notice one difference in the .xml file: in test1.xml, the introspectable interface is declared, in tde.xml it is not.

Using attached file test1.xml and running ``` dbusxml2qt3 test1.xml ``` prints out ``` [2019/05/04 22:05:10.933] ClassGenerator: processing interface 'org.freedesktop.DBus.Introspectable' [2019/05/04 22:05:10.933] ClassGenerator: processing interface 'org.freedesktop.DBus.ObjectManager' ``` but the file "introspectableInterface.h" is not created, although it is included into "objectmanagerNode.cpp". Using attached file tde.xml, the "introspectableInterface.h" file is correctly created. Notice one difference in the .xml file: in test1.xml, the introspectable interface is declared, in tde.xml it is not.
Poster
Owner
There is no content yet.
Collaborator

Hi Michele,
yes it is strange but not bad. If Introspectable is in the file it is not created. If it is not there, it is created on demand. It was like this from day one.

I use this in example 4e to create two interfaces. Since Introspectable is standard interface I do not need to create it twice.

However there might be more intuitive approach.

Hi Michele, yes it is strange but not bad. If Introspectable is in the file it is not created. If it is not there, it is created on demand. It was like this from day one. I use this in example 4e to create two interfaces. Since Introspectable is standard interface I do not need to create it twice. However there might be more intuitive approach.
Owner

I haven't tried it yet, but it's definitely not good for the file not to be generated, but regardless of it was listed as include.

I haven't tried it yet, but it's definitely not good for the file not to be generated, but regardless of it was listed as include.
Collaborator

I think it would be good to generate Introspectable if it is in the file and not generate it if it is not - so exactly the opposite now.

I think it would be good to generate Introspectable if it is in the file and not generate it if it is not - so exactly the opposite now.
Poster
Owner

I think the current behavior is wrong because it would cause FTBFS. Instead the correct behavior should be one of the following:

  1. always generate the Introspectable interface
  2. generate the Introspectable interface only if present in the xml file.
    My preference is for option 1. What do you think?
I think the current behavior is wrong because it would cause FTBFS. Instead the correct behavior should be one of the following: 1. always generate the Introspectable interface 2. generate the Introspectable interface only if present in the xml file. My preference is for option 1. What do you think?
Collaborator

If you look in the more complex examples 4d and 4e, it is obvious that Introspectable is expected only when there is another interface at this location.
Perhaps this was the motivation behind current situation, but things changed afterwards (perhaps in conjunction to -i and -n/-c options). I can not conclude now what is better. I fear 1) would have undesired results if you did not mean what I also mean - create always when there is another interface at this path location. Perhaps option 2) gives us more control and would correspond to what we receive from dbus if we execute Introspect. Imagine execute Introspectable.Introspect and receive Introspectable interface for the node - this will give you exactly the interfaces that you need to generate.

If you look in the more complex examples 4d and 4e, it is obvious that Introspectable is expected only when there is another interface at this location. Perhaps this was the motivation behind current situation, but things changed afterwards (perhaps in conjunction to -i and -n/-c options). I can not conclude now what is better. I fear 1) would have undesired results if you did not mean what I also mean - create always when there is another interface at this path location. Perhaps option 2) gives us more control and would correspond to what we receive from dbus if we execute Introspect. Imagine execute Introspectable.Introspect and receive Introspectable interface for the node - this will give you exactly the interfaces that you need to generate.
Poster
Owner

DBus specification (see here) say that "Objects instances may implement Introspect", so it seems it is not mandatory. In light of such description, option 2 seems more correct. Obviously, when Introspectable is not generated, it must not be included into the node .cpp file.

DBus specification (see [here](https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces)) say that "Objects instances *may* implement Introspect", so it seems it is not mandatory. In light of such description, option 2 seems more correct. Obviously, when Introspectable is not generated, it must not be included into the node .cpp file.
Collaborator

Hi Michele,
I did not have the time to check, but this was somewhere deep in my memories. Thank you for checking and sorting this out. However, please think with Slavek also what would be the impact on -i and perhaps -p, -n , -c. It should not generate if not requested in such case.

I still do not understand why it is been developed like it was - there might have been some reason behind, but I agree with you on option 2) - lets do it and see how it will work.

Hi Michele, I did not have the time to check, but this was somewhere deep in my memories. Thank you for checking and sorting this out. However, please think with Slavek also what would be the impact on -i and perhaps -p, -n , -c. It should not generate if not requested in such case. I still do not understand why it is been developed like it was - there might have been some reason behind, but I agree with you on option 2) - lets do it and see how it will work.
Poster
Owner

Yes, obviously the behavior has to be consistent. For what I have seen so far, interfaces are generated only with no parameters are given or when -i is specified. In both cases, we need to generate the Introspectable interface only if it is present in the xml file. In case -i is given, all interfaces will end up in a single file, but that is already the current behavior, so I think it will not require too much changes, hopefully.

Yes, obviously the behavior has to be consistent. For what I have seen so far, interfaces are generated only with no parameters are given or when -i is specified. In both cases, we need to generate the Introspectable interface only if it is present in the xml file. In case -i is given, all interfaces will end up in a single file, but that is already the current behavior, so I think it will not require too much changes, hopefully.
Poster
Owner

I have looked at this again. I believe the intention is to generate the introspectable interface in all cases, for semplicity.

  1. if it is not defined in the .xml file, the interface is already created on purpose, including the file.
  2. if introspectable interface is defined in the xml file, there is no need to create the interface on purpose again. For some reasons, the code is not creating the required file at all.

I have started to look at the code, I may have a solution by tomorrow hopefully

I have looked at this again. I believe the intention is to generate the introspectable interface in all cases, for semplicity. 1. if it is not defined in the .xml file, the interface is already created on purpose, including the file. 2. if introspectable interface is defined in the xml file, there is no need to create the interface on purpose again. For some reasons, the code is not creating the required file at all. I have started to look at the code, I may have a solution by tomorrow hopefully
Collaborator

On line 119 you have hasIntrospectable = true; and on line 292 if (!hasIntrospectable). I guess if we reverse the latter if (hasIntrospectable), it will generate whenever introspection is in the xml, but much better it would be to check if interfaces are defined, because AFAIR Introspection is implicit, but the Introspectable interface is optional and makes sense only if there are other interfaces defined in the node.

So I would do

 if ( !interfaces.isEmpty() && hasIntrospectable )

hope it helps

On line 119 you have hasIntrospectable = true; and on line 292 if (!hasIntrospectable). I guess if we reverse the latter if (hasIntrospectable), it will generate whenever introspection is in the xml, but much better it would be to check if interfaces are defined, because AFAIR Introspection is implicit, but the Introspectable interface is optional and makes sense only if there are other interfaces defined in the node. So I would do if ( !interfaces.isEmpty() && hasIntrospectable ) hope it helps
MicheleC closed this issue 5 years ago
Poster
Owner

The above commit fixes the issue. The code now generates exactly the same code and files regardless of whether the Introspectable interface is defined in the xml file or not.

I have thought at length whether or not to generate the Introspectable interface if not defined. Although optional, the answer was to generate it in all cases as the old code was trying to do. This is because the Introspectable interface is very nice to have and choosing to generate it only when defined in the xml file would have meant to have to make more changes to fix up tdelibs as well. Plus it is nice to have it automatically, since it is pretty much a de-facto "must have", although the spec say it is optional.

The above commit fixes the issue. The code now generates exactly the same code and files regardless of whether the Introspectable interface is defined in the xml file or not. I have thought at length whether or not to generate the Introspectable interface if not defined. Although optional, the answer was to generate it in all cases as the old code was trying to do. This is because the Introspectable interface is very nice to have and choosing to generate it only when defined in the xml file would have meant to have to make more changes to fix up tdelibs as well. Plus it is nice to have it automatically, since it is pretty much a de-facto "must have", although the spec say it is optional.
MicheleC added this to the R14.1.0 release milestone 5 years ago
MicheleC reopened this issue 5 years ago
Poster
Owner

It seems in my previous testing I missed one thing. If the Introspectable interface is not declared in the .xml file, the instrospectableInterface header is not included in the node file.

It seems in my previous testing I missed one thing. If the Introspectable interface is not declared in the .xml file, the instrospectableInterface header is not included in the node file.
MicheleC closed this issue 5 years ago
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TDE/dbus-1-tqt#17
Loading…
There is no content yet.