Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The big picture for 5.0 #1978

Closed
TheMarex opened this issue Feb 14, 2016 · 6 comments
Closed

The big picture for 5.0 #1978

TheMarex opened this issue Feb 14, 2016 · 6 comments
Milestone

Comments

@TheMarex
Copy link
Member

osrm-backend is currently a mix of different utilities and libraries combined with a huge testing suite. We depend on recent C++ compilers, LUA and Ruby for the backend and javascript for node-osrm and frontend.

The code base currently includes:

  • An own HTTP server (osrm-routed) which uses its own HTTP implementation (!) and query parsing
  • Various utilities with lots of options that use hand written boost program options code for each of those

The plan for 5.0 is to hit the following milestones:

Problems:

  • The split between libosrm and osrm-tools might lead to a similar disconnect as between node-osrm and osrm-backend right now. I feel we can address this in the following way:
    • Keep the master / develop convention.
    • Make osrm-tools/develop always use the latest liborm (via a triggered downstream build on merge to develop)
    • We need to fix this for node-osrm anyway - having more consumers just makes sure the approach is more robust
  • Dropping LUA eventually will lead to profile re-writes. I think in general they will probably be easily transformable

/cc @danpat @daniel-j-h @MoKob @freenerd @springmeyer @emiltin @lbud

@emiltin
Copy link
Contributor

emiltin commented Feb 14, 2016

You mention 'Remove cucumber dependencies'. Do you mean moving cucumber from Ruby to JS rather than actually removing it?

@emiltin
Copy link
Contributor

emiltin commented Feb 14, 2016

Moving cucumber (integration) tests move from osrm-backend to node-osrm and converting to JS is a good idea I think, as long as there's a convenient workflow for testing when you work on osrm-backend (libosrm). Ie. not having to compile different projects or the like.

@danpat
Copy link
Member

danpat commented Feb 18, 2016

@TheMarex I'm a big 👎 on splitting apart osrm-tools and libosrm as separate projects. Other than modular elegance, what purpose would the divide have?

Practically, there are very unlikely to be other consumers of libosrm other than osrm-tools and node-osrm. I would be OK with having osrm-tools and libosrm and node-osrm as separate build-trees inside the same repository, but I definitely do not think they should be separate projects. We do not iterate on them separately (and I can't really see that changing), and as @emiltin points out, we want the developer workflow to be convenient. If we replace osrm-routed with an expressjs driven Node interface that goes through node-osrm to libosrm as we've discussed, then I'll have absolutely no problem releasing a new OSRM version if we have to make changes to any level. A bug in osrm-extract that's written in JS is enough to justify a new release IMO.

My suggestion would be to have everything in one repo, and:

  • build libosrm.a/.so
  • build node-osrm and link it against that
  • osrm-extract/prepare/etc then become simple javascript wrappers that call node-osrm to get work done.
  • tests can be written in JS or C++ and use libosrm.a/node_osrm.so as appropriate

If other parties want to write other language bindings for libosrm.so, then that's fine, they can exist outside, but we should strongly consider embracing JS as a first-class part of OSM, and not making the daily workflow more cumbersome by trying to support secondary use-cases that may never eventuate.

/cc @springmeyer

@frodrigo
Copy link
Member

I'm not a gran fan of your all in JS plan. There is some issues with lua binding, but you want replace it with an other binding, and more over set it as a required/default bridge for build and serve .osrm files. Having basic tools in C++ make it simple to deploy and run..
After migrate to JS we will have chose between an JS/node powered OSRM and Java powered Graphhopper.

@danpat
Copy link
Member

danpat commented Feb 19, 2016

@frodrigo I think there's a good case to be made for osrm-extract, osrm-prepare to remain C++-only tools. The main concern we've discussed internally is osrm-routed: the DIY-http server works, but has had no strong security review, and parsing network packets is a notorious source of buffer-overflow problems. Replacing it with a Node-binding based HTTP server would offload that concern to a third party that has a much bigger set of eyeballs on it.

@willwhite willwhite added this to the 5.0 milestone Feb 23, 2016
@TheMarex
Copy link
Member Author

TheMarex commented Mar 9, 2016

We bailed on some points here:

  • no osrm-backend split at the moment
  • osrm-* tools remain in C++ for 5.0
  • node-osrm is not merged with osrm-backend
  • LUA -> JS switch is probably going to happen in 5.1

@TheMarex TheMarex closed this as completed Mar 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants