You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by GitBox <gi...@apache.org> on 2021/06/03 21:32:45 UTC

[GitHub] [mesos] cf-natali opened a new pull request #392: Updated picojson to version 1.3.1-dev.

cf-natali opened a new pull request #392:
URL: https://github.com/apache/mesos/pull/392


   In order to fix a compilation warning under recent gcc versions.
   It also contains a patch which we used to maintain ourselves.
   
   Note that since version 1.3.1 hasn't been released - picojson doesn't
   seem actively maintained - this is really just the current master, and
   I'm including the short commit hash in the version to indicate exactly
   which version we're using.
   
   Compilation warning:
   ```
   ../3rdparty/picojson-1.3.0/picojson.h: In member function ‘const T& picojson::value::get() const [with T = double]’:
   ../3rdparty/picojson-1.3.0/picojson.h:304:124: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
     304 |   GET(double, (type_ == int64_type && (const_cast<value*>(this)->type_ = number_type, const_cast<value*>(this)->u_.number_ = u_.int64_), u_.number_))
         |                                                                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   ../3rdparty/picojson-1.3.0/picojson.h:292:12: note: in definition of macro ‘GET’
     292 |     return var;       \
   ```
   
   Fix for the compilation warning:
   https://github.com/kazuho/picojson/commit/c8853367b0ad6ff6e76188e639a8e7f0706d4431
   
   The patch we used to maintain ourselves:
   https://github.com/apache/mesos/blob/master/3rdparty/picojson-1.3.0.patch
   
   Upstream commit corresponding to this patch:
   https://github.com/kazuho/picojson/commit/2f9d3cf33808615376493469c4be6942fc3c987a
   And follow-up commit:
   https://github.com/kazuho/picojson/commit/35ce0b6b03e0b60541258f5ad061003a696d9a34
   
   @asekretenko @qianzhangxa 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [mesos] qianzhangxa commented on pull request #392: Updated picojson to version 1.3.1-dev.

Posted by GitBox <gi...@apache.org>.
qianzhangxa commented on pull request #392:
URL: https://github.com/apache/mesos/pull/392#issuecomment-857715024


   I see the fix in https://github.com/kazuho/picojson/commit/c8853367b0ad6ff6e76188e639a8e7f0706d4431 is pretty small, so can we just add it into our patch https://github.com/apache/mesos/blob/master/3rdparty/picojson-1.3.0.patch?
   
   Bumping picojson to 1.3.1-dev will involve more changes which may have unknown impact.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [mesos] cf-natali commented on pull request #392: Updated picojson to version 1.3.1-dev.

Posted by GitBox <gi...@apache.org>.
cf-natali commented on pull request #392:
URL: https://github.com/apache/mesos/pull/392#issuecomment-857971389


   Yes I was wondering about that, but:
   - the patch doesn't apply cleanly, because in the meantime they've reformatted the code
   - updating would allow us to get rid of this patch we maintain ourselves
   - I think the patch we currently apply is actually incomplete, upstream has a subsequent fix applied more recently: https://github.com/kazuho/picojson/commit/35ce0b6b03e0b60541258f5ad061003a696d9a34
   
   So I can definitely apply the one-line change to our patch, just want to make sure that's the preferred option - @asekretenko  ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [mesos] cf-natali commented on pull request #392: Backported fix for picojson -Wparentheses warning with recent GCC.

Posted by GitBox <gi...@apache.org>.
cf-natali commented on pull request #392:
URL: https://github.com/apache/mesos/pull/392#issuecomment-860088425


   OK I updated to just backport the specific change, let's get this merged!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [mesos] asfgit closed pull request #392: Backported fix for picojson -Wparentheses warning with recent GCC.

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #392:
URL: https://github.com/apache/mesos/pull/392


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org