[GitHub] thrift issue #1412: [THRIFT-82] Add Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1412 Basically SBCL 1.3.14 (artful) works fine locally on Docker (and 1.3.1 had a bug with base strings according to SBCL devs - xenial). So the problem is probably something on environment hosting docker. I've noticed that some languages doesn't have docker tests enabled at all, if this problem persists is it acceptable to skip it for CL too? @jeking3 can you try running tests on Docker on your host locally to confirm that? What steps do you suggest in order to move forward? ---
[GitHub] thrift issue #1412: [THRIFT-82] Add Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1412 Could it be that `docker` sets a "fresh" environment for each command? SBCL keeps its cache files in ~/.cache/common-lisp and it looks like docker either runs commands as different users or that it wipes / fakes the home directory as a fresh one. ---
[GitHub] thrift issue #1412: [THRIFT-82] Add Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1412 It seems that SBCL has some problems when ran in docker (investigating that â sadly I can't reproduce this problem on my local machine). I'll install SBCL 1.3.1 on my machine to make 100% sure it is docker-specific. I'll let you know when I know more (and/or resolve the issue). ---
[GitHub] thrift issue #1412: [THRIFT-82] Add Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1412 and one more change: ensure-externals uses `[[` syntax, so it must be run with bash (not sh) â modified makefile and script to take this requirement into account. ---
[GitHub] thrift issue #1412: [THRIFT-82] Add Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1412 @jeking3 Thanks for the pointers. I've added requested info and updated Dockerfiles. Also rebased against master. ---
[GitHub] thrift issue #1412: [THRIFT-82] Add Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1412 any update on this? I'm fine with both options (given all parties agree on them) and have time to work on code forward. ---
[GitHub] thrift issue #1412: [THRIFT-82] Add Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1412 Good news, I have consent from @pc to change the header of the generator to match contributor guidelines. So next steps are: - merge changes to de.setf.thrift repository (so missing protocols are implemented) - publish de.setf.thrift on Quicklisp - adjust this pull request for Common Lisp inclusion should I close this pull request for now, or leave it as is until then? ---
[GitHub] thrift issue #1412: [THRIFT-82] Add Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1412 The bottom line is this: under the law of where mr. Anderson lives he can't yield the IP rights nor he sees a reason to do so, so we can't remove the copyrights put by him there. Code is licensed under the same license as the Thrift code though. ---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1410 Hello, I've send you an email a few hours ago with a question, if we can remove your "copyright" headers from the code found in de.setf.thrift, because that is what is required by Thrift team (I have included details in the email). Plese see: https://github.com/apache/thrift/pull/1410#discussion_r149655532 . ---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1410 see https://github.com/apache/thrift/pull/1412 ---
[GitHub] thrift issue #1412: [THRIFT-82] Add Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1412 squashed and rebased on top of the thrift master head. ---
[GitHub] thrift pull request #1412: [THRIFT-82] Add Common Lisp support
GitHub user dkochmanski opened a pull request: https://github.com/apache/thrift/pull/1412 [THRIFT-82] Add Common Lisp support There's framed and buffered socket transport, binary protocol, multiplex, simple server, cross-tests, self-tests, tutorial, CL library, CL code generator. Only SBCL is supported for now. You can merge this pull request into a Git repository by running: $ git pull https://github.com/TurtleWarePL/thrift develop Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1412.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1412 commit c5f8973b32bcf251fa2cc9a83fedd01000148ef8 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-19T07:16:43Z [THRIFT-82] Add Common Lisp support There's framed and buffered socket transport, binary protocol, multiplex, simple server, cross-tests, self-tests, tutorial, CL library, CL code generator. Only SBCL is supported for now. ---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1410 @jfarrell logged into travis a moment ago for the first time. @jeking3 I'm waiting for a response from @lisp if we can remove his copyright strings (we've removed ours). Should I close the PR and issue a new one before I receive a reply? ---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Github user dkochmanski commented on a diff in the pull request: https://github.com/apache/thrift/pull/1410#discussion_r149660082 --- Diff: lib/cl/framed-transport.lisp --- @@ -0,0 +1,136 @@ +(in-package #:org.apache.thrift.implementation) + + Copyright 2017 Rigetti Computing --- End diff -- I've send an email to Mr. Anderson â will let you know when / if I receive a reply. No, it is not possible, our work takes his repository[1] as base for further adjustments to meet the contribution requirements (missing protocols etc). [1] https://github.com/lisp/de.setf.thrift ---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Github user dkochmanski commented on a diff in the pull request: https://github.com/apache/thrift/pull/1410#discussion_r149592614 --- Diff: lib/cl/framed-transport.lisp --- @@ -0,0 +1,136 @@ +(in-package #:org.apache.thrift.implementation) + + Copyright 2017 Rigetti Computing --- End diff -- Right, thank you for pointing me to the document. Last commit removes all copyrights added by us. Note though, that files in lib/cl/ are based on work by James Anderson (and have copyrights as well) and I don't think I can remove them. Should I contact him and ask whenever we can remove his copyright stings too? ---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Github user dkochmanski commented on a diff in the pull request: https://github.com/apache/thrift/pull/1410#discussion_r149491753 --- Diff: lib/cl/ensure-externals.sh --- @@ -0,0 +1,12 @@ +#!/bin/sh + +set -e + +if [ ! -d "externals/" ] ; then --- End diff -- it is possible. Fixed in the pushed commit (some extra eval forms are explained in the commit message. ---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Github user dkochmanski commented on a diff in the pull request: https://github.com/apache/thrift/pull/1410#discussion_r149489770 --- Diff: lib/cl/server.lisp --- @@ -0,0 +1,230 @@ +(in-package #:org.apache.thrift.implementation) --- End diff -- it is a common practice in Common Lisp to have a package dedicated for usage by a programmer (which exports the protocol symbols - library API that is) and having no other symbols of its own and an implementation package which may have other internal symbols and interfaces. This allows avoiding for instance symbol name conflicts (for instance list vs list). ---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Github user dkochmanski commented on a diff in the pull request: https://github.com/apache/thrift/pull/1410#discussion_r149489004 --- Diff: lib/cl/framed-transport.lisp --- @@ -0,0 +1,136 @@ +(in-package #:org.apache.thrift.implementation) + + Copyright 2017 Rigetti Computing --- End diff -- Rigetti hired us to implement (and possibly merge) CL support to Thrift. All software under this delivery has to have license required by the project we are contributing to (that is Thrift). Not sure what is strange about that. ---
[GitHub] thrift issue #1410: Common Lisp support
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1410 Thank you for taking your time for the review ---
[GitHub] thrift pull request #1410: Common Lisp support
Github user dkochmanski commented on a diff in the pull request: https://github.com/apache/thrift/pull/1410#discussion_r148797236 --- Diff: lib/cl/framed-transport.lisp --- @@ -0,0 +1,136 @@ +(in-package #:org.apache.thrift.implementation) + + Copyright 2017 Rigetti Computing --- End diff -- Our work on Thrift is sponsored by Rigetti, so there is no need for that. ---
[GitHub] thrift pull request #1410: Common Lisp support
Github user dkochmanski commented on a diff in the pull request: https://github.com/apache/thrift/pull/1410#discussion_r148796969 --- Diff: lib/cl/externals/bundle-info.sexp --- @@ -0,0 +1,14 @@ +(:CREATION-TIME #A((20) BASE-CHAR . "2017-10-31T11:49:23Z") :REQUESTED-SYSTEMS --- End diff -- all things put in `externals` are checked in, so whole directory will be removed (this file included). ---
[GitHub] thrift pull request #1394: minor fixes to tests
Github user dkochmanski closed the pull request at: https://github.com/apache/thrift/pull/1394 ---
[GitHub] thrift issue #1394: minor fixes to tests
Github user dkochmanski commented on the issue: https://github.com/apache/thrift/pull/1394 sorry, wrong PR target. ---
[GitHub] thrift pull request #1394: minor fixes to tests
GitHub user dkochmanski opened a pull request: https://github.com/apache/thrift/pull/1394 minor fixes to tests You can merge this pull request into a Git repository by running: $ git pull https://github.com/TurtleWarePL/thrift develop-jd Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1394.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1394 commit b2ff80642f8f55a053b4d9d37f8b66654c8cc5f6 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-19T07:16:43Z Patch Thrift with de.setf.thrift commit ee922ebe75c2537898d915d3624f0c935cf2af28 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-19T07:16:56Z CL generator: fix and integrate it commit ebfffb4cafe127c917e491d6ed74fd36ac72aa58 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-19T08:42:55Z Remove non-existent packages and system dependencies commit dc8a36d07ad57a8eab3aea112062a9e28f152779 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-19T10:05:54Z Add namespace declarations for CL in tutorial .thrift files commit 9bf692896f48443ac0faffd668c8e02ce613c5ba Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-19T12:53:39Z Fix Thrift URIs commit 1c60bb1206487f125613e4dabba3ddd36d9beef4 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-19T13:20:10Z Cosmetic: Remove emacs file headers commit 8251142c6e037b9925efe892aa9d8fb04c3289e9 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-20T10:35:06Z Fix the handling of Thrift types The decoder should expect i32 when the field type is enum Rename i08 to i8 according to Thrift's expectations commit d921e46a73772d52c92d24e153cba13e1f868950 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-21T06:39:10Z Defined services should just be exported to the current *package* commit 319e07896b05183dbd62296a8d9f1bd918af2195 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-21T07:38:19Z Bugfix: wrong order of arguments in a function call commit 894fad0ee6f45a90a3367d45aa647a7d5292fde1 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-22T09:17:31Z Ensure users can use :common-lisp symbols when implementing services commit fe62d6a9f48b519309608a789bf7951cf2f3fa54 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-22T11:29:55Z CL generator: Generate ASDF systems for Thrift programs Also adds the CLI option not to generate .asd files, but the default is to generate them. commit 50d04e7c6f0a348886f30cd9ef52cfd4e07a1925 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-22T11:50:10Z Don't expect server implementation to exist when loading gen'd code We probably expect those to be declared later commit 665b7fa51d1f31d4d37f80998b48c0f381dbbaea Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-25T09:50:26Z CL generator: Put generated "programs" in separate directories commit 209cbffa780c012180b0e0fa15ae74c8cd381afd Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-25T11:20:14Z CL generator: copy the options string to comments in generated files commit dc8cb075766a2456e99e581c344aee167150ad2f Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-25T11:33:02Z CL generator: Fix the remaining warnings commit 6ce3ee2113e27ee222fa3f0097bc645b56358a4d Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-25T12:08:39Z Replace the float conversion code with `ieee-floats` from quicklisp commit ce124f7dcb79515614c2642be5abf2852c07693d Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-26T08:25:46Z CL generator: Add the option to change the ASDF system prefix Also: Cosmetic: add a newline after a generated (def-service) commit a21b328cf120fb171ac1b6b107dd931e30e632cb Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-27T08:32:52Z Fix the load order of thrift-test components commit b73b9a52571640f44437f17366e31277a3e5c45a Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-27T08:33:17Z Export vector-stream for use in thrift-test commit 72289ffd3509928d557555a9f80f0a720f095472 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-27T08:38:43Z Exclude definition-operators.lisp from compilation for now commit 6c77eda1a76ef0b71ebfde19df47b1d17b244d20 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-27T08:53:34Z Cosmetic: Typos and reindentation commit 6ff38d84b93b99c183f8c18915febf9fef0b02b4 Author: Tomek Kurcz <tomsand...@gmail.com> Date: 2017-09-27T09:01:14Z Fix vector-protocol.write-sequence commit 42cd93acb6abd9fdeb4ab563ce5502c1c81f2303 Author: Tomek Kurcz <tomsand...@gmai
