libosrmc icon indicating copy to clipboard operation
libosrmc copied to clipboard

Published 20 hours ago • verified publisher icon daniel-j-h

Made compatible with new osrm::engine::api::ResultT

Open akashihi opened this issue 6 years ago • 11 comments

Closes #11

Closes #5548

akashihi avatar Oct 11 '19 15:10 akashihi

If we merge these changes in we fix compilation against libosrm master but break compilation against stable releases and past libosrm releases. I the following steps moving forward:

  • the osrm folks make a proper release
  • we conditionally compile for their old / new api
  • libosrmc can then be kept api and abi compatible

daniel-j-h avatar Oct 11 '19 17:10 daniel-j-h

Ok, let's wait for OSRM release

akashihi avatar Oct 18 '19 08:10 akashihi

OSRM new version is released, should this one be merged?

akashihi avatar Oct 18 '20 17:10 akashihi

@akashihi I'm no longer involved with osrm and/or this library. If you want, I can give you commit access to this repo. so you are not blocked by me not being responsive here.

If we can keep the API/ABI stable here, that would be my number one goal. The implementation is free to change. But then we should conditionally compile the now code. What I mean is

  • if the user has the libosrm C++ headers for 5.23+ we need to compile with the new ResultT changes
  • if the user has the libosrm C++ headers for <5.23+ we need to compile with the old interface

or we say we only support 5.23+ from now on, but this breaks out understanding of ABI/API compatibility across OSRM versions.

daniel-j-h avatar Nov 10 '20 16:11 daniel-j-h

Not that i'm really working on OSRM, but i think i'm obliged to fix that incompatibility, cause i caused it. I'll check if i can maintain compatibility by using some metaprogramming, but in any case plan B will be to use simple preprocessor for that.

PS Yes, write access will definitely help.

akashihi avatar Nov 16 '20 09:11 akashihi

Coolio! I've added you as a collaborator to this repository :bow:

daniel-j-h avatar Nov 17 '20 15:11 daniel-j-h

Thank you! I'll try to find sometime for that issue in December

akashihi avatar Nov 18 '20 14:11 akashihi

Hi, can we get this pull request merged ? @daniel-j-h

mster429 avatar May 20 '21 17:05 mster429

Please read the conversation, we can not just merge it, it would break compilation against previous releases.

The osrm folks have changed their api and we need to conditionally compile against the old or new api now.

Until this is done, we can not merge this pull request.

daniel-j-h avatar May 20 '21 19:05 daniel-j-h

Got it. Thanks for your quick response @daniel-j-h

mster429 avatar May 20 '21 21:05 mster429

Hello is there any improvement un this commt? I want to process this repository for recent osrm version For not use https Kind regards

hogan-chu avatar May 25 '22 21:05 hogan-chu