-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
THRIFT-5766: Replace std::endl with "\n" #2943
Conversation
Looks good to me, albeit I do not have a strong opinion with respect to this change. I guess its a matter of personal taste whether |
(Personally)I don't think it is a matter of taste. I am also bringing up C++ Core Guidelines SL.io.50 (PS Which Bjarne commented on himself).
I would argue if it was only aesthetic why was the overload added in the first place... PS: I am probably passionate about the wrong things, shoot down the PR if this is 'out of scope' for this repo |
I doubled down and applied the changes to the whole codebase in my fork @ CJCombrink#1 |
Both the default constructor and operator== implementations reference certain member functions of the class' members. As an example, the default constructor references (i.e., "uses") the default constructors of its members. If a class contains a std::vector<Foo>, and Foo has only been *forward*- declared (which happens often in Thrift-generated code), this creates undefined behavior: The std::vector specification states that as long as Foo is an incomplete type, it is fine to reference std::vector<Foo>, but not any members (such as its default constructor). Thus, we must defer our default constructor's implementation (which references the default constructor of std::vector<Foo>) to a point where Foo is a complete type. That is the case in the .cpp file. The same holds for operator==.
Well we are certainly on the same page when it comes to In any case, I'm ok with the PR, but without a strong opinion pro or con on my side :-) |
That line was introduced intentionally in all generators. You may look up git and JIRA history and find out why, including the whole debate about why this is a good change, before removing it. I would also argue that forcing people into the literal inclusion of "\n" by removing the above most certainly will not be respected by people. Simply because nobody will think about it and just use endl instead, which due to the removal effectively results in the exact opposite of what is intended. You are setting it up for failure IMHO.
Not quite. Using std::endl is bad practice for the reasons linked above. But we're not doing that at all. That is the whole point why we have that endl constant, so nobody falls into that trap by accident. |
@Jens-G 11 Years later I am hoping that at least we can get to a point where sanity is restored and bad practices are not encouraged/entertained in such important code bases. PS: I come from a background where juniors used I am saying this again, I realise I am passionate about the wrong things thus I will not take offence if this/these PR(s) is/are declined, but I might keep stating the point :P
Depends on perspective: I see it as: "why do you need flushing" and my editor (VS Code with no custom modification) highlights |
+1 Go on then, change all the files and add a comment to the ticket, to put an end on this fruitless discussion. Question remains, why bad practice can't be fixed where it belongs - by properly deprecating
Usually deprecation and removal teaches quite well.
Agree, I wasn't fully aware of it. PS: Waiting for the great Rust rewrite, to save more walls. |
8471840
to
274ed34
Compare
I Updated this PR with replacing Do I still need to create a JIRA ticket for this change? I did request a user already thus waiting for that. |
@@ -221,11 +221,11 @@ void t_cl_generator::package_def(std::ostream &out) { | |||
} | |||
out << ")"; | |||
} | |||
out << ")" << endl << endl; | |||
out << ")\n\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in cases like this, the usage of endl
/std::endl
makes it much more obvious how many lines we are adding while \n
being part of the longer string just meshes everything together.
I'd suggest still explicitly adding every \n
individually, like this:
out << ")\n\n"; | |
out << ")" << "\n" << "\n"; |
but I don't feel strongly either way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fishy Thanks for the review and feedback.
Although I do understand the suggestion I specifically went through the effort to combine these. It does sort of stand out, but some editors color the \n
different and I deem that good enough. I find myself looking at the generated code much more often compared to the generator code thus there I quickly see how much (or few) newlines are added.
If there is enough pushback against the combination I can revert that combination change and (force)push the updates.
I don't think speed matters that much but I guess calling the operator<<
multiple times, more than the needed amount of time, would have a performance and speed implication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm slightly with @fishy here, as my comment in #2943 (comment) was actually going in the same direction: Having dedicated calls to << "\n"
jumps much more to the eye and conveyes how many newlines are added.
If you would be willing to change that, to make every newline jump out more, I'm two thumbs up for this PR! But without that change, I'm still ok to merge, albeit with minor hesitation...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@emmenlau Thank you again for the feedback.
I think there is enough reason to put back the << "\n
thus I will put in the effort (tonight) to 'revert' that change.
@CJCombrink from my point of view, everything else is good, thanks a lot for the nice work! The build failures are a continuous nightmare. The best you could do (and this is what I typically do) is check them individually, and see if the error seems related. Often builds fail because some third party dependency package can not be installed in some build machine, and those are the obvious ones to ignore. The others are harder, for those you can just compare the latest master build status with yours, to see if "you" broke it, or the person before you... In any case, we will also take a look at those before merging, but its great if you validate as much as you can upfront... |
44c4c59
to
b1abdc3
Compare
I updated the branch to use |
FYI the lib-java-kotlin failure in CI is being addressed in #2945 |
@fishy I think I have done what I can. I did check the test, master vs my changes for the parts that my environment is set up for and I don't see any regression. I have also compared the generated code for the unit tests with those against master and see no difference. What else can I do and what are the next steps for this PR? |
@CJCombrink sorry I'm in some family emergency and won't have time to review it any further for the next few weeks. None of my comment is blocking anyways :) Mario, Jens, and/or others should be able to guide you through the process. |
b1abdc3
to
12cdfa5
Compare
- Moves away from the bad habid of using endl which is known to flush - Remove references to endl and replace with line break
12cdfa5
to
ac117f7
Compare
I did some thinking... Or maybe just make the changes in the generators. I did go all the way with all other files, but baby steps might be the best or safest course of action. PS: I started a new ticket: THRIFT-5772 and if I have to choose I would rather have energy put there compared to this change, although this would be nice to have (in the C++ generator :P ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plus some files were totally missed
- contrib\fb303\TClientInfo.cpp
- lib\cpp\src\thrift\async\TEvhttpClientChannel.cpp
- lib\cpp\src\thrift\async\TEvhttpServer.cpp
- lib\cpp\src\thrift\transport\TFileTransport.cpp
- lib\cpp\test\concurrency\ThreadManagerTests.h
- lib\cpp\test\concurrency\TimerManagerTests.h
- lib\cpp\test\concurrency\Tests.cpp
Most of the missing single lines are cerr
outputs. I mean, if we want to save walls, we should be more consequent and include cerr
outputs as well, maybe doing the flush explicitly if needed. Or would cerr
be the exception from the rule that endl
is generally evil?
That's right but I'm fine in this particular case. If you could just add the commits re above changes at the end, that would make followup review easier for me. I will squash them before merge myself. |
Any news here? |
Hi Sorry, I was busy with a few other things. |
- PR feedback
- Braced init is a habit
- PR Review feedback - Initially skipped cerr but after some reading, cerr will flush after every << thus endl can also be avoided.
Initially I thought it might be good to keep cerr to flush. After your comment I did some thinking and reading and I see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks! Two minor bugs left though.
- Picked up in review
@Jens-G Thank you for your effort and meticulous attention to the details |
nice work @CJCombrink and (as usually) @Jens-G ! |
Patch: Carel Combrink This closes apache#2943
* THRIFT-5670: Fix wrong usage of GlobalOutput.perror() * Release 0.19.0 * Release 0.19.0 * THRIFT-5653: Fix Java UUID typeid It should be 16 not 17 according to the spec. Currently ENUM holds 16, it's not in TBinaryProtocol spec and seems to be a Java implementation detail somehow got mixed inside TType, move that to -1 for now. Someone more familiar with Java can probably remove it from TType completely in the future. client: java * reformat kotlin files * fix gradle format * Update supported go versions to 1.20 and 1.21 * fix kotlin cross test by downgrading to java 8 (apache#2840) * THRIFT-5731: Handle ErrAbandonRequest automatically Also add a test to verify the behavior. The test helped me to found a bug in TSimpleServer that didn't handle the ErrAbandonRequest case correctly, so fix the bug as well. client: go * THRIFT-5653: Fix Java UUID typeid It should be 16 not 17 according to the spec. Currently ENUM holds 16, it's not in TBinaryProtocol spec and seems to be a Java implementation detail somehow got mixed inside TType, move that to -1 for now. Someone more familiar with Java can probably remove it from TType completely in the future. client: java * release 0.19.0 * Bump com.github.spotbugs from 5.1.0 to 5.1.3 in /lib/java (apache#2848) Bumps com.github.spotbugs from 5.1.0 to 5.1.3. --- updated-dependencies: - dependency-name: com.github.spotbugs dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump com.diffplug.spotless from 6.20.0 to 6.21.0 in /lib/java (apache#2847) Bumps com.diffplug.spotless from 6.20.0 to 6.21.0. --- updated-dependencies: - dependency-name: com.diffplug.spotless dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * THRIFT-5733 Building code with circular includes crashes Thrift compiler Patch: Jens Geyer This closes apache#2851 * THRIFT-5734 generated code may lack required capitalization at class names CLient: haxe Patch: Jens Geyer * Release Management: added the missing EXTRA_DIST validation step * fix: fix NewTSocketConf comment error Client: ["go"] [skip ci] * THRIFT-5738: Remove unused variable * Bump com.diffplug.spotless from 6.21.0 to 6.22.0 in /lib/kotlin (apache#2859) Bumps com.diffplug.spotless from 6.21.0 to 6.22.0. --- updated-dependencies: - dependency-name: com.diffplug.spotless dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump actions/checkout from 3 to 4 (apache#2858) Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix building with gcc 13 This fixes the following compiler error. t_js_generator.cc:48:14: error: 'int64_t' does not name a type 48 | static const int64_t max_safe_integer = 0x1fffffffffffff; | ^~~~~~~ t_js_generator.cc:50:14: error: 'int64_t' does not name a type 50 | static const int64_t min_safe_integer = -max_safe_integer; | ^~~~~~~ * lib/cpp/test/Security*Test.cpp: Fix the check for OpenSSL version * build/cmake/DefinePlatformSpecifc.cmake: Separated MSVC and Clang-Cl settings for Windows * fix(php): exclude most part of the repo for php package * TProtocol.h: Be extra careful when including MSVC Windows-related headers to not pollute the c++ namespace * lib/cpp/test/concurrency/Tests.cpp: Very minor code cleanup (whitespace changes only) * build/cmake/GenerateConfigModule.cmake: Do not install 'FindLibevent.cmake' if libevent is disabled * Bump jvm from 1.8.22 to 1.9.10 in /lib/kotlin Bumps [jvm](https://github.com/JetBrains/kotlin) from 1.8.22 to 1.9.10. - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/v1.9.10/ChangeLog.md) - [Commits](JetBrains/kotlin@v1.8.22...v1.9.10) --- updated-dependencies: - dependency-name: jvm dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * rust to add uuid support * remove utf8 * fix domain socket * THRIFT-5660: Revert "lib: cpp: TTransportException: create thrift::numeric_cast" This reverts commit 6e9cbbd. See https://issues.apache.org/jira/browse/THRIFT-5660 for a discussion. * THRIFT-4606 LGPL license file still present (follow-up to THRIFT-3209) * use gradle 8.4 (apache#2869) * Bump @babel/traverse from 7.8.4 to 7.23.2 Bumps [@babel/traverse](https://github.com/babel/babel/tree/HEAD/packages/babel-traverse) from 7.8.4 to 7.23.2. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.23.2/packages/babel-traverse) --- updated-dependencies: - dependency-name: "@babel/traverse" dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * Bump browserify-sign from 4.0.4 to 4.2.2 in /lib/js Bumps [browserify-sign](https://github.com/crypto-browserify/browserify-sign) from 4.0.4 to 4.2.2. - [Changelog](https://github.com/browserify/browserify-sign/blob/main/CHANGELOG.md) - [Commits](browserify/browserify-sign@v4.0.4...v4.2.2) --- updated-dependencies: - dependency-name: browserify-sign dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * Bump browserify-sign from 4.0.4 to 4.2.2 in /lib/ts Bumps [browserify-sign](https://github.com/crypto-browserify/browserify-sign) from 4.0.4 to 4.2.2. - [Changelog](https://github.com/browserify/browserify-sign/blob/main/CHANGELOG.md) - [Commits](browserify/browserify-sign@v4.0.4...v4.2.2) --- updated-dependencies: - dependency-name: browserify-sign dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * Bump com.github.spotbugs from 5.1.3 to 5.2.1 in /lib/kotlin Bumps com.github.spotbugs from 5.1.3 to 5.2.1. --- updated-dependencies: - dependency-name: com.github.spotbugs dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Bump jvm from 1.9.10 to 1.9.20 in /lib/kotlin Bumps [jvm](https://github.com/JetBrains/kotlin) from 1.9.10 to 1.9.20. - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/master/ChangeLog.md) - [Commits](https://github.com/JetBrains/kotlin/commits) --- updated-dependencies: - dependency-name: jvm dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * THRIFT-5740 inherited interfaces should be explicitly listed in Delphi class decl Client: Delphi Patch: Jens Geyer * THRIFT-5741: use rust 1.65 (apache#2870) * use rust 1.65 * fix clippy * fix alert * fix protocol * fix one more dereference * fix more lint * fix over-fix * fix match &*server_type { * THRIFT-5742 Add addRange() function to Set helper classes to support adding data from arbitrary enumerable source containers Client: hx Patch: Jens Geyer * THRIFT-5743 add TLS1.3 to default protocols where available Client: netstd Patch: Jens Geyer * THRIFT-5746 Upgrade to net8 Client: netstd Patch: Jens Geyer * THRIFT-5746 Upgrade to net8 Client: netstd Patch: Jens Geyer This updates certain packages that became available now for 8.0.0 * THRIFT-5747 warning: The macro `AC_HELP_STRING' is obsolete. Patch: Jens Geyer * Bump com.diffplug.spotless from 6.22.0 to 6.23.2 in /lib/kotlin Bumps com.diffplug.spotless from 6.22.0 to 6.23.2. --- updated-dependencies: - dependency-name: com.diffplug.spotless dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Bump jvm from 1.9.20 to 1.9.21 in /lib/kotlin Bumps [jvm](https://github.com/JetBrains/kotlin) from 1.9.20 to 1.9.21. - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/master/ChangeLog.md) - [Commits](JetBrains/kotlin@v1.9.20...v1.9.21) --- updated-dependencies: - dependency-name: jvm dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * Bump actions/setup-java from 3 to 4 Bumps [actions/setup-java](https://github.com/actions/setup-java) from 3 to 4. - [Release notes](https://github.com/actions/setup-java/releases) - [Commits](actions/setup-java@v3...v4) --- updated-dependencies: - dependency-name: actions/setup-java dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * Bump com.github.spotbugs from 5.2.1 to 6.0.1 in /lib/kotlin Bumps com.github.spotbugs from 5.2.1 to 6.0.1. --- updated-dependencies: - dependency-name: com.github.spotbugs dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * Github Actions: Remove --disable-tests With --disable-tests, for example for Go the `make check` under `lib/go` would only run unit tests under `lib/go/thrift` but not the unit tests under `lib/go/test`. Also some changes in lib/go/test/fuzz/Makefile.am so it works in both go 1.20 and 1.21 (The current state breaks in 1.21 but because of `--disable-tests` we never noticed that). * Downgrade spotbugs to 5.2.5 6.x has breaking changes need extra care to upgrade. * Revert "lib: cpp: automake: ship thrift/numeric_cast.h" This reverts commit 779deab. * THRIFT-5749 Option to enable RTTI info Client: Delphi Patch: Jens Geyer * FIX: Unused import Map Client: hx Patch: Jens Geyer * THRIFT-5749 Option to enable RTTI info (2nd attempt) Client: Delphi Patch: Jens Geyer * Fix github actions for python3 tests Add a dummy test in test_sslsocket.py to workaround an issue in Python 3.12. * Bump actions/setup-go from 4 to 5 Bumps [actions/setup-go](https://github.com/actions/setup-go) from 4 to 5. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](actions/setup-go@v4...v5) --- updated-dependencies: - dependency-name: actions/setup-go dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * Bump com.diffplug.spotless from 6.23.2 to 6.23.3 in /lib/java Bumps com.diffplug.spotless from 6.23.2 to 6.23.3. --- updated-dependencies: - dependency-name: com.diffplug.spotless dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * Bump actions/setup-python from 4 to 5 Bumps [actions/setup-python](https://github.com/actions/setup-python) from 4 to 5. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v4...v5) --- updated-dependencies: - dependency-name: actions/setup-python dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * Bump jvm from 1.9.21 to 1.9.22 in /lib/kotlin Bumps [jvm](https://github.com/JetBrains/kotlin) from 1.9.21 to 1.9.22. - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/master/ChangeLog.md) - [Commits](JetBrains/kotlin@v1.9.21...v1.9.22) --- updated-dependencies: - dependency-name: jvm dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * Bump upload/download-artifacts from v3 to v4 This is the combination of apache#2910 & apache#2912. It looks like these 2 changes need to be done together, doing them individually will break CI. * Bump org.codehaus.plexus:plexus-utils in /contrib/thrift-maven-plugin Bumps [org.codehaus.plexus:plexus-utils](https://github.com/codehaus-plexus/plexus-utils) from 3.0.16 to 3.0.24. - [Release notes](https://github.com/codehaus-plexus/plexus-utils/releases) - [Commits](codehaus-plexus/plexus-utils@plexus-utils-3.0.16...plexus-utils-3.0.24) --- updated-dependencies: - dependency-name: org.codehaus.plexus:plexus-utils dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> * [THRIFT-5752] Add TTransportFactoryInterface * Revert "Bump upload/download-artifacts from v3 to v4" This reverts commit 8540066. actions/upload-artifact#478 will impact us. * THRIFT-5754: Fix PHP 8.1 deprecates passing null to non-nullable internal function parameters * THRIFT-5753 PHP 8.1 deprecated warning about return type in jsonSerialize functions * THRIFT-5750 deprecate "ansistr_binary_" option Client: delphi Patch: Jens Geyer * Fix ambigous typescript definitions * Bump com.diffplug.spotless from 6.23.3 to 6.25.0 in /lib/java Bumps com.diffplug.spotless from 6.23.3 to 6.25.0. --- updated-dependencies: - dependency-name: com.diffplug.spotless dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * bump version number * Updated manual version info and CHANGES * THRIFT-5744: Switch to slog for go library Client: go * THRIFT-5745: Implement slog.LogValuer on go TStructs Client: go Implement slog.LogValuer for all TStruct and TException generated by the compiler for go code. Also add SlogTStructWrapper in the library so we don't have to repeat it in the compiler generated go code. * THRIFT-5744: Switch to slog for go library Client: go * THRIFT-5745: Implement slog.LogValuer on go TStructs Client: go Implement slog.LogValuer for all TStruct and TException generated by the compiler for go code. Also add SlogTStructWrapper in the library so we don't have to repeat it in the compiler generated go code. * THRIFT-5688: Add PyPI publishing github actions This is tested with apache#2927, which published to https://test.pypi.org/project/thrift-test/. I tested locally with: (venv) fishy@penguin:~/work/test$ pip install -i https://test.pypi.org/simple/ thrift-test Looking in indexes: https://test.pypi.org/simple/ Collecting thrift-test Downloading https://test-files.pythonhosted.org/packages/e6/02/5885ea1406f560d0a23351f68acc2892d7f6495b16bfc2eeee8de4649777/thrift-test-0.21.0.tar.gz (62 kB) ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 62.3/62.3 kB 1.4 MB/s eta 0:00:00 Preparing metadata (setup.py) ... done Collecting six>=1.7.2 (from thrift-test) Downloading https://test-files.pythonhosted.org/packages/b3/b2/238e2590826bfdd113244a40d9d3eb26918bd798fc187e2360a8367068db/six-1.10.0.tar.gz (29 kB) Preparing metadata (setup.py) ... done Building wheels for collected packages: thrift-test, six Building wheel for thrift-test (setup.py) ... done Created wheel for thrift-test: filename=thrift_test-0.21.0-cp311-cp311-linux_x86_64.whl size=416914 sha256=3a972bc562be7ed19cb37399e444ed0d373cde5319023974080b625e550901d4 Stored in directory: /home/fishy/.cache/pip/wheels/45/20/1f/d3e1b869ac068d63ca2b2c13a2f4e33a80b360fae7091c8a9b Building wheel for six (setup.py) ... done Created wheel for six: filename=six-1.10.0-py2.py3-none-any.whl size=9942 sha256=74014380446ccf331366316cec0b1aaf40e0162e70307622b493e38e8451115f Stored in directory: /home/fishy/.cache/pip/wheels/e4/18/d0/e02474c90dcf14c511c0f52145d7e72e41ff3fb80b330ba58e Successfully built thrift-test six Installing collected packages: six, thrift-test Successfully installed six-1.10.0 thrift-test-0.21.0 (venv) fishy@penguin:~/work/test$ python3 Python 3.11.7 (main, Dec 8 2023, 14:22:46) [GCC 13.2.0] on linux Type "help", "copyright", "credits" or "license" for more information. >>> from thrift.transport import TSocket >>> transport = TSocket.TSocket('localhost', 9090) >>> transport <thrift.transport.TSocket.TSocket object at 0x785b18d83690> >>> transport.open() Could not connect to any of [('::1', 9090, 0, 0), ('127.0.0.1', 9090)] Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/home/fishy/work/test/venv/lib/python3.11/site-packages/thrift/transport/TSocket.py", line 149, in open raise TTransportException(type=TTransportException.NOT_OPEN, message=msg) thrift.transport.TTransport.TTransportException: Could not connect to any of [('::1', 9090, 0, 0), ('127.0.0.1', 9090)] >>> from thrift.protocol import fastbinary >>> fastbinary <module 'thrift.protocol.fastbinary' from '/home/fishy/work/test/venv/lib/python3.11/site-packages/thrift/protocol/fastbinary.cpython-311-x86_64-linux-gnu.so'> >>> fastbinary.decode_compact <built-in function decode_compact> >>> If we want to merge this version, I'll enable pending publishing with `pypi.yml` from this repo on pypi [1]. [1]: https://pypi.org/manage/account/publishing/ * THRIFT-5756 Run php tests in github actions * add ASF Header * THRIFT-5688: Add PyPI publishing github actions This is tested with apache#2927, which published to https://test.pypi.org/project/thrift-test/. I tested locally with: (venv) fishy@penguin:~/work/test$ pip install -i https://test.pypi.org/simple/ thrift-test Looking in indexes: https://test.pypi.org/simple/ Collecting thrift-test Downloading https://test-files.pythonhosted.org/packages/e6/02/5885ea1406f560d0a23351f68acc2892d7f6495b16bfc2eeee8de4649777/thrift-test-0.21.0.tar.gz (62 kB) ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 62.3/62.3 kB 1.4 MB/s eta 0:00:00 Preparing metadata (setup.py) ... done Collecting six>=1.7.2 (from thrift-test) Downloading https://test-files.pythonhosted.org/packages/b3/b2/238e2590826bfdd113244a40d9d3eb26918bd798fc187e2360a8367068db/six-1.10.0.tar.gz (29 kB) Preparing metadata (setup.py) ... done Building wheels for collected packages: thrift-test, six Building wheel for thrift-test (setup.py) ... done Created wheel for thrift-test: filename=thrift_test-0.21.0-cp311-cp311-linux_x86_64.whl size=416914 sha256=3a972bc562be7ed19cb37399e444ed0d373cde5319023974080b625e550901d4 Stored in directory: /home/fishy/.cache/pip/wheels/45/20/1f/d3e1b869ac068d63ca2b2c13a2f4e33a80b360fae7091c8a9b Building wheel for six (setup.py) ... done Created wheel for six: filename=six-1.10.0-py2.py3-none-any.whl size=9942 sha256=74014380446ccf331366316cec0b1aaf40e0162e70307622b493e38e8451115f Stored in directory: /home/fishy/.cache/pip/wheels/e4/18/d0/e02474c90dcf14c511c0f52145d7e72e41ff3fb80b330ba58e Successfully built thrift-test six Installing collected packages: six, thrift-test Successfully installed six-1.10.0 thrift-test-0.21.0 (venv) fishy@penguin:~/work/test$ python3 Python 3.11.7 (main, Dec 8 2023, 14:22:46) [GCC 13.2.0] on linux Type "help", "copyright", "credits" or "license" for more information. >>> from thrift.transport import TSocket >>> transport = TSocket.TSocket('localhost', 9090) >>> transport <thrift.transport.TSocket.TSocket object at 0x785b18d83690> >>> transport.open() Could not connect to any of [('::1', 9090, 0, 0), ('127.0.0.1', 9090)] Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/home/fishy/work/test/venv/lib/python3.11/site-packages/thrift/transport/TSocket.py", line 149, in open raise TTransportException(type=TTransportException.NOT_OPEN, message=msg) thrift.transport.TTransport.TTransportException: Could not connect to any of [('::1', 9090, 0, 0), ('127.0.0.1', 9090)] >>> from thrift.protocol import fastbinary >>> fastbinary <module 'thrift.protocol.fastbinary' from '/home/fishy/work/test/venv/lib/python3.11/site-packages/thrift/protocol/fastbinary.cpython-311-x86_64-linux-gnu.so'> >>> fastbinary.decode_compact <built-in function decode_compact> >>> If we want to merge this version, I'll enable pending publishing with `pypi.yml` from this repo on pypi [1]. [1]: https://pypi.org/manage/account/publishing/ * Add license header to pypi workflow file * Add license header to pypi workflow file * [THRIFT-5760] Update minimal version of php * [THRIFT-5758] PHP 8.2 Deprecate dynamic properties * Fix: fix readMessageBegin name type error Client: ["python"] * THRIFT-5761 Lib/json tests fail Fix the test to expect `typeId` and `class` inside the `type` object instead of on the root level. This is the way the compiler generates is. Old output: ```json "constants": [ { "name": "myNumberz", "typeId": "enum", "type": { "typeId": "enum", "class": "Numberz" }, "value": 1 } ], ``` New output: ``` "constants": [ { "name": "myNumberz", "typeId": "enum", "class": "Numberz", "value": 1 } ], ``` * [THRIFT-5757] Unit tests for php lib * Update lib/php/test/Unit/Lib/StringFunc/MbStringTest.php Co-authored-by: Pavel Kvach <pavel.kvach@gmail.com> * Update lib/php/test/Unit/Lib/StringFunc/CoreTest.php Co-authored-by: Pavel Kvach <pavel.kvach@gmail.com> * Update lib/php/test/Unit/Lib/ClassLoader/ThriftClassLoaderTest.php Co-authored-by: Pavel Kvach <pavel.kvach@gmail.com> * THRIFT-5762 Expose service result objects in Java Some libraries want to bypass the TServer class and handle the full service startup manually. For example when building a service that hosts multiple thrift services where the IFace type is unknown when handling a request. For example when you host multiple services on top of netty and through an HTTP path you want to route to the correct thrift service. In this situation you treat can treat an IFace as an Object and use the `getProcessMapView()` method to parse a byte array into a thrift message and pass let the `AsyncProcessFunction` handle the invocation. To return a correct thrift response it's necessary to write the `{service_name}_result` that contains the response args. While it is possible to get an incoming args object from the (Async)ProcessFunction its unfortunately not possible to get a result object without using reflection. This PR extends the (Async)ProcessFunction by adding a `getEmptyResultInstance` method that returns a new generic `A` (answer) that matches the `{service_name}_result` object. This allows thrift users to write the following processing code: ```java <I> void handleRequest( TProtocol in, TProtocol out, TBaseAsyncProcessor<I> processor, I asyncIface ) throws TException { final Map<String, AsyncProcessFunction<Object, TBase<?, ?>, TBase<?, ?>, TBase<?, ?>>> processMap = (Map) processor.getProcessMapView(); final var msg = in.readMessageBegin(); final var fn = processMap.get(msg.name); final var args = fn.getEmptyArgsInstance(); args.read(in); in.readMessageEnd(); if (fn.isOneway()) { return; } fn.start(asyncIface, args, new AsyncMethodCallback<>() { @OverRide public void onComplete(TBase<?, ?> o) { try { out.writeMessageBegin(new TMessage(fn.getMethodName(), TMessageType.REPLY, msg.getSeqid())); final var response_result = fn.getEmptyResultInstance(); final var success_field = response_result.fieldForId(SUCCESS_ID); ((TBase) response_result).setFieldValue(success_field, o); response_result.write(out); out.writeMessageEnd(); out.getTransport().flush(); } catch (TException e) { throw new RuntimeException(e); } } @OverRide public void onError(Exception e) { try { out.writeMessageBegin(new TMessage(fn.getMethodName(), TMessageType.EXCEPTION, msg.getSeqid())); ((TApplicationException) e).write(out); out.writeMessageEnd(); out.getTransport().flush(); } catch (TException ex) { throw new RuntimeException(ex); } } }); } ``` The above example code doesn't need any reference to the original types and can dynamically create the correct objects to return a correct response. * THRIFT-5764 Extra CTOR for TThriftBytesImpl Client: Delphi Patch: Jens Geyer * THRIFT-5765 Extra override for WriteBinary() to avoid unnecessary memory allocations when using COM types Client: Delphi Patch: JensG * Move default constructor and operator== implementation to CPP file Both the default constructor and operator== implementations reference certain member functions of the class' members. As an example, the default constructor references (i.e., "uses") the default constructors of its members. If a class contains a std::vector<Foo>, and Foo has only been *forward*- declared (which happens often in Thrift-generated code), this creates undefined behavior: The std::vector specification states that as long as Foo is an incomplete type, it is fine to reference std::vector<Foo>, but not any members (such as its default constructor). Thus, we must defer our default constructor's implementation (which references the default constructor of std::vector<Foo>) to a point where Foo is a complete type. That is the case in the .cpp file. The same holds for operator==. * Factor out duplicated code into helper function * Move generate_default_constructor call into generate_struct_definition This makes sure that helper structs like _args and _result also have their default constructors defined. * Always generate an initializer list * Fix ODR violations in cases where templates are involved * expose qt5 targets if available * [THRIFT-5757] Unit tests for php lib * Preparing 0.20.0 * Add "my own +1 vote" paragraph to mail template * THRIFT-5769: fix invalid size error on large messages Client: nodejs * THRIFT-5767: use string builder to parse strings with escaped quotes (apache#2946) Client: Go * THRIFT-5768 Add missing test in configure.ac for kotlin The Kotlin autoconfig script was missing a `test` causing it to try and execute `x/usr/local/bin/gradle` instead of test for its existence. This resulted in the following error: ``` ./configure: line 15049: x/usr/local/bin/gradle: No such file or directory ``` Adding `test` results in the configuration succeeding. Configure output now: ``` thrift 0.21.0 Building C (GLib) Library .... : yes Building C++ Library ......... : yes Building Common Lisp Library.. : yes Building D Library ........... : yes Building Dart Library ........ : yes Building .NET Standard Library : yes Building Erlang Library ...... : yes Building Go Library .......... : yes Building Haxe Library ........ : yes Building Java Library ........ : yes Building Kotlin Library ...... : yes Building Lua Library ......... : yes Building NodeJS Library ...... : yes Building Perl Library ........ : yes Building PHP Library ......... : yes Building Python Library ...... : yes Building Py3 Library ......... : yes Building Ruby Library ........ : yes Building Rust Library ........ : yes Building Swift Library ....... : yes ``` * THRIFT-5762 Fix spotless errors Run `gradlew :spotlessApply` to apply the correct coding style. Update kotlin compiler to support `getEmptyResultInstance` apache#2939 added the feature to create an instance of the result object without having to use the ProcessFunction. The Kotlin compiler re-uses the java lib so this commit udpates the Kotlin compiler to support this feature as well. * We have 2024 * [THRIFT-5757] Unit tests for php lib * Upgraded appveyor dockerfile reference to zlib to 1.2.13 Patch: JensG * Upgraded dockerfile reference to Win64OpenSSL-1_1_0l.exe * Bump jvm from 1.9.22 to 1.9.23 in /lib/kotlin Bumps [jvm](https://github.com/JetBrains/kotlin) from 1.9.22 to 1.9.23. - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/v1.9.23/ChangeLog.md) - [Commits](JetBrains/kotlin@v1.9.22...v1.9.23) --- updated-dependencies: - dependency-name: jvm dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * THRIFT-5750 Remove "ansistr_binary_" option Client: delphi Patch: Jens Geyer * THRIFT-5755 Docker image build fail This PR submits fixes to the focal and jammy docker images. * Bionic support was dropped becaused dotnet 8 no longer supports bionic (Ubuntu 18.04). Moved to `old/` like other unmaintained images. * Focal/Jammy used the wrong apt location for dotnet, endpoint was 18.04 instead of 20.04/22.04 * Jammy cannot build Erlang OPT 23 since it depends on OpenSSL 1.1 which was dropped in favor of 3.0. Using Erlang OPT 25 fixes the problem since it depends on OpenSSL 3.0 * Jammy was installing JDK 11 but lib/java requires Java 17 All containers used the `root` used to volume map the local files into the running container. This creates a hard to maintain working directory on Linux and MacOS since files form the local user and root user are mixed. To solve this the new docker files can be build using the UID and GID of the host so the files dont mix. The script uses UID and GID 1000 since these are the default ids for most Linux distros. Change the travis yml to build with 20.04 instead of 18.04. Removed all traces of 18.04 but it cant be tested locally. Updated the README to reflect the new `build/docker/` directory. * Remove `old/` directory from `build/docker` since its not maintained/used * Update GO in docker containers to 1.21 * Add Haxe and fix rust * Add specific PHP versions and additional dependencies * Update readme version table * Fix haxe setup, allow root users to be used and check for existing users * Fix EXTRA_DIST in php make file * Manually remove $(BUILT_SOURCES) since no-dist-built-sources doesnt work * Updated ReleaseManagement.md according to THRIFT-5755 changes * Bump org.jetbrains.kotlinx:kotlinx-coroutines-jdk8 in /lib/kotlin (apache#2941) Bumps [org.jetbrains.kotlinx:kotlinx-coroutines-jdk8](https://github.com/Kotlin/kotlinx.coroutines) from 1.7.3 to 1.8.0. - [Release notes](https://github.com/Kotlin/kotlinx.coroutines/releases) - [Changelog](https://github.com/Kotlin/kotlinx.coroutines/blob/master/CHANGES.md) - [Commits](Kotlin/kotlinx.coroutines@1.7.3...1.8.0) --- updated-dependencies: - dependency-name: org.jetbrains.kotlinx:kotlinx-coroutines-jdk8 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * THRIFT-5772: UUID support for c++ apache#2952 Client: cpp Patch: CJCombrink This closes apache#2952 * [THRIFT-5757] Unit tests for php lib Client: php Patch: Volodymyr Panivko This closes apache#2951 * THRIFT-5766 Replace std::endl with "\n" Patch: Carel Combrink This closes apache#2943 * THRIFT-5780 Prevent certain warnings related to net8 Client: netstd Patch: Jens Geyer This closes apache#2965 * THRIFT-5781 implement full deprecation support Client: netstd Patch: Jens Geyer * THRIFT-5782 implement full deprecation support Client: Delphi Patch: Jens Geyer plus a minor netstd fix * go: Define a bytePool for TRichTransport Client: go TBinaryProtocol and TCompactProtocol (and as an extension, THeaderProtocol) use TRichTransport's ReadByte/WriteByte functions a lot under the hood, and in some extreme cases those ReadByte/WriteByte calls can generate a lot of allocations for the byte they used. Use a resource pool to help reduce the allocations. * Revert "go: Define a bytePool for TRichTransport" This reverts commit 344498b. In our extreme case this actually made things worse. On 30s cpu profiles, although mallocgc reduced from 27.13s to 26.30s, the byte pool itself costed 11.9s. Looking at writeByte and readByte, writeByte increased from 3.69s to 5.89s, and readByte increased from 11.36s to 16.09s. * THRIFT-5783 drop net7 support Client: netstd Patch: Jens Geyer * THRIFT-5782 implement full deprecation support Client: Delphi Patch: Jens Geyer FIX: end-of-line comments may cause uncompileable code * go: Use errors.Is over == Client: go Fix 2 instances we are using == to check on error but should have used errors.Is instead. * THRIFT-5784: Add THeaderTransforms to TConfiguration Client: go While I'm here, also auto add compression transforms read (currently only zlib is supported) to writeTransforms so that a server will auto use the same compression on the responses as the client chose to use in the requests. * THRIFT-5781 implement full deprecation support -> fix uncompileable issue at deprecated enums Client: netstd Patch: Jens Geyer * go: Proper indent in compiler This is a "trivial" change for go compiler to always use the combination of indent_up, indent_down, and indent, over manual indentation (by adding 2 spaces at the beginning of the string). Also change go compiler's indent_str to tab over 2 spaces. While I'm here, also made a few minor tweaks on generated go code. * Remove deprecated has_rdoc from gemspec * THRIFT-5787 restoring binary compatibility of Factory constructor Client: netstd Patch: Steven Mitcham This closes apache#2979 * THRIFT-5788 Refactor and streamline hash set implementation Client: delphi Patch: Jens Geyer * THRIFT-5789 Refactor test suite client implementation Client: Delphi Patch: Jens Geyer * THRIFT-5788 Refactor and streamline hash set implementation Client: delphi Patch: Jens Geyer Follow-up: fix memory leak * THRIFT-5786: Full deprecation support in go Client: go This should got most of them. Also fixed an indentation bug from 4930ca. * fix: TProtocol::skip fix parameter type Client: php Patch: Ilya Urvachev & Volodymyr Panivko This closes apache#2983 * Bump org.jetbrains.kotlinx:kotlinx-coroutines-jdk8 in /lib/kotlin Bumps [org.jetbrains.kotlinx:kotlinx-coroutines-jdk8](https://github.com/Kotlin/kotlinx.coroutines) from 1.8.0 to 1.8.1. - [Release notes](https://github.com/Kotlin/kotlinx.coroutines/releases) - [Changelog](https://github.com/Kotlin/kotlinx.coroutines/blob/master/CHANGES.md) - [Commits](Kotlin/kotlinx.coroutines@1.8.0...1.8.1) --- updated-dependencies: - dependency-name: org.jetbrains.kotlinx:kotlinx-coroutines-jdk8 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * Update compact-protocol docs This updates the compact protocol documentation to simplify the explanation of Zigzag and ULEB128 encoding, along with links to wikipedia. In addition it uses varing instead of var int since the former is more common. * FIX: features documentation * go: Fix indentation in compiler This fixes a small case I missed in 4930cac. Before: func NewFoo() *Foo { return &Foo{ DefDef: 10, DefOpt: 11, } } After: func NewFoo() *Foo { return &Foo{ DefDef: 10, DefOpt: 11, } } * go: Improve efficiency with zlib in THeaderTransport When enabled zlib in THeaderTransport we observed very high cpu overhead, use a pool for zlib writers to improve efficiency. * Bump braces from 3.0.2 to 3.0.3 in /lib/js Bumps [braces](https://github.com/micromatch/braces) from 3.0.2 to 3.0.3. - [Changelog](https://github.com/micromatch/braces/blob/master/CHANGELOG.md) - [Commits](micromatch/braces@3.0.2...3.0.3) --- updated-dependencies: - dependency-name: braces dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * THRIFT-5773 Strong UUID wrapper for C++ Client: cpp/CMakeLists.txt Patch: Carel Combrink This closes apache#2958 * Bump braces from 3.0.2 to 3.0.3 in /lib/ts Bumps [braces](https://github.com/micromatch/braces) from 3.0.2 to 3.0.3. - [Changelog](https://github.com/micromatch/braces/blob/master/CHANGELOG.md) - [Commits](micromatch/braces@3.0.2...3.0.3) --- updated-dependencies: - dependency-name: braces dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * Bump ws from 6.2.2 to 6.2.3 in /lib/ts Bumps [ws](https://github.com/websockets/ws) from 6.2.2 to 6.2.3. - [Release notes](https://github.com/websockets/ws/releases) - [Commits](websockets/ws@6.2.2...6.2.3) --- updated-dependencies: - dependency-name: ws dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * optimizing performance issues under large-scale connection * THRIFT-5654 LNK4042 and LNK2019 in go_validator_generator.cc with VS2022 Patch: Eyüp Volkan & Jens Geyer * Bump ws from 6.2.2 to 6.2.3 in /lib/js Bumps [ws](https://github.com/websockets/ws) from 6.2.2 to 6.2.3. - [Release notes](https://github.com/websockets/ws/releases) - [Commits](websockets/ws@6.2.2...6.2.3) --- updated-dependencies: - dependency-name: ws dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * THRIFT-5777: python fix mismatched timeout exceptions Client: ["python"] * Use pip install instead of deprecated python setup.py install command * Remove trailing space from compiler generated go code Client: go * removing cend and cbegin * returning file to original state --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: stiga-huang <huangquanlong@gmail.com> Co-authored-by: Jens Geyer <jensg@apache.org> Co-authored-by: Yuxuan 'fishy' Wang <yuxuan.wang@reddit.com> Co-authored-by: Jiayu Liu <jiayu@hey.com> Co-authored-by: Jiayu Liu <Jimexist@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: bwangelme <bwangel.me@gmail.com> Co-authored-by: Jeff Alder <49817386+jeffalder@users.noreply.github.com> Co-authored-by: Biswapriyo Nath <nathbappai@gmail.com> Co-authored-by: Mario Emmenlauer <memmenlauer@biodataanalysis.de> Co-authored-by: Mario Emmenlauer <mario@emmenlauer.de> Co-authored-by: Ilya Urvachev <rtm@ctrlz.ru> Co-authored-by: Jiayu Liu <jiayu.liu@airbnb.com> Co-authored-by: Volodymyr Panivko <sveneld300@gmail.com> Co-authored-by: Pavel Kvach <pavel.kvach@gmail.com> Co-authored-by: adrianhelvikspond <adrian.helvik@spond.com> Co-authored-by: Volodymyr Panivko <sveneld300@gmailcom> Co-authored-by: vladimir.panivko <vladimir.panivko@together.com> Co-authored-by: Thomas <thomasbruggink@hotmail.com> Co-authored-by: Lukas Barth <mail@tinloaf.de> Co-authored-by: Tobias Weihs <t.weihs@mint-medical.de> Co-authored-by: Tuomo Jokimies <tuomo.jokimies@supercell.com> Co-authored-by: k-walton <113375522+k-walton@users.noreply.github.com> Co-authored-by: CJCombrink <carel.combrink@gmail.com> Co-authored-by: Nicholas La Roux <nicholas.laroux@shopify.com> Co-authored-by: Steven Mitcham <smitcham@blizzard.com> Co-authored-by: Alkis Evlogimenos <alkis.evlogimenos@databricks.com> Co-authored-by: uv747 <uv747@163.com> Co-authored-by: Charles Coulombe <charles.coulombe@calculquebec.ca>
Replaced usage of
endl
with\n
directlystatic const string endl = "\n"
(removed)[skip ci]
anywhere in the commit message to free up build resources.Reference to my comment here