[GitHub] qpid-proton issue #169: Clang format
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/169 __I 100% support having a consistent automated format for the proton codebase(s)__ However , I don't think this is quite it (IMO): - I think we should just leave the proton-c codebase with an indent of 2 - A lot of changes for marginal gain there. - We can have a 4 space indent for the C++ code, I don't think having slightly different settings is a problem if the format is automatically applied. - I think we can allow wider lines - it looks like it is wrapping at 80 chars, I think 100 would be fine (conservative even) for modern screens. - I'm not happy with the reformatting of the header files: - Especially reformatting the 2nd and subsequent parameter at a variable indent - not cool for variable width fonts!. - Seems to have reformatted blah* x(); to blah *x(); which is fine for C code, but not for C++ code. Another area where the setting should probably be different. - Connected to the first sub point: if the function name and first parameter is too long wrapping both: blah *pn_(pn_param1 *x); To blah * pn_(pn_param1 *x); I think the mess of the header files is actually the most important issue given that this is user visible documentation. It should look as nice as we can make it. Consistency is important, but it shouldn't be ugly to view - I currently opine that the reformat has made the header files ugly. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #161: NO-JIRA: Removed python upgrade for xcode7.3 travis ...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/161 Another thought - is there any way to pin the python version using brew? That would make the CI build instructions more robust. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #161: NO-JIRA: Removed python upgrade for xcode7.3 travis ...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/161 This looks good to me - it makes me wonder if we could do the same for the Xcode 9 build too. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #160: WIP: option version of connection_options getters
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/160 I definitely like making option a class in common rather than duplicating it. I think that this is the correct way to expose options in the API, but perhaps we could make the API a bit more easy to use. Perhaps we could have a C++11 only explicit bool conversion as well as is_set(). However we would need to experiment a bit as some of the options themselves are also bools so this might be confusing! Another thing we should consider is throwing an exception if get() is called when the option is not set. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #159: PROTON-1940: [c] normalize encoding of multip...
Github user astitcher commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/159#discussion_r220587685 --- Diff: c/src/core/codec.c --- @@ -529,15 +529,15 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list ap) case 'd': err = pn_data_put_double(data, va_arg(ap, double)); break; -case 'Z': +case 'Z': /* encode binary, must not be NULL */ --- End diff -- As much as I agree the format letters need documenting I think that doing it inline is *not* the right way to do it - IMO a single block comment above would be much easier to encompass all the different codes. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #157: WIP: extract connection_options_impl for tests and i...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/157 Honestly given the alternative, adding gets for the options seems fairly innocuous! To be (a little) less flippant Unit testing is actually a good reason to make the options readable too. Although I'm not sure if this is a good idea for password though. I think the original reason for not making the options readable was that it was unecessary as the client program should know what options it set, but unit testing is important too. Perhaps an alternative would be a "dump" method for the options (akin to ```pn_inspect()``` or __str__) which outputs the options in json for comparison although you'd have to parse it and compare to be safe. But hey you've got a json parser linked in anyway! --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #157: WIP: extract connection_options_impl for tests and i...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/157 Basically don't like this approach because it exposes implementation details to the library interface. As the unit tests don't only need to be built in the build environment. Myself I'd be happy with direct linking, but I'm not clear why you need/want to test the implementation when the actual external interface pretty much directly passes through anyway. Perhaps you can give a simple example of a test that can't use the exposed API. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #112: fix paths to test binaries
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/112 @matlo607 I think this may be fixed with change b172262577a79d8764b9fe06e27f71663446c309. If you have a chance can you check and close this PR if it is fixed. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #154: Replace user-defined macro WIN32 by compiler-defined...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/154 That's curious: When I do the same against upstream master I find WIN32 used in `c/include/proton/import_export.h`. Are you checking against the upstream master? I did think it was used in more places, but I realise those places are the CMake files so don't matter for this purpose. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #154: Replace user-defined macro WIN32 by compiler-defined...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/154 This looks fine, but you have ignored the c code in the tree - is there a reason for that? --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #152: PROTON-1886: Dump the thread's SSL error queue in th...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/152 This looks good - I'll pull it in. I wonder though if there is scope for some refactoring of the ssl error logic to make this error logic in common. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #148: PROTON-1354: Don't allow SASL mechanisms GSSA...
GitHub user astitcher opened a pull request: https://github.com/apache/qpid-proton/pull/148 PROTON-1354: Don't allow SASL mechanisms GSSAPI or GSS-SPNEGO by default Small change that fixes a pain point if you have kerberos on you client machine but aren't using it for proton. The SASL handling is changed so that if you want to use kerberos you now need to tell the pn_sasl_allowed_mechs() API on both the client and server that you allow GSSAPI and/or GSS-SPNEGO mechanisms (along with any other mechanisms you want). The logic here is that you are very unlikely to want GSSAPI unless you have specifically set it up and then you will know that and can allow it specifically. You can merge this pull request into a Git repository by running: $ git pull https://github.com/astitcher/qpid-proton proton-1354 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton/pull/148.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 #148 commit bad63dc878856bd9fb657f03eb008cad02f44098 Author: Andrew Stitcher Date: 2018-06-12T20:17:42Z PROTON-1354: Don't allow SASL mechanisms GSSAPI or GSS-SPNEGO by default - If you want to use these mechanisms they must be explicitly set in the allowed mechanisms list for server and client that are connecting --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #146: NO-JIRA: [c] prefer linking with static library in f...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/146 Ok - I see this is dependant on @alanconway 's recent work. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #146: NO-JIRA: [c] prefer linking with static library in f...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/146 I'm a little confused by this change - what creates the libqpid-proton-static lib that this changes uses? It doesn't seem that this changes creates that library anywhere. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #144: Python proactor
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/144 Pushed a new version of these changes according to feedback from @ssorj --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #144: Python proactor
GitHub user astitcher opened a pull request: https://github.com/apache/qpid-proton/pull/144 Python proactor This is a fairly large change to the structure of the Proton python binding. * Initially it removes support for really old versions of python - anything before 2.5. * Then it goes on to split up the monolithic __init__.py file into IMO) more manageable pieces. I've also used this to restrict the usable API to only those bits that are supposed to be API (again IMO). This is the initial work which I'm using as a base for more substantial changes, so I will be grateful for any comments and review. You can merge this pull request into a Git repository by running: $ git pull https://github.com/astitcher/qpid-proton python-proactor Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton/pull/144.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 #144 commit 4dcbd1fd456018d0921fec2f14dbb26fdde5b5f2 Author: Andrew Stitcher <astitcher@...> Date: 2018-04-24T14:38:57Z PROTON-1848: [Python] Remove Python 2.5 and earlier compatibility - Remove need for most compatibility hacks - Fix some seemingly odd conversion functions - Hidden all compatibility code in _compat module, but this still uses six to rename Queue->queue - could remove this later - will probably get rid of raise_ when rewriting reactor - can get rid of string_type by doing unicode type hack in reactor.py - leaving iteritems & unichr commit 68e2922584bbe2445303f32cca94cdba04e0fd48 Author: Andrew Stitcher <astitcher@...> Date: 2018-05-02T23:20:38Z PROTON-1850: Split up proton __init__.py into multiple files - Make sure that only what we want gets exported from __init__ by restricting what it imports - Move most of the reactor implementation specific code into reactor_impl.py --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #95: PROTON-1412 Add fuzzers to proton-c tests
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/95 I've now committed a slightly modified version of this PR. So it can be closed. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #139: NO-JIRA: Pinned the homebrew python version to 2 to ...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/139 This seems like an entirely unnecessary change - the Travis build job already only uses python2 - why do we need to change it? Homebrew building has no relation with Travis building does it? --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #132: PROTON-1728: WIP: Reorganize the source tree and rem...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/132 Can I suggest breaking this down into 3 separate changes (PRs branches etc.): * Removal of deprecated bindings * Removal of obsolete doc * Moving the remainder to new locations and fixing up build I think it'll be easier to see what's going on in the important part (the third) if there were fewer individual commits to look at. This would be easier I think if you just remove all the stuff from the tree. In any case the removals are much easier to review/test individually. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #134: PROTON-1730: add python version 3.6 to tox tests
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/134 Looks good to me --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #135: PROTON-1732: [OSX] Enable swig for the Travis CI bui...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/135 But failing that I'm happy with what's there now. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #135: PROTON-1732: [OSX] Enable swig for the Travis CI bui...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/135 Maybe I'm the one who misunderstands how env and matrix interact then. Only one other thing I might suggest is to just use pip2 everywhere without a variable at all - It looks like that should work on Ubuntu and will avoid bringing attention to this irrelevant detail. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #135: PROTON-1732: [OSX] Enable swig for the Travis...
Github user astitcher commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/135#discussion_r160062571 --- Diff: .travis.yml --- @@ -23,29 +23,37 @@ compiler: - gcc - clang +env: +- PIP='pip' + matrix: include: - os: linux env: - QPID_PROTON_CMAKE_ARGS='-DCMAKE_BUILD_TYPE=Coverage' +- PIP='pip' --- End diff -- I think we may be talking about the opposite case: - I'm suggesting that this line which part of a specific matrix build is not necessary because there is a line in the non matrix case. - I think you are saying that not having the non matrix line doesn't work. I agree with this. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #135: PROTON-1732: [OSX] Enable swig for the Travis...
Github user astitcher commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/135#discussion_r159936304 --- Diff: .travis.yml --- @@ -23,29 +23,37 @@ compiler: - gcc - clang +env: +- PIP='pip' + matrix: include: - os: linux env: - QPID_PROTON_CMAKE_ARGS='-DCMAKE_BUILD_TYPE=Coverage' +- PIP='pip' --- End diff -- You don't need this as it is the same as the default above. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #135: PROTON-1732: [OSX] Enable swig for the Travis CI bui...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/135 Hmm, not quite sure about this: * The actual change seems fine, although ISTR putting in an explicit turning off for PERL for apple builds some years ago, so I'm surprised you needed to do this (but not an actual problem). * All of the ruby tests are crashing, so maybe turning ruby off until we investigate and fix might be good, we can't have merging this change make the Travis master CI fail. * Even though cmake does find and use swig, the most important thing - the python tests - still don't build/run because cmake finds a mismatched libpython with a different version from the python interpreter it finds. So this is a useful step, but not a real advance over where are already are. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #131: PROTON-1711: SSL_IMPL none SASL_IMPL none build atte...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/131 @RoddieKieley I've fixed this issue now - can you close the PR (stupidly I forgot to add that to the commit). --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #131: PROTON-1711: SSL_IMPL none SASL_IMPL none build atte...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/131 Oops - I guess I did that when I added in the ssl tests. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #131: PROTON-1711: SSL_IMPL none SASL_IMPL none build atte...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/131 The tests should still pass if SSL_IMPL is none as there is a stub implementation. If not then we need to fix that bug. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #118: PROTON-1326: update python setup.py to avoid using i...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/118 @kgiusti I think this PR is now irrelevant - as 0.18 is out and supports openssl 1.0 and 1.1. Could you close it? --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #:
Github user astitcher commented on the pull request: https://github.com/apache/qpid-proton/commit/de3fd617210b5d5a2f2c3e384c33905dbf75ad58#commitcomment-25557336 i can see why you might think the way you do. But actually this is not correct. You are assuming that the final string needs to be zero terminated, but this is incorrect. This code copies the user name from an incoming zero terminated string, and puts it into a SASL ANONYMOUS frame without it being terminated. This frame does not use zero terminated strings. Also note that the original fix was doubly incorrect as it was strlen(username+1) *with the +1 inside the strlen* this actually allocates 2 characters too few from the point of view of your comment. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #128: PROTON-1342: CI on OS X
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/128 I like this latest version, but 2 remaining things: * Please, please rebase and squash this to be a single commit - this chain of development makes no sense as part of the project history. * A niggle really - you didn't follow the existing formatting of the yaml file. Specifically you indent follow on lists, this just pushes everything further right and makes it harder to read long lines. The existing style is like this: ```yaml before_install: - eval ... - if ... - if ... ``` --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #128: PROTON-1342: CI on OS X
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/128 A few points - * I'm not sure how familiar you are with git/github: but there seem to be a lot of fairly meaningless changes here. Could you squash all of these changes into a single change. That would make it easier to see what's actually changed. * I think you've introduced way too many builds to the CI - do we really need to build every possible version of Xcode? I'd prefer to pick just the current and earliest supported version - that should suffice for CI, shouldn't it? * I don't think you need to introduce a conditional to the before_script. The purpose of the QPID_PROTON_CMAKE_ARGS variable is to change exactly what you are changing - I think you can set it for the new matrix elements you are introducing. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #126: PROTON-1632: WIP: Split library versions
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/126 Looks good to me --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #120: PROTON-1369: Add deprecation warnings to the C++ bin...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/120 Otherwise this looks about right to me. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #120: PROTON-1369: Add deprecation warnings to the C++ bin...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/120 Now that I've merged the work_queue/work API changes I think you should deprecate the use of void_function0 and the work_queue/container APIs that use it. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #121: PROTON-1368: Remove parser from the public API
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/121 looks good. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #121: PROTON-1368: Remove parser from the public API
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/121 Note the build failures are from internal code including proton/parser.h. So even though I didn't find anything internally using the parser maybe my code browser isn't as good as I thought. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #120: PROTON-1369: Add deprecation warnings to the C++ bin...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/120 I'm not sure that it's actually worth putting out deprecation messages for including the files but not using the deprecated APIs there. If the APIs aren't used it will be trivial for anyone to just remove the ``#include`` when the header files are actually removed. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #120: PROTON-1369: Add deprecation warnings to the C++ bin...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/120 This is the CMake generated version of the deprecation macros: ``` # ifndef PN_DEPRECATED #if defined(PN_COMPILER_CXX_ATTRIBUTE_DEPRECATED) && PN_COMPILER_CXX_ATTRIBUTE_DEPRECATED # define PN_DEPRECATED [[deprecated]] # define PN_DEPRECATED_MSG(MSG) [[deprecated(MSG)]] #elif PN_COMPILER_IS_GNU || PN_COMPILER_IS_Clang # define PN_DEPRECATED __attribute__((__deprecated__)) # define PN_DEPRECATED_MSG(MSG) __attribute__((__deprecated__(MSG))) #elif PN_COMPILER_IS_MSVC # define PN_DEPRECATED __declspec(deprecated) # define PN_DEPRECATED_MSG(MSG) __declspec(deprecated(MSG)) #else # define PN_DEPRECATED # define PN_DEPRECATED_MSG(MSG) #endif # endif ``` I note that it doesn't include the declaration itself - I take this to mean that you can choose the correct place to put the macro do it works for all the supported compilers - This certainly seems neater to me. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #120: PROTON-1369: Add deprecation warnings to the C++ bin...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/120 Note the #pragma is portable in that compilers are required to ignore pragmas they don't understand. You can use _Pragma to use macros to expand to #pragma to make it more platform portable. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #121: PROTON-1368: Remove parser from the public API
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/121 I'm not sure what the logic of this change is: This parser API is not used internally (as far as I can tell) in proton-c so removing the external API whilst leaving the internal code intact probably makes no sense unless you are trying to maintain ABI compatibility. But in this case there is no need to even keep the header file internally as there is no internal code that uses this API. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #:
Github user astitcher commented on the pull request: https://github.com/apache/qpid-proton/commit/29272ca3948233a417ebc04ba80dedaefc7a4104#commitcomment-24146519 In CMakeLists.txt: In CMakeLists.txt on line 177: I agree with @jdanekrh - very many (most really) of the proton-c tests are in python so disabling the python tests enormously diminishes the sanitiser coverage. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #119: NO-JIRA Fix CppCheck warnings
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/119 I've merged this now (with an extra fix) --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #119: NO-JIRA Fix CppCheck warnings
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/119 @michaelandrepearce can you paste in the JIRA report urls here then so they can be connected. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #119: NO-JIRA Fix CppCheck warnings
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/119 @jdanekrh ok - I'll merge and commit this - also note we need a JIRA for this too - it's not a simple typo (or something trivial) that doesn't need it - I'll create one unless @michaelandrepearce has already created one. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #119: NO-JIRA Fix CppCheck warnings
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/119 Do you need me to merge these changes? (I forget if you are a committer or not) --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #119: NO-JIRA Fix CppCheck warnings
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/119#discussion_r137036761
--- Diff: proton-c/src/proactor/libuv.c ---
@@ -1179,7 +1179,10 @@ pn_proactor_t *pn_proactor() {
pn_proactor_t *p = (pn_proactor_t*)calloc(1, sizeof(pn_proactor_t));
p->collector = pn_collector();
p->batch.next_event = _batch_next;
- if (!p->collector) return NULL;
+ if (!p->collector) {
+free(p);
--- End diff --
Looks good to me, though stylistically I'd move line 1181 after the 'if'
statement.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #118: PROTON-1326: update python setup.py to avoid using i...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/118 I'm assuming you are proposing to check this in branching from the 0.17 release? (It's difficult to tell from the PR). Note that the CI builds both fail - you will want to check this isn't due to this change. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #116: PROTON-1532: Allow insecure mechanism in SASL
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/116 @alanconway you may want to take a look at this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #111: Fix mapping of LONG for Ruby 2.4+
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/111 I applied this fix - that's why it's closed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #115: PROTON-1533: Fix swig deprecation warnings
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/115#discussion_r132225370
--- Diff: proton-c/bindings/php/CMakeLists.txt ---
@@ -34,7 +34,7 @@ list(APPEND SWIG_MODULE_cproton_EXTRA_DEPS
${CMAKE_SOURCE_DIR}/proton-c/include/proton/cproton.i
${PROTON_HEADERS}
)
-swig_add_module(cproton php ${CMAKE_CURRENT_SOURCE_DIR}/php.i)
+swig_add_library(cproton LANGUAGE php SOURCES
${CMAKE_CURRENT_SOURCE_DIR}/php.i)
--- End diff --
Not directly related to your change, but I think the
${CMAKE_CURRENT_SOURCE_DIR} is extraneous here and is different from all the
other uses of swig_add_library in the other bindings.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #112: fix paths to test binaries
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/112 @matlo607 Sorry this change looks good, but it makes the regular CI build fail because now it can't find the same binaries! So Look at the Travis output above and try to figure out a change that will work in all cases. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #108: PROTON-1500: Implement plugin interface for S...
Github user astitcher closed the pull request at: https://github.com/apache/qpid-proton/pull/108 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #108: PROTON-1500: Implement plugin interface for SASL imp...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/108 @grs @rgodfrey @alanconway @ssorj You may want to take a look at this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #108: PROTON-1500: Implement plugin interface for S...
GitHub user astitcher opened a pull request: https://github.com/apache/qpid-proton/pull/108 PROTON-1500: Implement plugin interface for SASL implementations - Allow sasl implementation to be selected per connection - Compile both default and cyrus implementation (if we can) - No current change in behaviour (get cyrus is available and default otherwise) - But now have frqamework to solve PROTON-1209; Stil to do for this: - Need to have API to turn on cyrus, with default being the initial implementation - Need to deprecate cyrus specific APIs: -- pn_sasl_extended -- pn_sasl_config_path -- pn_sasl_config_name - Need to deprecate pn_sasl_done You can merge this pull request into a Git repository by running: $ git pull https://github.com/astitcher/qpid-proton sasl-interceptor Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton/pull/108.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 #108 commit 21a74d980298d3eeb5b4c57c93815a5e6643cb38 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-06-07T05:42:28Z PROTON-1500: Implement plugin interface for SASL implementations - Allow sasl implementation to be selected per connection - Compile both default and cyrus implementation (if we can) - No current change in behaviour (get cyrus is available and default otherwise) - But now have frqamework to solve PROTON-1209; Stil to do for this: - Need to have API to turn on cyrus, with default being the initial implementation - Need to deprecate cyrus specific APIs: -- pn_sasl_extended -- pn_sasl_config_path -- pn_sasl_config_name - Need to deprecate pn_sasl_done --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #105: Port of C++ binding to Proton Proactor
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/105 I've now updated this with what I intend to be the final rebase before merging. Everything is functionally there except for reconnect, which will be addressed in the 0.19 timescale. * Multhtreaded container is now implemented * The (still) experimental connection_driver now works. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #105: Port of C++ binding to Proton Proactor
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/105 I've now rebased and slightly modified some the previous commits. Substantively I've addressed the work_queue header guard issue; finished the work dealing with scheduling work to occur in different contexts; renamed proton::defer to proton::schedule_work. Note that the commits in the branch *are not* in the order display in this PR because they have been heavily reworked and reordered in rebases. So the creation time does not correspond to the commit order. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #105: Port of C++ binding to Proton Proactor
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/105#discussion_r117555738
--- Diff: proton-c/bindings/cpp/include/proton/message.hpp ---
@@ -329,11 +329,7 @@ class message {
mutable annotation_map message_annotations_;
mutable annotation_map delivery_annotations_;
-/// Decode the message corresponding to a delivery from a link.
--- End diff --
This is a private function that didn't need exposing. So I moved it to the
internals.
I can't remember anymore if this change was enabled by other things changed
or not. But the code moved into message_adapter.cpp which also changed a lot in
the same change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #105: Port of C++ binding to Proton Proactor
Github user astitcher commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/105#discussion_r117554762 --- Diff: proton-c/bindings/cpp/include/proton/work_queue.hpp --- @@ -0,0 +1,368 @@ +#ifndef PROTON_EVENT_LOOP_HPP --- End diff -- Will fix. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #105: Port of C++ binding to Proton Proactor
GitHub user astitcher opened a pull request: https://github.com/apache/qpid-proton/pull/105 Port of C++ binding to Proton Proactor This set of changes reimplements the C++ binding do that it no longer uses the Proton Reactor library at all and now only links to the Proton Core and Proton Proactor libraries. It is not 100% finished yet, missing pieces include: * Multi thread support - Currently the API is thread safe if you obey the thread safety rules, but only one thread is used to service handler callbacks - This will be fixed soon. * Reconnect is not implemented - If a network connection fails no reconnect action is attempted - This will also be fixed, but ntoe that the C++ binding implementation of reconnect is subject to change as we don't have a lot of experience with it yet. Included in this PR is a reworking of the mechanism for injecting work into safe contexts. The concepts have been renamed for clarity (`proton::work_queue` & `proton::work`) and some convenience functions have been added to help those (poor benighted souls) using C++03. There is some more work to come which should also improve injecting work using the scheduler. You can merge this pull request into a Git repository by running: $ git pull https://github.com/astitcher/qpid-proton cpp-proactor Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton/pull/105.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 #105 commit 96435b5cfac13283c612ba60feefd73901bc9c4f Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-01-23T17:32:50Z PROTON-1400: [C++ binding] Removed proton_event and proton_handler - Removed old low level proton event handling completely - Now directly dispatch to the messaging_handler - Moved private message::decode directly into message handling code commit 84dcd7bcaa97106ef43a6a8b44a7f3e819f07107 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-01-25T04:36:03Z PROTON-1400: WIP Use the mt broker example as the example instead of the previous st broker - The st broker didn't correctly respect the object access constraints from within handlers commit b95f252ede1aaa20c8a5fd1abd42fdbe2c7c3ebe Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-02-08T07:32:36Z PROTON-1400: [C++ binding] Proactor container implementation - Remove all reactor use - Rearrange object context code - Change container includes to proactor container includes commit ea7f0e989ba5b90aa322b38336dfbbb089fae899 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-02-09T01:12:36Z PROTON-1400: [C++ binding] Remove reactor container implementation files. commit a0c9e9a756d452bc7c992e4a9875f681b69d5c44 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-03-03T20:51:58Z NO-JIRA: Header file corrections commit 6ed1d1ab4a68f0a132f44b59a09b1582a2f6b4ff Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-03-28T17:45:28Z PROTON-1400: [C++ example] Rework broker to use container event loops commit db92f4c73444d186899e5fa28e638195f3adf5f4 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-04-14T06:13:12Z PROTON-1400: [C+ binding] Change semantic for incoming xx_open - If not overridden then you get automatic outgoing matching xx_open - If overridden then it is assumed you will handle the outgoing open yourself. commit a52e5d16363caa736062359dab0733b4db6b8992 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-04-20T19:20:40Z PROTON-1400: Implement container level event_loops commit 95a7ded6410e599f6327adedbf5386b5d8b4fca1 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-04-27T17:05:14Z PROTON-1400: [C++ binding] Use pn_proactor_now() to implement timestamp::now() commit 2da87d20cff09b00503dd70341fde73b5ae36bd5 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-05-15T05:27:54Z PROTON-1481: [C++ binding] Rename event_loop API to work_queue commit efd5ec09f1a7cfc311fe8f612d48160ce16d7e2b Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-05-15T05:36:52Z PROTON-1481: [C++ binding] simpify work_queue code by introducing work type - The work type can be created from std::function<void()> or void_function0 - and so pushes those c++1/C++03 differences into a single place commit e252354b82f3a4273275a15da614abda3859a2c9 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-05-15T05:45:43Z PROTON-1481: [C++ binding] Split out general work deferring functions to the work_queue header Add efficient C++11 versions of work factories Reorder defer arguments to be like std::bind Add extra overloads for defer like std::bind (for free functions a
[GitHub] qpid-proton pull request #104: Port of thre C++ binding to the Proton Proact...
Github user astitcher closed the pull request at: https://github.com/apache/qpid-proton/pull/104 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #104: Port of thre C++ binding to the Proton Proact...
GitHub user astitcher opened a pull request: https://github.com/apache/qpid-proton/pull/104 Port of thre C++ binding to the Proton Proactor library This set of changes reimplements the C++ binding do that it no longer uses the Proton Reactor library at all and now only links to the Proton Core and Proton Proactor libraries. It is not 100% finished yet, missing pieces include: * Multi thread support - Currently the API is thread safe if you obey the thread safety rules, but only one thread is used to service handler callbacks - This will be fixed soon. * Reconnect is not implemented - If a network connection fails no reconnect action is attempted - This will also be fixed, but ntoe that the C++ binding implementation of reconnect is subject to change as we don't have a lot of experience with it yet. Included in this PR is a reworking of the mechanism for injecting work into safe contexts. The concepts have been renamed for clarity (`proton::work_queue` & `proton::work`) and some convenience functions have been added to help those (poor benighted souls) using C++03. There is some more work to come which should also improve injecting work using the scheduler. You can merge this pull request into a Git repository by running: $ git pull https://github.com/astitcher/qpid-proton cpp-proactor Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton/pull/104.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 #104 commit 085aa00f54be1609f936d5bb93a0bf1437829f3a Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-02-08T07:32:36Z PROTON-1400: [C++ binding] Proactor container implementation - Remove all reactor use - Rearrange object context code - Change container includes to proactor container includes commit 602429bcfd15edaebcb370f5dee010903e3b4d03 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-02-09T01:12:36Z PROTON-1400: [C++ binding] Remove reactor container implementation files. commit fc6b05b1e6762f680310ee80f5fc2afa883e4d71 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-04-27T17:05:14Z PROTON-1400: [C++ binding] Use pn_proactor_now() to implement timestamp::now() commit ca95aa36b271ca63293a7287afcedaab575183a9 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-05-16T20:02:09Z PROTON-1400: Fixup commit 6f24b7c88b72363b966f6855ed689c46d82682b0 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-01-23T17:32:50Z PROTON-1400: [C++ binding] Removed proton_event and proton_handler - Removed old low level proton event handling completely - Now directly dispatch to the messaging_handler - Moved private message::decode directly into message handling code commit c4f5ac3b091a37cf4423eca373f41ca382d2f45f Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-01-25T04:36:03Z PROTON-1400: WIP Use the mt broker example as the example instead of the previous st broker - The st broker didn't correctly respect the object access constraints from within handlers commit d622676f8abe46d9351236e1ddc55336f524051d Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-03-03T20:51:58Z NO-JIRA: Header file corrections commit e9bf06254d2904192cfe27de291ea2b8ae2443a1 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-04-14T06:13:12Z PROTON-1400: [C+ binding] Change semantic for incoming xx_open - If not overridden then you get automatic outgoing matching xx_open - If overridden then it is assumed you will handle the outgoing open yourself. commit 6b231dc2857eb8fdb5bb948aa2709da742a09b9f Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-03-28T17:45:28Z PROTON-1400: [C++ example] Rework broker to use container event loops commit 238324b1e636c1c632cef8b45fdd04611cf61aaa Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-04-20T19:20:40Z PROTON-1400: Implement container level event_loops commit 6f00ac312f0a359195aaf005a301657228290063 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-05-15T05:27:54Z PROTON-1481: [C++ binding] Rename event_loop API to work_queue commit f3b84b5ab8bbdaa3422a5e628f8b210371a9cd9a Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-05-15T05:36:52Z PROTON-1481: [C++ binding] simpify work_queue code by introducing work type - The work type can be created from std::function<void()> or void_function0 - and so pushes those c++1/C++03 differences into a single place commit 557a16bf0544a0faa4d1c9aa2d92e7a864264f40 Author: Andrew Stitcher <astitc...@apache.org> Date: 2017-05-15T05:45:43Z PROTON-1481: [C++ binding] Split out general work deferring functions to the work_queue header
[GitHub] qpid-proton issue #101: PROTON-1453: properly terminate output string buffer
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/101 @alanconway If the calls are documented to always return a value in the string (even upon failure) I won't complain. As a matter of API design though, a filing call should not alter anything. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #99: PROTON-1408 -- fix for msg id rollover bug
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/99 Also I *strongly suggest* you test this code on both 32 and 64 bit machines to make sure that the casting works equally well when uintptr_t is 32 vs 64 bits. This is independent of whatever solution you end up with. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #99: PROTON-1408 -- fix for msg id rollover bug
Github user astitcher commented on the issue:
https://github.com/apache/qpid-proton/pull/99
Having looked a bit more it seems less clear that the bug is in pn_hash!
Essentially the lower 32 bit masking is used as a more determinate way to
cast the int32_t of the pn_sequence_t to the uintptr_t (uint64_t) of the hash
code I don't understand (yet) why this casting sometimes gives different
results, but I *strongly suggest* creating a better named inline function to
express the intent rather than doing the masking with no real explanation.
Viz:
static inline uintptr_t pni_sequence_make_hash(pn_sequence_t i) {
return i & 0xUL;
}
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #99: PROTON-1408 -- fix for msg id rollover bug
Github user astitcher commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/99#discussion_r108212167 --- Diff: proton-c/src/core/transport.c --- @@ -82,9 +82,11 @@ void pn_delivery_map_free(pn_delivery_map_t *db) pn_free(db->deliveries); } +#define BOTTOM_32_BITS 0x --- End diff -- Isn't this just UINT32_MAX? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #99: PROTON-1408 -- fix for msg id rollover bug
Github user astitcher commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/99#discussion_r108192064 --- Diff: proton-c/src/core/transport.c --- @@ -1620,12 +1622,12 @@ static int pn_scan_error(pn_data_t *data, pn_condition_t *condition, const char } /* - This operator, copied from code for the qpid cpp broker, gives the correct + This operator, inspired by code for the qpid cpp broker, gives the correct result when comparing sequence numbers implemented in a signed integer type. */ -static bool sequence_less_than ( pn_sequence_t a, pn_sequence_t b ) +static bool sequence_cmp ( pn_sequence_t a, pn_sequence_t b ) --- End diff -- This should be ```inline``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #99: PROTON-1408 -- partial fix for msg id rollover...
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/99#discussion_r107258218
--- Diff: proton-c/src/core/transport.c ---
@@ -1648,7 +1659,7 @@ int pn_do_disposition(pn_transport_t *transport,
uint8_t frame_type, uint16_t ch
bool remote_data = (pn_data_next(transport->disp_data) &&
pn_data_get_list(transport->disp_data) > 0);
- for (pn_sequence_t id = first; id <= last; id++) {
+ for (pn_sequence_t id = first; sequence_less_than(id, last) || (id ==
last); id++) {
--- End diff --
Of course this test will still do the wrong thing if the wrap-around is big
enough (ie if first > last at teh start of the loop)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #99: PROTON-1408 -- partial fix for msg id rollover...
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/99#discussion_r107257116
--- Diff: proton-c/src/core/transport.c ---
@@ -1619,6 +1619,17 @@ static int pn_scan_error(pn_data_t *data,
pn_condition_t *condition, const char
return 0;
}
+/*
+ This operator, copied from code for the qpid cpp broker, gives the
correct
+ result when comparing sequence numbers implemented in a signed integer
type.
+*/
+static bool sequence_less_than ( pn_sequence_t a, pn_sequence_t b )
--- End diff --
ie code would be
static int sequence_cmp(pn_sequence_t a, pn_sequence_t b) {
return a-b;
}
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #99: PROTON-1408 -- partial fix for msg id rollover...
Github user astitcher commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/99#discussion_r107255885 --- Diff: proton-c/src/core/transport.c --- @@ -1619,6 +1619,17 @@ static int pn_scan_error(pn_data_t *data, pn_condition_t *condition, const char return 0; } +/* + This operator, copied from code for the qpid cpp broker, gives the correct + result when comparing sequence numbers implemented in a signed integer type. +*/ +static bool sequence_less_than ( pn_sequence_t a, pn_sequence_t b ) --- End diff -- Why not return less than 0 for less than 0 for equal and greater than 0 for greater than (like strcmp) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #99: PROTON-1408 -- partial fix for msg id rollover...
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/99#discussion_r107256041
--- Diff: proton-c/src/core/transport.c ---
@@ -1648,7 +1659,7 @@ int pn_do_disposition(pn_transport_t *transport,
uint8_t frame_type, uint16_t ch
bool remote_data = (pn_data_next(transport->disp_data) &&
pn_data_get_list(transport->disp_data) > 0);
- for (pn_sequence_t id = first; id <= last; id++) {
+ for (pn_sequence_t id = first; sequence_less_than(id, last) || (id ==
last); id++) {
--- End diff --
Then this test would be sequence_cmp(id, last)<=0
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #98: PROTON-1434: add test blacklist to tox tests
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/98 Looks like a good idea to me --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #95: PROTON-1412 Add fuzzers to proton-c tests
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/95 Ah, sorry, I didn't read it through to the end (it is perhaps a little wordy). If the libs are interchangeable then which one is used need to be selectable from CMake. I'll see if I can make this work. I've some other small changes to do anyway. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #95: PROTON-1412 Add fuzzers to proton-c tests
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/95 @jdanekrh I'm looking at this PR now. And I don't understand the purpose in renaming libFuzzer -> libFuzzingEngine. Could you explain why you do that. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #95: [REVIEW] Add fuzzers to proton-c tests
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/95 Ok, if the function is *useful* without the fuzzer then it makes sense to compile it without the fuzzer. Sold. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #95: [REVIEW] Add fuzzers to proton-c tests
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/95 Assuming that any compatible fuzzer has to have the same interface (assuming they all use LLVMFuzzerTestOneInput) then they all need to supply a compatible definition. It's not as if this function is useful to compile unless there is a fuzzer present. So I don't really buy that argument. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #95: [REVIEW] Add fuzzers to proton-c tests
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/95 Also, in general I'm not usually happy to check in generated (especially) binary files to a source code repository. I understand the purpose here, but it seems messy to me. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #95: [REVIEW] Add fuzzers to proton-c tests
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/95 In general though it's more important to get this into the repo so it can be used going forward and then maybe it can be tidied up later. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #95: [REVIEW] Add fuzzers to proton-c tests
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/95 I'd be happy with just hex files, but the binary files are virtually impossible to use sensibly without extra tools as a developer. It might be a bit better with the fuzzing code checked in as you could just feed the failing test into the tester, but without it I essentially had to construct tests with a hex editor, as the binary sequences in the bug reports couldn't be simply injected into a running example program. I really found and fixed the fuzzing bugs by reviewing the failing code! After looking at the stack traces. But I had great difficulty in reproducing the original bugs so that I could show that I'd fixed them. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #95: [REVIEW] Add fuzzers to proton-c tests
Github user astitcher commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/95#discussion_r102986492 --- Diff: proton-c/src/tests/fuzz/fuzz-sniff-header.c --- @@ -0,0 +1,15 @@ +#include +#include + +#include "core/autodetect.h" + +#ifdef __cplusplus --- End diff -- The declaration only has to be extern "C" so you could put: #ifdef __cplusplus extern "C" #endif int LLVMFuzzerTestOneInput(uint8_t *data, size_t size); In a header file. That is sufficient to give the function the correct linkage. In fact I'm surprised there isn't already a declaration like this in libfuzzer. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #95: [REVIEW] Add fuzzers to proton-c tests
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/95 I really like the idea of putting fuzzing tests into the source tree (guarded appropriately by cmake tests). I'm not very keen about putting binary only files into the corpus. Could we use hex files perhaps? I'd even prefer in the future to use a text format like used by my project https://github.com/astitcher/amqp-value-parser.git --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #95: [REVIEW] Add fuzzers to proton-c tests
Github user astitcher commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/95#discussion_r102975218 --- Diff: proton-c/src/tests/fuzz/fuzz-sniff-header.c --- @@ -0,0 +1,15 @@ +#include +#include + +#include "core/autodetect.h" + +#ifdef __cplusplus --- End diff -- Even so, you shouldn't need this outside header files. whether the internal symbol names are C++ or C is not important unless they are visible externally. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #93: PROTON-1312: fix memory leak on BlockingConnec...
Github user astitcher commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/93#discussion_r95459311 --- Diff: proton-c/bindings/python/proton/__init__.py --- @@ -2581,6 +2581,9 @@ def close(self): """ self._update_cond() pn_connection_close(self._impl) +if hasattr(self, '_session_policy'): --- End diff -- The issue with just using 'self._session_policy = None' is that it would create a useless attribute at his point if it doesn't already exist. But either solution works. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #92: PROTON-1379: [C++ binding] Add #if'd #include
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/92 Yep this looks like a mistake - I 'll pull it in. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #91: PROTON-1375: add explicit cast to avoid clang 4.0 war...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/91 This PR should be fixed by 79d06f07 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #90: PROTON-1355: Set ssl.peer_hostname to virtual_...
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/90#discussion_r88679501
--- Diff: proton-c/bindings/python/proton/reactor.py ---
@@ -565,7 +565,7 @@ def _connect(self, connection, reactor):
if not self.ssl_domain:
raise SSLUnavailable("amqps: SSL libraries not found")
self.ssl = SSL(transport, self.ssl_domain)
-self.ssl.peer_hostname = url.host
+self.ssl.peer_hostname = self.virtual_host if
self.virtual_host != None else url.host
--- End diff --
actually you can write it even more clearly ...
... = self.virtual_host or url.host
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #88: PROTON-1330: [python] bundle the C source in the pyth...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/88 I like this approach. I was expecting to see the file lists substituted in setup.py.in somewhere. I didn't see that - how are the CMake file lists used inside setup.py to generate the source bundle? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #88: PROTON-1330: [python] bundle the C source in t...
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/88#discussion_r88113144
--- Diff: proton-c/bindings/python/CMakeLists.txt ---
@@ -122,3 +129,55 @@ install(TARGETS ${SWIG_MODULE_cproton_REAL_NAME}
COMPONENT Python)
set_directory_properties(PROPERTIES ADDITIONAL_MAKE_CLEAN_FILES
"html;tutorial")
+
+#
+# Set up the directory 'dist' for building the python native package
+# source distribution for Pypi/pip
+#
+
+set(py_dist_dir ${CMAKE_CURRENT_BINARY_DIR}/dist)
+
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/setup.py.in
+ ${py_dist_dir}/setup.py
+)
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/README.rst.in
+ ${py_dist_dir}/README.rst
+)
+
+file(COPY ${py_dist_files} DESTINATION ${py_dist_dir})
+
+file(MAKE_DIRECTORY ${py_dist_dir}/proton)
+file(COPY ${pysrc} DESTINATION ${py_dist_dir}/proton)
+
+#file(COPY ${py_dist_proton_c_files}
--- End diff --
Do you really want to add commented out lines?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #87: Proton-c core split
Github user astitcher closed the pull request at: https://github.com/apache/qpid-proton/pull/87 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #87: Proton-c core split
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/87 @gemmellr I've been rebasing continually, so it's a surprise the dates are so long ago - I'll reset them before commit. Thanks for pointing this out. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #87: Proton-c core split
GitHub user astitcher opened a pull request: https://github.com/apache/qpid-proton/pull/87 Proton-c core split This is mostly for comment by @alanconway and @cliffjansen. It is a rearrangement of the Proton-c source tree, and an introduction of a core OS and IO independent qpid-proton-core library. You can merge this pull request into a Git repository by running: $ git pull https://github.com/astitcher/qpid-proton proton-core Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton/pull/87.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 #87 commit 82a590e7cf8d8c9f16e66282703b0e5fc8b3c3e3 Author: Andrew Stitcher <astitc...@apache.org> Date: 2016-08-30T20:36:19Z PROTON-1350 PROTON-1351: Introduce proton-c core library - Created new core proton library qpid-proton-core which only contains protocol processsing and no IO. - Rearranged source tree to separate core protocol code and io/reactor/extra code - Rearranged code so that compiler dependent code is isolated and platform (OS) dependent code is isolated This is a large change, but the majority is moving files around and fixing up the header includes. There is a small amount of internal API changing so support the core searation. commit 8336cd284b273fa5395c9424d05ae9bb36ae081e Author: Andrew Stitcher <astitc...@apache.org> Date: 2016-10-10T19:32:15Z Make go binding only depend on qpid-proton-core --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #85: PROTON-1330: [python] bundle the C source in the pyth...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/85 A couple of comments: * It seems entirely arbitrary (to me anyway) to move generating encoding.h and protocol.h from the C build process to generating them when producing the C part of the python install. We know that we have python available at the time of build the C code (well duh!) so we could keep on generating at C build time when we do the python install. This seems to me to minimise the divergence between the packaged C source for the python package and the real C source tree. * Given the c code built by the python install is somewhat selective anyway - it makes sense (again to me anyway) only to package C source which is actually used in the library used by the python installation. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #84: Adds cmake libatomic check to examplex/cpp
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/84 this should be fixed in SHA 76285f04e36cdbff68e98877cbc01bf4fb53 (which should be in the 0.15 release). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #84: Adds cmake libatomic check to examplex/cpp
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/84 Is this against master? I'm pretty sure @alanconway already fixed this by completely avoiding 8 byte atomics on 32 bit platforms. The place where 64 bit atomics are used just doesn't need them, being a simple sequence number. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #83: [Go binding] Replaced c handler based flowcont...
Github user astitcher closed the pull request at: https://github.com/apache/qpid-proton/pull/83 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #83: [Go binding] Replaced c handler based flowcontroller ...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/83 @alanconway Thanks for pointing out the stupid Cism in my go code. I've fixed it now. I'd much porefer to check this iin with sone sort of test - any clue how I should go about that? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #83: [Go binding] Replaced c handler based flowcont...
GitHub user astitcher opened a pull request: https://github.com/apache/qpid-proton/pull/83 [Go binding] Replaced c handler based flowcontroller with native go - The go binding now has no dependency on the proton-c reactor code @alanconway - could you take a look at this an tell me what you think. You can merge this pull request into a Git repository by running: $ git pull https://github.com/astitcher/qpid-proton go-flowcontroller Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton/pull/83.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 #83 commit ae1ee66ad46512acffae710f8d6f2d4126fc7d2c Author: Andrew Stitcher <astitc...@apache.org> Date: 2016-10-10T19:31:25Z [Go binding] Replaced c handler based flowcontroller with native go - The go binding now has no dependency on the proton-c reactor code --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #81: PROTON-1303: Replace go binding URL parser with versi...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/81 ":1234" is not allowed by the go URL parser - that is what is rejecting it. It gets rejected before our code has anything to do with it. [That is because it is invalid as a URL - you can't have a leading ":" without a scheme. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton issue #81: PROTON-1303: Replace go binding URL parser with versi...
Github user astitcher commented on the issue: https://github.com/apache/qpid-proton/pull/81 @alanconway Did you note the change in meaning for a few URLs? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton pull request #81: PROTON-1303: Replace go binding URL parser wit...
GitHub user astitcher opened a pull request: https://github.com/apache/qpid-proton/pull/81 PROTON-1303: Replace go binding URL parser with version that doesn't ⦠This PR is specially for @alanconway to review. You can merge this pull request into a Git repository by running: $ git pull https://github.com/astitcher/qpid-proton go-url-parser Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton/pull/81.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 #81 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
