You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/11/10 08:48:37 UTC

[GitHub] [nifi-minifi-cpp] lordgamez opened a new pull request #935: MINIFICPP-1404 Add option to disable unity build of AWS library

lordgamez opened a new pull request #935:
URL: https://github.com/apache/nifi-minifi-cpp/pull/935


   Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [ ] Is there a JIRA ticket associated with this PR? Is it referenced
        in the commit message?
   
   - [ ] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
   
   - [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
   
   - [ ] Is your initial contribution a single, squashed commit?
   
   ### For code changes:
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
   - [ ] If applicable, have you updated the LICENSE file?
   - [ ] If applicable, have you updated the NOTICE file?
   
   ### For documentation related changes:
   - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
   
   ### Note:
   Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
   


----------------------------------------------------------------
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] [nifi-minifi-cpp] lordgamez commented on pull request #935: MINIFICPP-1404 Add option to disable unity build of AWS library

Posted by GitBox <gi...@apache.org>.

lordgamez commented on pull request #935:
URL: https://github.com/apache/nifi-minifi-cpp/pull/935#issuecomment-724797290

> Tested, works, but I'm not sure this change adds a lot of value to the project. I would expect a third party lib to only change when it's updated to a newer version, which is a rare occurrence. Since it's behind an option, it doesn't hurt too much either. Treat my approval as a verification that this change is successfully achieving its goals but I'm neutral on whether we need this feature.

It was my personal experience that lead to this change, as working on the AWS features every newly introduced test change or minor code change triggered the regeneration and recompilation of the AWS SDK if unity build was enabled. This became tiresome after a while as changes took disproportionately long time to compile because of this. I did not want to disable the unity build by default because it makes the binary 20-30MB smaller in my experience, so I chose this option, but I opened the PR separately from other changes to be open for discussions whether it's okay have this option or just locally disable it in the CMake files when developing the AWS extension. This may be more explicit and could be found more easily with some explanation if anyone else experiences the same problem.

----------------------------------------------------------------
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] [nifi-minifi-cpp] lordgamez commented on pull request #935: MINIFICPP-1404 Add option to disable unity build of AWS library

Posted by GitBox <gi...@apache.org>.

lordgamez commented on pull request #935:
URL: https://github.com/apache/nifi-minifi-cpp/pull/935#issuecomment-724805060


   We currently have 2 dependencies from the sdk, the aws-cpp-sdk-s3 and the aws-cpp-sdk-core libraries. When we trigger a build after a dependent change the unity build generates new ub_S3.cpp and a ub_core.cpp files from the source files of those libraries. CMake only checks the timestamp of these source files so even with if the content is unchanged, they are rebuilt.


----------------------------------------------------------------
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] [nifi-minifi-cpp] szaszm commented on pull request #935: MINIFICPP-1404 Add option to disable unity build of AWS library

Posted by GitBox <gi...@apache.org>.

szaszm commented on pull request #935:
URL: https://github.com/apache/nifi-minifi-cpp/pull/935#issuecomment-724800185


   I wonder why changes in our aws code triggers recompilation of the third party aws lib. It's as is libaws had a dependency on the minifi aws extension rather than the other way around.


----------------------------------------------------------------
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] [nifi-minifi-cpp] lordgamez edited a comment on pull request #935: MINIFICPP-1404 Add option to disable unity build of AWS library

Posted by GitBox <gi...@apache.org>.

lordgamez edited a comment on pull request #935:
URL: https://github.com/apache/nifi-minifi-cpp/pull/935#issuecomment-724797290

It was my personal experience that lead to this change, as working on the AWS features every newly introduced test change or minor code change triggered the regeneration and recompilation of the AWS SDK if unity build was enabled. This became tiresome after a while as changes took disproportionately long time to compile because of this. I did not want to disable the unity build by default because it makes the binary 20-30MB smaller in my experience, so I chose this option. I also felt that this may not have a lot of added value so I opened the PR separately from other changes to be open for discussion whether it's okay have this option or just locally disable it in the CMake files when developing the extension. Having this option may be more explicit and could be found more easily with some explanation if anyone else experiences the same problem.

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

[GitHub] [nifi-minifi-cpp] szaszm closed pull request #935: MINIFICPP-1404 Add option to disable unity build of AWS library

Posted by GitBox <gi...@apache.org>.

szaszm closed pull request #935:
URL: https://github.com/apache/nifi-minifi-cpp/pull/935


   


----------------------------------------------------------------
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