Created attachment 178234 [details] osrm.shar This adds support for osrm http://project-osrm.org/ to ports. Please let me know if I am doing something incorrectly.
Created attachment 184501 [details] Newest version, port renamed to mirror name in GH project This updates to latest version: 5.8.0, tried and tested in production. It also renames the port to osrm-backend to better match the project name in Project-OSRM. If this is in the incorrect category, let me know and I will change the port category.
Created attachment 184968 [details] Update to 5.9.0
portlint: root@newtop:/usr/ports/www/osrm-backend # portlint looks fine.
I might need help to choose a proper uid/gid for this one as well.
Created attachment 184985 [details] 5.9.0 added user and group As per koobs wishes here are some changes to submittal of the port.
Created attachment 184987 [details] Updated with choosen user and group
Review items: PORTVERSION is invalid. Use DISTVERSIONPREFIX for the 'v' prefix of the GitHub tagname. GH_TAGNAME (doesnt need to be set explicitly) will then be set correctly. MASTER_SITES=GH is not required when using USE_GITHUB=yes See Also: https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/book.html#makefile-master_sites-github-ex3 Pending QA (poudriere) testing/confirmation
Created attachment 185111 [details] Poudriere log
Created attachment 185112 [details] Fully working Changed user, corrected UIDs file.
Created attachment 185113 [details] Update to diff Removes GITHUB_PROJECT and +@${MKDIR} ${WRKSRC}/build as per suggestions by koobs.
Created attachment 185115 [details] Update to 5.10.0
Created attachment 185127 [details] Passes portlint -AC This diff makes the Makefile pass portlint -AC
Created attachment 185143 [details] Corrects maintainer
Ping
testbuild@work
Builds are fine on 11a, 12a, but fail on 10.3i: http://people.freebsd.org/~pi/logs/www__osrm-backend-10i-1502349156.txt
Thanks Kurt! Andreas: please update the patch to correct the build-issues. I will have a look at it than.
Upstream issue (may or may not be exactly the same issue, please verify), closed without changes: Invalid use of non-static data member 'offset' https://github.com/Project-OSRM/osrm-backend/issues/3719 Also noted: * -std=c++1y is included in compiler args CC dim who may be able to shed light on (or remind us about) FreeBSD 10.x (10.3 in this case), Clang and C++11 support or issues
I could make it build with using llvm40 BUILD_DEPENDS+= clang++40:devel/llvm40 BUILD_DEPENDS+= clang++40:devel/llvm40 BUILD_DEPENDS+= llvm-config40:devel/llvm40 and CC= clang40 CXX= clang++40 But then there's: https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/dads-cc.html advising against it. What's the best way to proceed with this? Is it okay for me to submit a patch with the above lines?
I took a look at the compilation error, and it seems to have been fixed in newer versions of clang, but it's rather hard to come up with a small test case, and figuring out which exact upstream revision fixed it. So for now the easiest workaround would be to build it with a newer clang from ports, or gcc.
Created attachment 185331 [details] Fixes build issue on 10.3 Fixes build issue on 10.3. Also tested with 11.1, not tested with 12.
Testbuilds are fine on 12a, 11a and 10i 8-)
* Because you are using DISTVERSIONPREFIX, should probably use DISTVERSION instead of PORTVERSION. * Add LICENSE_FILE if it is available in the source. * Instead of calling var with append flag with *_DEPENDS, this should be like so LIB_DEPENDS= foo.so:devel/foo \ <tab> <tab> bar.so:devel/bar \ ... ... and so on. * CMAKE_BUILD_TYPE Should be after USES section. Would have to test the port to verify the compiler_type bit after bsd.port.pre.mk. I think there is a better way to set the minimum version of clang. Pretty sure testing COMPILER_TYPE should be CHOSEN_COMPILER_TYPE, but not positive on this.
Richard I have in my local copy fixed most of the issues you commented on. I would like to get help with specifying the compiler version properly. Ideally I would like to be able to set a variable for this, such as COMPILER= clang and COMPILER_VERSION= >=40 or of it's likes. But looking at compiler.mk it seems to to only be used for comparisons. What is a cleaner way of doing this?
Created attachment 185402 [details] New Makefile For some reason my ports tree seems messed up and doesn't catch the changes so I am not getting the full diff. Is there some weird caching going on with svnlite? deleting tree and and checking it out again did not help. Hence attaching Makefile. Seems that when I do compiler:c++14-lang ports pulls down llvm40 all by it self. Magic. Hence there is no more need for the COMPILER/COMPILER_VERSION conditional or overriding CC and CXX vars. I also added LICENSE_FILE and re-arranged and changed vars in the Makefile as per Richards suggestions.
Created attachment 185405 [details] Cleanup * Pulls in llvm40 by specifying compiler:c++14-lang. * Removes conditional, adjusts LIB_DEPENDS as per Richards suggestions. * Adds LICENSE_FILE. * portlint -AC gives no warnings/errors. * poudriere porttest runs fine on 10.3 and 11.
(In reply to Andreas Andersson from comment #27) That was one solution I was considering may fix the problem. I'm not sure if the port actually requires c++14 though, but it is better than setting cc and cxx. > For some reason my ports tree seems messed up and doesn't catch the changes so I am not getting the full diff. Is there some weird caching going on with svnlite? deleting tree and and checking it out again did not help. I'm not sure I really understand, Usually svnlite diff foo bar devel/foobar >foobar.diff Assuming it has already been added svnlite add ... A couple other things, * OSRMSHAREDIR is not needed, ${DATADIR} by default is set to ${PREFIX}/share/${PORTNAME}. * I'm not sure I understand the osrm_options variable in in the rc file. It is overridable and doesn't really set anything so the vars inside it should probably just be moved to command_args. * install -o $osrm_user /dev/null ${pidfile}. Why is this creating a file in /dev/null? this shouldn't be this services task. * Should this services default shell be /bin/sh or /usr/sbin/nologin? Does the program need this specifically? Same question with the set home directory.
(In reply to Richard Gallamore from comment #28) > * OSRMSHAREDIR is not needed, ${DATADIR} by default is set to ${PREFIX}/share/${PORTNAME}. Will be fixing this shortly > * install -o $osrm_user /dev/null ${pidfile}. Why is this creating a file in /dev/null? this shouldn't be this services task. What do you suggest instead? > * Should this services default shell be /bin/sh or /usr/sbin/nologin? Does the program need this specifically? Same question with the set home directory. I'll try without those two.
(In reply to Andreas Andersson from comment #29) Err, I wasn't properly caffeinated when looking at this this morning. The install pid looks fine. Could probably have more restrictive perms like 644. I was actually one line off, chown ${osrm_user}:${osrm_group} ${osrm_file} this command will fail without testing osrm_file variable is set or setting this to a default. Should have a test before running. [ -n "${osrm_file" ] && chown ...
Created attachment 185417 [details] More cleanup Turns out there's even more magic in Cmake / ports. * Removed some of the manual copying of files and dir creation. * Added check in osrm rc.d script to make sure file exist before trying to chown datafiles. * Changed osrm user to use /usr/sbin/nologin * Set /nonexistent as dir for osrm user. * Set permissions 644 on the datadir.
I think you forgot the osrm_options, this isn't really doing anything and can be user modifiable. It should probably be removed.
(In reply to Richard Gallamore from comment #32) >I think you forgot the osrm_options, this isn't really doing anything and can be user modifiable. It should probably be removed. It is intended to be used when for instance one would want to modify the number of threads osrm would use, or to set the listening ip, for instance if you would like to use shared memory. osrm-routed <base.osrm> [<options>]: Options: -v [ --version ] Show version -h [ --help ] Show this help message --trial [=arg(=1)] Quit after initialization Configuration: -i [ --ip ] arg (=0.0.0.0) IP address -p [ --port ] arg (=5000) TCP/IP port -t [ --threads ] arg (=8) Number of threads to use -s [ --shared-memory ] [=arg(=1)] (=0) Load data from shared memory -a [ --algorithm ] arg (=CH) Algorithm to use for the data. Can be CH, CoreCH, MLD. --max-viaroute-size arg (=500) Max. locations supported in viaroute query --max-trip-size arg (=100) Max. locations supported in trip query --max-table-size arg (=100) Max. locations supported in distance table query --max-matching-size arg (=100) Max. locations supported in map matching query --max-nearest-size arg (=100) Max. results supported in nearest query --max-alternatives arg (=3) Max. number of alternatives supported in the MLD route query
So - did you reach a conclusion here? Is the port in a shape it could be committed?
Here is the problem with the rc script: /etc/rc.conf osrm_enable="YES" osrm_flags="-i 10.0.0.2" osrm_file="/usr/local/etc/myfile" osrm_options="--max-table-size 50" The command ran would translate to this: /usr/sbin/daemon -f -c -p /var/run/osrm.pid /usr/local/bin/osrm-routed --max-table-size 50 As you can see, osrm_flags and osrm_file have been ignored because osrm_options defaults to using osrm_flags and osrm_files. Changing osrm_options makes these variables unused in the script. The _flags and _file option should be moved to command_args with _option being empty by default. Also, something I missed : ${osrm_enable:="NO"} should be the first user modifiable variable for this script. Also, some other things I just found glancing at the diff, * CMAKE_BUILD_TYPE= Release, this line is redundant when defining cmake in USES and should be removed. * Consider adding DEBUG to OPTIONS_DEFINE, this will make it possible to select the option and will change BUILD_TYPE to Debug. (In reply to Torsten Zuehlsdorff from comment #34) Thanks Torsten, forgot to reply back. Its getting close assuming I didn't miss anything, which is always possible.
(In reply to Richard Gallamore from comment #35) Hi, thanks for your input. I will look at how to implement this better. Basically one of the things I thought of. If you are running with --shared--memory setting you do **not** need to setup an osrm-file. What is happening with --shared-memory setting is that you want to load the osrm into memory by using osrm-datastore and then just start osrm-routed with osrm-routed --shared-memory. See https://github.com/Project-OSRM/osrm-backend/wiki/Configuring-and-using-Shared-Memory One of the things I've been thinking about how to solve this is to ignore the osrm file flag if osrm_shared_memory is set to "YES". Does it make sense to have osrm_options for all the different settings of osrm-routed or only for the shared memory setting?
Meanwhile I am also awating 5.11.0 which is in rc3 right now.
(In reply to Andreas Andersson from comment #36) I think it would be better to have the shared_memory variable. Also, can you please take a look at [1] and follow the example a bit more closely? The main item I refer to is the comment section at the top. This will help dispel any confusion one may have. Add a note that the shared_memory variable will override X. I think the _option variable should be removed, those can be defined in flags. Making it clear in the comment about shared_memory option should be enough. Thanks again for your work on this. [1] https://www.freebsd.org/doc/en/books/porters-handbook/rc-scripts.html
Created attachment 185581 [details] Update to makefile, rc-script This update to diff does the following: * Updates to 5.11.0 * Removes CMAKE_BUILD_TYPE= Release * Adds DEBUG option RC-script changes: Checks if osrm_shared_memory is enabled, ignores osrm_file if osrm_shared_memory is set to yes. If not it checks if file exists, exits with error message if it doesnt.
Created attachment 185582 [details] Diff
Created attachment 185583 [details] Update to rc-script
Created attachment 185584 [details] Clarification in rc-script Clarification rc script. portlint -AC runs without errors. poudriere testport ok on 10 and 11.
Looks good to me, only thing that comes to mind is I'm not sure the post-extract is actually necessary.
(In reply to Richard Gallamore from comment #43) IIRC when I first created the port back in december I needed to remove those files or cmake would fail.
Created attachment 185608 [details] Remove post extract Just tried without post-extract and stopped removing those files. Seems to work, poudriere testport fine on 10 and 11.
(In reply to Andreas Andersson from comment #45) Yeah, that's what I thought. cmake:outsource usually takes care of all that.
So, is this ready to be committed now? :)
(In reply to Andreas Andersson from comment #47) > So, is this ready to be committed now? :) Hey, i was going to say that! ;)
Everything looks good to me, port looks beautiful.
Aloha, can you please update the patch? It did not apply anymore to the ports-tree. UID and GID files are rejected. I'm planning to have a lock at it this week and get it finally committed. Greetings, Torsten
Created attachment 186062 [details] Update GIDs and UIDs Due to another project taking the UID and GID I choose (zap) I am updating the diff with another UID and GID.
The port doesn't build on FreeBSD 11.0 amd64 nor i386. Did you test it only for 10.3 after the last change?
(In reply to Torsten Zuehlsdorff from comment #52) This seems to be because of clang 3.8.0. Appearantly the `compiler:c++14-lang` flag does not cause LLVM40 to be installed in 11.0 while it does so for 10.3 ?
(In reply to Andreas Andersson from comment #53) > This seems to be because of clang 3.8.0. Appearantly the `compiler:c++14-lang` > flag does not cause LLVM40 to be installed in 11.0 while it does so for 10.3 ? Yes - it seems so. That is quite irritating. So you need to use a switch based on the FreeBSD version :( Question beside: does it build with GCC 6? This is now the new default and i saw it being mentioned in the thread.
Hello Andreas, are you still working at the port? Greetings, Torsten
Hello Andreas, i did a test through all plattforms. It works on 10.3, 10.4, 11.1 but not on 11.0. Since the support for 11.0 ends next friday, i have no problem to commit it than. I needed to update GID and UID for the test. Since the current release is 5.13: would you mind to update the patch for next week? Greetings, Torsten
(In reply to Torsten Zuehlsdorff from comment #56) Hi, I can certainly do that. Expect a new patch tomorrow. Sorry for not responding earlier. But I've had a really busy few months at work due to merger.
(In reply to Andreas Andersson from comment #57) > Hi, I can certainly do that. Expect a new patch tomorrow. Sorry for not > responding earlier. But I've had a really busy few months at work due to > merger. I was busy the last few months, too. No problem! If you come up with a new patch i'm going to commit it. Greetings, Torsten
Any news here? Greetings, Torsten
(In reply to Torsten Zuehlsdorff from comment #59) Yes! Currently doing porttest on 10.3, 11.1 already passed. When 10.3 is done I will test on 10.4. This is with the newest version (5.15.2). When that is done I'll submit a new pr with updated GID and UID as well. This might take a few hours though because I don't have a 10.4 jail yet and I am assuming it will build all packages for me for that jail.
I am unable to test 10.4. Compilation of LLVM50 ends with this: -- Installing: /wrkdirs/usr/ports/devel/llvm50/work/stage/usr/local/llvm50/share/doc/clang/html/./_images/DriverArchitecture.png -- Installing: /wrkdirs/usr/ports/devel/llvm50/work/stage/usr/local/llvm50/share/doc/clang/html/./DiagnosticsReference.html CMake Error at tools/clang/docs/cmake_install.cmake:36 (file): file INSTALL cannot copy file "/wrkdirs/usr/ports/devel/llvm50/work/.build/tools/clang/docs/html/./DiagnosticsReference.html" to "/wrkdirs/usr/ports/devel/llvm50/work/stage/usr/local/llvm50/share/doc/clang/html/./DiagnosticsReference.html". Call Stack (most recent call first): tools/clang/cmake_install.cmake:60 (include) tools/cmake_install.cmake:43 (include) cmake_install.cmake:61 (include) FAILED: CMakeFiles/install/strip.util cd /wrkdirs/usr/ports/devel/llvm50/work/.build && /usr/local/bin/cmake -DCMAKE_INSTALL_DO_STRIP=1 -P cmake_install.cmake ninja: build stopped: subcommand failed. *** Error code 1 Stop. make: stopped in /usr/ports/devel/llvm50 =>> Cleaning up wrkdir ===> Cleaning for llvm50-5.0.1 build of devel/llvm50 | llvm50-5.0.1 ended at Sat Feb 10 16:27:30 UTC 2018 build time: 00:10:19 !!! build failure encountered !!! Any ideas?
(In reply to Andreas Andersson from comment #61) 10.3 and 11.1 working fine though.
Created attachment 190493 [details] Update to 1.15.2 update UID and GID Updates to 1.15.2 , sets osrm UID and GID to 228
(In reply to Andreas Andersson from comment #63) Builds fine on 10.3, 10.4 and 11.1
Also builds on 12-CURRENT
The pkg-plist file is empty, looks strange ?
Created attachment 190501 [details] Populate pkg-plist Here is the correct diff _with_ pkg-plist. Sorry about that.
testbuilds are fine.
A commit references this bug: Author: pi Date: Tue Feb 13 19:56:38 UTC 2018 New revision: 461738 URL: https://svnweb.freebsd.org/changeset/ports/461738 Log: New port: www/osrm-backend This is a port of osrm-backend of the open-streetmap project. WWW: http://project-osrm.org/ PR: 215524 Submitted by: Andreas Andersson <a.andersson.thn@gmail.com> Reviewed by: koobs, ultima, tz Changes: head/GIDs head/UIDs head/www/Makefile head/www/osrm-backend/ head/www/osrm-backend/Makefile head/www/osrm-backend/distinfo head/www/osrm-backend/files/ head/www/osrm-backend/files/osrm.in head/www/osrm-backend/pkg-descr head/www/osrm-backend/pkg-plist
Committed, thanks for your patience!