JSON-RPC: automatically create JSON schema API description #4309

Merged
merged 3 commits into from Apr 30, 2014

Conversation

Projects
None yet
7 participants
@Montellese
Member

Montellese commented Mar 2, 2014

Right now when editing the JSON schema API description of our JSON-RPC API we need to adjust one of the *.json files in xbmc/interfaces/json-rpc and we need to make the same changes in xbmc/interfaces/json-rpc/ServiceDescription.h which is read by the JSON-RPC implementation. But editing that file by hand is very tedious (which is why we have the separate *.json files in the first place). @topfs2 and I have written a tool to do that conversion and it's located at https://github.com/Montellese/jsd_builder. It takes the *.json files, a license.txt and a version.txt file to build the ServiceDescription.h. But right now this is a manual task.

This adds the necessary bits to the win32 build system to automatically create ServiceDescription.h during the build (similar to how it's done for the python code generation).
Would be nice if someone with the necessary linux bootstrap/depends/Makefile foo could add the necessary bits to also automate this in the linux buildsystem. IMO it can be done the same way as for the python code generation (i.e. during bootstrapping IIRC).

Obviously this is Gotham + 1.

@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone Mar 3, 2014

@Montellese Montellese modified the milestones: WIP / RFC (code improvements needed), Pending for inclusion Apr 26, 2014

@Montellese Montellese added the RFC label Apr 26, 2014

@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Apr 28, 2014

Member

@wsnipex mind taking a look?

Member

jmarshallnz commented Apr 28, 2014

@wsnipex mind taking a look?

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Apr 28, 2014

Member

Thanks :)

jenkins build this please

Member

jmarshallnz commented Apr 28, 2014

Thanks :)

jenkins build this please

@jmarshallnz jmarshallnz modified the milestones: Helix 14.0-alpha1, WIP / RFC (code improvements needed) Apr 28, 2014

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Apr 28, 2014

Member

@wsnipex: Thanks. I've added some additional linux-magic which I haven't tested yet so please wait with merging. I'm also not 100% sure if we could/should add some additional logic to Xcode like I did for win32. Will have to check how it's done for the python code generation stuff.

Member

Montellese commented Apr 28, 2014

@wsnipex: Thanks. I've added some additional linux-magic which I haven't tested yet so please wait with merging. I'm also not 100% sure if we could/should add some additional logic to Xcode like I did for win32. Will have to check how it's done for the python code generation stuff.

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Apr 28, 2014

Member

I've removed the additional commit I added because it doesn't work. Ideally there'd be some logic in xbmc/interfaces/json-rpc/Makefile that would automatically trigger a re-generation of ServiceDescription.h if one of the *.json files changed. I tried to do it the same as it is done for the python code generation but that caused some problems which are probably due to the fact that codegenerator.mk now contains logic for two different auto-generated things.

jenkins build this please

Member

Montellese commented Apr 28, 2014

I've removed the additional commit I added because it doesn't work. Ideally there'd be some logic in xbmc/interfaces/json-rpc/Makefile that would automatically trigger a re-generation of ServiceDescription.h if one of the *.json files changed. I tried to do it the same as it is done for the python code generation but that caused some problems which are probably due to the fact that codegenerator.mk now contains logic for two different auto-generated things.

jenkins build this please

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Apr 29, 2014

Member

it would be easy to move the codegen to its own file, if that solves your issue.

Member

wsnipex commented Apr 29, 2014

it would be easy to move the codegen to its own file, if that solves your issue.

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Apr 29, 2014

Member

Yeah I just didn't have time yesterday at 11 pm to give that a try. I'll do that this evening.

Member

Montellese commented Apr 29, 2014

Yeah I just didn't have time yesterday at 11 pm to give that a try. I'll do that this evening.

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Apr 29, 2014

Member

fixed it up to (hopefully) do what you need and sent you a new PR

Member

wsnipex commented Apr 29, 2014

fixed it up to (hopefully) do what you need and sent you a new PR

@Montellese Montellese added Improvement and removed RFC labels Apr 29, 2014

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Apr 29, 2014

Member

@wsnipex: Yup works exactly like I wanted. Thanks a lot.

jenkins build this please (and this time also work on osx et al.)

Member

Montellese commented Apr 29, 2014

@wsnipex: Yup works exactly like I wanted. Thanks a lot.

jenkins build this please (and this time also work on osx et al.)

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Apr 29, 2014

Member

jenkins acting up again: Unrecognized macro 'ghprbPullId' in 'xbmc-pr${ghprbPullId}-${GIT_REVISION,length=7}'

lets try again: jenkins build this please

Member

wsnipex commented Apr 29, 2014

jenkins acting up again: Unrecognized macro 'ghprbPullId' in 'xbmc-pr${ghprbPullId}-${GIT_REVISION,length=7}'

lets try again: jenkins build this please

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Apr 29, 2014

Member

@wsnipex: Hm looks like this approach doesn't work for osx and android as it tries to cross-compile when being called through xbmc/interfaces/json-rpc/Makefile. Any easy way to get around this? If not we'll just go back to the initial bootstrap-only approach for linux et al.

Member

Montellese commented Apr 29, 2014

@wsnipex: Hm looks like this approach doesn't work for osx and android as it tries to cross-compile when being called through xbmc/interfaces/json-rpc/Makefile. Any easy way to get around this? If not we'll just go back to the initial bootstrap-only approach for linux et al.

@davilla

This comment has been minimized.

Show comment
Hide comment
@davilla

davilla Apr 29, 2014

Contributor

Build system tools should be added to depends in the native area. Lets not make another mess like TexturePacker please.

Contributor

davilla commented Apr 29, 2014

Build system tools should be added to depends in the native area. Lets not make another mess like TexturePacker please.

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Apr 29, 2014

Member

yeah, guess it needs to go to depends for cross compile. sigh..

Member

wsnipex commented Apr 29, 2014

yeah, guess it needs to go to depends for cross compile. sigh..

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Apr 29, 2014

Member

win32 build works now with jenkins: http://jenkins.xbmc.org/job/XBMC-WIN-32/798/

Member

Montellese commented Apr 29, 2014

win32 build works now with jenkins: http://jenkins.xbmc.org/job/XBMC-WIN-32/798/

@davilla

This comment has been minimized.

Show comment
Hide comment
@davilla

davilla Apr 29, 2014

Contributor

add it to depends/native is trivial. depends knows how to pull from a git hash.
see tools/depends/target/xbmc-pvr-addons/Makefile
then add a tools/depends/native/jsd_builder, add jsd_builder to tools/depends/native/Makefile, done.

Contributor

davilla commented Apr 29, 2014

add it to depends/native is trivial. depends knows how to pull from a git hash.
see tools/depends/target/xbmc-pvr-addons/Makefile
then add a tools/depends/native/jsd_builder, add jsd_builder to tools/depends/native/Makefile, done.

@davilla

This comment has been minimized.

Show comment
Hide comment
@davilla

davilla Apr 29, 2014

Contributor

look at tools/depends/native/yasm-native/Makefile, it's pretty simple. c/p then change to pull from a git hash.

Contributor

davilla commented Apr 29, 2014

look at tools/depends/native/yasm-native/Makefile, it's pretty simple. c/p then change to pull from a git hash.

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Apr 30, 2014

Member

@wsnipex: Thanks for moving the jsd_builder tool to depends. Let's give this another try.

jenkins build this please

Member

Montellese commented Apr 30, 2014

@wsnipex: Thanks for moving the jsd_builder tool to depends. Let's give this another try.

jenkins build this please

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Apr 30, 2014

Member

All platform builds finally succeeded. Thanks again @wsnipex for your help.

Member

Montellese commented Apr 30, 2014

All platform builds finally succeeded. Thanks again @wsnipex for your help.

@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Apr 30, 2014

Member

Are you gonna merge it? :)

Member

jmarshallnz commented Apr 30, 2014

Are you gonna merge it? :)

Montellese added a commit that referenced this pull request Apr 30, 2014

Merge pull request #4309 from Montellese/jsonrpc_schema_creation
JSON-RPC: automatically create JSON schema API description

@Montellese Montellese merged commit c47fb53 into xbmc:master Apr 30, 2014

1 check passed

default Merged build #560 succeeded in 1 hr 36 min
Details

@Montellese Montellese deleted the Montellese:jsonrpc_schema_creation branch Apr 30, 2014

@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Apr 30, 2014

Member

:) Saves me editing the .h in my branch now ;)

Member

jmarshallnz commented Apr 30, 2014

:) Saves me editing the .h in my branch now ;)

@stefansaraev

This comment has been minimized.

Show comment
Hide comment
@stefansaraev

stefansaraev May 1, 2014

Contributor

you include a pre-built (binary x86_64) JsonSchemaBuilder. this shouldnt be there unless you have a very very good reason. there is none or I am blind...

Contributor

stefansaraev commented May 1, 2014

you include a pre-built (binary x86_64) JsonSchemaBuilder. this shouldnt be there unless you have a very very good reason. there is none or I am blind...

@wsnipex

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex May 1, 2014

Member

omg, good catch. Shame on me. To my defense, I didn't git commit -a, so I must have added it manually...
will remove asap

Member

wsnipex commented May 1, 2014

omg, good catch. Shame on me. To my defense, I didn't git commit -a, so I must have added it manually...
will remove asap

@MartijnKaijser MartijnKaijser modified the milestones: Gotham13.0-rc1, Helix 14.0-alpha1 May 21, 2014

@Memphiz

This comment has been minimized.

Show comment
Hide comment
@Memphiz

Memphiz Jun 2, 2014

Member

@wsnipex - is it by intention to pass CFLAGS to CXX instead of CXXFLAGS and/or CPPFLAGS - also shouldn't it read LDFLAGS instead of LFLAGS? (same below for the .cpp.o branch).

@wsnipex - is it by intention to pass CFLAGS to CXX instead of CXXFLAGS and/or CPPFLAGS - also shouldn't it read LDFLAGS instead of LFLAGS? (same below for the .cpp.o branch).

This comment has been minimized.

Show comment
Hide comment
@wsnipex

wsnipex Jun 2, 2014

Member

agreed, this looks wrong, although it probably doesn't make a difference here. LDFLAGS isn't even used as there are no external deps.
But I didn't write this, just copied JsonSchemaBuilder in here.
@topfs2, @Montellese: ping

Member

wsnipex replied Jun 2, 2014

agreed, this looks wrong, although it probably doesn't make a difference here. LDFLAGS isn't even used as there are no external deps.
But I didn't write this, just copied JsonSchemaBuilder in here.
@topfs2, @Montellese: ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment